Merge pull request #2556 from transcom/ren-167443089-update-deletion-…

…to-soft-delete

Update uploads/documents deletion to use soft delete

cypress/integration/office/documentViewer.js

@@ -196,7 +196,9 @@ describe('The document viewer', function() {
 
       cy.get('select[name="moveDocument.move_document_type"]').select('Expense');
       cy.get('select[name="moveDocument.moving_expense_type"]').select('Contracted expense');
-      cy.get('input[name="moveDocument.requested_amount_cents"]').type('4,999.92');
+      cy.get('input[name="moveDocument.requested_amount_cents"]')
+        .clear()
+        .type('4,999.92');
       cy.get('select[name="moveDocument.payment_method"]').select('GTCC');
       cy.get('select[name="moveDocument.status"]').select('OK');
 

migrations/20190730225048_add_deleted_at_to_document_tables.up.sql

@@ -0,0 +1,5 @@
+ALTER TABLE documents ADD COLUMN deleted_at timestamp with time zone;
+ALTER TABLE move_documents ADD COLUMN deleted_at timestamp with time zone;
+ALTER TABLE uploads ADD COLUMN deleted_at timestamp with time zone;
+ALTER TABLE weight_ticket_set_documents ADD COLUMN deleted_at timestamp with time zone;
+ALTER TABLE moving_expense_documents ADD COLUMN deleted_at timestamp with time zone;

migrations/20190829152347_create-deleted-at-indices-for-document-tables.up.sql

@@ -0,0 +1,5 @@
+CREATE INDEX documents_deleted_at_idx ON documents USING btree(deleted_at);
+CREATE INDEX move_documents_deleted_at_idx ON move_documents USING btree(deleted_at);
+CREATE INDEX uploads_deleted_at_idx ON uploads USING btree(deleted_at);
+CREATE INDEX weight_ticket_set_documents_deleted_at_idx ON weight_ticket_set_documents USING btree(deleted_at);
+CREATE INDEX moving_expense_documents_deleted_at_idx ON moving_expense_documents USING btree(deleted_at);

migrations_manifest.txt

@@ -336,6 +336,7 @@
 20190724193344_add_more_system_admin_users.sql
 20190726151515_grant_all_ecs_user.up.sql
 20190730194820_pp3_2019_tspp_data.sql
+20190730225048_add_deleted_at_to_document_tables.up.sql
 20190731163048_remove_is_superuser.up.fizz
 20190805161006_disable_truss_user.sql
 20190807134704_admin_user_fname_lname_required.up.fizz
@@ -345,3 +346,4 @@
 20190814223800_add_john_gedeon.sql
 20190815172746_disable_mikena.up.sql
 20190821192638_add_office_user.up.sql
+20190829152347_create-deleted-at-indices-for-document-tables.up.sql

pkg/db/utilities/utilities.go

@@ -0,0 +1,121 @@
+package utilities
+
+import (
+	"errors"
+	"reflect"
+	"time"
+
+	"github.com/gobuffalo/pop"
+	"github.com/gobuffalo/validate"
+
+	"github.com/gofrs/uuid"
+)
+
+const deletedAt = "DeletedAt"
+const modelsPkgPath = "github.com/transcom/mymove/pkg/models"
+
+// SoftDestroy soft deletes a record and all foreign key associations from the database
+func SoftDestroy(c *pop.Connection, model interface{}) error {
+	verrs := validate.NewErrors()
+	var err error
+
+	if !IsModel(model) {
+		return errors.New("can only soft delete type model")
+	}
+
+	modelValue := reflect.ValueOf(model).Elem()
+	deletedAtField := modelValue.FieldByName(deletedAt)
+
+	if deletedAtField.IsValid() {
+		if deletedAtField.CanSet() {
+			now := time.Now()
+			reflectTime := reflect.ValueOf(&now)
+			deletedAtField.Set(reflectTime)
+			verrs, err = c.ValidateAndSave(model)
+
+			if err != nil || verrs.HasAny() {
+				return errors.New("error updating model")
+			}
+		} else {
+			return errors.New("can not soft delete this model")
+		}
+	} else {
+		return errors.New("this model does not have deleted_at field")
+	}
+
+	associations := GetForeignKeyAssociations(c, model)
+	if len(associations) > 0 {
+		for _, association := range associations {
+			err = SoftDestroy(c, association)
+			if err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+// IsModel verifies if the given interface is a model
+func IsModel(model interface{}) bool {
+	pkgPath := reflect.TypeOf(model).Elem().PkgPath()
+	return pkgPath == modelsPkgPath
+}
+
+// GetForeignKeyAssociations fetches all the foreign key associations the model has
+func GetForeignKeyAssociations(c *pop.Connection, model interface{}) []interface{} {
+	var foreignKeyAssociations []interface{}
+	c.Load(model)
+
+	modelValue := reflect.ValueOf(model).Elem()
+	modelType := modelValue.Type()
+
+	for pos := 0; pos < modelValue.NumField(); pos++ {
+		fieldValue := modelValue.Field(pos)
+
+		if fieldValue.CanInterface() {
+			association := fieldValue.Interface()
+
+			if association != nil {
+				hasOneTag := modelType.Field(pos).Tag.Get("has_one")
+				hasManyTag := modelType.Field(pos).Tag.Get("has_many")
+
+				if hasOneTag != "" && GetHasOneForeignKeyAssociation(association) != nil {
+					foreignKeyAssociations = append(foreignKeyAssociations, association)
+				}
+
+				if hasManyTag != "" {
+					foreignKeyAssociations = append(foreignKeyAssociations, GetHasManyForeignKeyAssociations(association)...)
+				}
+			}
+		}
+	}
+	return foreignKeyAssociations
+}
+
+// GetHasOneForeignKeyAssociation fetches the "has_one" foreign key association if not an empty model
+func GetHasOneForeignKeyAssociation(model interface{}) interface{} {
+	modelValue := reflect.ValueOf(model).Elem()
+	idField := modelValue.FieldByName("ID")
+
+	if idField.CanInterface() && idField.Interface() != uuid.Nil {
+		return model
+	}
+	return nil
+}
+
+// GetHasManyForeignKeyAssociations fetches the "has_many" foreing key association if not an empty model
+func GetHasManyForeignKeyAssociations(model interface{}) []interface{} {
+	var hasManyForeignKeyAssociations []interface{}
+	associations := reflect.ValueOf(model)
+
+	for pos := 0; pos < associations.Len(); pos++ {
+		association := associations.Index(pos)
+		idField := association.FieldByName("ID")
+
+		if idField.CanInterface() && idField.Interface() != uuid.Nil {
+			associationPtr := association.Addr().Interface()
+			hasManyForeignKeyAssociations = append(hasManyForeignKeyAssociations, associationPtr)
+		}
+	}
+	return hasManyForeignKeyAssociations
+}

pkg/db/utilities/utilities_test.go

@@ -0,0 +1,159 @@
+package utilities_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+
+	"github.com/transcom/mymove/pkg/db/utilities"
+	"github.com/transcom/mymove/pkg/models"
+	"github.com/transcom/mymove/pkg/services/mocks"
+	"github.com/transcom/mymove/pkg/testdatagen"
+	"github.com/transcom/mymove/pkg/testingsuite"
+	"github.com/transcom/mymove/pkg/unit"
+)
+
+type UtilitiesSuite struct {
+	testingsuite.PopTestSuite
+}
+
+func (suite *UtilitiesSuite) SetupTest() {
+	suite.DB().TruncateAll()
+}
+
+func TestUtilitiesSuite(t *testing.T) {
+	hs := &UtilitiesSuite{
+		PopTestSuite: testingsuite.NewPopTestSuite(testingsuite.CurrentPackage()),
+	}
+	suite.Run(t, hs)
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_NotModel() {
+	accessCodeFetcher := &mocks.AccessCodeFetcher{}
+
+	err := utilities.SoftDestroy(suite.DB(), &accessCodeFetcher)
+
+	suite.Equal("can only soft delete type model", err.Error())
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_ModelWithoutDeletedAtWithoutAssociations() {
+	//model without deleted_at with no associations
+	user := testdatagen.MakeDefaultUser(suite.DB())
+
+	err := utilities.SoftDestroy(suite.DB(), &user)
+
+	suite.Equal("this model does not have deleted_at field", err.Error())
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_ModelWithDeletedAtWithoutAssociations() {
+	//model with deleted_at with no associations
+	expenseDocumentModel := testdatagen.MakeMovingExpenseDocument(suite.DB(), testdatagen.Assertions{
+		MovingExpenseDocument: models.MovingExpenseDocument{
+			MovingExpenseType:    models.MovingExpenseTypeCONTRACTEDEXPENSE,
+			PaymentMethod:        "GTCC",
+			RequestedAmountCents: unit.Cents(10000),
+		},
+	})
+
+	suite.MustSave(&expenseDocumentModel)
+	suite.Nil(expenseDocumentModel.DeletedAt)
+
+	err := utilities.SoftDestroy(suite.DB(), &expenseDocumentModel)
+	suite.NoError(err)
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_ModelWithoutDeletedAtWithAssociations() {
+	// model without deleted_at with associations
+	serviceMember := testdatagen.MakeDefaultServiceMember(suite.DB())
+	suite.MustSave(&serviceMember)
+
+	err := utilities.SoftDestroy(suite.DB(), &serviceMember)
+	suite.Equal("this model does not have deleted_at field", err.Error())
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_ModelWithDeletedAtWithHasOneAssociations() {
+	// model with deleted_at with "has one" associations
+	ppm := testdatagen.MakePPM(suite.DB(), testdatagen.Assertions{
+		PersonallyProcuredMove: models.PersonallyProcuredMove{
+			Status: models.PPMStatusPAYMENTREQUESTED,
+		},
+	})
+	move := ppm.Move
+	moveDoc := testdatagen.MakeMoveDocument(suite.DB(),
+		testdatagen.Assertions{
+			MoveDocument: models.MoveDocument{
+				MoveID:                   move.ID,
+				Move:                     move,
+				PersonallyProcuredMoveID: &ppm.ID,
+				MoveDocumentType:         models.MoveDocumentTypeWEIGHTTICKETSET,
+				Status:                   models.MoveDocumentStatusOK,
+			},
+		})
+	suite.MustSave(&moveDoc)
+	suite.Nil(moveDoc.DeletedAt)
+
+	emptyWeight := unit.Pound(1000)
+	fullWeight := unit.Pound(2500)
+	weightTicketSetDocument := models.WeightTicketSetDocument{
+		MoveDocumentID:           moveDoc.ID,
+		MoveDocument:             moveDoc,
+		EmptyWeight:              &emptyWeight,
+		EmptyWeightTicketMissing: false,
+		FullWeight:               &fullWeight,
+		FullWeightTicketMissing:  false,
+		VehicleNickname:          "My Car",
+		VehicleOptions:           "CAR",
+		WeightTicketDate:         &testdatagen.NextValidMoveDate,
+		TrailerOwnershipMissing:  false,
+	}
+	suite.MustSave(&weightTicketSetDocument)
+	suite.Nil(weightTicketSetDocument.DeletedAt)
+
+	err := utilities.SoftDestroy(suite.DB(), &moveDoc)
+
+	suite.NoError(err)
+	suite.NotNil(moveDoc.DeletedAt)
+	suite.NotNil(moveDoc.WeightTicketSetDocument.DeletedAt)
+}
+
+func (suite *UtilitiesSuite) TestSoftDestroy_ModelWithDeletedAtWithHasManyAssociations() {
+	// model with deleted_at with "has many" associations
+	serviceMember := testdatagen.MakeDefaultServiceMember(suite.DB())
+
+	document := testdatagen.MakeDocument(suite.DB(), testdatagen.Assertions{
+		Document: models.Document{
+			ServiceMemberID: serviceMember.ID,
+			ServiceMember:   serviceMember,
+		},
+	})
+	suite.MustSave(&document)
+	suite.Nil(document.DeletedAt)
+
+	upload := models.Upload{
+		DocumentID:  &document.ID,
+		UploaderID:  document.ServiceMember.UserID,
+		Filename:    "test.pdf",
+		Bytes:       1048576,
+		ContentType: "application/pdf",
+		Checksum:    "ImGQ2Ush0bDHsaQthV5BnQ==",
+	}
+	upload2 := models.Upload{
+		DocumentID:  &document.ID,
+		UploaderID:  document.ServiceMember.UserID,
+		Filename:    "test2.pdf",
+		Bytes:       1048576,
+		ContentType: "application/pdf",
+		Checksum:    "ImGQ2Ush0bDHsaQthV5BnQ==",
+	}
+	suite.MustSave(&upload)
+	suite.MustSave(&upload2)
+	suite.Nil(upload.DeletedAt)
+	suite.Nil(upload2.DeletedAt)
+
+	err := utilities.SoftDestroy(suite.DB(), &document)
+
+	suite.NoError(err)
+	suite.NotNil(document.DeletedAt)
+	suite.NotNil(document.Uploads[0].DeletedAt)
+	suite.NotNil(document.Uploads[1].DeletedAt)
+}

pkg/handlers/internalapi/documents.go

@@ -92,7 +92,7 @@ func (h ShowDocumentHandler) Handle(params documentop.ShowDocumentParams) middle
 		return handlers.ResponseForError(logger, err)
 	}
 
-	document, err := models.FetchDocument(ctx, h.DB(), session, documentID)
+	document, err := models.FetchDocument(ctx, h.DB(), session, documentID, false)
 	if err != nil {
 		return handlers.ResponseForError(logger, err)
 	}

pkg/handlers/internalapi/move_documents.go

@@ -173,7 +173,7 @@ func (h IndexMoveDocumentsHandler) Handle(params movedocop.IndexMoveDocumentsPar
 		return handlers.ResponseForError(logger, err)
 	}
 
-	moveDocs, err := move.FetchAllMoveDocumentsForMove(h.DB())
+	moveDocs, err := move.FetchAllMoveDocumentsForMove(h.DB(), false)
 	if err != nil {
 		return handlers.ResponseForError(logger, err)
 	}

pkg/handlers/internalapi/personally_procured_move.go

@@ -562,7 +562,7 @@ func (h RequestPPMExpenseSummaryHandler) Handle(params ppmop.RequestPPMExpenseSu
 
 	// Fetch all approved expense documents for a PPM
 	status := models.MoveDocumentStatusOK
-	moveDocsExpense, err := models.FetchMoveDocuments(h.DB(), session, ppmID, &status, models.MoveDocumentTypeEXPENSE)
+	moveDocsExpense, err := models.FetchMoveDocuments(h.DB(), session, ppmID, &status, models.MoveDocumentTypeEXPENSE, false)
 	if err != nil {
 		return handlers.ResponseForError(logger, err)
 	}

pkg/handlers/internalapi/uploads.go

@@ -61,7 +61,7 @@ func (h CreateUploadHandler) Handle(params uploadop.CreateUploadParams) middlewa
 		}
 
 		// Fetch document to ensure user has access to it
-		document, docErr := models.FetchDocument(ctx, h.DB(), session, documentID)
+		document, docErr := models.FetchDocument(ctx, h.DB(), session, documentID, false)
 		if docErr != nil {
 			return handlers.ResponseForError(logger, docErr)
 		}

pkg/handlers/internalapi/uploads_test.go

@@ -158,6 +158,7 @@ func (suite *HandlerSuite) TestDeleteUploadHandlerSuccess() {
 	fakeS3 := storageTest.NewFakeS3Storage(true)
 
 	upload := testdatagen.MakeDefaultUpload(suite.DB())
+	suite.Nil(upload.DeletedAt)
 
 	file := suite.Fixture("test.pdf")
 	fakeS3.Store(upload.StorageKey, file.Data, "somehash")
@@ -179,13 +180,15 @@ func (suite *HandlerSuite) TestDeleteUploadHandlerSuccess() {
 
 	queriedUpload := models.Upload{}
 	err := suite.DB().Find(&queriedUpload, upload.ID)
-	suite.NotNil(err)
+	suite.Nil(err)
+	suite.NotNil(queriedUpload.DeletedAt)
 }
 
 func (suite *HandlerSuite) TestDeleteUploadsHandlerSuccess() {
 	fakeS3 := storageTest.NewFakeS3Storage(true)
 
 	upload1 := testdatagen.MakeDefaultUpload(suite.DB())
+	suite.Nil(upload1.DeletedAt)
 
 	upload2Assertions := testdatagen.Assertions{
 		Upload: models.Upload{
@@ -219,5 +222,6 @@ func (suite *HandlerSuite) TestDeleteUploadsHandlerSuccess() {
 
 	queriedUpload := models.Upload{}
 	err := suite.DB().Find(&queriedUpload, upload1.ID)
-	suite.NotNil(err)
+	suite.Nil(err)
+	suite.NotNil(queriedUpload.DeletedAt)
 }