fixes blank tiles due to unsupported content #755

2018-11-21T07:29:06+01:00

jessopb commented

2018-11-21 07:29:06 +01:00

(Migrated from github.com)

This is a draft of a solution.

What I'd like to do is have a central list of supported type strings ( 'image/jpg' ); passing this list allows for future filtering. Would it be better for supported types be passed or require()d?
Also, this function could be renamed or duplicated to GetAllMatchingChannelClaims, GetAllSupportedChannelClaims, or just GetChannelClaims. I don't think it makes sense to duplicate it just to get at currently unsupported content.

This is a draft of a solution. 1) What I'd like to do is have a central list of supported type strings ( 'image/jpg' ); passing this list allows for future filtering. Would it be better for supported types be passed or require()d? 2) Also, this function could be renamed or duplicated to GetAllMatchingChannelClaims, GetAllSupportedChannelClaims, or just GetChannelClaims. I don't think it makes sense to duplicate it just to get at currently unsupported content.

skhameneh (Migrated from github.com) approved these changes 2018-11-21 17:58:07 +01:00

skhameneh (Migrated from github.com) requested changes 2018-11-21 17:59:51 +01:00

server/chainquery/queries/claimQueries.js

					
				@ -68,15 +68,35 @@ export default (db, table, sequelize) => ({

				    });

				  },

skhameneh (Migrated from github.com) commented

2018-11-21 17:59:43 +01:00

This needs to be optional, chainquery functions should not be opinionated on Spee.ch content types.

skhameneh (Migrated from github.com) requested changes 2018-11-26 17:21:00 +01:00

server/chainquery/queries/claimQueries.js

					
				@ -77,2 +73,2 @@

				        { bid_state: 'Accepted' },

				      ],

				    const defaultWhereClauses = {

skhameneh (Migrated from github.com) commented

2018-11-26 17:20:51 +01:00

See here and the original signature of getAllChannelClaims: async (channelClaimId, bidState), this allows all claims to be fetched. The changes break this pattern and force Spee.ch filtering on code that should be unopinionated.

See here and the original signature of `getAllChannelClaims: async (channelClaimId, bidState)`, this allows all claims to be fetched. The changes break this pattern and force Spee.ch filtering on code that should be unopinionated.

jessopb (Migrated from github.com) reviewed 2018-11-27 05:05:07 +01:00

server/chainquery/queries/claimQueries.js

					
				@ -77,2 +73,2 @@

				        { bid_state: 'Accepted' },

				      ],

				    const defaultWhereClauses = {

jessopb (Migrated from github.com) commented

2018-11-27 05:05:06 +01:00

I had read that once you go beyond 1 optional parameter, you're better off passing an object. With bidState already being optional and not passed from the controller, contentTypes would have to be a second optional parameter, meaning the function, I think, would have to be called with ( first, null, third ). Is there a better way to make contentTypes optional? I'm leading toward a GetMatchingChannelClaims function in chainquery instead. This would pave the way for future db queries.

skhameneh commented

2018-11-27 17:21:58 +01:00

(Migrated from github.com)

Merging to get a fix in ASAP, will make a clean-up task.

Sign in to join this conversation.

No reviewers

skhameneh

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/spee.ch#755