Int b 21480 full ppm counseling #14572

samaysofo · 2025-01-13T14:52:56Z

B-21480

Summary

These changes finds the closest transportation office that provides counseling in the GBLOC of the given duty location for oconus/conus duty locations then assign them to an order with a full PPM shipment that do not provide counseling. It also disables the counseling fee checkbox for full ppm shipments.

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

NOTE: I had to set the duty location for zip code (32222) provides_counseling to false in other to get a duty location with that status.

Run this to get all duty_location:
select * from duty_locations ;
Get the Id of one of the duty_locations and run this :
UPDATE duty_locations SET provides_services_counseling = false WHERE id = 'Provide_The_ID_You_Wish_TO_Update'
Double check if the record was updated
select * from duty_locations dl where dl.id ='The_ID_You_Updated'

How to test

Access the app
Login as a customer
Create a move and make sure the duty location does not provide service counseling (e.g NS Mayport FL , 32228)
Add a PPM shipment (make sure this is the only shipment associated with the move and the duty location is "NS Mayport FL") and submit.
Login as a Service Counselor (GBLOC = CNNQ)
Navigate to the "Counseling Queue".
Should see the move created in step 3 with status "Needs Counseling".
Login as a Task Ordering Office
Navigate to the Task Order Queue
Should not see the move created in step 3.
Click on the "Search" nav and search for the move created in step 3 (select "Move Code" search option)
Click on the move to navigate to the "Move details" page.
The counseling option(checkbox) under MTO service items should be disabled.

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

https://github.com/user-attachments/assets/77d9a562-4a6b-4111-8ffc-0d64835ab4d5
https://github.com/user-attachments/assets/d769ae4d-ee2e-4409-9183-707f294a57eb

…-Counseling

…move into B-21551-NTSR-Estimated-Price

…sciated tests

…-Counseling

…to MAIN-B-21480-Full-PPM-Counseling

samaysofo · 2025-01-15T15:02:55Z

fyi, working on adding the screenshots. the duty location (NS Mayport, FL) that was being used during development is no longer there in the location lookup.

danieljordan-caci · 2025-01-15T15:05:04Z

fyi, working on adding the screenshots. the duty location (NS Mayport, FL) that was being used during development is no longer there in the location lookup.

Yeah they are getting away from Prime counseled moves. My advice would be to just edit the provides_counseling value in whatever duty location you use to get the desired result.

samaysofo · 2025-01-15T19:28:47Z

PR has been updated with videos.

samaysofo · 2025-01-15T19:34:19Z

fyi, working on adding the screenshots. the duty location (NS Mayport, FL) that was being used during development is no longer there in the location lookup.

Yeah they are getting away from Prime counseled moves. My advice would be to just edit the provides_counseling value in whatever duty location you use to get the desired result.

I ended up setting one of the duty locations provides_counseling value i wanted to use in the db.

msaki-caci · 2025-01-15T18:50:02Z

pkg/services/transportation_office/transportation_office_fetcher.go

@@ -305,3 +306,95 @@ func findOconusGblocDepartmentIndicator(appCtx appcontext.AppContext, dutyLocati
 	return nil, apperror.NewImplementationError(fmt.Sprintf("Error: Cannot determine GBLOC -- serviceMember.Affiliation: %s, DutyLocaton: %s, departmentIndicator: %s, dutyLocation.Address.ID: %s",
 		serviceMember.Affiliation, dutyLocation.Name, *departmentIndicator, dutyLocation.Address.ID))
 }
+
+// Return the closeset transportation office in the GBLOC of the given duty location for oconus/conus duty locations


spelling on closest

This pr is still open for review, when anyone gets the chance. Waiting on making the typo change if by chance someone else has some recommended changes to make all at once.

robot-mymove · 2025-01-16T20:56:44Z

Test coverage on server failed.

Last run: Mon 20 Jan 2025 02:36:09 PM UTC
Refer to this report to see coverage details for the server app.
Documentation for test coverage can be found at this link.

ajlusk · 2025-01-20T18:08:09Z

pkg/services/mto_service_item/mto_service_item_creator_test.go

 	planner := &mocks.Planner{}
 	planner.On("ZipTransitDistance",
 		mock.AnythingOfType("*appcontext.appContext"),
 		mock.Anything,
 		mock.Anything,
 		false,
 		false,
+		false,
+		false,


ZipTransitDistance only takes 5 arguments. I can't tell why it takes a 4th and 5th but definitely doesn't need a 6th and 7th. Seems like maybe a wonky merge thing.

yeah, looks like a wonky merge got me

ajlusk · 2025-01-20T18:23:00Z

src/components/Office/RequestedShipments/SubmittedRequestedShipments.test.jsx

@@ -177,18 +177,17 @@ describe('RequestedShipments', () => {
      moveCode: 'TE5TC0DE',
    };

-    it('does not render the "Add service items to move" section or Counseling option when all shipments are PPM', () => {
+    it('should disable the counseling checkbox when full ppm shipment', () => {


Basically inconsequential but while you're fixing typos, it seems the previous language at the end of this was more accurate. "Full PPM" describes a move, not a shipment. There's a similar rephrasing in a few places.

brianmanley-caci · 2025-01-20T21:08:28Z

pkg/services/move/move_router.go

 	for i := range move.MTOShipments {
 		// if it's a PPMShipment update both the mto and ppm shipment level statuses
 		if move.MTOShipments[i].ShipmentType == models.MTOShipmentTypePPM {
 			move.MTOShipments[i].Status = models.MTOShipmentStatusSubmitted
 			move.MTOShipments[i].PPMShipment.Status = models.PPMShipmentStatusSubmitted
 			// actual expense reimbursement is always true for civilian moves
 			move.MTOShipments[i].PPMShipment.IsActualExpenseReimbursement = models.BoolPointer(isCivilian)
+			if move.IsPPMOnly() && !orders.OriginDutyLocation.ProvidesServicesCounseling {


Could you add a test for this logic in move_router_test.go?

brianmanley-caci · 2025-01-20T21:11:15Z

pkg/services/transportation_office.go

@@ -13,4 +13,5 @@ type TransportationOfficesFetcher interface {
 	GetTransportationOffice(appCtx appcontext.AppContext, transportationOfficeID uuid.UUID, includeOnlyPPMCloseoutOffices bool) (*models.TransportationOffice, error)
 	GetAllGBLOCs(appCtx appcontext.AppContext) (*models.GBLOCs, error)
 	GetCounselingOffices(appCtx appcontext.AppContext, dutyLocationID uuid.UUID) (*models.TransportationOffices, error)
+	FindClosestCounselingOffice(appCtx appcontext.AppContext, dutyLocationID uuid.UUID) (models.TransportationOffice, error)


To be consistent with the other functions, could you return a pointer to the transportation office?
FindClosestCounselingOffice(appCtx appcontext.AppContext, dutyLocationID uuid.UUID) (*models.TransportationOffice, error)

brianmanley-caci · 2025-01-20T21:20:23Z

pkg/services/transportation_office/transportation_office_fetcher_test.go

+
+func (suite *TransportationOfficeServiceSuite) Test_FindClosestCounselingOfficeCONUS() {
+	suite.toFetcher = NewTransportationOfficesFetcher()
+	customAddress1 := models.Address{


Can you use address_factory.BuildAddress to create these addresses?

brianmanley-caci · 2025-01-20T21:21:37Z

pkg/services/transportation_office/transportation_office_fetcher_test.go

+		models.AffiliationSPACEFORCE}
+
+	setupServiceMember := func(serviceMemberAffiliation models.ServiceMemberAffiliation) models.ServiceMember {
+		customServiceMember := models.ServiceMember{


I think you could use service_member_factory here.

ajlusk · 2025-01-21T17:14:47Z

pkg/services/transportation_office/transportation_office_fetcher_test.go

+				},
+			},
+		}, nil)
+		suite.MustSave(&origDutyLocation)


I believe the suite.MustSave here is redundant and superfluous.

It's needed for the address above but since you already used the factory with a DB connection for the duty location it isn't doing anything. It's very easy to copy and paste from somewhere else. I was recently corrected on the same thing.

loganwc and others added 30 commits October 29, 2024 16:07

estimated prices now calculates for NTS-R shipments

a89fca0

updated comment

78efbd4

now using 110% of estimated weight for NTS-R

0c62a14

Merge branch 'main' into B-21551-NTSR-Estimated-Price

fa2cd85

most of the tests but they don't work

f419625

tests now workgit add .

6783ebc

Merge branch 'main' into B-21551-NTSR-Estimated-Price

2811129

someone deleted a thing i wanted

529b140

Update mto_service_item_creator_test.go

ab2950c

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

406e7a4

…-Counseling

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

1404941

…-Counseling

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

2a5d7a1

…-Counseling

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

4e7c5aa

…-Counseling

Disables counseling checkbox

7e9693a

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

7a4692d

…-Counseling

Merge branch 'main' into B-21551-NTSR-Estimated-Price

22f8556

Changes moveRouter struct to add transportationOfficesFetcher

945cf21

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

20d3169

…-Counseling

fixed origindutylocation providesServicesCounseling nil issue

e5a6ef0

code clean up

fe1793e

Merge branch 'main' into B-21551-NTSR-Estimated-Price

56dec62

Merge branch 'main' into B-21551-NTSR-Estimated-Price

44caebe

update tests so they work with Dan the Mans changes

0cb1e19

Merge branch 'B-21551-NTSR-Estimated-Price' of github.com:transcom/my…

ccacf68

…move into B-21551-NTSR-Estimated-Price

Merge branch 'main' into B-21551-NTSR-Estimated-Price

46fe8c2

more daniel madness

7608779

Added function for finding the closeset counseling office with its as…

78581e2

…sciated tests

Added test for CONUS/OCONUS for finding the closest counseling offices.

faa41a8

Merge remote-tracking branch 'origin/main' into MAIN-B-21480-Full-PPM…

deee183

…-Counseling

Merge remote-tracking branch 'origin/B-21551-NTSR-Estimated-Price' in…

805efd3

…to MAIN-B-21480-Full-PPM-Counseling

samaysofo requested review from pambecker, brianmanley-caci, elizabeth-perkins, msaki-caci, ryan-mchugh and stevengleason-caci January 15, 2025 14:07

samaysofo marked this pull request as ready for review January 15, 2025 14:08

samaysofo requested review from a team as code owners January 15, 2025 14:08

Merge branch 'integrationTesting' into INT-B-21480-Full-PPM-Counseling

37ef056

msaki-caci reviewed Jan 15, 2025

View reviewed changes

Merge branch 'integrationTesting' into INT-B-21480-Full-PPM-Counseling

80f7650

samaysofo requested a review from begrohmann January 15, 2025 22:31

samaysofo added the database label Jan 15, 2025

Merge branch 'integrationTesting' into INT-B-21480-Full-PPM-Counseling

8a0a5c8

samaysofo added 2 commits January 17, 2025 07:58

Merge branch 'integrationTesting' into INT-B-21480-Full-PPM-Counseling

4547870

Merge branch 'integrationTesting' into INT-B-21480-Full-PPM-Counseling

70be164

samaysofo requested a review from danieljordan-caci January 20, 2025 14:14

ajlusk reviewed Jan 20, 2025

View reviewed changes

brianmanley-caci requested changes Jan 20, 2025

View reviewed changes

ajlusk reviewed Jan 21, 2025

View reviewed changes

begrohmann approved these changes Jan 21, 2025

View reviewed changes

samaysofo closed this Jan 22, 2025

samaysofo deleted the INT-B-21480-Full-PPM-Counseling branch February 28, 2025 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Int b 21480 full ppm counseling #14572

Int b 21480 full ppm counseling #14572

samaysofo commented Jan 13, 2025 •

edited

Loading

samaysofo commented Jan 15, 2025

danieljordan-caci commented Jan 15, 2025

samaysofo commented Jan 15, 2025

samaysofo commented Jan 15, 2025

msaki-caci Jan 15, 2025

samaysofo Jan 17, 2025

robot-mymove commented Jan 16, 2025 •

edited

Loading

ajlusk Jan 20, 2025

samaysofo Jan 20, 2025

ajlusk Jan 20, 2025 •

edited

Loading

brianmanley-caci Jan 20, 2025

brianmanley-caci Jan 20, 2025

brianmanley-caci Jan 20, 2025

brianmanley-caci Jan 20, 2025

ajlusk Jan 21, 2025 •

edited

Loading

Int b 21480 full ppm counseling #14572

Int b 21480 full ppm counseling #14572

Conversation

samaysofo commented Jan 13, 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

samaysofo commented Jan 15, 2025

danieljordan-caci commented Jan 15, 2025

samaysofo commented Jan 15, 2025

samaysofo commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robot-mymove commented Jan 16, 2025 • edited Loading

Test coverage on server failed.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajlusk Jan 20, 2025 • edited Loading

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

ajlusk Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

samaysofo commented Jan 13, 2025 •

edited

Loading

robot-mymove commented Jan 16, 2025 •

edited

Loading

ajlusk Jan 20, 2025 •

edited

Loading

ajlusk Jan 21, 2025 •

edited

Loading