Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#Refocus api token database updates #97

Merged
merged 5 commits into from
Nov 16, 2016
Merged

#Refocus api token database updates #97

merged 5 commits into from
Nov 16, 2016

Conversation

pallavi2209
Copy link
Contributor

@pallavi2209 pallavi2209 commented Nov 11, 2016

New Model: Token

Columns:

  • name - allowNull: false, validate using db/constants.nameRegex, use db/constants.fieldlen.normalName for field length
  • token - allowNull: false, string
  • isDeleted - dataTypes.BIGINT, defaultValue: 0, allowNull: false
  • isDisabled - follow the same pattern we use for "isDeleted" so a record is considered "disabled" when this field has a non-zero value

Indexes:

  • unique on lower(name)+createdBy+isDeleted

Associations:

  • createdBy association to User

Scopes:

  • default scope should exclude the token field and should order by name ascending

Hooks:

  • beforeCreate - reuse db/helpers/userUtils.hashPassword to store the encrypted version of the token

Permissions:

  • Profile checks will be in the API layer, not the DB layer

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 83.975% when pulling a5ac0eb on token-db-updates into c8424e5 on master.

name: {
type: dataTypes.STRING(constants.fieldlen.normalName),
allowNull: false,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a name field ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user has multiple tokens, the name field makes it possible to identify a particular token one might wish to disable or delete

type: dataTypes.STRING,
allowNull: false,
unique: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamigo @pallavi2209 Any thoughts on having a default values for all the data types? We might be able to get ride of the responsify function at one point of time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shriram can you create a spike for team to discuss this? (But let's not let it hold up this particular PR.)

Copy link
Contributor

@iamigo iamigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will approve after fixing reference to lens/lenses

},
},
name: {
singular: 'Lens',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 83.975% when pulling 652ed73 on token-db-updates into 4df2b8f on master.

@pallavi2209
Copy link
Contributor Author

@iamigo @shriramshankar updated the PR.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 83.975% when pulling 7d69ea0 on token-db-updates into 2337cc5 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 82.208% when pulling 6ce4698 on token-db-updates into a73a4aa on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 82.208% when pulling c819876 on token-db-updates into ca2ac27 on master.

@iamigo iamigo merged commit 4148fbb into master Nov 16, 2016
@iamigo iamigo deleted the token-db-updates branch November 16, 2016 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants