-
Notifications
You must be signed in to change notification settings - Fork 35
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
B 21934 int #14629
B 21934 int #14629
Changes from 17 commits
9db96ef
4e79c09
13764bc
6204fed
5dbc184
62a3b44
f88c16a
6f96ae7
c1c37b5
0cc7a3a
9070c36
b4a649f
0e728d4
2bd35bd
c3a23c4
a25402d
c17a897
5f26682
6061c7b
3853b1c
5b9f7ec
637cdab
cf6ac4c
3200c4e
21612c9
07cb5cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
ALTER TABLE entitlements | ||
ADD COLUMN IF NOT EXISTS admin_restricted_weight_location BOOLEAN; | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
ALTER TABLE entitlements | ||
ALTER COLUMN admin_restricted_weight_location TYPE boolean USING (COALESCE(admin_restricted_weight_location, false)), | ||
ALTER COLUMN admin_restricted_weight_location SET DEFAULT false, | ||
ALTER COLUMN admin_restricted_weight_location SET NOT NULL; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Recommending remove this boolean column and instead basing whether or not the entitlement is restricted based on
entitlements.weight_restriction
being null or not null.Reason for recommendation is because this bool column would just be storing whether or not weight restriction is null or not and also we end up with the possibility of
true
weight restriction without any integer providedScreen.Recording.2025-01-23.at.8.59.38.AM.mov
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.
Thinking.. im going to read up on the feature and weigh the pros/cons
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.
@transcom/mountain-movers would also love others thoughts on this
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 wouldn't be opposed to just enforcing the constraight of if bool true that weight restriction int must also be present - but we'll want some method of enforcing that a restriction exists
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.
actually.. I don't hate it.. refactoring now
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.
ill just remove that column and variable, and the UI will read from the weightRestriction value being null or not
and if they check it.. they have to enter a value
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.
Yeah I agree with this approach! Nice catch Cam!
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.
Also agree! Seems clean
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 - give it a re-review
I do like this much better