Skip to content

Conversation

@livzorn
Copy link
Contributor

@livzorn livzorn commented Jul 23, 2021

JIRA link

HEEDLS-557

Description

I created a migration that changed the maximum length of the Email column on AdminUsers table from 250 to 255 characters, so it is consistent with the Candidates table. The migration also deletes and recreates the AdminUsers_Email index accordingly as it depends the Email column.

I also modified two view models so the form input for Email will have the same max length.

I will document these changes in the release notes after merge!

Copy link
Contributor

@AlexJacksonDS AlexJacksonDS left a comment

Choose a reason for hiding this comment

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

The change itself looks fine, but question for @stellake.

I can see that you've decided the centre fields are out of scope for this ticket in the HLD discussion, but now we are in the situation where the Centre manager can have an email that is 255 in the AdminUsers table, but only 250 in the Centres table. Do we have a ticket in the future to fix this? (Centre website email is 100 characters too, but doesn't necessarily match an admin like the CentreManager field should)

@stellake
Copy link
Contributor

I can see that you've decided the centre fields are out of scope for this ticket in the HLD discussion, but now we are in the situation where the Centre manager can have an email that is 255 in the AdminUsers table, but only 250 in the Centres table. Do we have a ticket in the future to fix this? (Centre website email is 100 characters too, but doesn't necessarily match an admin like the CentreManager field should)

The centre manager details are completely independent of the admin user that has centre manager permissions. The reason we're changing the email lengths of the users is that it affects the logins/core functionality/how we've combined these users into one account. The centre manager details are there as additional contact details display for centre, so we're good to leave it (and this way we don't need to introduce extra risk or spend unnecessary budget either). I suspect in reality, no one's going to specify their centre contact details as something with 200+ characters :)

@livzorn livzorn force-pushed the HEEDLS-557-email-length-consistency branch from 6600605 to 60640dd Compare July 26, 2021 09:17
@livzorn livzorn force-pushed the HEEDLS-557-email-length-consistency branch from 60640dd to 55f5cc3 Compare July 27, 2021 08:36
Copy link
Contributor

@stellake stellake left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@livzorn livzorn merged commit e374963 into master Jul 27, 2021
@stellake stellake deleted the HEEDLS-557-email-length-consistency branch August 11, 2021 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants