Merge pull request #11953 from transcom/B-18193

B-18193 API Fix Delete Weight Ticket

pkg/handlers/internalapi/weight_ticket.go

@@ -1,19 +1,15 @@
 package internalapi
 
 import (
-	"database/sql"
-
 	"github.com/go-openapi/runtime/middleware"
 	"github.com/gofrs/uuid"
 	"go.uber.org/zap"
 
 	"github.com/transcom/mymove/pkg/appcontext"
 	"github.com/transcom/mymove/pkg/apperror"
-	"github.com/transcom/mymove/pkg/db/utilities"
 	weightticketops "github.com/transcom/mymove/pkg/gen/internalapi/internaloperations/ppm"
 	"github.com/transcom/mymove/pkg/handlers"
 	"github.com/transcom/mymove/pkg/handlers/internalapi/internal/payloads"
-	"github.com/transcom/mymove/pkg/models"
 	"github.com/transcom/mymove/pkg/services"
 )
 
@@ -168,42 +164,11 @@ func (h DeleteWeightTicketHandler) Handle(params weightticketops.DeleteWeightTic
 				return weightticketops.NewDeleteWeightTicketForbidden(), noServiceMemberIDErr
 			}
 
-			// Make sure the service member is not modifying another service member's PPM
 			ppmID := uuid.FromStringOrNil(params.PpmShipmentID.String())
-			var ppmShipment models.PPMShipment
-			err := appCtx.DB().Scope(utilities.ExcludeDeletedScope()).
-				EagerPreload(
-					"Shipment.MoveTaskOrder.Orders",
-					"WeightTickets",
-				).
-				Find(&ppmShipment, ppmID)
-			if err != nil {
-				if err == sql.ErrNoRows {
-					return weightticketops.NewDeleteWeightTicketNotFound(), err
-				}
-				return weightticketops.NewDeleteWeightTicketInternalServerError(), err
-			}
-			if ppmShipment.Shipment.MoveTaskOrder.Orders.ServiceMemberID != appCtx.Session().ServiceMemberID {
-				wrongServiceMemberIDErr := apperror.NewSessionError("Attempted delete by wrong service member")
-				appCtx.Logger().Error("internalapi.DeleteWeightTicketHandler", zap.Error(wrongServiceMemberIDErr))
-				return weightticketops.NewDeleteWeightTicketForbidden(), wrongServiceMemberIDErr
-			}
 
 			weightTicketID := uuid.FromStringOrNil(params.WeightTicketID.String())
-			found := false
-			for _, lineItem := range ppmShipment.WeightTickets {
-				if lineItem.ID == weightTicketID {
-					found = true
-					break
-				}
-			}
-			if !found {
-				mismatchedPPMShipmentAndWeightTicketIDErr := apperror.NewSessionError("Weight ticket does not exist on ppm shipment")
-				appCtx.Logger().Error("internalapi.DeleteWeightTicketHandler", zap.Error(mismatchedPPMShipmentAndWeightTicketIDErr))
-				return weightticketops.NewDeleteWeightTicketNotFound(), mismatchedPPMShipmentAndWeightTicketIDErr
-			}
 
-			err = h.weightTicketDeleter.DeleteWeightTicket(appCtx, weightTicketID)
+			err := h.weightTicketDeleter.DeleteWeightTicket(appCtx, ppmID, weightTicketID)
 			if err != nil {
 				appCtx.Logger().Error("internalapi.DeleteWeightTicketHandler", zap.Error(err))
 

pkg/handlers/internalapi/weight_ticket_test.go

@@ -423,6 +423,7 @@ func (suite *HandlerSuite) TestDeleteWeightTicketHandler() {
 		mockDeleter.On("DeleteWeightTicket",
 			mock.AnythingOfType("*appcontext.appContext"),
 			mock.AnythingOfType("uuid.UUID"),
+			mock.AnythingOfType("uuid.UUID"),
 		).Return(err)
 
 		// Use createS3HandlerConfig for the HandlerConfig because we are required to upload a doc

pkg/services/weight_ticket.go

@@ -32,5 +32,5 @@ type WeightTicketUpdater interface {
 //
 //go:generate mockery --name WeightTicketDeleter
 type WeightTicketDeleter interface {
-	DeleteWeightTicket(appCtx appcontext.AppContext, weightTicketID uuid.UUID) error
+	DeleteWeightTicket(appCtx appcontext.AppContext, ppmID uuid.UUID, weightTicketID uuid.UUID) error
 }

pkg/services/weight_ticket/weight_ticket_deleter.go

@@ -1,10 +1,15 @@
 package weightticket
 
 import (
+	"database/sql"
+
 	"github.com/gofrs/uuid"
+	"go.uber.org/zap"
 
 	"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"
 	"github.com/transcom/mymove/pkg/services/ppmshipment"
 )
@@ -21,7 +26,40 @@ func NewWeightTicketDeleter(fetcher services.WeightTicketFetcher, estimator serv
 	}
 }
 
-func (d *weightTicketDeleter) DeleteWeightTicket(appCtx appcontext.AppContext, weightTicketID uuid.UUID) error {
+func (d *weightTicketDeleter) DeleteWeightTicket(appCtx appcontext.AppContext, ppmID uuid.UUID, weightTicketID uuid.UUID) error {
+	var ppmShipment models.PPMShipment
+	err := appCtx.DB().Scope(utilities.ExcludeDeletedScope()).
+		EagerPreload(
+			"Shipment.MoveTaskOrder.Orders",
+			"WeightTickets",
+		).
+		Find(&ppmShipment, ppmID)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return apperror.NewNotFoundError(weightTicketID, "while looking for WeightTicket")
+		}
+		return apperror.NewQueryError("WeightTicket fetch original", err, "")
+	}
+
+	if ppmShipment.Shipment.MoveTaskOrder.Orders.ServiceMemberID != appCtx.Session().ServiceMemberID && !appCtx.Session().IsOfficeUser() {
+		wrongServiceMemberIDErr := apperror.NewForbiddenError("Attempted delete by wrong service member")
+		appCtx.Logger().Error("internalapi.DeleteWeightTicketHandler", zap.Error(wrongServiceMemberIDErr))
+		return wrongServiceMemberIDErr
+	}
+
+	found := false
+	for _, lineItem := range ppmShipment.WeightTickets {
+		if lineItem.ID == weightTicketID {
+			found = true
+			break
+		}
+	}
+	if !found {
+		mismatchedPPMShipmentAndWeightTicketIDErr := apperror.NewNotFoundError(weightTicketID, "Weight ticket does not exist on ppm shipment")
+		appCtx.Logger().Error("internalapi.DeleteWeightTicketHandler", zap.Error(mismatchedPPMShipmentAndWeightTicketIDErr))
+		return mismatchedPPMShipmentAndWeightTicketIDErr
+	}
+
 	weightTicket, err := d.GetWeightTicket(appCtx, weightTicketID)
 	if err != nil {
 		return err

pkg/services/weight_ticket/weight_ticket_deleter_test.go

@@ -75,11 +75,12 @@ func (suite *WeightTicketSuite) TestDeleteWeightTicket() {
 	}
 	suite.Run("Returns an error if the original doesn't exist", func() {
 		notFoundWeightTicketID := uuid.Must(uuid.NewV4())
+		ppmID := uuid.Must(uuid.NewV4())
 		fetcher := NewWeightTicketFetcher()
 		estimator := mocks.PPMEstimator{}
 		deleter := NewWeightTicketDeleter(fetcher, &estimator)
 
-		err := deleter.DeleteWeightTicket(suite.AppContextForTest(), notFoundWeightTicketID)
+		err := deleter.DeleteWeightTicket(suite.AppContextForTest(), ppmID, notFoundWeightTicketID)
 
 		if suite.Error(err) {
 			suite.IsType(apperror.NotFoundError{}, err)
@@ -98,14 +99,16 @@ func (suite *WeightTicketSuite) TestDeleteWeightTicket() {
 			ServiceMemberID: originalWeightTicket.EmptyDocument.ServiceMemberID,
 		})
 
+		ppmID := originalWeightTicket.PPMShipmentID
+
 		fetcher := NewWeightTicketFetcher()
 		estimator := mocks.PPMEstimator{}
 		mockIncentive := unit.Cents(10000)
 		estimator.On("FinalIncentiveWithDefaultChecks", appCtx, mock.AnythingOfType("models.PPMShipment"), mock.AnythingOfType("*models.PPMShipment")).Return(&mockIncentive, nil)
 		deleter := NewWeightTicketDeleter(fetcher, &estimator)
 
 		suite.Nil(originalWeightTicket.DeletedAt)
-		err := deleter.DeleteWeightTicket(appCtx, originalWeightTicket.ID)
+		err := deleter.DeleteWeightTicket(appCtx, ppmID, originalWeightTicket.ID)
 		suite.NoError(err)
 
 		var weightTicketInDB models.WeightTicket
@@ -145,6 +148,7 @@ func (suite *WeightTicketSuite) TestDeleteWeightTicket() {
 			ServiceMemberID: originalWeightTicket.EmptyDocument.ServiceMemberID,
 		})
 
+		ppmID := originalWeightTicket.PPMShipmentID
 		fetcher := NewWeightTicketFetcher()
 		estimator := mocks.PPMEstimator{}
 		mockIncentive := unit.Cents(10000)
@@ -153,7 +157,7 @@ func (suite *WeightTicketSuite) TestDeleteWeightTicket() {
 			mock.AnythingOfType("models.PPMShipment"),
 			mock.AnythingOfType("*models.PPMShipment")).Return(&mockIncentive, nil).Once()
 		deleter := NewWeightTicketDeleter(fetcher, &estimator)
-		err := deleter.DeleteWeightTicket(appCtx, originalWeightTicket.ID)
+		err := deleter.DeleteWeightTicket(appCtx, ppmID, originalWeightTicket.ID)
 		suite.NoError(err)
 
 		estimator.AssertCalled(suite.T(), "FinalIncentiveWithDefaultChecks",