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

Added non-null constraint to temporary_label_id #3408

Merged
merged 10 commits into from
Oct 24, 2023
Merged

Conversation

dylanbun
Copy link
Collaborator

Resolves #2133

Changed the temporary_label_id column in the label and audit task interaction table to be from null to non-null.

  • I've written a descriptive PR title.

@dylanbun dylanbun self-assigned this Oct 18, 2023
@misaugstad misaugstad self-assigned this Oct 22, 2023
@misaugstad
Copy link
Member

Ahh, so we actually only wanted to change this for the label table. Not all interactions in the audit_task_interaction table are tied to a label, so it should be an option type there. And I don't believe that there are any instances where it is null in the label table. I'm okay with the check there just in case; could you just add a comment to say that there are no instances of this in the live databases at the time it was run?

@dylanbun
Copy link
Collaborator Author

yup, updated it. ready for review!

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Oh you still need to update the Label Scala class to use an Int instead of Option[Int]!

And since you're already making an update, making the comment in the SQL file match the style would be nice :)

@dylanbun
Copy link
Collaborator Author

ready for review (assuming you just wanted a period added to the sql file).

@misaugstad
Copy link
Member

And starting with a capital letter, but I can change that if everything else checks out ;)

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Great, thanks @dylanbun!!

@misaugstad misaugstad merged commit 5eb98f4 into develop Oct 24, 2023
@misaugstad misaugstad deleted the 2133-temp-label branch October 24, 2023 00:02
@misaugstad misaugstad mentioned this pull request Nov 1, 2023
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.

temporary_label_id stored as Option[Int] in Database
2 participants