B 21934 int #14553

deandreJones · 2025-01-09T18:50:57Z

B-21934

Summary

Is there anything you would like reviewers to give additional scrutiny?

this article explains more about the approach used.

Verification Steps for the Author

These are to be checked by the author.

Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
Have the Agility acceptance criteria been met for this change?

Verification Steps for Reviewers

These are to be checked by a reviewer.

Has the branch been pulled in and checked out?
Have the BL acceptance criteria been met for this change?
Was the CircleCI build successful?
Has the code been reviewed from a standards and best practices point of view?

Setup to Run the Code

How to test

Access the Customer app
create a shipment
submit to counselor
login as counselor
search for move that was just created
click Allowances on left side
click on Admin Restricted Weight Location
the weight Restriction box should appear
notice the verbiage change from “Weight allowance” has been changed to “Standard weight allowance”.
add a weight to Weight Restriction and see its saved and viewable on allowances tile
submit the move to TOO(add orders info)
login as TOO search for the move and click approve with shipment(s) selected
on the popup see that the admin restricted weight location value is yes and the weight restriction value is populated
click Approve and send
log in as prime simulator role
in a separate tab go to http://officelocal:3000/swagger-ui/prime.html#/moveTaskOrder/getMoveTaskOrder
review the response in the entitlements object to see that the weight restriction weight is included

Frontend

There are no aXe warnings for UI.
This works in Supported Browsers and their phone views (Chrome, Firefox, Edge).
There are no new console errors in the browser devtools.
There are no new console errors in the test output.
If this PR adds a new component to Storybook, it ensures the component is fully responsive, OR if it is intentionally not, a wrapping div using the officeApp class or custom min-width styling is used to hide any states the would not be visible to the user.
This change meets the standards for Section 508 compliance.

Backend

Code follows the guidelines for Logging.
The requirements listed in Querying the Database Safely have been satisfied.

Database

Any new migrations/schema changes:

Follows our guidelines for Zero-Downtime Deploys.
Have been communicated to #g-database.
Secure migrations have been tested following the instructions in our docs.

Screenshots

… B-21934-INT

remove dup- from merge conflict

danieljordan-caci

Couple bugs:

When I uncheck the admin restricted weight location box after putting in a value, it still saves that value on form submission. See video:
https://github.com/user-attachments/assets/16a2039e-4aff-4d5a-a74e-264924d8e1c5
I can put in a negative value and it saves:
Can you swip swap some boxes around so all of the checkboxes are all in order? Just requires switching the placement of the standard weight allowance box:

migrations/app/schema/20250107184554_add_weight_restriction.up.sql

pkg/handlers/ghcapi/internal/payloads/model_to_payload.go

pkg/models/ghc_entitlements.go

src/components/Office/AllowancesDetailForm/AllowancesDetailForm.jsx

deandreJones · 2025-01-14T16:58:08Z

Couple bugs:

When I uncheck the admin restricted weight location box after putting in a value, it still saves that value on form submission. See video:
https://github.com/user-attachments/assets/16a2039e-4aff-4d5a-a74e-264924d8e1c5

I can put in a negative value and it saves:

Can you swip swap some boxes around so all of the checkboxes are all in order? Just requires switching the placement of the standard weight allowance box:

what ... what order you thinking? not sure what you mean

danieljordan-caci · 2025-01-14T16:59:43Z

Couple bugs:

When I uncheck the admin restricted weight location box after putting in a value, it still saves that value on form submission. See video:
https://github.com/user-attachments/assets/16a2039e-4aff-4d5a-a74e-264924d8e1c5

I can put in a negative value and it saves:

Can you swip swap some boxes around so all of the checkboxes are all in order? Just requires switching the placement of the standard weight allowance box:

what ... what order you thinking? not sure what you mean

check the screenshot - I got it arranged how I'd like it

… B-21934-INT

deandreJones · 2025-01-14T17:03:58Z

Couple bugs:

When I uncheck the admin restricted weight location box after putting in a value, it still saves that value on form submission. See video:
https://github.com/user-attachments/assets/16a2039e-4aff-4d5a-a74e-264924d8e1c5

I can put in a negative value and it saves:

Can you swip swap some boxes around so all of the checkboxes are all in order? Just requires switching the placement of the standard weight allowance box:

what ... what order you thinking? not sure what you mean

check the screenshot - I got it arranged how I'd like it

oh- putting the standard weight allowance above all the boxes

TevinAdams · 2025-01-14T17:34:31Z

Test Case Passed without edge cases:

danieljordan-caci

Getting an error when trying to create a move as a SC:

ERROR   ghcapi/orders.go:276    Data received from requester is bad: BAD_DATA: Error saving entitlement       {"git_branch": "B-21934-INT", "git_commit": "36ed644a365fe4497cf354c22d1bdc3381862220", "host": "officelocal:3000", "milmove_trace_id": "39076976-fcc0-429e-8bff-ea3b02c55279", "session_id": "dcktUPQ_9Bf5YC1vl0Cfnj-PoRilHVkB0ugj-y_57iA"}
github.com/transcom/mymove/pkg/handlers/ghcapi.(*CreateOrderHandler).Handle.CreateOrderHandler.Handle.func1
        /Users/daniel.jordan_cn/mymove/pkg/handlers/ghcapi/orders.go:276
github.com/transcom/mymove/pkg/handlers.(*Config).AuditableAppContextFromRequestWithErrors.func1
        /Users/daniel.jordan_cn/mymove/pkg/handlers/config.go:152
github.com/transcom/mymove/pkg/appcontext.(*appContext).NewTransaction.func1
        /Users/daniel.jordan_cn/mymove/pkg/appcontext/context.go:67

Ran make db_dev_fresh prior to this as well

danieljordan-caci · 2025-01-14T22:10:45Z

pkg/models/ghc_entitlements.go

@@ -31,6 +31,8 @@ type Entitlement struct {
 	OrganizationalClothingAndIndividualEquipment bool             `db:"organizational_clothing_and_individual_equipment"`
 	ProGearWeight                                int              `db:"pro_gear_weight"`
 	ProGearWeightSpouse                          int              `db:"pro_gear_weight_spouse"`
+	AdminRestrictedWeightLocation                *bool            `db:"admin_restricted_weight_location"`
+	WeightRestriction                            *int             `db:"weight_restriction"`


Looking at your migrations:

+ 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

AdminRestrictedWeightLocation is NOT NULL so it shouldn't be a pointer
WeightRestriction can be NULL so it needs to be a pointer

cameroncaci

Requesting feedback - this ticket didn't have an upstream set properly (Which doesn't break too much thankfully), and the upstream had a change to the db columns for weight restrictions. Switching from entitlements having a bool column of if weight restricted being true is the biggest change from the upstream - the weight restrictor is minimal code changes.

The frontend would need to be modified to not track/send payloads of this bool and instead be under the logic of if weight restricted amount is present (Not NULL), then weight restriction is true and then show the conditional fields.

Also since the migrations aren't deployed yet, suggesting to merge in the upstream migrations to shorten it down to a single migration file

cameroncaci · 2025-01-15T14:28:08Z

migrations/app/schema/20250106190758_admin_weight_restriction_convert_bool.up.sql

+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;


Hey there, so this ticket was missing an upstream (Minimal impact), but in the db it was decided that the boolean value is not needed if we have a not null weight_restriction amount

See #14525 (comment)

cameroncaci · 2025-01-15T14:29:26Z

migrations/app/schema/20250106153750_admin_weight_restriction.up.sql

+ALTER TABLE entitlements
+ADD COLUMN IF NOT EXISTS admin_restricted_weight_location BOOLEAN;


Since this is the bool, ref'ing the other file at migrations/app/schema/20250106190758_admin_weight_restriction_convert_bool.up.sql with #14525 (comment)

cameroncaci · 2025-01-15T14:30:03Z

pkg/factory/entitlement_factory.go

+		AdminRestrictedWeightLocation:                setBoolPtr(cEntitlement.AdminRestrictedWeightLocation, false),
+		WeightRestriction:                            &weightRestriction,


If weight restriction doesn't exist, this would probably be better off left as nil instead of set to 0

cameroncaci · 2025-01-15T14:30:40Z

pkg/handlers/primeapi/payloads/model_to_payload.go

+		TotalWeight:       totalWeight,
+		WeightRestriction: int64(*entitlement.WeightRestriction),
+		ETag:              etag.GenerateEtag(entitlement.UpdatedAt),


If switching to nil, this weight restriction returns empty not nil

Once the swagger gets updated I'm sure the IDE intellisense will point out these instances for faster bread crumbing

cameroncaci · 2025-01-15T14:32:41Z

pkg/services/order/order_updater.go

+	weightRestriction := int(payload.WeightRestriction)
+	order.Entitlement.WeightRestriction = &weightRestriction


Thoughts on importing the weight restrictor service for use here?

mymove/pkg/services/entitlements/weight_restrictor.go

Line 22 in 2772bed

func (wr *weightRestrictor) ApplyWeightRestrictionToEntitlement(appCtx appcontext.AppContext, entitlement models.Entitlement, weightRestriction int, eTag string) (*models.Entitlement, error) {

deandreJones · 2025-01-15T14:46:18Z

closing due to newly found upstream

deandreJones and others added 7 commits January 7, 2025 17:08

B-21934

31fd371

Merge remote-tracking branch 'origin/B-21934' into B-21934-INT

a21714d

add weight restriction

633df9d

Merge remote-tracking branch 'origin/main' into B-21934

0f3269c

add weight restriction

667d8b4

B-21934 add validation

a1e1fb0

Merge branch 'integrationTesting' into B-21934-INT

336c02b

deandreJones added Mountain Movers Movin' Mountains 1 Sprint at a time INTEGRATION Slated for Integration Testing labels Jan 10, 2025

deandreJones added 2 commits January 10, 2025 17:29

fix tests

1d69b42

Merge branch 'B-21934-INT' of https://github.com/transcom/mymove into…

e5c2c40

… B-21934-INT

deandreJones marked this pull request as ready for review January 10, 2025 17:49

deandreJones requested review from a team as code owners January 10, 2025 17:49

deandreJones and others added 4 commits January 10, 2025 11:49

Merge branch 'integrationTesting' into B-21934-INT

4d5344f

more tests

2440a23

Merge branch 'B-21934-INT' of https://github.com/transcom/mymove into…

b756b98

… B-21934-INT

remove comment

341c8fa

deandreJones requested review from a team, danieljordan-caci, TevinAdams, cameroncaci and traskowskycaci January 10, 2025 21:44

deandreJones added 2 commits January 13, 2025 14:14

Merge branch 'integrationTesting' into B-21934-INT

9bfa6a2

Update ghc.yaml

36ed644

remove dup- from merge conflict

danieljordan-caci requested changes Jan 13, 2025

View reviewed changes

pointing

84afa63

Merge branch 'B-21934-INT' of https://github.com/transcom/mymove into…

464e4cf

… B-21934-INT

deandreJones and others added 2 commits January 14, 2025 11:08

cupid shuffle

87ab429

Merge branch 'integrationTesting' into B-21934-INT

6b044c6

deandreJones requested a review from danieljordan-caci January 14, 2025 17:13

danieljordan-caci requested changes Jan 14, 2025

View reviewed changes

cameroncaci requested changes Jan 15, 2025

View reviewed changes

deandreJones closed this Jan 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

B 21934 int #14553

B 21934 int #14553

deandreJones commented Jan 9, 2025 •

edited

Loading

danieljordan-caci left a comment

deandreJones commented Jan 14, 2025

danieljordan-caci commented Jan 14, 2025

deandreJones commented Jan 14, 2025

TevinAdams commented Jan 14, 2025

danieljordan-caci left a comment

danieljordan-caci Jan 14, 2025

cameroncaci left a comment

cameroncaci Jan 15, 2025

cameroncaci Jan 15, 2025

cameroncaci Jan 15, 2025

cameroncaci Jan 15, 2025

cameroncaci Jan 15, 2025

cameroncaci Jan 15, 2025

deandreJones commented Jan 15, 2025

		ALTER TABLE entitlements
		ADD COLUMN IF NOT EXISTS admin_restricted_weight_location BOOLEAN;

		AdminRestrictedWeightLocation: setBoolPtr(cEntitlement.AdminRestrictedWeightLocation, false),
		WeightRestriction: &weightRestriction,

		weightRestriction := int(payload.WeightRestriction)
		order.Entitlement.WeightRestriction = &weightRestriction

B 21934 int #14553

B 21934 int #14553

Conversation

deandreJones commented Jan 9, 2025 • edited Loading

Summary

Verification Steps for the Author

Verification Steps for Reviewers

Setup to Run the Code

How to test

Frontend

Backend

Database

Any new migrations/schema changes:

Screenshots

danieljordan-caci left a comment

Choose a reason for hiding this comment

deandreJones commented Jan 14, 2025

danieljordan-caci commented Jan 14, 2025

deandreJones commented Jan 14, 2025

TevinAdams commented Jan 14, 2025

danieljordan-caci left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cameroncaci left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deandreJones commented Jan 15, 2025

deandreJones commented Jan 9, 2025 •

edited

Loading