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

Int b 22693 #14957

Open
wants to merge 37 commits into
base: integrationTesting
Choose a base branch
from
Open

Int b 22693 #14957

wants to merge 37 commits into from

Conversation

brianmanley-caci
Copy link
Contributor

Agility ticket

Summary

In B-22692 we added a new DB procedure for populating the prime_acknowledge_at columns on the moves and shipments table. In this story we are adding a new endpoint prime/v1/move-task-orders/acknowledge to the prime API. This endpoint allows the prime to send the move/shipment data with the primeAcknowledgedAt values, and we will then pass that to our DB procedure to save those values.

Verification Steps for the Author

These are to be checked by the author.

  • 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 Local build successful?
  • Has the code been reviewed from a standards and best practices point of view?

How to test

  1. Start your local server.
  2. Login to the office app as a Prime Simulator.
  3. Open up the prime v1 swagger UI (http://officelocal:3000/swagger-ui/prime.html).
  4. Scroll down to the acknowledgeMovesAndShipments endpoint and click Try it out.
  5. Open DBeaver and execute the following query to find some move/shipment data:
    select m.id as move_id, ms.id as shipment_id, m.prime_acknowledged_at as move_prime_acknowledged_at, ms.prime_acknowledged_at as shipment_prime_acknowledged_at from moves m join mto_shipments ms on m.id = ms.move_id;
  6. Go back to swagger and plug a move id, shipment id, and primeAcknowledgeAt values into the payload.
  7. Click Execute.
  8. Verify that you receive a success message.
  9. Go back to DBeaver, and verify that the prime_acknowledged_at column has been populated with the correct values.

Backend

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

Screenshot 2025-03-04 at 5 22 25 PM
Screenshot 2025-03-04 at 5 23 47 PM
Screenshot 2025-03-04 at 5 25 52 PM

@brianmanley-caci brianmanley-caci added INTEGRATION Slated for Integration Testing Go-Rillaz Go-Rillaz labels Mar 4, 2025
@brianmanley-caci brianmanley-caci self-assigned this Mar 4, 2025
@brianmanley-caci brianmanley-caci requested a review from a team as a code owner March 4, 2025 23:35
@ryan-mchugh ryan-mchugh self-requested a review March 5, 2025 17:59
suite.Assertions.IsType(&movetaskorderops.AcknowledgeMovesAndShipmentsOK{}, handlerResponse)
})

suite.Run("Unsuccessful Acknowledge Moves and Shipments - 200", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) 200->500 (internal server error)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to the correct http code.

return movetaskorderops.NewAcknowledgeMovesAndShipmentsInternalServerError(), err
}
responsePayload := &primemessages.AcknowledgeMovesShipmentsSuccessResponse{
Message: "Successfully updated acknowledgement for moves and shipments",
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) can check the message on tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a check to validate the success message.

}
moves, verrs := payloads.MovesModelFromAcknowledgeMovesAndShipments(&payload)
if verrs != nil && verrs.HasAny() {
return movetaskorderops.NewAcknowledgeMovesAndShipmentsUnprocessableEntity().WithPayload(payloads.ValidationError(
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor) can check the unprocessable type on tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually not as minor as you thought. I added the test, but realized I wasn't properly returning this error in the case of a bad payload. Good catch 🙇 Should be good to go now.

@ryan-mchugh ryan-mchugh self-requested a review March 5, 2025 18:17
@ryan-mchugh
Copy link
Contributor

Local Testing
image
image
image
image

@@ -1118,6 +1118,72 @@ func (suite *MTOServiceItemServiceSuite) TestUpdateMTOServiceItemData() {
}
})

suite.Run("SITDepartureDate - Does not error or update shipment auth end date when set after the authorized end date - international", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

am getting a failing test

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And here I was attempting to fix that failure 😞 I'll take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this test was essentially duplicated on main, but not on int. Somehow after removing the duplicate from my main B branch it got added back to my INT-B branch 😕 I removed it from this branch only as it doesn't exists on my main one.

Copy link
Contributor

@ryan-mchugh ryan-mchugh left a comment

Choose a reason for hiding this comment

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

failing server test

schema:
$ref: '#/definitions/AcknowledgeMovesShipmentsSuccessResponse'
'401':
$ref: '#/responses/PermissionDenied'
Copy link
Contributor

Choose a reason for hiding this comment

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

you're only handling unprocessable, error and ok. could you include some checks for these other responses if they might be needed or you could just remove them from here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I removed them here.

@brianmanley-caci brianmanley-caci requested a review from loganwc March 7, 2025 17:58
Copy link
Contributor

@danieljordan-caci danieljordan-caci left a comment

Choose a reason for hiding this comment

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

Just some little things

Comment on lines 17 to 19
// NewPortLocationFetcher returns a new port location fetcher
func NewMoveAndShipmentAcknowledgementUpdater() services.MoveAndShipmentAcknowledgementUpdater {
return &moveAndShipmentAcknowledgementUpdater{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Got a copy pasta thang here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in here.

Comment on lines 2 to 40
AcknowledgeShipment:
type: object
properties:
id:
type: string
format: uuid
example: 1f2270c7-7166-40ae-981e-b200ebdf3054
primeAcknowledgedAt:
type: string
format: date-time
example: 2025-04-13T14:15:33Z

AcknowledgeMove:
type: object
properties:
id:
type: string
format: uuid
example: a502b4f1-b9c4-4faf-8bdd-68292501bf26
primeAcknowledgedAt:
type: string
format: date-time
example: 2025-04-13T14:15:22Z
mtoShipments:
type: array
items:
$ref: '#/definitions/AcknowledgeShipment'

AcknowledgeMoves:
type: array
items:
$ref: '#/definitions/AcknowledgeMove'

AcknowledgeMovesShipmentsSuccessResponse:
type: object
properties:
message:
type: string
example: 'Moves/Shipments acknowledgement successfully completed'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you break these up into their own .yaml files?

Copy link
Contributor

Choose a reason for hiding this comment

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

So having an AcknowledgeShipment.yaml, AcknowledgeMove.yaml, AcknowledgeMoves.yaml and AcknowledgeMovesShipmentsSuccessResponse.yaml

This keeps up with how we have things currently here in the definitions folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking was that they weren't going to be terribly reusable, so maybe keep them together. But I also don't think it would hurt to break them up for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got those separated out in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Go-Rillaz Go-Rillaz INTEGRATION Slated for Integration Testing
Development

Successfully merging this pull request may close these issues.

7 participants