Merge pull request #11779 from transcom/B-18192-moving-expense-api-de…

…lete

B-18192 move query logic from handler to service object

pkg/handlers/internalapi/moving_expense.go

@@ -9,11 +9,9 @@ import (
 
 	"github.com/transcom/mymove/pkg/appcontext"
 	"github.com/transcom/mymove/pkg/apperror"
-	"github.com/transcom/mymove/pkg/db/utilities"
 	movingexpenseops "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"
 )
 
@@ -186,39 +184,10 @@ func (h DeleteMovingExpenseHandler) Handle(params movingexpenseops.DeleteMovingE
 
 			// 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",
-					"MovingExpenses",
-				).
-				Find(&ppmShipment, ppmID)
-			if err != nil {
-				if err == sql.ErrNoRows {
-					return movingexpenseops.NewDeleteMovingExpenseNotFound(), err
-				}
-				return movingexpenseops.NewDeleteMovingExpenseInternalServerError(), err
-			}
-			if ppmShipment.Shipment.MoveTaskOrder.Orders.ServiceMemberID != appCtx.Session().ServiceMemberID {
-				wrongServiceMemberIDErr := apperror.NewSessionError("Attempted delete by wrong service member")
-				appCtx.Logger().Error("internalapi.DeleteMovingExpenseHandler", zap.Error(wrongServiceMemberIDErr))
-				return movingexpenseops.NewDeleteMovingExpenseForbidden(), wrongServiceMemberIDErr
-			}
 
 			movingExpenseID := uuid.FromStringOrNil(params.MovingExpenseID.String())
-			found := false
-			for _, lineItem := range ppmShipment.MovingExpenses {
-				if lineItem.ID == movingExpenseID {
-					found = true
-					break
-				}
-			}
-			if !found {
-				mismatchedPPMShipmentAndMovingExpenseIDErr := apperror.NewSessionError("Moving expense does not exist on ppm shipment")
-				appCtx.Logger().Error("internalapi.DeleteMovingExpenseHandler", zap.Error(mismatchedPPMShipmentAndMovingExpenseIDErr))
-				return movingexpenseops.NewDeleteMovingExpenseNotFound(), mismatchedPPMShipmentAndMovingExpenseIDErr
-			}
-			err = h.movingExpenseDeleter.DeleteMovingExpense(appCtx, movingExpenseID)
+
+			err := h.movingExpenseDeleter.DeleteMovingExpense(appCtx, ppmID, movingExpenseID)
 			if err != nil {
 				appCtx.Logger().Error("internalapi.DeleteMovingExpenseHandler", zap.Error(err))
 

pkg/handlers/internalapi/moving_expense_test.go

@@ -425,6 +425,7 @@ func (suite *HandlerSuite) TestDeleteMovingExpenseHandler() {
 		mockDeleter.On("DeleteMovingExpense",
 			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/moving_expense.go

@@ -25,5 +25,5 @@ type MovingExpenseUpdater interface {
 //
 //go:generate mockery --name MovingExpenseDeleter
 type MovingExpenseDeleter interface {
-	DeleteMovingExpense(appCtx appcontext.AppContext, movingExpenseID uuid.UUID) error
+	DeleteMovingExpense(appCtx appcontext.AppContext, ppmID uuid.UUID, movingExpenseID uuid.UUID) error
 }

pkg/services/moving_expense/moving_expense_deleter.go

@@ -1,10 +1,15 @@
 package movingexpense
 
 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"
 )
 
@@ -15,7 +20,40 @@ func NewMovingExpenseDeleter() services.MovingExpenseDeleter {
 	return &movingExpenseDeleter{}
 }
 
-func (d *movingExpenseDeleter) DeleteMovingExpense(appCtx appcontext.AppContext, movingExpenseID uuid.UUID) error {
+func (d *movingExpenseDeleter) DeleteMovingExpense(appCtx appcontext.AppContext, ppmID uuid.UUID, movingExpenseID uuid.UUID) error {
+	var ppmShipment models.PPMShipment
+	err := appCtx.DB().Scope(utilities.ExcludeDeletedScope()).
+		EagerPreload(
+			"Shipment.MoveTaskOrder.Orders",
+			"MovingExpenses",
+		).
+		Find(&ppmShipment, ppmID)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return apperror.NewNotFoundError(movingExpenseID, "while looking for MovingExpense")
+		}
+		return apperror.NewQueryError("MovingExpense fetch original", err, "")
+	}
+
+	if ppmShipment.Shipment.MoveTaskOrder.Orders.ServiceMemberID != appCtx.Session().ServiceMemberID {
+		wrongServiceMemberIDErr := apperror.NewForbiddenError("Attempted delete by wrong service member")
+		appCtx.Logger().Error("internalapi.DeleteMovingExpenseHandler", zap.Error(wrongServiceMemberIDErr))
+		return wrongServiceMemberIDErr
+	}
+
+	found := false
+	for _, lineItem := range ppmShipment.MovingExpenses {
+		if lineItem.ID == movingExpenseID {
+			found = true
+			break
+		}
+	}
+	if !found {
+		mismatchedPPMShipmentAndMovingExpenseIDErr := apperror.NewNotFoundError(movingExpenseID, "Moving expense does not exist on ppm shipment")
+		appCtx.Logger().Error("internalapi.DeleteMovingExpenseHandler", zap.Error(mismatchedPPMShipmentAndMovingExpenseIDErr))
+		return mismatchedPPMShipmentAndMovingExpenseIDErr
+	}
+
 	movingExpense, err := FetchMovingExpenseByID(appCtx, movingExpenseID)
 	if err != nil {
 		return err

pkg/services/moving_expense/moving_expense_deleter_test.go

@@ -65,9 +65,10 @@ func (suite *MovingExpenseSuite) TestDeleteMovingExpense() {
 	}
 	suite.Run("Returns an error if the original doesn't exist", func() {
 		notFoundMovingExpenseID := uuid.Must(uuid.NewV4())
+		ppmID := uuid.Must(uuid.NewV4())
 		deleter := NewMovingExpenseDeleter()
 
-		err := deleter.DeleteMovingExpense(suite.AppContextWithSessionForTest(&auth.Session{}), notFoundMovingExpenseID)
+		err := deleter.DeleteMovingExpense(suite.AppContextWithSessionForTest(&auth.Session{}), ppmID, notFoundMovingExpenseID)
 
 		if suite.Error(err) {
 			suite.IsType(apperror.NotFoundError{}, err)
@@ -81,11 +82,12 @@ func (suite *MovingExpenseSuite) TestDeleteMovingExpense() {
 
 	suite.Run("Successfully deletes as a customer's moving expense", func() {
 		originalMovingExpense := setupForTest(nil, true)
-
 		deleter := NewMovingExpenseDeleter()
 
 		suite.Nil(originalMovingExpense.DeletedAt)
-		err := deleter.DeleteMovingExpense(suite.AppContextWithSessionForTest(&auth.Session{}), originalMovingExpense.ID)
+		err := deleter.DeleteMovingExpense(suite.AppContextWithSessionForTest(&auth.Session{
+			ServiceMemberID: originalMovingExpense.Document.ServiceMemberID,
+		}), originalMovingExpense.PPMShipmentID, originalMovingExpense.ID)
 		suite.NoError(err)
 
 		var movingExpenseInDB models.MovingExpense