LF-4037: add default to last name #3092
Conversation
antsgar
requested review from
SayakaOno and
kathyavini
and removed request for
a team
|
|
||
| export const up = async function (knex) { | ||
| await knex.schema.alterTable('users', (table) => { | ||
| table.string('last_name').notNullable().defaultTo('').alter(); |
There was a problem hiding this comment.
What existed before was really weird... notNullable().defaultTo(null) 😂 . Almost makes me think it was a typo.
My question is why default to empty string instead of just allowing null with .nullable()? It might make it easier to deal with on the frontend but it seems weird to me https://stackoverflow.com/a/44890240
There was a problem hiding this comment.
Well the not nullable with a null default is not that weird, essentially null is the default if there's no default if that makes sense. Essentially if you set a column to not nullable and you don't set a default, you're expecting that whoever's inserting a new record will always specify that field, which is totally fine if the field is required and that's what you want to enforce.
To the second point, I'd totally prefer to make this nullable, but that seems higher risk especially in the context of a hotfix since there might be places where the client is expecting a string and is not prepared with nullish coalescing to handle the null scenario, so this seems like the safest choice.
There was a problem hiding this comment.
Ok I did not realize it was hotfix 👍
| @@ -158,6 +158,7 @@ const loginController = { | |||
| isInvited: user?.status_id === 2, | |||
| }); | |||
| } catch (err) { | |||
| console.error(err); | |||
There was a problem hiding this comment.
Ohhh so that's why the new error message didn't come back 🙂
Duncan-Brain
deleted the
LF-4037-SSO-account-cannot-be-created-without-last-name
branch
…be-created-without-last-name LF-4037: add default to last name
Description
Add default of
''tolast_namecolum inuserstable.We're currently not setting a default at the db level and have a fallback of '' on the client when we send out the user information from an invitation. To avoid having to set this fallback in each place where we add a user, I defaulted the column and made the field not required at the model level.
Jira link:
https://lite-farm.atlassian.net/browse/LF-4037
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Tested bug by creating a Google account without a last name and verifying that I could use it to log into Litefarm.
Checklist: