-
Notifications
You must be signed in to change notification settings - Fork 9
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
#167619168 Create Sessions Table #18
Conversation
- Create migration from session table - Add demo seed file for session details [Finishes #167619168]
- Create migration from session table - Add demo seed file for session details [Finishes #167619168]
3611c93
to
958ef82
Compare
958ef82
to
612accb
Compare
612accb
to
42ab2aa
Compare
type: Sequelize.STRING | ||
}, | ||
userAgent: { | ||
allowNull: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow null here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
type: Sequelize.DATE | ||
}, | ||
ipAddress: { | ||
allowNull: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow null here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think allow null here means that the field should be nullable, or am I wrong?
userId: DataTypes.INTEGER, | ||
active: DataTypes.BOOLEAN, | ||
devicePlatform: DataTypes.STRING, | ||
expiresAt: DataTypes.DATE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't sequelize have a date time type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is datetime, confirmed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add model tests
Okay. On it. |
42ab2aa
to
2fe96bb
Compare
2fe96bb
to
1021d4f
Compare
- Create migration for session table - Add demo seed file for session details [Finishes #167619168]
1021d4f
to
d8cb777
Compare
|
d8cb777
to
6914c9b
Compare
type: Sequelize.DATE | ||
}, | ||
ipAddress: { | ||
allowNull: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think allow null here means that the field should be nullable, or am I wrong?
6914c9b
to
f3f7b32
Compare
- Create migration from session table - Add demo seed file for session details [Finishes #167619168]
f3f7b32
to
84febf5
Compare
- Create migration from session table - Add demo seed file for session details [Finishes #167619168]
What does this PR do?
Description of Task proposed in this pull request?
How should this be manually tested (Quality Assurance)?
-Add the database connection string in .env file
npm run db:seed
What are the relevant pivotal tracker stories?
Any background context you want to add (Operations Impact)?
What I have learned working on this feature:
Screenshots: [If you made some visual changes to the application please upload screenshots here, or remove this section]