From cc382faaeb025bb5f8fff9c913449cc6f2db0b13 Mon Sep 17 00:00:00 2001 From: josiahzimmerman-caci Date: Wed, 8 May 2024 17:29:58 +0000 Subject: [PATCH 01/15] debian 11 force to sha --- Dockerfile | 2 +- Dockerfile.dp3 | 2 +- Dockerfile.local | 2 +- Dockerfile.reviewapp | 2 +- Dockerfile.tasks | 2 +- Dockerfile.tasks_dp3 | 2 +- Dockerfile.tasks_local | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 78b3c34db31..a372bf2f4ea 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,7 +8,7 @@ RUN apt-get install -y ca-certificates --no-install-recommends RUN update-ca-certificates # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a COPY --from=build-env /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt COPY bin/rds-ca-rsa4096-g1.pem /bin/rds-ca-rsa4096-g1.pem diff --git a/Dockerfile.dp3 b/Dockerfile.dp3 index 3691aae35fa..175d5291583 100644 --- a/Dockerfile.dp3 +++ b/Dockerfile.dp3 @@ -1,5 +1,5 @@ # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a #AWS GovCloud RDS cert COPY bin/rds-ca-rsa4096-g1.pem /bin/rds-ca-rsa4096-g1.pem diff --git a/Dockerfile.local b/Dockerfile.local index c74e1a5b496..aafc2dca195 100644 --- a/Dockerfile.local +++ b/Dockerfile.local @@ -20,7 +20,7 @@ RUN rm -f bin/milmove && make bin/milmove ######### # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a COPY --from=builder --chown=root:root /home/circleci/project/bin/rds-ca-rsa4096-g1.pem /bin/rds-ca-rsa4096-g1.pem COPY --from=builder --chown=root:root /home/circleci/project/bin/rds-ca-2019-root.pem /bin/rds-ca-2019-root.pem diff --git a/Dockerfile.reviewapp b/Dockerfile.reviewapp index 3c3f9a99933..32247a239b4 100644 --- a/Dockerfile.reviewapp +++ b/Dockerfile.reviewapp @@ -106,7 +106,7 @@ RUN set -x \ ######### # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 as milmove +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a as milmove COPY --from=server_builder /build/bin/rds-ca-2019-root.pem /bin/rds-ca-2019-root.pem COPY --from=server_builder /build/bin/milmove /bin/milmove diff --git a/Dockerfile.tasks b/Dockerfile.tasks index 7bd005a8399..9740e0f094e 100644 --- a/Dockerfile.tasks +++ b/Dockerfile.tasks @@ -8,7 +8,7 @@ RUN apt-get install -y ca-certificates --no-install-recommends RUN update-ca-certificates # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a COPY --from=build-env /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt COPY config/tls/milmove-cert-bundle.p7b /config/tls/milmove-cert-bundle.p7b diff --git a/Dockerfile.tasks_dp3 b/Dockerfile.tasks_dp3 index 81a83bbd6bd..8c933d58a20 100644 --- a/Dockerfile.tasks_dp3 +++ b/Dockerfile.tasks_dp3 @@ -1,5 +1,5 @@ # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a # Demo Environment Certs COPY config/tls/api.demo.dp3.us.chain.der.p7b /config/tls/api.demo.dp3.us.chain.der.p7b diff --git a/Dockerfile.tasks_local b/Dockerfile.tasks_local index 39addf8ba43..2898f2d99c9 100644 --- a/Dockerfile.tasks_local +++ b/Dockerfile.tasks_local @@ -19,7 +19,7 @@ RUN rm -f bin/milmove-tasks && make bin/milmove-tasks ######### # hadolint ignore=DL3007 -FROM gcr.io/distroless/base:latest@sha256:9bc3117a99c731a41200a28774405125cb6fbda1819f4a1af88bd3bfad5dcf32 +FROM gcr.io/distroless/base-debian11@sha256:84bb9d5e7d4dc9a21460c376fe32f3adfc333ff4b32df1c7c50a30cb2e282d7a COPY --from=builder --chown=root:root /home/circleci/project/config/tls/milmove-cert-bundle.p7b /config/tls/milmove-cert-bundle.p7b COPY --from=builder --chown=root:root /home/circleci/project/bin/rds-ca-2019-root.pem /bin/rds-ca-2019-root.pem From 314ccf1456f5ba1e16de09a7a6b33b454e66ae9f Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Tue, 9 Apr 2024 16:31:35 +0000 Subject: [PATCH 02/15] initial commit, checking move status on frontend when getting move --- pkg/handlers/ghcapi/api.go | 1 + pkg/handlers/ghcapi/move.go | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/handlers/ghcapi/api.go b/pkg/handlers/ghcapi/api.go index 9508feacc24..d2becf189ef 100644 --- a/pkg/handlers/ghcapi/api.go +++ b/pkg/handlers/ghcapi/api.go @@ -86,6 +86,7 @@ func NewGhcAPIHandler(handlerConfig handlers.HandlerConfig) *ghcops.MymoveAPI { ghcAPI.MoveGetMoveHandler = GetMoveHandler{ HandlerConfig: handlerConfig, MoveFetcher: move.NewMoveFetcher(), + MoveRouter: moveRouter, } ghcAPI.MoveGetMoveHistoryHandler = GetMoveHistoryHandler{ diff --git a/pkg/handlers/ghcapi/move.go b/pkg/handlers/ghcapi/move.go index ecbcc0f89b1..0f5f082a574 100644 --- a/pkg/handlers/ghcapi/move.go +++ b/pkg/handlers/ghcapi/move.go @@ -21,6 +21,7 @@ import ( type GetMoveHandler struct { handlers.HandlerConfig services.MoveFetcher + services.MoveRouter } // Handle handles the getMove by locator request @@ -34,7 +35,6 @@ func (h GetMoveHandler) Handle(params moveop.GetMoveParams) middleware.Responder } move, err := h.FetchMove(appCtx, locator, nil) - if err != nil { appCtx.Logger().Error("Error retrieving move by locator", zap.Error(err)) switch err.(type) { @@ -45,6 +45,14 @@ func (h GetMoveHandler) Handle(params moveop.GetMoveParams) middleware.Responder } } + // this is a frontloading check to make sure that if a move has been left in "Approvals Requested" status + // after actions have been taken, it should change to "Move approved" after loading the move + _, err = h.MoveRouter.ApproveOrRequestApproval(appCtx, *move) + if err != nil { + appCtx.Logger().Error("Error evaluating move status of ApprovalOrRequestApproval", zap.Error(err)) + return moveop.NewGetMoveInternalServerError(), err + } + payload := payloads.Move(move) return moveop.NewGetMoveOK().WithPayload(payload), nil }) From 691ad84f621701073759ecac573fbab2eabbfcf4 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Tue, 9 Apr 2024 16:55:27 +0000 Subject: [PATCH 03/15] added shipment status check --- pkg/services/move/move_router.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/services/move/move_router.go b/pkg/services/move/move_router.go index a58784929cc..8c6a8e6f0ed 100644 --- a/pkg/services/move/move_router.go +++ b/pkg/services/move/move_router.go @@ -303,7 +303,8 @@ func approvable(move models.Move) bool { moveHasAcknowledgedOrdersAmendment(move.Orders) && moveHasAcknowledgedExcessWeightRisk(move) && allSITExtensionsAreReviewed(move) && - allShipmentAddressUpdatesAreReviewed(move) + allShipmentAddressUpdatesAreReviewed(move) && + allShipmentsAreApproved(move) } func statusSliceContains(statusSlice []models.MoveStatus, status models.MoveStatus) bool { @@ -359,6 +360,16 @@ func allSITExtensionsAreReviewed(move models.Move) bool { return true } +func allShipmentsAreApproved(move models.Move) bool { + for _, shipment := range move.MTOShipments { + if shipment.Status == models.MTOShipmentStatusSubmitted { + return false + } + } + + return true +} + func allShipmentAddressUpdatesAreReviewed(move models.Move) bool { for _, shipment := range move.MTOShipments { if shipment.DeliveryAddressUpdate != nil && shipment.DeliveryAddressUpdate.Status == models.ShipmentAddressUpdateStatusRequested { From e2066e4cc5064d091663b20c129ffc9850f41d10 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Tue, 9 Apr 2024 20:09:39 +0000 Subject: [PATCH 04/15] removed check on get move and added check when approving shipments --- pkg/handlers/ghcapi/api.go | 2 +- pkg/handlers/ghcapi/move.go | 9 ------ pkg/handlers/ghcapi/mto_shipment_test.go | 2 ++ .../shipment_diversion_approver.go | 28 ++++++++++++++----- .../shipment_diversion_approver_test.go | 7 +++-- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/pkg/handlers/ghcapi/api.go b/pkg/handlers/ghcapi/api.go index d2becf189ef..b8e25c2fe2b 100644 --- a/pkg/handlers/ghcapi/api.go +++ b/pkg/handlers/ghcapi/api.go @@ -86,7 +86,6 @@ func NewGhcAPIHandler(handlerConfig handlers.HandlerConfig) *ghcops.MymoveAPI { ghcAPI.MoveGetMoveHandler = GetMoveHandler{ HandlerConfig: handlerConfig, MoveFetcher: move.NewMoveFetcher(), - MoveRouter: moveRouter, } ghcAPI.MoveGetMoveHistoryHandler = GetMoveHistoryHandler{ @@ -343,6 +342,7 @@ func NewGhcAPIHandler(handlerConfig handlers.HandlerConfig) *ghcops.MymoveAPI { handlerConfig, mtoshipment.NewShipmentDiversionApprover( mtoshipment.NewShipmentRouter(), + moveRouter, ), shipmentSITStatus, } diff --git a/pkg/handlers/ghcapi/move.go b/pkg/handlers/ghcapi/move.go index 0f5f082a574..c1b2b4ce266 100644 --- a/pkg/handlers/ghcapi/move.go +++ b/pkg/handlers/ghcapi/move.go @@ -21,7 +21,6 @@ import ( type GetMoveHandler struct { handlers.HandlerConfig services.MoveFetcher - services.MoveRouter } // Handle handles the getMove by locator request @@ -45,14 +44,6 @@ func (h GetMoveHandler) Handle(params moveop.GetMoveParams) middleware.Responder } } - // this is a frontloading check to make sure that if a move has been left in "Approvals Requested" status - // after actions have been taken, it should change to "Move approved" after loading the move - _, err = h.MoveRouter.ApproveOrRequestApproval(appCtx, *move) - if err != nil { - appCtx.Logger().Error("Error evaluating move status of ApprovalOrRequestApproval", zap.Error(err)) - return moveop.NewGetMoveInternalServerError(), err - } - payload := payloads.Move(move) return moveop.NewGetMoveOK().WithPayload(payload), nil }) diff --git a/pkg/handlers/ghcapi/mto_shipment_test.go b/pkg/handlers/ghcapi/mto_shipment_test.go index b3600c7acfe..23fef3f61e9 100644 --- a/pkg/handlers/ghcapi/mto_shipment_test.go +++ b/pkg/handlers/ghcapi/mto_shipment_test.go @@ -25,6 +25,7 @@ import ( "github.com/transcom/mymove/pkg/services/fetch" "github.com/transcom/mymove/pkg/services/ghcrateengine" "github.com/transcom/mymove/pkg/services/mocks" + moverouter "github.com/transcom/mymove/pkg/services/move" moveservices "github.com/transcom/mymove/pkg/services/move" movetaskorder "github.com/transcom/mymove/pkg/services/move_task_order" mtoserviceitem "github.com/transcom/mymove/pkg/services/mto_service_item" @@ -1180,6 +1181,7 @@ func (suite *HandlerSuite) TestApproveShipmentDiversionHandler() { officeUser := factory.BuildOfficeUserWithRoles(nil, nil, []roles.RoleType{roles.RoleTypeTOO}) approver := mtoshipment.NewShipmentDiversionApprover( mtoshipment.NewShipmentRouter(), + moverouter.NewMoveRouter(), ) req := httptest.NewRequest("POST", fmt.Sprintf("/shipments/%s/approve-diversion", shipment.ID.String()), nil) diff --git a/pkg/services/mto_shipment/shipment_diversion_approver.go b/pkg/services/mto_shipment/shipment_diversion_approver.go index 2b82e74a193..e74ae6107cd 100644 --- a/pkg/services/mto_shipment/shipment_diversion_approver.go +++ b/pkg/services/mto_shipment/shipment_diversion_approver.go @@ -12,19 +12,20 @@ import ( ) type shipmentDiversionApprover struct { - router services.ShipmentRouter + router services.ShipmentRouter + moveRouter services.MoveRouter } // NewShipmentDiversionApprover creates a new struct with the service dependencies -func NewShipmentDiversionApprover(router services.ShipmentRouter) services.ShipmentDiversionApprover { +func NewShipmentDiversionApprover(router services.ShipmentRouter, moveRouter services.MoveRouter) services.ShipmentDiversionApprover { return &shipmentDiversionApprover{ - router, + router, moveRouter, } } // ApproveShipmentDiversion Approves the shipment diversion func (f *shipmentDiversionApprover) ApproveShipmentDiversion(appCtx appcontext.AppContext, shipmentID uuid.UUID, eTag string) (*models.MTOShipment, error) { - shipment, err := FindShipment(appCtx, shipmentID) + shipment, err := FindShipment(appCtx, shipmentID, "MoveTaskOrder") if err != nil { return nil, err } @@ -34,9 +35,22 @@ func (f *shipmentDiversionApprover) ApproveShipmentDiversion(appCtx appcontext.A return &models.MTOShipment{}, apperror.NewPreconditionFailedError(shipmentID, query.StaleIdentifierError{StaleIdentifier: eTag}) } - err = f.router.ApproveDiversion(appCtx, shipment) - if err != nil { - return nil, err + transactionError := appCtx.NewTransaction(func(txnAppCtx appcontext.AppContext) error { + err = f.router.ApproveDiversion(appCtx, shipment) + if err != nil { + return err + } + + move := shipment.MoveTaskOrder + if _, err = f.moveRouter.ApproveOrRequestApproval(txnAppCtx, move); err != nil { + return err + } + + return nil + }) + + if transactionError != nil { + return nil, transactionError } verrs, err := appCtx.DB().ValidateAndSave(shipment) diff --git a/pkg/services/mto_shipment/shipment_diversion_approver_test.go b/pkg/services/mto_shipment/shipment_diversion_approver_test.go index fd64be90d66..18942087c7d 100644 --- a/pkg/services/mto_shipment/shipment_diversion_approver_test.go +++ b/pkg/services/mto_shipment/shipment_diversion_approver_test.go @@ -12,11 +12,13 @@ import ( "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/services/mocks" + moverouter "github.com/transcom/mymove/pkg/services/move" ) func (suite *MTOShipmentServiceSuite) TestApproveShipmentDiversion() { router := NewShipmentRouter() - approver := NewShipmentDiversionApprover(router) + moveRouter := moverouter.NewMoveRouter() + approver := NewShipmentDiversionApprover(router, moveRouter) suite.Run("If the shipment diversion is approved successfully, it should update the shipment status in the DB", func() { shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ @@ -107,7 +109,8 @@ func (suite *MTOShipmentServiceSuite) TestApproveShipmentDiversion() { suite.Run("It calls ApproveDiversion on the ShipmentRouter", func() { shipmentRouter := &mocks.ShipmentRouter{} - approver := NewShipmentDiversionApprover(shipmentRouter) + moveRouter := moverouter.NewMoveRouter() + approver := NewShipmentDiversionApprover(shipmentRouter, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ { Model: models.MTOShipment{ From cffdc9302e618f8491a99051a354fc9b5ceb5a8b Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Tue, 9 Apr 2024 20:10:47 +0000 Subject: [PATCH 05/15] removed check on get move and added check when approving shipments --- pkg/handlers/ghcapi/mto_shipment_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/handlers/ghcapi/mto_shipment_test.go b/pkg/handlers/ghcapi/mto_shipment_test.go index 23fef3f61e9..ec1d6434e95 100644 --- a/pkg/handlers/ghcapi/mto_shipment_test.go +++ b/pkg/handlers/ghcapi/mto_shipment_test.go @@ -25,7 +25,6 @@ import ( "github.com/transcom/mymove/pkg/services/fetch" "github.com/transcom/mymove/pkg/services/ghcrateengine" "github.com/transcom/mymove/pkg/services/mocks" - moverouter "github.com/transcom/mymove/pkg/services/move" moveservices "github.com/transcom/mymove/pkg/services/move" movetaskorder "github.com/transcom/mymove/pkg/services/move_task_order" mtoserviceitem "github.com/transcom/mymove/pkg/services/mto_service_item" @@ -1181,7 +1180,7 @@ func (suite *HandlerSuite) TestApproveShipmentDiversionHandler() { officeUser := factory.BuildOfficeUserWithRoles(nil, nil, []roles.RoleType{roles.RoleTypeTOO}) approver := mtoshipment.NewShipmentDiversionApprover( mtoshipment.NewShipmentRouter(), - moverouter.NewMoveRouter(), + moveservices.NewMoveRouter(), ) req := httptest.NewRequest("POST", fmt.Sprintf("/shipments/%s/approve-diversion", shipment.ID.String()), nil) From 1e7d15cae97aad3fc2c8586a399712ca7ca9293f Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Wed, 10 Apr 2024 13:29:54 +0000 Subject: [PATCH 06/15] updating old incorrect comments --- pkg/handlers/ghcapi/mto_shipment.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/handlers/ghcapi/mto_shipment.go b/pkg/handlers/ghcapi/mto_shipment.go index ded5e64929c..fd19693d30f 100644 --- a/pkg/handlers/ghcapi/mto_shipment.go +++ b/pkg/handlers/ghcapi/mto_shipment.go @@ -695,14 +695,14 @@ func (h RejectShipmentHandler) triggerShipmentRejectionEvent(appCtx appcontext.A } } -// RequestShipmentCancellationHandler Requests a shipment diversion +// RequestShipmentCancellationHandler Requests a shipment cancellation type RequestShipmentCancellationHandler struct { handlers.HandlerConfig services.ShipmentCancellationRequester services.ShipmentSITStatus } -// Handle Requests a shipment diversion +// Handle Requests a shipment cancellation func (h RequestShipmentCancellationHandler) Handle(params shipmentops.RequestShipmentCancellationParams) middleware.Responder { return h.AuditableAppContextFromRequestWithErrors(params.HTTPRequest, func(appCtx appcontext.AppContext) (middleware.Responder, error) { From 9ac1e9e6a749322212a9d7a86579562fe16f1956 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Wed, 10 Apr 2024 15:10:21 +0000 Subject: [PATCH 07/15] adding in check for deleting shipments --- pkg/handlers/ghcapi/api.go | 2 +- pkg/handlers/internalapi/api.go | 2 +- pkg/services/mto_shipment/shipment_deleter.go | 10 +++++++++- pkg/services/mto_shipment/shipment_deleter_test.go | 14 +++++++++----- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/pkg/handlers/ghcapi/api.go b/pkg/handlers/ghcapi/api.go index b8e25c2fe2b..d66918303e5 100644 --- a/pkg/handlers/ghcapi/api.go +++ b/pkg/handlers/ghcapi/api.go @@ -317,7 +317,7 @@ func NewGhcAPIHandler(handlerConfig handlers.HandlerConfig) *ghcops.MymoveAPI { ghcAPI.ShipmentDeleteShipmentHandler = DeleteShipmentHandler{ handlerConfig, - mtoshipment.NewShipmentDeleter(moveTaskOrderUpdater), + mtoshipment.NewShipmentDeleter(moveTaskOrderUpdater, moveRouter), } ghcAPI.ShipmentApproveShipmentHandler = ApproveShipmentHandler{ diff --git a/pkg/handlers/internalapi/api.go b/pkg/handlers/internalapi/api.go index 5ff0f119deb..a0732c8fef0 100644 --- a/pkg/handlers/internalapi/api.go +++ b/pkg/handlers/internalapi/api.go @@ -213,7 +213,7 @@ func NewInternalAPI(handlerConfig handlers.HandlerConfig) *internalops.MymoveAPI internalAPI.MtoShipmentDeleteShipmentHandler = DeleteShipmentHandler{ handlerConfig, - mtoshipment.NewShipmentDeleter(moveTaskOrderUpdater), + mtoshipment.NewShipmentDeleter(moveTaskOrderUpdater, moveRouter), } internalAPI.PpmCreateMovingExpenseHandler = CreateMovingExpenseHandler{handlerConfig, movingexpense.NewMovingExpenseCreator()} diff --git a/pkg/services/mto_shipment/shipment_deleter.go b/pkg/services/mto_shipment/shipment_deleter.go index e068aa40903..9f363b27622 100644 --- a/pkg/services/mto_shipment/shipment_deleter.go +++ b/pkg/services/mto_shipment/shipment_deleter.go @@ -13,13 +13,15 @@ import ( type shipmentDeleter struct { checks []validator moveTaskOrderUpdater services.MoveTaskOrderUpdater + moveRouter services.MoveRouter } // NewShipmentDeleter creates a new struct with the service dependencies -func NewShipmentDeleter(moveTaskOrderUpdater services.MoveTaskOrderUpdater) services.ShipmentDeleter { +func NewShipmentDeleter(moveTaskOrderUpdater services.MoveTaskOrderUpdater, moveRouter services.MoveRouter) services.ShipmentDeleter { return &shipmentDeleter{ checks: []validator{checkDeleteAllowed()}, moveTaskOrderUpdater: moveTaskOrderUpdater, + moveRouter: moveRouter, } } @@ -57,10 +59,16 @@ func (f *shipmentDeleter) DeleteShipment(appCtx appcontext.AppContext, shipmentI } // Update PPMType once shipment gets created. _, err = f.moveTaskOrderUpdater.UpdatePPMType(txnAppCtx, shipment.MoveTaskOrderID) + if err != nil { + return err + } + // if the shipment had any actions for the TOO we can remove these by checking if the move status should change + _, err = f.moveRouter.ApproveOrRequestApproval(appCtx, shipment.MoveTaskOrder) if err != nil { return err } + return nil }) diff --git a/pkg/services/mto_shipment/shipment_deleter_test.go b/pkg/services/mto_shipment/shipment_deleter_test.go index 4ca68cefc76..8a2d1134f07 100644 --- a/pkg/services/mto_shipment/shipment_deleter_test.go +++ b/pkg/services/mto_shipment/shipment_deleter_test.go @@ -33,7 +33,7 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { moveRouter, ) suite.Run("Returns an error when shipment is not found", func() { - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) id := uuid.Must(uuid.NewV4()) session := suite.AppContextWithSessionForTest(&auth.Session{ ApplicationName: auth.OfficeApp, @@ -47,7 +47,8 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { }) suite.Run("Returns an error when the Move is neither in Draft nor in NeedsServiceCounseling status", func() { - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + moveRouter := moveservices.NewMoveRouter() + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), nil, nil) move := shipment.MoveTaskOrder move.Status = models.MoveStatusServiceCounselingCompleted @@ -65,7 +66,8 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { }) suite.Run("Soft deletes the shipment when it is found", func() { - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + moveRouter := moveservices.NewMoveRouter() + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), nil, nil) validStatuses := []struct { @@ -168,7 +170,8 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { }) suite.Run("Returns not found error when the shipment is already deleted", func() { - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + moveRouter := moveservices.NewMoveRouter() + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), nil, nil) session := suite.AppContextWithSessionForTest(&auth.Session{ ApplicationName: auth.OfficeApp, @@ -184,7 +187,8 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { }) suite.Run("Soft deletes the associated PPM shipment", func() { - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + moveRouter := moveservices.NewMoveRouter() + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) ppmShipment := factory.BuildPPMShipment(suite.DB(), []factory.Customization{ { Model: models.Move{ From ca9df175299707d4a77b84a37a5a084249809550 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 11 Apr 2024 12:50:28 +0000 Subject: [PATCH 08/15] updated shipment diversion approver test --- pkg/handlers/ghcapi/move.go | 1 + .../shipment_diversion_approver.go | 9 +++++-- .../shipment_diversion_approver_test.go | 25 +++++++++++-------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/handlers/ghcapi/move.go b/pkg/handlers/ghcapi/move.go index c1b2b4ce266..ecbcc0f89b1 100644 --- a/pkg/handlers/ghcapi/move.go +++ b/pkg/handlers/ghcapi/move.go @@ -34,6 +34,7 @@ func (h GetMoveHandler) Handle(params moveop.GetMoveParams) middleware.Responder } move, err := h.FetchMove(appCtx, locator, nil) + if err != nil { appCtx.Logger().Error("Error retrieving move by locator", zap.Error(err)) switch err.(type) { diff --git a/pkg/services/mto_shipment/shipment_diversion_approver.go b/pkg/services/mto_shipment/shipment_diversion_approver.go index e74ae6107cd..b1195607537 100644 --- a/pkg/services/mto_shipment/shipment_diversion_approver.go +++ b/pkg/services/mto_shipment/shipment_diversion_approver.go @@ -42,8 +42,10 @@ func (f *shipmentDiversionApprover) ApproveShipmentDiversion(appCtx appcontext.A } move := shipment.MoveTaskOrder - if _, err = f.moveRouter.ApproveOrRequestApproval(txnAppCtx, move); err != nil { - return err + if move.Status == models.MoveStatusAPPROVALSREQUESTED || move.Status == models.MoveStatusAPPROVED { + if _, err = f.moveRouter.ApproveOrRequestApproval(appCtx, move); err != nil { + return err + } } return nil @@ -59,6 +61,9 @@ func (f *shipmentDiversionApprover) ApproveShipmentDiversion(appCtx appcontext.A return nil, invalidInputError } + if err != nil { + return nil, err + } return shipment, err } diff --git a/pkg/services/mto_shipment/shipment_diversion_approver_test.go b/pkg/services/mto_shipment/shipment_diversion_approver_test.go index 18942087c7d..73195c53c4e 100644 --- a/pkg/services/mto_shipment/shipment_diversion_approver_test.go +++ b/pkg/services/mto_shipment/shipment_diversion_approver_test.go @@ -4,14 +4,12 @@ import ( "time" "github.com/gofrs/uuid" - "github.com/stretchr/testify/mock" "github.com/transcom/mymove/pkg/apperror" "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/etag" "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/models" - "github.com/transcom/mymove/pkg/services/mocks" moverouter "github.com/transcom/mymove/pkg/services/move" ) @@ -108,10 +106,14 @@ func (suite *MTOShipmentServiceSuite) TestApproveShipmentDiversion() { }) suite.Run("It calls ApproveDiversion on the ShipmentRouter", func() { - shipmentRouter := &mocks.ShipmentRouter{} - moveRouter := moverouter.NewMoveRouter() + shipmentRouter := NewShipmentRouter() approver := NewShipmentDiversionApprover(shipmentRouter, moveRouter) + move := factory.BuildAvailableToPrimeMove(suite.DB(), nil, nil) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ + { + Model: move, + LinkOnly: true, + }, { Model: models.MTOShipment{ Status: models.MTOShipmentStatusSubmitted, @@ -124,15 +126,16 @@ func (suite *MTOShipmentServiceSuite) TestApproveShipmentDiversion() { ApplicationName: auth.OfficeApp, OfficeUserID: uuid.Must(uuid.NewV4()), }) - createdShipment := models.MTOShipment{} - err := suite.DB().Find(&createdShipment, shipment.ID) - suite.FatalNoError(err) - - shipmentRouter.On("ApproveDiversion", mock.AnythingOfType("*appcontext.appContext"), &createdShipment).Return(nil) - _, err = approver.ApproveShipmentDiversion(session, shipment.ID, eTag) + _, err := approver.ApproveShipmentDiversion(session, shipment.ID, eTag) + suite.NoError(err) + createdShipment := models.MTOShipment{} + err = suite.DB().Find(&createdShipment, shipment.ID) suite.NoError(err) - shipmentRouter.AssertNumberOfCalls(suite.T(), "ApproveDiversion", 1) + + suite.FatalNoError(err) + // if the created shipment has a status of approved, then ApproveDiversion was successful + suite.Equal(models.MTOShipmentStatusApproved, createdShipment.Status) }) } From a6cf94f3e04f2d78e23b7e7d52532138d27d4787 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 11 Apr 2024 14:08:19 +0000 Subject: [PATCH 09/15] updated one more test --- pkg/services/mto_shipment/shipment_deleter_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/services/mto_shipment/shipment_deleter_test.go b/pkg/services/mto_shipment/shipment_deleter_test.go index 8a2d1134f07..d85ba587e62 100644 --- a/pkg/services/mto_shipment/shipment_deleter_test.go +++ b/pkg/services/mto_shipment/shipment_deleter_test.go @@ -108,8 +108,9 @@ func (suite *MTOShipmentServiceSuite) TestShipmentDeleter() { }) suite.Run("Soft deletes the shipment when it is found and check if shipment_seq_num changed", func() { + moveRouter := moveservices.NewMoveRouter() move := factory.BuildMove(suite.DB(), nil, nil) - shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater) + shipmentDeleter := NewShipmentDeleter(moveTaskOrderUpdater, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ { Model: move, From f556624dfaa1b1dae0b49daa562f8ed7d6768691 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 11 Apr 2024 15:44:56 +0000 Subject: [PATCH 10/15] added move status check when deleting --- pkg/services/mto_shipment/shipment_deleter.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/services/mto_shipment/shipment_deleter.go b/pkg/services/mto_shipment/shipment_deleter.go index 9f363b27622..f10a834c303 100644 --- a/pkg/services/mto_shipment/shipment_deleter.go +++ b/pkg/services/mto_shipment/shipment_deleter.go @@ -7,6 +7,7 @@ import ( "github.com/transcom/mymove/pkg/appcontext" "github.com/transcom/mymove/pkg/apperror" "github.com/transcom/mymove/pkg/db/utilities" + "github.com/transcom/mymove/pkg/models" "github.com/transcom/mymove/pkg/services" ) @@ -64,9 +65,12 @@ func (f *shipmentDeleter) DeleteShipment(appCtx appcontext.AppContext, shipmentI } // if the shipment had any actions for the TOO we can remove these by checking if the move status should change - _, err = f.moveRouter.ApproveOrRequestApproval(appCtx, shipment.MoveTaskOrder) - if err != nil { - return err + move := shipment.MoveTaskOrder + if move.Status == models.MoveStatusAPPROVALSREQUESTED || move.Status == models.MoveStatusAPPROVED { + _, err = f.moveRouter.ApproveOrRequestApproval(txnAppCtx, shipment.MoveTaskOrder) + if err != nil { + return err + } } return nil From 6c5fa94a3c7fbb6e274cf7a19e710247713f83e8 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 25 Apr 2024 18:55:00 +0000 Subject: [PATCH 11/15] initial commit, all should be well --- pkg/handlers/ghcapi/api.go | 1 + pkg/handlers/ghcapi/mto_shipment_test.go | 1 + .../mto_shipment/mto_shipment_updater.go | 17 ++++--- .../shipment_cancellation_requester.go | 47 ++++++++++++++----- .../shipment_cancellation_requester_test.go | 21 +++++---- 5 files changed, 59 insertions(+), 28 deletions(-) diff --git a/pkg/handlers/ghcapi/api.go b/pkg/handlers/ghcapi/api.go index d66918303e5..3f8e039c0e7 100644 --- a/pkg/handlers/ghcapi/api.go +++ b/pkg/handlers/ghcapi/api.go @@ -358,6 +358,7 @@ func NewGhcAPIHandler(handlerConfig handlers.HandlerConfig) *ghcops.MymoveAPI { handlerConfig, mtoshipment.NewShipmentCancellationRequester( mtoshipment.NewShipmentRouter(), + moveRouter, ), shipmentSITStatus, } diff --git a/pkg/handlers/ghcapi/mto_shipment_test.go b/pkg/handlers/ghcapi/mto_shipment_test.go index ec1d6434e95..a1a55d997ae 100644 --- a/pkg/handlers/ghcapi/mto_shipment_test.go +++ b/pkg/handlers/ghcapi/mto_shipment_test.go @@ -1803,6 +1803,7 @@ func (suite *HandlerSuite) TestRequestShipmentCancellationHandler() { officeUser := factory.BuildOfficeUserWithRoles(nil, nil, []roles.RoleType{roles.RoleTypeTOO}) canceler := mtoshipment.NewShipmentCancellationRequester( mtoshipment.NewShipmentRouter(), + moveservices.NewMoveRouter(), ) req := httptest.NewRequest("POST", fmt.Sprintf("/shipments/%s/request-cancellation", shipment.ID.String()), nil) diff --git a/pkg/services/mto_shipment/mto_shipment_updater.go b/pkg/services/mto_shipment/mto_shipment_updater.go index d3911c1a93a..17bfd5bcdd7 100644 --- a/pkg/services/mto_shipment/mto_shipment_updater.go +++ b/pkg/services/mto_shipment/mto_shipment_updater.go @@ -612,12 +612,7 @@ func (f *mtoShipmentUpdater) updateShipmentRecord(appCtx appcontext.AppContext, // excess weight risk depending on the weight allowance and other shipment estimated weights if newShipment.PrimeEstimatedWeight != nil { if dbShipment.PrimeEstimatedWeight == nil || *newShipment.PrimeEstimatedWeight != *dbShipment.PrimeEstimatedWeight { - /* - TODO: If the move was already in risk of excess we need to set the status back to APPROVED if - the new shipment estimated weight drops it out of the range. Can potentially reuse - moveRouter.ApproveAmmendedOrders if we also add checks for excess weight there and orders - acknowledgement - */ + // checking if the total of shipment weight & new prime estimated weight is 90% or more of allowed weight move, verrs, err := f.moveWeights.CheckExcessWeight(txnAppCtx, dbShipment.MoveTaskOrderID, *newShipment) if verrs != nil && verrs.HasAny() { return errors.New(verrs.Error()) @@ -627,9 +622,13 @@ func (f *mtoShipmentUpdater) updateShipmentRecord(appCtx appcontext.AppContext, } existingMoveStatus := move.Status - err = f.moveRouter.SendToOfficeUser(txnAppCtx, move) - if err != nil { - return err + // if the move is in excess weight risk and the TOO has not acknowledge that, need to change move status to "Approvals Requested" + // this will trigger the TOO to acknowledge the excess right, which populates ExcessWeightAcknowledgedAt + if move.ExcessWeightQualifiedAt != nil && move.ExcessWeightAcknowledgedAt == nil { + err = f.moveRouter.SendToOfficeUser(txnAppCtx, move) + if err != nil { + return err + } } if existingMoveStatus != move.Status { diff --git a/pkg/services/mto_shipment/shipment_cancellation_requester.go b/pkg/services/mto_shipment/shipment_cancellation_requester.go index 8e298503f13..ec32903d1c7 100644 --- a/pkg/services/mto_shipment/shipment_cancellation_requester.go +++ b/pkg/services/mto_shipment/shipment_cancellation_requester.go @@ -12,19 +12,21 @@ import ( ) type shipmentCancellationRequester struct { - router services.ShipmentRouter + router services.ShipmentRouter + moveRouter services.MoveRouter } // NewShipmentCancellationRequester creates a new struct with the service dependencies -func NewShipmentCancellationRequester(router services.ShipmentRouter) services.ShipmentCancellationRequester { +func NewShipmentCancellationRequester(router services.ShipmentRouter, moveRouter services.MoveRouter) services.ShipmentCancellationRequester { return &shipmentCancellationRequester{ router, + moveRouter, } } // RequestShipmentCancellation Requests the shipment diversion func (f *shipmentCancellationRequester) RequestShipmentCancellation(appCtx appcontext.AppContext, shipmentID uuid.UUID, eTag string) (*models.MTOShipment, error) { - shipment, err := FindShipment(appCtx, shipmentID) + shipment, err := FindShipment(appCtx, shipmentID, "MoveTaskOrder") if err != nil { return nil, err } @@ -34,16 +36,39 @@ func (f *shipmentCancellationRequester) RequestShipmentCancellation(appCtx appco return &models.MTOShipment{}, apperror.NewPreconditionFailedError(shipmentID, query.StaleIdentifierError{StaleIdentifier: eTag}) } - err = f.router.RequestCancellation(appCtx, shipment) - if err != nil { - return nil, err - } + transactionError := appCtx.NewTransaction(func(txnAppCtx appcontext.AppContext) error { + + // this changes the shipment status to "CANCELLATION_REQUESTED" but only on an approved shipment + err = f.router.RequestCancellation(appCtx, shipment) + if err != nil { + return err + } + + // save the shipment to the db + verrs, err := appCtx.DB().ValidateAndSave(shipment) + if verrs != nil && verrs.HasAny() { + invalidInputError := apperror.NewInvalidInputError(shipment.ID, nil, verrs, "Could not validate shipment while requesting the shipment cancellation.") + + return invalidInputError + } + if err != nil { + return err + } + + // checking if the move still requires action by the TOO + move := shipment.MoveTaskOrder + if move.Status == models.MoveStatusAPPROVALSREQUESTED || move.Status == models.MoveStatusAPPROVED { + _, err = f.moveRouter.ApproveOrRequestApproval(txnAppCtx, shipment.MoveTaskOrder) + if err != nil { + return err + } + } - verrs, err := appCtx.DB().ValidateAndSave(shipment) - if verrs != nil && verrs.HasAny() { - invalidInputError := apperror.NewInvalidInputError(shipment.ID, nil, verrs, "Could not validate shipment while requesting the shipment cancellation.") + return nil + }) - return nil, invalidInputError + if transactionError != nil { + return nil, transactionError } return shipment, err diff --git a/pkg/services/mto_shipment/shipment_cancellation_requester_test.go b/pkg/services/mto_shipment/shipment_cancellation_requester_test.go index c5270557e8b..f2cb1c6b587 100644 --- a/pkg/services/mto_shipment/shipment_cancellation_requester_test.go +++ b/pkg/services/mto_shipment/shipment_cancellation_requester_test.go @@ -4,19 +4,19 @@ import ( "time" "github.com/gofrs/uuid" - "github.com/stretchr/testify/mock" "github.com/transcom/mymove/pkg/apperror" "github.com/transcom/mymove/pkg/auth" "github.com/transcom/mymove/pkg/etag" "github.com/transcom/mymove/pkg/factory" "github.com/transcom/mymove/pkg/models" - "github.com/transcom/mymove/pkg/services/mocks" + moveservices "github.com/transcom/mymove/pkg/services/move" ) func (suite *MTOShipmentServiceSuite) TestRequestShipmentCancellation() { router := NewShipmentRouter() - requester := NewShipmentCancellationRequester(router) + moveRouter := moveservices.NewMoveRouter() + requester := NewShipmentCancellationRequester(router, moveRouter) suite.Run("If the shipment diversion is requested successfully, it should update the shipment status in the DB", func() { shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ @@ -102,8 +102,9 @@ func (suite *MTOShipmentServiceSuite) TestRequestShipmentCancellation() { }) suite.Run("It calls RequestCancellation on the ShipmentRouter", func() { - shipmentRouter := &mocks.ShipmentRouter{} - requester := NewShipmentCancellationRequester(shipmentRouter) + shipmentRouter := NewShipmentRouter() + moveRouter := moveservices.NewMoveRouter() + requester := NewShipmentCancellationRequester(shipmentRouter, moveRouter) shipment := factory.BuildMTOShipmentMinimal(suite.DB(), []factory.Customization{ { Model: models.MTOShipment{ @@ -120,11 +121,15 @@ func (suite *MTOShipmentServiceSuite) TestRequestShipmentCancellation() { err := suite.DB().Find(&createdShipment, shipment.ID) suite.FatalNoError(err) - shipmentRouter.On("RequestCancellation", mock.AnythingOfType("*appcontext.appContext"), &createdShipment).Return(nil) - _, err = requester.RequestShipmentCancellation(session, shipment.ID, eTag) suite.NoError(err) - shipmentRouter.AssertNumberOfCalls(suite.T(), "RequestCancellation", 1) + dbShipment := models.MTOShipment{} + err = suite.DB().Find(&dbShipment, shipment.ID) + suite.NoError(err) + + suite.FatalNoError(err) + // if the created shipment has a status of cancellation requested, then RequestCancellation was successful + suite.Equal(models.MTOShipmentStatusCancellationRequested, dbShipment.Status) }) } From 3d8180157e0a3d0fc015425f3626121b5613518c Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 25 Apr 2024 19:25:57 +0000 Subject: [PATCH 12/15] fixing err shadowing --- pkg/services/mto_shipment/shipment_cancellation_requester.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/mto_shipment/shipment_cancellation_requester.go b/pkg/services/mto_shipment/shipment_cancellation_requester.go index ec32903d1c7..d0540093333 100644 --- a/pkg/services/mto_shipment/shipment_cancellation_requester.go +++ b/pkg/services/mto_shipment/shipment_cancellation_requester.go @@ -45,13 +45,13 @@ func (f *shipmentCancellationRequester) RequestShipmentCancellation(appCtx appco } // save the shipment to the db - verrs, err := appCtx.DB().ValidateAndSave(shipment) + verrs, saveErr := appCtx.DB().ValidateAndSave(shipment) if verrs != nil && verrs.HasAny() { invalidInputError := apperror.NewInvalidInputError(shipment.ID, nil, verrs, "Could not validate shipment while requesting the shipment cancellation.") return invalidInputError } - if err != nil { + if saveErr != nil { return err } From e20917fe3c4722a7c8c459951ded570f4a48d546 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Thu, 25 Apr 2024 19:42:07 +0000 Subject: [PATCH 13/15] updated comment --- pkg/services/mto_shipment/shipment_cancellation_requester.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/services/mto_shipment/shipment_cancellation_requester.go b/pkg/services/mto_shipment/shipment_cancellation_requester.go index d0540093333..cb3b864bc32 100644 --- a/pkg/services/mto_shipment/shipment_cancellation_requester.go +++ b/pkg/services/mto_shipment/shipment_cancellation_requester.go @@ -56,6 +56,7 @@ func (f *shipmentCancellationRequester) RequestShipmentCancellation(appCtx appco } // checking if the move still requires action by the TOO + // if no action is needed, then the move status will stay in APPROVED or APPROVALS_REQUESTED move := shipment.MoveTaskOrder if move.Status == models.MoveStatusAPPROVALSREQUESTED || move.Status == models.MoveStatusAPPROVED { _, err = f.moveRouter.ApproveOrRequestApproval(txnAppCtx, shipment.MoveTaskOrder) From 95c8f89b2f82e04b6ed11e90e12a60866bdcc7a9 Mon Sep 17 00:00:00 2001 From: Daniel Jordan Date: Fri, 26 Apr 2024 14:57:36 +0000 Subject: [PATCH 14/15] updated comment --- pkg/services/mto_shipment/mto_shipment_updater.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/mto_shipment/mto_shipment_updater.go b/pkg/services/mto_shipment/mto_shipment_updater.go index 17bfd5bcdd7..94dbb92a8dd 100644 --- a/pkg/services/mto_shipment/mto_shipment_updater.go +++ b/pkg/services/mto_shipment/mto_shipment_updater.go @@ -623,7 +623,7 @@ func (f *mtoShipmentUpdater) updateShipmentRecord(appCtx appcontext.AppContext, existingMoveStatus := move.Status // if the move is in excess weight risk and the TOO has not acknowledge that, need to change move status to "Approvals Requested" - // this will trigger the TOO to acknowledge the excess right, which populates ExcessWeightAcknowledgedAt + // this will trigger the TOO to acknowledged the excess right, which populates ExcessWeightAcknowledgedAt if move.ExcessWeightQualifiedAt != nil && move.ExcessWeightAcknowledgedAt == nil { err = f.moveRouter.SendToOfficeUser(txnAppCtx, move) if err != nil { From 825f61fdaee621a808dc27d3e69ac7d5b64e85cc Mon Sep 17 00:00:00 2001 From: AaronW Date: Thu, 9 May 2024 22:09:45 -0500 Subject: [PATCH 15/15] separated jobs from the method that creates service items --- .../mto_service_item_creator.go | 458 ++++++++++-------- 1 file changed, 250 insertions(+), 208 deletions(-) diff --git a/pkg/services/mto_service_item/mto_service_item_creator.go b/pkg/services/mto_service_item/mto_service_item_creator.go index 00d3ca40c2d..dbc5ac5dd34 100644 --- a/pkg/services/mto_service_item/mto_service_item_creator.go +++ b/pkg/services/mto_service_item/mto_service_item_creator.go @@ -61,161 +61,172 @@ func (o *mtoServiceItemCreator) calculateSITDeliveryMiles(appCtx appcontext.AppC // CreateMTOServiceItem creates a MTO Service Item func (o *mtoServiceItemCreator) CreateMTOServiceItem(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem) (*models.MTOServiceItems, *validate.Errors, error) { - var verrs *validate.Errors - var err error var requestedServiceItems models.MTOServiceItems // used in case additional service items need to be auto-created var createdServiceItems models.MTOServiceItems - + var mtoShipment models.MTOShipment var move models.Move - moveID := serviceItem.MoveTaskOrderID - queryFilters := []services.QueryFilter{ - query.NewQueryFilter("id", "=", moveID), - } - // check if Move exists - err = o.builder.FetchOne(appCtx, &move, queryFilters) - if err != nil { - switch err { - case sql.ErrNoRows: - return nil, nil, apperror.NewNotFoundError(moveID, "in Moves") - default: - return nil, nil, apperror.NewQueryError("Move", err, "") - } + + if err := o.checkMoveStatus(appCtx, serviceItem, &move); err != nil { + return nil, nil, err } - // Service items can only be created if a Move's status is either Approved - // or Approvals Requested, so check and fail early. - if move.Status != models.MoveStatusAPPROVED && move.Status != models.MoveStatusAPPROVALSREQUESTED { - return nil, nil, apperror.NewConflictError( - move.ID, - fmt.Sprintf("Cannot create service items before a move has been approved. The current status for the move with ID %s is %s", move.ID, move.Status), - ) + if err := o.tryGetReServiceInfo(appCtx, serviceItem); err != nil { + return nil, nil, err } - // find the re service code id - var reService models.ReService - reServiceCode := serviceItem.ReService.Code - queryFilters = []services.QueryFilter{ - query.NewQueryFilter("code", "=", reServiceCode), + if verrs, err := o.tryCreateSupportingServiceItems(appCtx, serviceItem, &createdServiceItems); err != nil { + return nil, nil, err + } else if verrs != nil { + return nil, verrs, nil } - err = o.builder.FetchOne(appCtx, &reService, queryFilters) - if err != nil { - switch err { - case sql.ErrNoRows: - return nil, nil, apperror.NewNotFoundError(uuid.Nil, fmt.Sprintf("for service item with code: %s", reServiceCode)) - default: - return nil, nil, apperror.NewQueryError("ReService", err, "") - } + + // TODO: Once customer onboarding is built, we can revisit to figure out which service items goes under each type of shipment + + if err := o.checkShipment(appCtx, serviceItem, &mtoShipment); err != nil { + return nil, nil, err } - // set re service fields for service item - serviceItem.ReServiceID = reService.ID - serviceItem.ReService.Name = reService.Name - // We can have two service items that come in from a MTO approval that do not have an MTOShipmentID - // they are MTO level service items. This should capture that and create them accordingly, they are thankfully - // also rather basic. - if serviceItem.MTOShipmentID == nil { - if serviceItem.ReService.Code == models.ReServiceCodeMS || serviceItem.ReService.Code == models.ReServiceCodeCS { - serviceItem.Status = "APPROVED" - } - verrs, err = o.builder.CreateOne(appCtx, serviceItem) - if verrs != nil { - return nil, verrs, nil - } - if err != nil { - return nil, nil, err - } + o.harmonizeDestAddress(serviceItem, &mtoShipment) - createdServiceItems = append(createdServiceItems, *serviceItem) + var errSITValidation error + if serviceItem, errSITValidation = o.validateSIT(appCtx, serviceItem); errSITValidation != nil { + return nil, nil, errSITValidation + } - return &createdServiceItems, nil, nil + if err := o.checkCustomerContacts(appCtx, serviceItem); err != nil { + return nil, nil, err } - // By the time the serviceItem model object gets here to the creator it should have a status attached to it. - // If for some reason that isn't the case we will set it - if serviceItem.Status == "" { - serviceItem.Status = models.MTOServiceItemStatusSubmitted + // These service items should be created as part of a group and not individually + if o.isDeliveryItem(serviceItem.ReService.Code) { + verrs := validate.NewErrors() + verrs.Add("reServiceCode", fmt.Sprintf("%s cannot be created", serviceItem.ReService.Code)) + return nil, nil, apperror.NewInvalidInputError(serviceItem.ID, nil, verrs, + fmt.Sprintf("A service item with reServiceCode %s cannot be manually created.", serviceItem.ReService.Code)) } - // TODO: Once customer onboarding is built, we can revisit to figure out which service items goes under each type of shipment - // check if shipment exists linked by MoveTaskOrderID - var mtoShipment models.MTOShipment - mtoShipmentID := *serviceItem.MTOShipmentID - queryFilters = []services.QueryFilter{ - query.NewQueryFilter("id", "=", mtoShipmentID), - query.NewQueryFilter("move_id", "=", moveID), + var updatedShipmentPickupAddress *bool + var err error + if updatedShipmentPickupAddress, err = o.checkShipmentAddress(appCtx, &requestedServiceItems, &mtoShipment, serviceItem); err != nil { + return nil, nil, err } - err = o.builder.FetchOne(appCtx, &mtoShipment, queryFilters) - if err != nil { - switch err { - case sql.ErrNoRows: - return nil, nil, apperror.NewNotFoundError(mtoShipmentID, fmt.Sprintf("for mtoShipment with moveID: %s", moveID.String())) - default: - return nil, nil, apperror.NewQueryError("MTOShipment", err, "") + + requestedServiceItems = append(requestedServiceItems, *serviceItem) + + // create new items in a transaction in case of failure + if transactionErr := appCtx.NewTransaction(func(txnAppCtx appcontext.AppContext) error { + if err := o.checkRequestedServiceItems(txnAppCtx, &requestedServiceItems, &createdServiceItems); err != nil { + return err } - } - // checking to see if the service item being created is a destination SIT - // if so, we want the destination address to be the same as the shipment's - // which will later populate the additional dest SIT service items as well - if serviceItem.ReService.Code == models.ReServiceCodeDDFSIT && mtoShipment.DestinationAddressID != nil { - serviceItem.SITDestinationFinalAddress = mtoShipment.DestinationAddress - serviceItem.SITDestinationFinalAddressID = mtoShipment.DestinationAddressID - } + // If updates were made to shipment, save update in the database + if *updatedShipmentPickupAddress { + if verrs, err := o.builder.UpdateOne(txnAppCtx, mtoShipment.PickupAddress, nil); verrs != nil || err != nil { + return fmt.Errorf("failed to update mtoShipment.PickupAddress: %#v %e", verrs, err) + } + } - if serviceItem.ReService.Code == models.ReServiceCodeDOASIT { - // DOASIT must be associated with shipment that has DOFSIT - serviceItem, err = o.validateSITStandaloneServiceItem(appCtx, serviceItem, models.ReServiceCodeDOFSIT) - if err != nil { - return nil, nil, err + if _, err := o.moveRouter.ApproveOrRequestApproval(txnAppCtx, move); err != nil { + return err } + + return nil + }); transactionErr != nil { + return nil, nil, transactionErr } - if serviceItem.ReService.Code == models.ReServiceCodeDDASIT { - // DDASIT must be associated with shipment that has DDFSIT - serviceItem, err = o.validateSITStandaloneServiceItem(appCtx, serviceItem, models.ReServiceCodeDDFSIT) - if err != nil { - return nil, nil, err + return &createdServiceItems, nil, nil +} + +func (o *mtoServiceItemCreator) checkRequestedServiceItems(txnAppCtx appcontext.AppContext, requestedServiceItems *models.MTOServiceItems, createdServiceItems *models.MTOServiceItems) error { + for serviceItemIndex := range *requestedServiceItems { + requestedServiceItem := (*requestedServiceItems)[serviceItemIndex] + + if requestedServiceItem.SITOriginHHGActualAddress != nil { + address := requestedServiceItem.SITOriginHHGActualAddress + if address.ID == uuid.Nil { + if verrs, err := o.builder.CreateOne(txnAppCtx, address); verrs != nil || err != nil { + return fmt.Errorf("failed to save SITOriginHHGActualAddress: %#v %e", verrs, err) + } + } + requestedServiceItem.SITOriginHHGActualAddressID = &address.ID } - } - for index := range serviceItem.CustomerContacts { - createCustContacts := &serviceItem.CustomerContacts[index] - err = validateTimeMilitaryField(appCtx, createCustContacts.TimeMilitary) - if err != nil { - return nil, nil, apperror.NewInvalidInputError(serviceItem.ID, err, nil, err.Error()) + if requestedServiceItem.SITOriginHHGOriginalAddress != nil { + address := requestedServiceItem.SITOriginHHGOriginalAddress + if address.ID == uuid.Nil { + if verrs, err := o.builder.CreateOne(txnAppCtx, address); verrs != nil || err != nil { + return fmt.Errorf("failed to save SITOriginHHGOriginalAddress: %#v %e", verrs, err) + } + } + requestedServiceItem.SITOriginHHGOriginalAddressID = &address.ID } - } - if serviceItem.ReService.Code == models.ReServiceCodeDDDSIT || serviceItem.ReService.Code == models.ReServiceCodeDOPSIT || - serviceItem.ReService.Code == models.ReServiceCodeDDSFSC || serviceItem.ReService.Code == models.ReServiceCodeDOSFSC { - verrs = validate.NewErrors() - verrs.Add("reServiceCode", fmt.Sprintf("%s cannot be created", serviceItem.ReService.Code)) - return nil, nil, apperror.NewInvalidInputError(serviceItem.ID, nil, verrs, - fmt.Sprintf("A service item with reServiceCode %s cannot be manually created.", serviceItem.ReService.Code)) + // create SITDestinationFinalAddress address if ID (UUID) is Nil + if requestedServiceItem.SITDestinationFinalAddress != nil { + address := requestedServiceItem.SITDestinationFinalAddress + if address.ID == uuid.Nil { + if verrs, err := o.builder.CreateOne(txnAppCtx, address); verrs != nil || err != nil { + return fmt.Errorf("failed to save SITOriginHHGOriginalAddress: %#v %e", verrs, err) + } + } + requestedServiceItem.SITDestinationFinalAddressID = &address.ID + } + + // create customer contacts if any + for index := range requestedServiceItem.CustomerContacts { + createCustContact := &requestedServiceItem.CustomerContacts[index] + if createCustContact.ID == uuid.Nil { + if verrs, err := o.builder.CreateOne(txnAppCtx, createCustContact); verrs != nil || err != nil { + return fmt.Errorf("%#v %e", verrs, err) + } + } + } + + if verrs, err := o.builder.CreateOne(txnAppCtx, requestedServiceItem); verrs != nil || err != nil { + return fmt.Errorf("%#v %e", verrs, err) + } + + *createdServiceItems = append(*createdServiceItems, requestedServiceItem) + + // create dimensions if any + for index := range requestedServiceItem.Dimensions { + createDimension := &requestedServiceItem.Dimensions[index] + createDimension.MTOServiceItemID = requestedServiceItem.ID + if verrs, err := o.builder.CreateOne(txnAppCtx, createDimension); verrs != nil && verrs.HasAny() { + return apperror.NewInvalidInputError(uuid.Nil, nil, verrs, "Failed to create dimensions") + } else if err != nil { + return fmt.Errorf("%e", err) + } + } } + return nil +} - updateShipmentPickupAddress := false +func (o *mtoServiceItemCreator) checkShipmentAddress(appCtx appcontext.AppContext, requestedServiceItems *models.MTOServiceItems, mtoShipment *models.MTOShipment, serviceItem *models.MTOServiceItem) (*bool, error) { + var extraServiceItems *models.MTOServiceItems + result := false if serviceItem.ReService.Code == models.ReServiceCodeDDFSIT || serviceItem.ReService.Code == models.ReServiceCodeDOFSIT { - extraServiceItems, errSIT := o.validateFirstDaySITServiceItem(appCtx, serviceItem) - if errSIT != nil { - return nil, nil, errSIT + var err error + if extraServiceItems, err = o.validateFirstDaySITServiceItem(appCtx, serviceItem); err != nil { + return &result, err } // update HHG origin address for ReServiceCodeDOFSIT service item if serviceItem.ReService.Code == models.ReServiceCodeDOFSIT { // When creating a DOFSIT, the prime must provide an HHG actual address for the move/shift in origin (pickup address) if serviceItem.SITOriginHHGActualAddress == nil { - verrs = validate.NewErrors() + verrs := validate.NewErrors() verrs.Add("reServiceCode", fmt.Sprintf("%s cannot be created", serviceItem.ReService.Code)) - return nil, nil, apperror.NewInvalidInputError(serviceItem.ID, nil, verrs, + return &result, apperror.NewInvalidInputError(serviceItem.ID, nil, verrs, fmt.Sprintf("A service item with reServiceCode %s must have the sitHHGActualOrigin field set.", serviceItem.ReService.Code)) } - county, errCounty := models.FindCountyByZipCode(appCtx.DB(), serviceItem.SITOriginHHGActualAddress.PostalCode) - if errCounty != nil { - return nil, nil, errCounty + if county, err := models.FindCountyByZipCode(appCtx.DB(), serviceItem.SITOriginHHGActualAddress.PostalCode); err != nil { + return &result, err + } else { + serviceItem.SITOriginHHGActualAddress.County = county } - serviceItem.SITOriginHHGActualAddress.County = county // update the SIT service item to track/save the HHG original pickup address (that came from the // MTO shipment @@ -227,9 +238,6 @@ func (o *mtoServiceItemCreator) CreateMTOServiceItem(appCtx appcontext.AppContex mtoShipment.PickupAddress = serviceItem.SITOriginHHGActualAddress.Copy() mtoShipment.PickupAddress.ID = *mtoShipment.PickupAddressID // Keep to same ID to be updated with new values - // changes were made to the shipment, needs to be saved to the database - updateShipmentPickupAddress = true - // Find the DOPSIT service item and update the SIT related address fields. These fields // will be used for pricing when a payment request is created for DOPSIT for itemIndex := range *extraServiceItems { @@ -258,7 +266,7 @@ func (o *mtoServiceItemCreator) CreateMTOServiceItem(appCtx appcontext.AppContex } } - milesCalculated, errCalcSITDelivery := o.calculateSITDeliveryMiles(appCtx, serviceItem, mtoShipment) + milesCalculated, errCalcSITDelivery := o.calculateSITDeliveryMiles(appCtx, serviceItem, *mtoShipment) // only calculate SITDeliveryMiles for DOPSIT and DOSFSC origin service items if serviceItem.ReService.Code == models.ReServiceCodeDOFSIT && milesCalculated != 0 { @@ -286,113 +294,147 @@ func (o *mtoServiceItemCreator) CreateMTOServiceItem(appCtx appcontext.AppContex } } - requestedServiceItems = append(requestedServiceItems, *extraServiceItems...) + *requestedServiceItems = append(*requestedServiceItems, *extraServiceItems...) } - requestedServiceItems = append(requestedServiceItems, *serviceItem) - // create new items in a transaction in case of failure - transactionErr := appCtx.NewTransaction(func(txnAppCtx appcontext.AppContext) error { + // If this is reached, then changes were made to the shipment which need to be saved to the DB + result = true + return &result, nil +} - if err != nil { - txnAppCtx.Logger().Error(fmt.Sprintf("error starting txn: %v", err)) - return err +func (o *mtoServiceItemCreator) isDeliveryItem(code models.ReServiceCode) bool { + return code == models.ReServiceCodeDDDSIT || code == models.ReServiceCodeDOPSIT || + code == models.ReServiceCodeDDSFSC || code == models.ReServiceCodeDOSFSC +} + +func (o *mtoServiceItemCreator) validateSIT(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem) (*models.MTOServiceItem, error) { + if serviceItem.ReService.Code == models.ReServiceCodeDOASIT { + // DOASIT must be associated with shipment that has DOFSIT + if serviceItemResult, err := o.validateSITStandaloneServiceItem(appCtx, serviceItem, models.ReServiceCodeDOFSIT); err != nil { + return nil, err + } else { + return serviceItemResult, nil } - for serviceItemIndex := range requestedServiceItems { - requestedServiceItem := &requestedServiceItems[serviceItemIndex] - - // create address if ID (UUID) is Nil - if requestedServiceItem.SITOriginHHGActualAddress != nil { - address := requestedServiceItem.SITOriginHHGActualAddress - if address.ID == uuid.Nil { - verrs, err = o.builder.CreateOne(txnAppCtx, address) - if verrs != nil || err != nil { - return fmt.Errorf("failed to save SITOriginHHGActualAddress: %#v %e", verrs, err) - } - } - requestedServiceItem.SITOriginHHGActualAddressID = &address.ID - } + } - // create address if ID (UUID) is Nil - if requestedServiceItem.SITOriginHHGOriginalAddress != nil { - address := requestedServiceItem.SITOriginHHGOriginalAddress - if address.ID == uuid.Nil { - verrs, err = o.builder.CreateOne(txnAppCtx, address) - if verrs != nil || err != nil { - return fmt.Errorf("failed to save SITOriginHHGOriginalAddress: %#v %e", verrs, err) - } - } - requestedServiceItem.SITOriginHHGOriginalAddressID = &address.ID - } + if serviceItem.ReService.Code == models.ReServiceCodeDDASIT { + // DDASIT must be associated with shipment that has DDFSIT + if serviceItemResult, err := o.validateSITStandaloneServiceItem(appCtx, serviceItem, models.ReServiceCodeDDFSIT); err != nil { + return nil, err + } else { + return serviceItemResult, nil + } + } + return serviceItem, nil +} - // create SITDestinationFinalAddress address if ID (UUID) is Nil - if requestedServiceItem.SITDestinationFinalAddress != nil { - address := requestedServiceItem.SITDestinationFinalAddress - if address.ID == uuid.Nil { - verrs, err = o.builder.CreateOne(txnAppCtx, address) - if verrs != nil || err != nil { - return fmt.Errorf("failed to save SITOriginHHGOriginalAddress: %#v %e", verrs, err) - } - } - requestedServiceItem.SITDestinationFinalAddressID = &address.ID - } +func (o *mtoServiceItemCreator) checkCustomerContacts(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem) error { + for index := range serviceItem.CustomerContacts { + createCustContacts := &serviceItem.CustomerContacts[index] + if err := validateTimeMilitaryField(appCtx, createCustContacts.TimeMilitary); err != nil { + return apperror.NewInvalidInputError(serviceItem.ID, err, nil, err.Error()) + } + } + return nil +} - // create customer contacts if any - for index := range requestedServiceItem.CustomerContacts { - createCustContact := &requestedServiceItem.CustomerContacts[index] - if createCustContact.ID == uuid.Nil { - verrs, err = o.builder.CreateOne(txnAppCtx, createCustContact) - if verrs != nil || err != nil { - return fmt.Errorf("%#v %e", verrs, err) - } - } - } +func (o *mtoServiceItemCreator) harmonizeDestAddress(serviceItem *models.MTOServiceItem, mtoShipment *models.MTOShipment) { + if serviceItem.ReService.Code == models.ReServiceCodeDDFSIT && mtoShipment.DestinationAddressID != nil { + serviceItem.SITDestinationFinalAddress = mtoShipment.DestinationAddress + serviceItem.SITDestinationFinalAddressID = mtoShipment.DestinationAddressID + } +} - verrs, err = o.builder.CreateOne(txnAppCtx, requestedServiceItem) - if verrs != nil || err != nil { - return fmt.Errorf("%#v %e", verrs, err) - } +func (o *mtoServiceItemCreator) checkShipment(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem, mtoShipment *models.MTOShipment) error { + // check if shipment exists linked by MoveTaskOrderID + mtoShipmentID := *serviceItem.MTOShipmentID + queryFilters := []services.QueryFilter{ + query.NewQueryFilter("id", "=", mtoShipmentID), + query.NewQueryFilter("move_id", "=", serviceItem.MoveTaskOrderID), + } + err := o.builder.FetchOne(appCtx, &mtoShipment, queryFilters) + if err != nil { + switch err { + case sql.ErrNoRows: + return apperror.NewNotFoundError(mtoShipmentID, fmt.Sprintf("for mtoShipment with moveID: %s", serviceItem.MoveTaskOrder.ID.String())) + default: + return apperror.NewQueryError("MTOShipment", err, "") + } + } + return nil +} - createdServiceItems = append(createdServiceItems, *requestedServiceItem) +func (o *mtoServiceItemCreator) tryCreateSupportingServiceItems(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem, createdServiceItems *models.MTOServiceItems) (*validate.Errors, error) { + if serviceItem.MTOShipmentID == nil { + if serviceItem.ReService.Code == models.ReServiceCodeMS || serviceItem.ReService.Code == models.ReServiceCodeCS { + serviceItem.Status = "APPROVED" + } + verrs, err := o.builder.CreateOne(appCtx, serviceItem) + if verrs != nil { + return verrs, nil + } + if err != nil { + return nil, err + } - // create dimensions if any - for index := range requestedServiceItem.Dimensions { - createDimension := &requestedServiceItem.Dimensions[index] - createDimension.MTOServiceItemID = requestedServiceItem.ID - verrs, err = o.builder.CreateOne(txnAppCtx, createDimension) - if verrs != nil && verrs.HasAny() { - return apperror.NewInvalidInputError(uuid.Nil, nil, verrs, "Failed to create dimensions") - } - if err != nil { - return fmt.Errorf("%e", err) - } - } + *createdServiceItems = append(*createdServiceItems, *serviceItem) + return nil, nil + } - } + // By the time the serviceItem model object gets here to the creator it should have a status attached to it. + // If for some reason that isn't the case we will set it + if serviceItem.Status == "" { + serviceItem.Status = models.MTOServiceItemStatusSubmitted + } + return nil, nil +} - // If updates were made to shipment, save update in the database - if updateShipmentPickupAddress { - verrs, err = o.builder.UpdateOne(txnAppCtx, mtoShipment.PickupAddress, nil) - if verrs != nil || err != nil { - return fmt.Errorf("failed to update mtoShipment.PickupAddress: %#v %e", verrs, err) - } +func (o *mtoServiceItemCreator) tryGetReServiceInfo(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem) error { + var reService models.ReService + reServiceCode := serviceItem.ReService.Code + queryFilters := []services.QueryFilter{ + query.NewQueryFilter("code", "=", reServiceCode), + } + err := o.builder.FetchOne(appCtx, &reService, queryFilters) + if err != nil { + switch err { + case sql.ErrNoRows: + return apperror.NewNotFoundError(uuid.Nil, fmt.Sprintf("for service item with code: %s", reServiceCode)) + default: + return apperror.NewQueryError("ReService", err, "") } + } + // set re service fields for service item + serviceItem.ReServiceID = reService.ID + serviceItem.ReService.Name = reService.Name + return nil +} - if _, err = o.moveRouter.ApproveOrRequestApproval(txnAppCtx, move); err != nil { - return err +func (o *mtoServiceItemCreator) checkMoveStatus(appCtx appcontext.AppContext, serviceItem *models.MTOServiceItem, move *models.Move) error { + moveID := serviceItem.MoveTaskOrderID + queryFilters := []services.QueryFilter{ + query.NewQueryFilter("id", "=", moveID), + } + // check if Move exists + err := o.builder.FetchOne(appCtx, &move, queryFilters) + if err != nil { + switch err { + case sql.ErrNoRows: + return apperror.NewNotFoundError(moveID, "in Moves") + default: + return apperror.NewQueryError("Move", err, "") } - - return nil - }) - - if transactionErr != nil { - return nil, nil, transactionErr - } else if verrs != nil && verrs.HasAny() { - return nil, verrs, nil - } else if err != nil { - return nil, verrs, apperror.NewQueryError("unknown", err, "") } - return &createdServiceItems, nil, nil + // Service items can only be created if a Move's status is either Approved + // or Approvals Requested, so check and fail early. + if move.Status != models.MoveStatusAPPROVED && move.Status != models.MoveStatusAPPROVALSREQUESTED { + return apperror.NewConflictError( + move.ID, + fmt.Sprintf("Cannot create service items before a move has been approved. The current status for the move with ID %s is %s", move.ID, move.Status), + ) + } + return nil } // checkDuplicateServiceCodes checks if the move or shipment has a duplicate service item with the same code as the one