Sequelize queries #234

2017-11-01T04:46:12+01:00

bones7242 commented

2017-11-01 04:46:12 +01:00

(Migrated from github.com)

updated models to paramterize sql queries

- updated models to paramterize sql queries

kauffj (Migrated from github.com) reviewed 2017-11-03 18:47:07 +01:00

kauffj (Migrated from github.com) left a comment

Oops, I typed all these up a few days ago and never hit submit on the review.

models/certificate.js

					
				@ -1,4 +1,31 @@

				module.exports = (sequelize, { STRING, BOOLEAN, INTEGER, TEXT, ARRAY, DECIMAL, DOUBLE }) => {

				const logger = require('winston');

kauffj (Migrated from github.com) commented

2017-11-01 14:33:06 +01:00

Would it make sense for this to be Channel rather than Certificate?

Would it make sense for this to be `Channel` rather than `Certificate`?

models/certificate.js

					
				@ -2,0 +2,4 @@

				const NO_CHANNEL = 'NO_CHANNEL';

				function sortResult (result, longId) {

				  let claimIndex;

kauffj (Migrated from github.com) commented

2017-11-01 14:29:47 +01:00

This function is called sortResult but it appears to return a short Id.

This function is called `sortResult` but it appears to return a short Id.

models/claim.js

					
				@ -2,0 +2,4 @@

				const NO_CLAIM = 'NO_CLAIM';

				function sortResult (result, longId) {

				  let claimIndex;

kauffj (Migrated from github.com) commented

2017-11-01 14:31:42 +01:00

This function seems to contain code that could be shared with the other sortResult.

(This function has same naming problem as well.)

This function seems to contain code that could be shared with the other `sortResult`. (This function has same naming problem as well.)

routes/api-routes.js

					
				@ -147,7 +147,8 @@ module.exports = (app) => {

				  // route to get a short claim id from long claim Id

				  app.get('/api/shortClaimId/:longId/:name', ({ originalUrl, ip, params }, res) => {

kauffj (Migrated from github.com) commented

2017-11-01 14:34:49 +01:00

I'm guessing this code isn't actually missing a colon or it wouldn't work, but what's the difference here?

bones7242 (Migrated from github.com) reviewed 2017-11-04 00:43:11 +01:00

models/certificate.js

					
				@ -1,4 +1,31 @@

				module.exports = (sequelize, { STRING, BOOLEAN, INTEGER, TEXT, ARRAY, DECIMAL, DOUBLE }) => {

				const logger = require('winston');

bones7242 (Migrated from github.com) commented

2017-11-04 00:43:10 +01:00

I think Certificate is accurate because it is actually storing the certificate claims from the blockchain. We have a Channel table for the Channels that are created on spee.ch. It parallels how we have a Claim table for all the stream claims in the blockchain (i guess this table should be Streams to be fully parallel) and a File table for all the files held locally by spee.ch.

bones7242 (Migrated from github.com) reviewed 2017-11-04 01:06:47 +01:00

routes/api-routes.js

					
				@ -147,7 +147,8 @@ module.exports = (app) => {

				  // route to get a short claim id from long claim Id

				  app.get('/api/shortClaimId/:longId/:name', ({ originalUrl, ip, params }, res) => {

bones7242 (Migrated from github.com) commented

2017-11-04 01:06:47 +01:00

the line break was accidental; difference was just adding Claim because getShortClaimId... is now a method of Claim

kauffj (Migrated from github.com) reviewed 2017-11-06 17:58:11 +01:00

models/certificate.js

					
				@ -1,4 +1,31 @@

				module.exports = (sequelize, { STRING, BOOLEAN, INTEGER, TEXT, ARRAY, DECIMAL, DOUBLE }) => {

				const logger = require('winston');

kauffj (Migrated from github.com) commented

2017-11-06 17:58:11 +01:00

@billbitt got it

Sign in to join this conversation.

No reviewers

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#234