From 58d4bbdd33a6af6b771933d510a48e4a0be2911d Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 22 Jan 2024 14:19:50 -0500 Subject: [PATCH 01/27] Working on object batch deletions --- pgmodels/intellectual_object.go | 12 ++++ pgmodels/intellectual_object_test.go | 28 +++++++++ pgmodels/work_item.go | 7 +++ pgmodels/work_item_test.go | 21 +++++++ views/deletions/already_approved.html | 4 +- views/deletions/already_cancelled.html | 4 +- views/deletions/review.html | 19 +++++- .../admin/intellectual_objects_controller.go | 39 ++++++++++++ web/webui/deletion.go | 59 ++++++++++++++++--- web/webui/deletion_requests_controller.go | 13 ++-- 10 files changed, 186 insertions(+), 20 deletions(-) diff --git a/pgmodels/intellectual_object.go b/pgmodels/intellectual_object.go index d5b867a8..99b4ff61 100644 --- a/pgmodels/intellectual_object.go +++ b/pgmodels/intellectual_object.go @@ -74,6 +74,18 @@ func IntellectualObjectSelect(query *Query) ([]*IntellectualObject, error) { return objects, err } +// CountObjectsThatCanBeDeleted returns the number of active objects in the +// list of object IDs that belong to the specified institution. We use this +// when running batch deletions to ensure that no objects belong to an +// institution other than the one requesting the deletion. +// +// If we get a list of 100 ids, the return value should be 100. If it's not +// some object in the ID list was already deleted, or it belongs to someone +// else. +func CountObjectsThatCanBeDeleted(institutionID int64, objIDs []int64) (int, error) { + return common.Context().DB.Model((*IntellectualObject)(nil)).Where(`institution_id = ? and state = 'A' and id in (?)`, institutionID, pg.In(objIDs)).Count() +} + // Save saves this object to the database. This will peform an insert // if IntellectualObject.ID is zero. Otherwise, it updates. func (obj *IntellectualObject) Save() error { diff --git a/pgmodels/intellectual_object_test.go b/pgmodels/intellectual_object_test.go index 476756da..cb530f40 100644 --- a/pgmodels/intellectual_object_test.go +++ b/pgmodels/intellectual_object_test.go @@ -94,6 +94,34 @@ func TestObjHasActiveFiles(t *testing.T) { } +func TestCountObjectsThatCanBeDeleted(t *testing.T) { + // These are the ids of non-deleted objects belonging to institution 3. + // These are loaded from fixture data. + idsBelongingToInst3 := []int64{4, 5, 6, 12, 13} + + // All five items should be OK to delete, because all five + // belong to inst 3 and are in active state. + numberOkToDelete, err := pgmodels.CountObjectsThatCanBeDeleted(3, idsBelongingToInst3) + require.NoError(t, err) + assert.Equal(t, len(idsBelongingToInst3), numberOkToDelete) + + // We should get zero here, because none of these objects + // belong to inst2. + numberOkToDelete, err = pgmodels.CountObjectsThatCanBeDeleted(2, idsBelongingToInst3) + require.NoError(t, err) + assert.Equal(t, 0, numberOkToDelete) + + // In this set, the first three items belong to inst 3 and + // are active. ID 14 is already deleted, and items 1 and 2 + // belong to a different institution. So we should get three + // because only the first three items belong to inst 3 AND + // are currently active. + miscIds := []int64{4, 5, 6, 14, 1, 2} + numberOkToDelete, err = pgmodels.CountObjectsThatCanBeDeleted(3, miscIds) + require.NoError(t, err) + assert.Equal(t, 3, numberOkToDelete) +} + func TestObjLastIngestEvent(t *testing.T) { obj, err := pgmodels.IntellectualObjectByID(6) require.Nil(t, err) diff --git a/pgmodels/work_item.go b/pgmodels/work_item.go index e93f4263..d90431bc 100644 --- a/pgmodels/work_item.go +++ b/pgmodels/work_item.go @@ -9,6 +9,7 @@ import ( "github.com/APTrust/registry/common" "github.com/APTrust/registry/constants" v "github.com/asaskevich/govalidator" + "github.com/go-pg/pg/v10" "github.com/jinzhu/copier" "github.com/stretchr/stew/slice" ) @@ -116,6 +117,12 @@ func WorkItemsPendingForObject(instID int64, bagName string) ([]*WorkItem, error return WorkItemSelect(query) } +// WorkItemsPendingForObjectBatch returns the number of WorkItems pending +func WorkItemsPendingForObjectBatch(objIDs []int64) (int, error) { + completed := common.InterfaceList(constants.CompletedStatusValues) + return common.Context().DB.Model((*WorkItem)(nil)).Where(`intellectual_object_id in (?) and status not in (?)`, pg.In(objIDs), pg.In(completed)).Count() +} + // WorkItemsPendingForFile returns a list of in-progress WorkItems // for the GenericFile with the specified ID. func WorkItemsPendingForFile(fileID int64) ([]*WorkItem, error) { diff --git a/pgmodels/work_item_test.go b/pgmodels/work_item_test.go index fd4a759f..b517323a 100644 --- a/pgmodels/work_item_test.go +++ b/pgmodels/work_item_test.go @@ -439,6 +439,27 @@ func TestNewDeletionItem(t *testing.T) { require.Nil(t, item6) } +func TestWorkItemsPendingForObjectBatch(t *testing.T) { + db.LoadFixtures() + + // These objects belong to institution 3 + // and have no pending WorkItems in the fixture data. + objIDs := []int64{5, 6, 12, 13} + itemCount, err := pgmodels.WorkItemsPendingForObjectBatch(objIDs) + require.NoError(t, err) + assert.Equal(t, 0, itemCount) + + // Now add in Intel Obj 4, which has three + // pending WorkItems. The function should return + // the number of unfinished work items for this + // batch of objects. + objIDs = []int64{4, 5, 6, 12, 13} + itemCount, err = pgmodels.WorkItemsPendingForObjectBatch(objIDs) + require.NoError(t, err) + assert.Equal(t, 3, itemCount) + +} + func TestIsRestorationSpotTest(t *testing.T) { // This is not a spot test because it's not even a restoration. diff --git a/views/deletions/already_approved.html b/views/deletions/already_approved.html index 9b93c86c..f970c976 100644 --- a/views/deletions/already_approved.html +++ b/views/deletions/already_approved.html @@ -5,9 +5,9 @@

Deletion Previously Confirmed

-

The deletion of file {{ .itemIdentifier }} was approved by {{ .deletionRequest.ConfirmedBy.Name }} ({{ .deletionRequest.ConfirmedBy.Email }}) on {{ dateUS .deletionRequest.ConfirmedAt }}.

+

The deletion of {{ .itemIdentifier }} was approved by {{ .deletionRequest.ConfirmedBy.Name }} ({{ .deletionRequest.ConfirmedBy.Email }}) on {{ dateUS .deletionRequest.ConfirmedAt }}.

-

If the file has not yet been deleted, it will be soon.

+

If the items have not yet been deleted, they will be soon.

Back to Deletions List
diff --git a/views/deletions/already_cancelled.html b/views/deletions/already_cancelled.html index 8c1e20b9..520e2f44 100644 --- a/views/deletions/already_cancelled.html +++ b/views/deletions/already_cancelled.html @@ -5,9 +5,9 @@

Deletion Previously Cancelled

-

The deletion of file {{ .itemIdentifier }} was cancelled by {{ .deletionRequest.CancelledBy.Name }} ({{ .deletionRequest.CancelledBy.Email }} on {{ dateUS .deletionRequest.CancelledAt }}.

+

The deletion of {{ .itemIdentifier }} was cancelled by {{ .deletionRequest.CancelledBy.Name }} ({{ .deletionRequest.CancelledBy.Email }} on {{ dateUS .deletionRequest.CancelledAt }}.

-

This deletion request will not be executed. Unless the file was deleted by a subsequent request, this file should still exist.

+

This deletion request will not be executed. Unless the files or objects were deleted by a subsequent request, these items should still exist.

Back to Deletions List
diff --git a/views/deletions/review.html b/views/deletions/review.html index 4f6a9668..0b40f22f 100644 --- a/views/deletions/review.html +++ b/views/deletions/review.html @@ -5,8 +5,9 @@

Review Deletion Request

-

User {{ .deletionRequest.RequestedBy.Name }} ({{ .deletionRequest.RequestedBy.Email }}) wants to delete the following item:

+

User {{ .deletionRequest.RequestedBy.Name }} ({{ .deletionRequest.RequestedBy.Email }}) wants to delete the following items:

+ {{ if (eq .itemType "file") }}

Generic File

@@ -18,7 +19,9 @@

Generic File

Updated: {{ dateUS .file.UpdatedAt }}

- {{ else }} + + + {{ else if (eq .itemType "single object") }}

Intellectual Object

@@ -32,9 +35,19 @@

Intellectual Object

Updated: {{ dateUS .object.UpdatedAt }}

+ + + {{ else if (eq .itemType "object list") }} + +

Intellectual Objects

+ + {{ range $index, $obj := .objectList }} +

{{ $obj.Identifier }}

+ {{ end }} + {{ end }} -

Do you want to approve or cancel this request? If you approve, the file(s) will be deleted as soon as possible. Deletion cannot be undone. If you cancel, the file(s) will stay and no one else will be able to approve this request.

+

Do you want to approve or cancel this request? If you approve, the items(s) will be deleted as soon as possible. Deletion cannot be undone. If you cancel, the file(s) will stay and no one else will be able to approve this request.

diff --git a/web/api/admin/intellectual_objects_controller.go b/web/api/admin/intellectual_objects_controller.go index 27171ac9..59c2d702 100644 --- a/web/api/admin/intellectual_objects_controller.go +++ b/web/api/admin/intellectual_objects_controller.go @@ -2,6 +2,7 @@ package admin_api import ( "net/http" + "strconv" "github.com/APTrust/registry/common" "github.com/APTrust/registry/constants" @@ -53,6 +54,32 @@ func IntellectualObjectInitRestore(c *gin.Context) { c.JSON(http.StatusCreated, workItem) } +// IntellectualObjectInitBatchDelete creates an deletion request for +// multiple objects. This request must be approved by an administrator +// at the depositing institution before the deletion will actually be queued. +// +// POST /objects/init_batch_delete/:id +func IntellectualObjectInitBatchDelete(c *gin.Context) { + req := api.NewRequest(c) + objectIDs, err := StringSliceToInt64Slice(c.Request.PostForm["objectID"]) + if api.AbortIfError(c, err) { + return + } + institutionID, err := strconv.ParseInt(c.Request.PostFormValue("institutionID"), 10, 64) + if api.AbortIfError(c, err) { + return + } + del, err := webui.NewDeletionForObjectBatch(institutionID, objectIDs, req.CurrentUser, req.BaseURL()) + if api.AbortIfError(c, err) { + return + } + _, err = del.CreateRequestAlert() + if api.AbortIfError(c, err) { + return + } + c.JSON(http.StatusCreated, del.DeletionRequest) +} + // IntellectualObjectDelete marks an object record as deleted. // It also creates a deletion premis event. Before it does any of // that, it checks a number of pre-conditions. See the @@ -127,3 +154,15 @@ func CoerceObjectStorageOption(existingObject, submittedObject *pgmodels.Intelle submittedObject.StorageOption = existingObject.StorageOption } } + +func StringSliceToInt64Slice(strSlice []string) ([]int64, error) { + var err error + ints := make([]int64, len(strSlice)) + for i, strValue := range strSlice { + ints[i], err = strconv.ParseInt(strValue, 10, 64) + if err != nil { + break + } + } + return ints, err +} diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 5fe49689..0b448c90 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -76,7 +76,46 @@ func NewDeletionForObject(objID int64, currentUser *pgmodels.User, baseURL strin baseURL: baseURL, currentUser: currentUser, } - err = del.initObjectDeletionRequest(objID) + err = del.initObjectDeletionRequest(obj.InstitutionID, []int64{objID}) + if err != nil { + return nil, err + } + err = del.loadInstAdmins() + return del, err +} + +// NewDeletionForObjectBatch creates a new DeletionRequest for a batch of +// IntellectualObjects and returns the Deletion object. This constructor +// is only for initializing new DeletionRequests, not for reviewing, approving +// or cancelling existing requests. +func NewDeletionForObjectBatch(institutionID int64, objIDs []int64, currentUser *pgmodels.User, baseURL string) (*Deletion, error) { + + // Make sure that all objects belong to the specified institution. + validObjectCount, err := pgmodels.CountObjectsThatCanBeDeleted(institutionID, objIDs) + if err != nil { + return nil, err + } + if validObjectCount != len(objIDs) { + common.Context().Log.Error().Msgf("Batch deletion requested for %d objects, of which only %d are valid. InstitutionID = %d. Current user = %s. IDs: %v", + len(objIDs), validObjectCount, institutionID, currentUser.Email, objIDs) + return nil, fmt.Errorf("one or more object ids is invalid") + } + + // Make sure there are no pending work items for these objects. + pendingWorkItems, err := pgmodels.WorkItemsPendingForObjectBatch(objIDs) + if err != nil { + return nil, err + } + if pendingWorkItems > 0 { + common.Context().Log.Warn().Msgf("Some objects in batch deletion request have pending work items. Object IDs: %v", objIDs) + return nil, common.ErrPendingWorkItems + } + + del := &Deletion{ + baseURL: baseURL, + currentUser: currentUser, + } + err = del.initObjectDeletionRequest(institutionID, objIDs) if err != nil { return nil, err } @@ -168,20 +207,22 @@ func (del *Deletion) initFileDeletionRequest(genericFileID int64) error { // We do not save the plaintext version of the token, // only the encrypted version. When this new DeletionRequest goes out of // scope, there's no further access to the token, so get it while you can. -func (del *Deletion) initObjectDeletionRequest(objID int64) error { - obj, err := pgmodels.IntellectualObjectByID(objID) - if err != nil { - return err - } - +func (del *Deletion) initObjectDeletionRequest(institutionID int64, objIDs []int64) error { deletionRequest, err := pgmodels.NewDeletionRequest() if err != nil { return err } - deletionRequest.InstitutionID = obj.InstitutionID + deletionRequest.InstitutionID = institutionID deletionRequest.RequestedByID = del.currentUser.ID deletionRequest.RequestedAt = time.Now().UTC() - deletionRequest.AddObject(obj) + + for _, objID := range objIDs { + obj, err := pgmodels.IntellectualObjectByID(objID) + if err != nil { + return err + } + deletionRequest.AddObject(obj) + } err = deletionRequest.Save() if err != nil { return err diff --git a/web/webui/deletion_requests_controller.go b/web/webui/deletion_requests_controller.go index a79e20b7..02675efa 100644 --- a/web/webui/deletion_requests_controller.go +++ b/web/webui/deletion_requests_controller.go @@ -77,13 +77,18 @@ func DeletionRequestReview(c *gin.Context) { req.TemplateData["deletionRequest"] = del.DeletionRequest req.TemplateData["token"] = c.Query("token") - if len(del.DeletionRequest.IntellectualObjects) > 0 { - req.TemplateData["itemType"] = "object" - req.TemplateData["itemIdentifier"] = del.DeletionRequest.IntellectualObjects[0].Identifier + if len(del.DeletionRequest.IntellectualObjects) == 1 { + req.TemplateData["itemType"] = "single object" + req.TemplateData["itemIdentifier"] = fmt.Sprintf("object %s", del.DeletionRequest.IntellectualObjects[0].Identifier) req.TemplateData["object"] = del.DeletionRequest.IntellectualObjects[0] + } else if len(del.DeletionRequest.IntellectualObjects) > 1 { + // Bulk object deletion + req.TemplateData["itemType"] = "object list" + req.TemplateData["itemIdentifier"] = fmt.Sprintf("%d objects", len(del.DeletionRequest.IntellectualObjects)) + req.TemplateData["objectList"] = del.DeletionRequest.IntellectualObjects } else if len(del.DeletionRequest.GenericFiles) > 0 { req.TemplateData["itemType"] = "file" - req.TemplateData["itemIdentifier"] = del.DeletionRequest.GenericFiles[0].Identifier + req.TemplateData["itemIdentifier"] = fmt.Sprintf("file %s", del.DeletionRequest.GenericFiles[0].Identifier) req.TemplateData["file"] = del.DeletionRequest.GenericFiles[0] } else { common.Context().Log.Info().Msgf("DeletionRequest with ID %d has no associated files or objects.", req.Auth.ResourceID) From 667d82f92c77d1997b1a65d37b9dedd211eabfac Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 22 Jan 2024 15:59:25 -0500 Subject: [PATCH 02/27] Working on permissions for bulk deletion --- constants/permissions.go | 3 + middleware/authorization_map.go | 97 ++++++++++--------- .../admin/intellectual_objects_controller.go | 10 +- .../intellectual_objects_controller_test.go | 32 ++++++ web/webui/deletion.go | 15 ++- 5 files changed, 105 insertions(+), 52 deletions(-) diff --git a/constants/permissions.go b/constants/permissions.go index 6bc1d1cf..e1558a4c 100644 --- a/constants/permissions.go +++ b/constants/permissions.go @@ -47,6 +47,7 @@ const ( InstitutionRead = "InstitutionRead" InstitutionUpdate = "InstitutionUpdate" InstitutionUpdatePrefs = "InstitutionUpdatePrefs" + IntellectualObjectBatchDelete = "IntellectualObjectBatchDelete" IntellectualObjectCreate = "IntellectualObjectCreate" IntellectualObjectDelete = "IntellectualObjectDelete" IntellectualObjectFinishBulkDelete = "IntellectualObjectFinishBulkDelete" @@ -124,6 +125,7 @@ var Permissions = []Permission{ InstitutionRead, InstitutionUpdate, InstitutionUpdatePrefs, + IntellectualObjectBatchDelete, IntellectualObjectCreate, IntellectualObjectDelete, IntellectualObjectFinishBulkDelete, @@ -311,6 +313,7 @@ func initPermissions() { sysAdmin[InstitutionRead] = true sysAdmin[InstitutionUpdate] = true sysAdmin[InstitutionUpdatePrefs] = true + sysAdmin[IntellectualObjectBatchDelete] = true sysAdmin[IntellectualObjectCreate] = true sysAdmin[IntellectualObjectDelete] = true // preserv workers do this with sys admin account sysAdmin[IntellectualObjectFinishBulkDelete] = true // not implemented yet diff --git a/middleware/authorization_map.go b/middleware/authorization_map.go index 275e285a..57d466d8 100644 --- a/middleware/authorization_map.go +++ b/middleware/authorization_map.go @@ -31,54 +31,55 @@ type AuthMetadata struct { // requests that hit an unguarded route will return an internal server // error. var AuthMap = map[string]AuthMetadata{ - "AlertCreate": {"Alert", constants.AlertCreate}, - "AlertDelete": {"Alert", constants.AlertDelete}, - "AlertIndex": {"Alert", constants.AlertRead}, - "AlertNew": {"Alert", constants.AlertCreate}, - "AlertShow": {"Alert", constants.AlertRead}, - "AlertUpdate": {"Alert", constants.AlertUpdate}, - "AlertMarkAsReadXHR": {"Alert", constants.AlertUpdate}, - "AlertMarkAllAsRead": {"Alert", constants.AlertUpdate}, - "AlertMarkAsUnreadXHR": {"Alert", constants.AlertUpdate}, - "BillingReportShow": {"DepositStats", constants.BillingReportShow}, - "ChecksumCreate": {"Checksum", constants.ChecksumCreate}, - "ChecksumDelete": {"Checksum", constants.ChecksumDelete}, - "ChecksumIndex": {"Checksum", constants.ChecksumRead}, - "ChecksumNew": {"Checksum", constants.ChecksumCreate}, - "ChecksumShow": {"Checksum", constants.ChecksumRead}, - "ChecksumUpdate": {"Checksum", constants.ChecksumUpdate}, - "DashboardShow": {"Dashboard", constants.DashboardShow}, - "DeletionRequestApprove": {"DeletionRequest", constants.DeletionRequestApprove}, - "DeletionRequestCancel": {"DeletionRequest", constants.DeletionRequestApprove}, - "DeletionRequestIndex": {"DeletionRequest", constants.DeletionRequestList}, - "DeletionRequestReview": {"DeletionRequest", constants.DeletionRequestApprove}, - "DeletionRequestShow": {"DeletionRequest", constants.DeletionRequestShow}, - "DepositReportShow": {"DepositStats", constants.DepositReportShow}, - "GenericFileCreate": {"GenericFile", constants.FileCreate}, - "GenericFileCreateBatch": {"GenericFile", constants.FileCreate}, - "GenericFileDelete": {"GenericFile", constants.FileDelete}, - "GenericFileFinishBulkDelete": {"GenericFile", constants.FileFinishBulkDelete}, - "GenericFileIndex": {"GenericFile", constants.FileRead}, - "GenericFileInitDelete": {"GenericFile", constants.FileRequestDelete}, - "GenericFileInitRestore": {"GenericFile", constants.FileRestore}, - "GenericFileNew": {"GenericFile", constants.FileCreate}, - "GenericFileRequestDelete": {"GenericFile", constants.FileRequestDelete}, - "GenericFileRequestRestore": {"GenericFile", constants.FileRestore}, - "GenericFileShow": {"GenericFile", constants.FileRead}, - "GenericFileUpdate": {"GenericFile", constants.FileUpdate}, - "InstitutionCreate": {"Institution", constants.InstitutionCreate}, - "InstitutionDelete": {"Institution", constants.InstitutionDelete}, - "InstitutionEdit": {"Institution", constants.InstitutionUpdate}, - "InstitutionEditPrefs": {"Institution", constants.InstitutionUpdatePrefs}, - "InstitutionIndex": {"Institution", constants.InstitutionList}, - "InstitutionNew": {"Institution", constants.InstitutionCreate}, - "InstitutionShow": {"Institution", constants.InstitutionRead}, - "InstitutionUndelete": {"Institution", constants.InstitutionUpdate}, - "InstitutionUpdate": {"Institution", constants.InstitutionUpdate}, - "InstitutionUpdatePrefs": {"Institution", constants.InstitutionUpdatePrefs}, - "IntellectualObjectCreate": {"IntellectualObject", constants.IntellectualObjectCreate}, - "IntellectualObjectDelete": {"IntellectualObject", constants.IntellectualObjectDelete}, - "IntellectualObjectEvents": {"PremisEvent", constants.EventRead}, + "AlertCreate": {"Alert", constants.AlertCreate}, + "AlertDelete": {"Alert", constants.AlertDelete}, + "AlertIndex": {"Alert", constants.AlertRead}, + "AlertNew": {"Alert", constants.AlertCreate}, + "AlertShow": {"Alert", constants.AlertRead}, + "AlertUpdate": {"Alert", constants.AlertUpdate}, + "AlertMarkAsReadXHR": {"Alert", constants.AlertUpdate}, + "AlertMarkAllAsRead": {"Alert", constants.AlertUpdate}, + "AlertMarkAsUnreadXHR": {"Alert", constants.AlertUpdate}, + "BillingReportShow": {"DepositStats", constants.BillingReportShow}, + "ChecksumCreate": {"Checksum", constants.ChecksumCreate}, + "ChecksumDelete": {"Checksum", constants.ChecksumDelete}, + "ChecksumIndex": {"Checksum", constants.ChecksumRead}, + "ChecksumNew": {"Checksum", constants.ChecksumCreate}, + "ChecksumShow": {"Checksum", constants.ChecksumRead}, + "ChecksumUpdate": {"Checksum", constants.ChecksumUpdate}, + "DashboardShow": {"Dashboard", constants.DashboardShow}, + "DeletionRequestApprove": {"DeletionRequest", constants.DeletionRequestApprove}, + "DeletionRequestCancel": {"DeletionRequest", constants.DeletionRequestApprove}, + "DeletionRequestIndex": {"DeletionRequest", constants.DeletionRequestList}, + "DeletionRequestReview": {"DeletionRequest", constants.DeletionRequestApprove}, + "DeletionRequestShow": {"DeletionRequest", constants.DeletionRequestShow}, + "DepositReportShow": {"DepositStats", constants.DepositReportShow}, + "GenericFileCreate": {"GenericFile", constants.FileCreate}, + "GenericFileCreateBatch": {"GenericFile", constants.FileCreate}, + "GenericFileDelete": {"GenericFile", constants.FileDelete}, + "GenericFileFinishBulkDelete": {"GenericFile", constants.FileFinishBulkDelete}, + "GenericFileIndex": {"GenericFile", constants.FileRead}, + "GenericFileInitDelete": {"GenericFile", constants.FileRequestDelete}, + "GenericFileInitRestore": {"GenericFile", constants.FileRestore}, + "GenericFileNew": {"GenericFile", constants.FileCreate}, + "GenericFileRequestDelete": {"GenericFile", constants.FileRequestDelete}, + "GenericFileRequestRestore": {"GenericFile", constants.FileRestore}, + "GenericFileShow": {"GenericFile", constants.FileRead}, + "GenericFileUpdate": {"GenericFile", constants.FileUpdate}, + "InstitutionCreate": {"Institution", constants.InstitutionCreate}, + "InstitutionDelete": {"Institution", constants.InstitutionDelete}, + "InstitutionEdit": {"Institution", constants.InstitutionUpdate}, + "InstitutionEditPrefs": {"Institution", constants.InstitutionUpdatePrefs}, + "InstitutionIndex": {"Institution", constants.InstitutionList}, + "InstitutionNew": {"Institution", constants.InstitutionCreate}, + "InstitutionShow": {"Institution", constants.InstitutionRead}, + "InstitutionUndelete": {"Institution", constants.InstitutionUpdate}, + "InstitutionUpdate": {"Institution", constants.InstitutionUpdate}, + "InstitutionUpdatePrefs": {"Institution", constants.InstitutionUpdatePrefs}, + "IntellectualObjectInitBatchDelete": {"IntellectualObject", constants.IntellectualObjectBatchDelete}, + "IntellectualObjectCreate": {"IntellectualObject", constants.IntellectualObjectCreate}, + "IntellectualObjectDelete": {"IntellectualObject", constants.IntellectualObjectDelete}, + "IntellectualObjectEvents": {"PremisEvent", constants.EventRead}, // IntellectualObjectFiles gets an object ID and will look up that object to check // it's institution. The permission, however, is FileReade, because this endpoint // returns files. https://trello.com/c/n5asx3bj diff --git a/web/api/admin/intellectual_objects_controller.go b/web/api/admin/intellectual_objects_controller.go index 59c2d702..3c10c621 100644 --- a/web/api/admin/intellectual_objects_controller.go +++ b/web/api/admin/intellectual_objects_controller.go @@ -69,7 +69,15 @@ func IntellectualObjectInitBatchDelete(c *gin.Context) { if api.AbortIfError(c, err) { return } - del, err := webui.NewDeletionForObjectBatch(institutionID, objectIDs, req.CurrentUser, req.BaseURL()) + requestorID, err := strconv.ParseInt(c.Request.PostFormValue("requestorID"), 10, 64) + if api.AbortIfError(c, err) { + return + } + + common.Context().Log.Warn().Msgf("Creating batch deletion request on behalf of user %d for %d objects belonging to institution %d. Current user is %s.", + requestorID, len(objectIDs), institutionID, req.CurrentUser.Email) + + del, err := webui.NewDeletionForObjectBatch(requestorID, institutionID, objectIDs, req.BaseURL()) if api.AbortIfError(c, err) { return } diff --git a/web/api/admin/intellectual_objects_controller_test.go b/web/api/admin/intellectual_objects_controller_test.go index 009d5096..6fe004d1 100644 --- a/web/api/admin/intellectual_objects_controller_test.go +++ b/web/api/admin/intellectual_objects_controller_test.go @@ -291,3 +291,35 @@ func TestObjectInitRestore(t *testing.T) { WithHeader(constants.APIKeyHeader, "password"). Expect().Status(http.StatusForbidden) } + +func TestObjectBatchDelete(t *testing.T) { + // START HERE + + // Test permissions. Only APTrust admin should be allowed to do this. + + // Ensure that we get failure if we include an object + // with a pending WorkItem. + + // Ensure that we get failure if we include an object + // that belongs to another institution. + + // Ensure that we get failure if requestorID belongs + // to an inst user rather than inst admin. + + // Ensure we get success with valid params: + // inst id, user id, object ids. + + // Check post conditions. There should be a deletion + // request with all expected properties and with the + // right list of object IDs. + + // Check the text of the alert. It should include + // all of the object identifiers. + + // Confirm the alert, and then test that the correct + // WorkItems were created and that no spurious work + // items were created. + + // TODO: Create & test bulk delete ENV token from + // parameter store? +} diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 0b448c90..8f79a648 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -88,7 +88,16 @@ func NewDeletionForObject(objID int64, currentUser *pgmodels.User, baseURL strin // IntellectualObjects and returns the Deletion object. This constructor // is only for initializing new DeletionRequests, not for reviewing, approving // or cancelling existing requests. -func NewDeletionForObjectBatch(institutionID int64, objIDs []int64, currentUser *pgmodels.User, baseURL string) (*Deletion, error) { +func NewDeletionForObjectBatch(requestorID, institutionID int64, objIDs []int64, baseURL string) (*Deletion, error) { + + requestingUser, err := pgmodels.UserByID(requestorID) + if err != nil { + return nil, err + } + if requestingUser.InstitutionID != institutionID || requestingUser.Role != constants.RoleInstAdmin { + common.Context().Log.Error().Msgf("Requesting user %d is not admin at institution %d. Rejecting bulk deletion request.", requestorID, institutionID) + return nil, fmt.Errorf("invalid requestor id") + } // Make sure that all objects belong to the specified institution. validObjectCount, err := pgmodels.CountObjectsThatCanBeDeleted(institutionID, objIDs) @@ -97,7 +106,7 @@ func NewDeletionForObjectBatch(institutionID int64, objIDs []int64, currentUser } if validObjectCount != len(objIDs) { common.Context().Log.Error().Msgf("Batch deletion requested for %d objects, of which only %d are valid. InstitutionID = %d. Current user = %s. IDs: %v", - len(objIDs), validObjectCount, institutionID, currentUser.Email, objIDs) + len(objIDs), validObjectCount, institutionID, requestingUser.Email, objIDs) return nil, fmt.Errorf("one or more object ids is invalid") } @@ -113,7 +122,7 @@ func NewDeletionForObjectBatch(institutionID int64, objIDs []int64, currentUser del := &Deletion{ baseURL: baseURL, - currentUser: currentUser, + currentUser: requestingUser, } err = del.initObjectDeletionRequest(institutionID, objIDs) if err != nil { From 1a6c18431bae08152b8b5f4069ecb022e3555e92 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Tue, 23 Jan 2024 10:32:53 -0500 Subject: [PATCH 03/27] Added config option BATCH_DELETE_ENABLED --- .env.dev | 5 +++++ .env.docker | 1 + .env.integration | 5 +++++ .env.test | 5 +++++ .env.travis | 5 +++++ .env.uidemo | 5 +++++ common/config.go | 20 ++++++++++--------- .../admin/intellectual_objects_controller.go | 6 ++++++ 8 files changed, 43 insertions(+), 9 deletions(-) diff --git a/.env.dev b/.env.dev index 0cd8452a..01f555ba 100644 --- a/.env.dev +++ b/.env.dev @@ -45,6 +45,11 @@ HTTPS_COOKIES=false # NSQ_URL='http://localhost:4151' +# +# Should we enable batch deletions in the current environment? +# +BATCH_DELETE_ENABLED=true + # # Two-Factor auth settings. # diff --git a/.env.docker b/.env.docker index b40aae33..e9699c20 100644 --- a/.env.docker +++ b/.env.docker @@ -16,6 +16,7 @@ # AUTHY_API_KEY # AWS_SES_PWD # AWS_SES_USER +# BATCH_DELETE_ENABLED # COOKIE_BLOCK_KEY # COOKIE_DOMAIN # COOKIE_HASH_KEY diff --git a/.env.integration b/.env.integration index e1710e0b..1a396b48 100644 --- a/.env.integration +++ b/.env.integration @@ -45,6 +45,11 @@ HTTPS_COOKIES=false # items into queues. NSQ_URL='http://localhost:4151' +# +# Should we enable batch deletions in the current environment? +# +BATCH_DELETE_ENABLED=true + # # Two-Factor auth settings. # diff --git a/.env.test b/.env.test index bca82a3c..c8b8425b 100644 --- a/.env.test +++ b/.env.test @@ -46,6 +46,11 @@ HTTPS_COOKIES=false # items into queues. NSQ_URL='http://localhost:4151' +# +# Should we enable batch deletions in the current environment? +# +BATCH_DELETE_ENABLED=true + # # Two-Factor auth settings. # diff --git a/.env.travis b/.env.travis index 52e440f7..14ae6f8c 100644 --- a/.env.travis +++ b/.env.travis @@ -45,6 +45,11 @@ HTTPS_COOKIES=false # NSQ_URL='http://localhost:4151' +# +# Should we enable batch deletions in the current environment? +# +BATCH_DELETE_ENABLED=true + # # Two-Factor auth settings. # diff --git a/.env.uidemo b/.env.uidemo index eb8cc7de..9cd63d06 100644 --- a/.env.uidemo +++ b/.env.uidemo @@ -50,6 +50,11 @@ HTTPS_COOKIES=true # NSQ_URL='http://localhost:4151' +# +# Should we enable batch deletions in the current environment? +# +BATCH_DELETE_ENABLED=true + # # Two-Factor auth settings. # diff --git a/common/config.go b/common/config.go index 9a1d4cec..ccf2cffe 100644 --- a/common/config.go +++ b/common/config.go @@ -79,14 +79,15 @@ type RedisConfig struct { } type Config struct { - Cookies *CookieConfig - DB *DBConfig - EnvName string - Logging *LoggingConfig - NsqUrl string - TwoFactor *TwoFactorConfig - Email *EmailConfig - Redis *RedisConfig + Cookies *CookieConfig + DB *DBConfig + EnvName string + Logging *LoggingConfig + NsqUrl string + TwoFactor *TwoFactorConfig + Email *EmailConfig + Redis *RedisConfig + BatchDeleteEnabled bool } // Returns a new config based on APT_ENV @@ -181,7 +182,8 @@ func loadConfig() *Config { FlashCookie: v.GetString("FLASH_COOKIE_NAME"), PrefsCookie: v.GetString("PREFS_COOKIE_NAME"), }, - NsqUrl: nsqUrl, + NsqUrl: nsqUrl, + BatchDeleteEnabled: v.GetBool("BATCH_DELETE_ENABLED"), TwoFactor: &TwoFactorConfig{ AuthyAPIKey: v.GetString("AUTHY_API_KEY"), AuthyEnabled: v.GetBool("ENABLE_TWO_FACTOR_AUTHY"), diff --git a/web/api/admin/intellectual_objects_controller.go b/web/api/admin/intellectual_objects_controller.go index 3c10c621..681bffd5 100644 --- a/web/api/admin/intellectual_objects_controller.go +++ b/web/api/admin/intellectual_objects_controller.go @@ -1,6 +1,7 @@ package admin_api import ( + "fmt" "net/http" "strconv" @@ -60,6 +61,11 @@ func IntellectualObjectInitRestore(c *gin.Context) { // // POST /objects/init_batch_delete/:id func IntellectualObjectInitBatchDelete(c *gin.Context) { + if !common.Context().Config.BatchDeleteEnabled { + common.Context().Log.Error().Msgf("Rejecting attempt to run batch deletion when config says BatchDeleteEnabled=false") + api.AbortIfError(c, fmt.Errorf("No!")) + return + } req := api.NewRequest(c) objectIDs, err := StringSliceToInt64Slice(c.Request.PostForm["objectID"]) if api.AbortIfError(c, err) { From b5c8375bd2cf1f977411ca28aec4eaaecd95389c Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Tue, 23 Jan 2024 13:51:40 -0500 Subject: [PATCH 04/27] Changed BatchDeleteEnabled to BatchDeletionKey --- .env.dev | 4 ++-- .env.docker | 2 +- .env.integration | 4 ++-- .env.test | 4 ++-- .env.travis | 4 ++-- .env.uidemo | 4 ++-- common/config.go | 22 +++++++++++----------- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/.env.dev b/.env.dev index 01f555ba..8497c0f2 100644 --- a/.env.dev +++ b/.env.dev @@ -46,9 +46,9 @@ HTTPS_COOKIES=false NSQ_URL='http://localhost:4151' # -# Should we enable batch deletions in the current environment? +# Secret key required to perform batch deletions. # -BATCH_DELETE_ENABLED=true +BATCH_DELETION_KEY="00000000-0000-0000-0000-000000000000" # # Two-Factor auth settings. diff --git a/.env.docker b/.env.docker index e9699c20..245cdcfb 100644 --- a/.env.docker +++ b/.env.docker @@ -16,7 +16,7 @@ # AUTHY_API_KEY # AWS_SES_PWD # AWS_SES_USER -# BATCH_DELETE_ENABLED +# BATCH_DELETION_KEY # COOKIE_BLOCK_KEY # COOKIE_DOMAIN # COOKIE_HASH_KEY diff --git a/.env.integration b/.env.integration index 1a396b48..62800317 100644 --- a/.env.integration +++ b/.env.integration @@ -46,9 +46,9 @@ HTTPS_COOKIES=false NSQ_URL='http://localhost:4151' # -# Should we enable batch deletions in the current environment? +# Secret key required to perform batch deletions. # -BATCH_DELETE_ENABLED=true +BATCH_DELETION_KEY="00000000-0000-0000-0000-000000000000" # # Two-Factor auth settings. diff --git a/.env.test b/.env.test index c8b8425b..44a5f15c 100644 --- a/.env.test +++ b/.env.test @@ -47,9 +47,9 @@ HTTPS_COOKIES=false NSQ_URL='http://localhost:4151' # -# Should we enable batch deletions in the current environment? +# Secret key required to perform batch deletions. # -BATCH_DELETE_ENABLED=true +BATCH_DELETION_KEY="00000000-0000-0000-0000-000000000000" # # Two-Factor auth settings. diff --git a/.env.travis b/.env.travis index 14ae6f8c..5fdded01 100644 --- a/.env.travis +++ b/.env.travis @@ -46,9 +46,9 @@ HTTPS_COOKIES=false NSQ_URL='http://localhost:4151' # -# Should we enable batch deletions in the current environment? +# Secret key required to perform batch deletions. # -BATCH_DELETE_ENABLED=true +BATCH_DELETION_KEY="00000000-0000-0000-0000-000000000000" # # Two-Factor auth settings. diff --git a/.env.uidemo b/.env.uidemo index 9cd63d06..017bf359 100644 --- a/.env.uidemo +++ b/.env.uidemo @@ -51,9 +51,9 @@ HTTPS_COOKIES=true NSQ_URL='http://localhost:4151' # -# Should we enable batch deletions in the current environment? +# Secret key required to perform batch deletions. # -BATCH_DELETE_ENABLED=true +BATCH_DELETION_KEY="00000000-0000-0000-0000-000000000000" # # Two-Factor auth settings. diff --git a/common/config.go b/common/config.go index ccf2cffe..2c8527ae 100644 --- a/common/config.go +++ b/common/config.go @@ -79,15 +79,15 @@ type RedisConfig struct { } type Config struct { - Cookies *CookieConfig - DB *DBConfig - EnvName string - Logging *LoggingConfig - NsqUrl string - TwoFactor *TwoFactorConfig - Email *EmailConfig - Redis *RedisConfig - BatchDeleteEnabled bool + Cookies *CookieConfig + DB *DBConfig + EnvName string + Logging *LoggingConfig + NsqUrl string + TwoFactor *TwoFactorConfig + Email *EmailConfig + Redis *RedisConfig + BatchDeletionKey string } // Returns a new config based on APT_ENV @@ -182,8 +182,8 @@ func loadConfig() *Config { FlashCookie: v.GetString("FLASH_COOKIE_NAME"), PrefsCookie: v.GetString("PREFS_COOKIE_NAME"), }, - NsqUrl: nsqUrl, - BatchDeleteEnabled: v.GetBool("BATCH_DELETE_ENABLED"), + NsqUrl: nsqUrl, + BatchDeletionKey: v.GetString("BATCH_DELETION_KEY"), TwoFactor: &TwoFactorConfig{ AuthyAPIKey: v.GetString("AUTHY_API_KEY"), AuthyEnabled: v.GetBool("ENABLE_TWO_FACTOR_AUTHY"), From 047fabdf7299981a2bfef1d8572bec1d6ed69458 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Tue, 23 Jan 2024 13:52:01 -0500 Subject: [PATCH 05/27] Starting batch deletion tests --- app/application.go | 1 + .../intellectual_objects_batch_delete_test.go | 142 ++++++++++++++++++ .../admin/intellectual_objects_controller.go | 93 ++++++++++-- .../intellectual_objects_controller_test.go | 32 ---- 4 files changed, 223 insertions(+), 45 deletions(-) create mode 100644 web/api/admin/intellectual_objects_batch_delete_test.go diff --git a/app/application.go b/app/application.go index 54e08fdc..caa2cc31 100644 --- a/app/application.go +++ b/app/application.go @@ -379,6 +379,7 @@ func initRoutes(router *gin.Engine) { adminAPI.PUT("/objects/update/:id", admin_api.IntellectualObjectUpdate) adminAPI.DELETE("/objects/delete/:id", admin_api.IntellectualObjectDelete) adminAPI.POST("/objects/init_restore/:id", admin_api.IntellectualObjectInitRestore) + adminAPI.POST("/objects/init_batch_delete", admin_api.IntellectualObjectInitBatchDelete) // Premis Events adminAPI.POST("/events/create", admin_api.PremisEventCreate) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go new file mode 100644 index 00000000..2652e8b3 --- /dev/null +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -0,0 +1,142 @@ +package admin_api_test + +import ( + "net/http" + "net/url" + "os" + "strconv" + "testing" + + "github.com/APTrust/registry/common" + "github.com/APTrust/registry/constants" + "github.com/APTrust/registry/db" + admin_api "github.com/APTrust/registry/web/api/admin" + tu "github.com/APTrust/registry/web/testutil" + "github.com/stretchr/testify/require" +) + +func TestObjectBatchDelete(t *testing.T) { + + os.Setenv("APT_ENV", "test") + + err := db.ForceFixtureReload() + require.Nil(t, err) + tu.InitHTTPTests(t) + + idsThatCanBeDeleted := []int64{5, 6, 12, 13} + idAlreadyDeleted := int64(14) + idWithPendingWorkItems := int64(4) + idBelongingToOtherInst := int64(1) + + // validParams := url.Values{} + // validParams.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) + // validParams.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) + // for _, id := range idsThatCanBeDeleted { + // validParams.Add("objectID", strconv.FormatInt(id, 10)) + // } + + validParams := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst2Admin.ID, + ObjectIDs: idsThatCanBeDeleted, + SecretKey: common.Context().Config.BatchDeletionKey, + } + + // Inst admin can request batch deletion, but inst user cannot. + paramsBadRequestorRole := url.Values{} + paramsBadRequestorRole.Set("institutionID", strconv.FormatInt(tu.Inst2User.InstitutionID, 10)) + paramsBadRequestorRole.Set("requestorID", strconv.FormatInt(tu.Inst2User.ID, 10)) + for _, id := range idsThatCanBeDeleted { + paramsBadRequestorRole.Add("objectID", strconv.FormatInt(id, 10)) + } + + // This inst admin is at the wrong institution + paramsBadRequestorInst := url.Values{} + paramsBadRequestorInst.Set("institutionID", strconv.FormatInt(tu.Inst1Admin.InstitutionID, 10)) + paramsBadRequestorInst.Set("requestorID", strconv.FormatInt(tu.Inst1Admin.ID, 10)) + for _, id := range idsThatCanBeDeleted { + paramsBadRequestorInst.Add("objectID", strconv.FormatInt(id, 10)) + } + + // This batch contains one id referring to an object that + // has already been deleted. + paramsBadIdAlreadyDeleted := url.Values{} + paramsBadIdAlreadyDeleted.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) + paramsBadIdAlreadyDeleted.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) + for _, id := range idsThatCanBeDeleted { + paramsBadIdAlreadyDeleted.Add("objectID", strconv.FormatInt(id, 10)) + } + paramsBadIdAlreadyDeleted.Add("objectID", strconv.FormatInt(idAlreadyDeleted, 10)) + + // This batch contains one id that has pending work items. + paramsBadPendingWorkItem := url.Values{} + paramsBadPendingWorkItem.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) + paramsBadPendingWorkItem.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) + for _, id := range idsThatCanBeDeleted { + paramsBadPendingWorkItem.Add("objectID", strconv.FormatInt(id, 10)) + } + paramsBadPendingWorkItem.Add("objectID", strconv.FormatInt(idWithPendingWorkItems, 10)) + + // This batch contains one object that belongs to another institution. + paramsBadOtherInstItem := url.Values{} + paramsBadOtherInstItem.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) + paramsBadOtherInstItem.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) + for _, id := range idsThatCanBeDeleted { + paramsBadOtherInstItem.Add("objectID", strconv.FormatInt(id, 10)) + } + paramsBadOtherInstItem.Add("objectID", strconv.FormatInt(idBelongingToOtherInst, 10)) + + testObjectBatchDeletePermissions(t, validParams) + + // START HERE + + // Test permissions. Only APTrust admin should be allowed to do this. + + // Ensure that we get failure if we include an object + // with a pending WorkItem. + + // Ensure that we get failure if we include an object + // that belongs to another institution. + + // Ensure that we get failure if requestorID belongs + // to an inst user rather than inst admin. + + // Ensure we get success with valid params: + // inst id, user id, object ids. + + // Check post conditions. There should be a deletion + // request with all expected properties and with the + // right list of object IDs. + + // Check the text of the alert. It should include + // all of the object identifiers. + + // Confirm the alert, and then test that the correct + // WorkItems were created and that no spurious work + // items were created. + + // TODO: Create & test bulk delete ENV token from + // parameter store? +} + +func testObjectBatchDeletePermissions(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + + tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect().Status(http.StatusCreated) + + tu.Inst1AdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.Inst2Admin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect().Status(http.StatusForbidden) + + tu.Inst1UserClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.Inst2User.Email). + WithJSON(params). + WithHeader(constants.APIKeyHeader, "password"). + Expect().Status(http.StatusForbidden) + +} diff --git a/web/api/admin/intellectual_objects_controller.go b/web/api/admin/intellectual_objects_controller.go index 681bffd5..a871b9ba 100644 --- a/web/api/admin/intellectual_objects_controller.go +++ b/web/api/admin/intellectual_objects_controller.go @@ -1,7 +1,9 @@ package admin_api import ( - "fmt" + "encoding/json" + "errors" + "io" "net/http" "strconv" @@ -13,6 +15,29 @@ import ( "github.com/gin-gonic/gin" ) +// ObjectBatchDeleteParams contains info about which objects to +// delete in an object batch delete operation. +// +// We use this struct for two reasons: +// +// 1. JSON is easier to craft than a form with a thousand values. +// +// 2. Because the httptest library is lame and cannot properly +// create multiple form values with the same name due to a +// problem with the underlying github.com/ajg/form library. +// It turns all params into a flat map. This is documented. +// So instead of getting objectIds = [1,2,3,4] as url.Values +// would give it to us, we get objectIds.0 = 1, objectIds.1 = 2, +// objectIds.3 = 2, etc. That's worthless in a testing library +// that needs to be able to pass values in the standard format +// that the back end expects. +type ObjectBatchDeleteParams struct { + InstitutionID int64 `json:"institutionId"` + RequestorID int64 `json:"requestorId"` + ObjectIDs []int64 `json:"objectIds"` + SecretKey string `json:"secretKey"` +} + // IntellectualObjectCreate creates a new object record. // // POST /admin-api/v3/objects/create/:institution_id @@ -59,38 +84,80 @@ func IntellectualObjectInitRestore(c *gin.Context) { // multiple objects. This request must be approved by an administrator // at the depositing institution before the deletion will actually be queued. // -// POST /objects/init_batch_delete/:id +// Note that becaue this is part of the admin API, access to this call +// is restricted to APTrust admins. +// +// POST /objects/init_batch_delete func IntellectualObjectInitBatchDelete(c *gin.Context) { - if !common.Context().Config.BatchDeleteEnabled { - common.Context().Log.Error().Msgf("Rejecting attempt to run batch deletion when config says BatchDeleteEnabled=false") - api.AbortIfError(c, fmt.Errorf("No!")) + req := api.NewRequest(c) + + // We want to log this because this is a dangerous operation. + // We should never hit this line unless the request was submitted + // by an APTrust admin. + common.Context().Log.Warn().Msgf("Got batch deletion request from user %s, IP address %s", req.CurrentUser.Email, c.RemoteIP()) + + // If the batch deletion key is not set in the config, bail + // because this is unsafe. + if !common.LooksLikeUUID(common.Context().Config.BatchDeletionKey) { + message := "Configuration setting for BatchDeletionKey is missing or invalid" + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Rejecting object batch deletion request: %s", message) + api.AbortIfError(c, errors.New(message)) return } - req := api.NewRequest(c) - objectIDs, err := StringSliceToInt64Slice(c.Request.PostForm["objectID"]) + + // The request body will be JSON, not a normal post form. + // See note on ObjectBatchDeleteParams above. + // First, read the request body into a byte slice. + requestJson, err := io.ReadAll(c.Request.Body) if api.AbortIfError(c, err) { + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Could not read JSON from request body: %v", err) return } - institutionID, err := strconv.ParseInt(c.Request.PostFormValue("institutionID"), 10, 64) + + // Now parse the json bytes. + params := ObjectBatchDeleteParams{} + err = json.Unmarshal(requestJson, ¶ms) if api.AbortIfError(c, err) { + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Error parsing JSON from request body: %v", err) return } - requestorID, err := strconv.ParseInt(c.Request.PostFormValue("requestorID"), 10, 64) - if api.AbortIfError(c, err) { + + // OK, if the request doesn't include the secret key, reject it. + // We don't want anyone maliciously deleting files. + if params.SecretKey != common.Context().Config.BatchDeletionKey { + message := "Request secret key does not match configuration's BatchDeletionKey" + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Rejecting object batch deletion request: %s", message) + api.AbortIfError(c, errors.New(message)) return } - common.Context().Log.Warn().Msgf("Creating batch deletion request on behalf of user %d for %d objects belonging to institution %d. Current user is %s.", - requestorID, len(objectIDs), institutionID, req.CurrentUser.Email) + // If we made it this far, we're going to proceed with the request. + // Let the logs know. + common.Context().Log.Warn().Msgf("IntellectualObjectInitBatchDelete: Creating batch deletion request on behalf of user %d for %d objects belonging to institution %d. Current user is %s.", + params.RequestorID, len(params.ObjectIDs), params.InstitutionID, req.CurrentUser.Email) - del, err := webui.NewDeletionForObjectBatch(requestorID, institutionID, objectIDs, req.BaseURL()) + // Create the batch deletion request. Note that this will fail if + // certain internal checks fail. E.g. RequestorID does not belong + // to an inst admin, one or more files in the batch belongs to another + // institution, or has already been deleted. + del, err := webui.NewDeletionForObjectBatch(params.RequestorID, params.InstitutionID, params.ObjectIDs, req.BaseURL()) if api.AbortIfError(c, err) { + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Creating batch deletion failed: %v", err) return } + + // Now create the alert email to the institutional admin so they + // can review and approve or cancel the request. This last line + // of human intervention ensures batch deletions won't happen + // silently or without explicit approval. _, err = del.CreateRequestAlert() if api.AbortIfError(c, err) { + common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Batch deletion created, but creation of confirmation email failed: %v", err) return } + + // Now send the JSON response. This will be a pretty hefty chunk of JSON, + // since it will include an object record for each object in the batch. c.JSON(http.StatusCreated, del.DeletionRequest) } diff --git a/web/api/admin/intellectual_objects_controller_test.go b/web/api/admin/intellectual_objects_controller_test.go index 6fe004d1..009d5096 100644 --- a/web/api/admin/intellectual_objects_controller_test.go +++ b/web/api/admin/intellectual_objects_controller_test.go @@ -291,35 +291,3 @@ func TestObjectInitRestore(t *testing.T) { WithHeader(constants.APIKeyHeader, "password"). Expect().Status(http.StatusForbidden) } - -func TestObjectBatchDelete(t *testing.T) { - // START HERE - - // Test permissions. Only APTrust admin should be allowed to do this. - - // Ensure that we get failure if we include an object - // with a pending WorkItem. - - // Ensure that we get failure if we include an object - // that belongs to another institution. - - // Ensure that we get failure if requestorID belongs - // to an inst user rather than inst admin. - - // Ensure we get success with valid params: - // inst id, user id, object ids. - - // Check post conditions. There should be a deletion - // request with all expected properties and with the - // right list of object IDs. - - // Check the text of the alert. It should include - // all of the object identifiers. - - // Confirm the alert, and then test that the correct - // WorkItems were created and that no spurious work - // items were created. - - // TODO: Create & test bulk delete ENV token from - // parameter store? -} From e0c891d8aa5f0e7d23478766af3aca4f6e82c20b Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Tue, 23 Jan 2024 14:50:10 -0500 Subject: [PATCH 06/27] Started work on detailed batch deletion tests --- .../intellectual_objects_batch_delete_test.go | 203 ++++++++++++------ 1 file changed, 142 insertions(+), 61 deletions(-) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 2652e8b3..0a6f7c04 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -1,40 +1,37 @@ package admin_api_test import ( + "encoding/json" + "fmt" + "io" "net/http" - "net/url" - "os" - "strconv" "testing" "github.com/APTrust/registry/common" "github.com/APTrust/registry/constants" "github.com/APTrust/registry/db" + "github.com/APTrust/registry/pgmodels" admin_api "github.com/APTrust/registry/web/api/admin" tu "github.com/APTrust/registry/web/testutil" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestObjectBatchDelete(t *testing.T) { - os.Setenv("APT_ENV", "test") + //os.Setenv("APT_ENV", "test") err := db.ForceFixtureReload() require.Nil(t, err) tu.InitHTTPTests(t) idsThatCanBeDeleted := []int64{5, 6, 12, 13} - idAlreadyDeleted := int64(14) - idWithPendingWorkItems := int64(4) - idBelongingToOtherInst := int64(1) - - // validParams := url.Values{} - // validParams.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) - // validParams.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) - // for _, id := range idsThatCanBeDeleted { - // validParams.Add("objectID", strconv.FormatInt(id, 10)) - // } + // idAlreadyDeleted := int64(14) + // idWithPendingWorkItems := int64(4) + // idBelongingToOtherInst := int64(1) + // These params are valid and will create a bulk deletion request + // if submitted by APTrust admin user. validParams := admin_api.ObjectBatchDeleteParams{ InstitutionID: tu.Inst2Admin.InstitutionID, RequestorID: tu.Inst2Admin.ID, @@ -42,51 +39,49 @@ func TestObjectBatchDelete(t *testing.T) { SecretKey: common.Context().Config.BatchDeletionKey, } - // Inst admin can request batch deletion, but inst user cannot. - paramsBadRequestorRole := url.Values{} - paramsBadRequestorRole.Set("institutionID", strconv.FormatInt(tu.Inst2User.InstitutionID, 10)) - paramsBadRequestorRole.Set("requestorID", strconv.FormatInt(tu.Inst2User.ID, 10)) - for _, id := range idsThatCanBeDeleted { - paramsBadRequestorRole.Add("objectID", strconv.FormatInt(id, 10)) - } + // // Inst admin can request batch deletion, but inst user cannot. + // paramsBadRequestorRole := admin_api.ObjectBatchDeleteParams{ + // InstitutionID: tu.Inst2User.InstitutionID, + // RequestorID: tu.Inst2User.ID, + // ObjectIDs: idsThatCanBeDeleted, + // SecretKey: common.Context().Config.BatchDeletionKey, + // } - // This inst admin is at the wrong institution - paramsBadRequestorInst := url.Values{} - paramsBadRequestorInst.Set("institutionID", strconv.FormatInt(tu.Inst1Admin.InstitutionID, 10)) - paramsBadRequestorInst.Set("requestorID", strconv.FormatInt(tu.Inst1Admin.ID, 10)) - for _, id := range idsThatCanBeDeleted { - paramsBadRequestorInst.Add("objectID", strconv.FormatInt(id, 10)) - } + // // This inst admin is at the wrong institution + // paramsBadRequestorInst := admin_api.ObjectBatchDeleteParams{ + // InstitutionID: tu.Inst1Admin.InstitutionID, + // RequestorID: tu.Inst1Admin.ID, + // ObjectIDs: idsThatCanBeDeleted, + // SecretKey: common.Context().Config.BatchDeletionKey, + // } - // This batch contains one id referring to an object that - // has already been deleted. - paramsBadIdAlreadyDeleted := url.Values{} - paramsBadIdAlreadyDeleted.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) - paramsBadIdAlreadyDeleted.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) - for _, id := range idsThatCanBeDeleted { - paramsBadIdAlreadyDeleted.Add("objectID", strconv.FormatInt(id, 10)) - } - paramsBadIdAlreadyDeleted.Add("objectID", strconv.FormatInt(idAlreadyDeleted, 10)) - - // This batch contains one id that has pending work items. - paramsBadPendingWorkItem := url.Values{} - paramsBadPendingWorkItem.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) - paramsBadPendingWorkItem.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) - for _, id := range idsThatCanBeDeleted { - paramsBadPendingWorkItem.Add("objectID", strconv.FormatInt(id, 10)) - } - paramsBadPendingWorkItem.Add("objectID", strconv.FormatInt(idWithPendingWorkItems, 10)) - - // This batch contains one object that belongs to another institution. - paramsBadOtherInstItem := url.Values{} - paramsBadOtherInstItem.Set("institutionID", strconv.FormatInt(tu.Inst2Admin.InstitutionID, 10)) - paramsBadOtherInstItem.Set("requestorID", strconv.FormatInt(tu.Inst2Admin.ID, 10)) - for _, id := range idsThatCanBeDeleted { - paramsBadOtherInstItem.Add("objectID", strconv.FormatInt(id, 10)) - } - paramsBadOtherInstItem.Add("objectID", strconv.FormatInt(idBelongingToOtherInst, 10)) + // // This batch contains one id referring to an object that + // // has already been deleted. + // paramsBadIdAlreadyDeleted := admin_api.ObjectBatchDeleteParams{ + // InstitutionID: tu.Inst2Admin.InstitutionID, + // RequestorID: tu.Inst2Admin.ID, + // ObjectIDs: append(idsThatCanBeDeleted, idAlreadyDeleted), + // SecretKey: common.Context().Config.BatchDeletionKey, + // } + + // // This batch contains one id that has pending work items. + // paramsBadPendingWorkItem := admin_api.ObjectBatchDeleteParams{ + // InstitutionID: tu.Inst2Admin.InstitutionID, + // RequestorID: tu.Inst2Admin.ID, + // ObjectIDs: append(idsThatCanBeDeleted, idWithPendingWorkItems), + // SecretKey: common.Context().Config.BatchDeletionKey, + // } + + // // This batch contains one object that belongs to another institution. + // paramsBadOtherInstItem := admin_api.ObjectBatchDeleteParams{ + // InstitutionID: tu.Inst2Admin.InstitutionID, + // RequestorID: tu.Inst2Admin.ID, + // ObjectIDs: append(idsThatCanBeDeleted, idBelongingToOtherInst), + // SecretKey: common.Context().Config.BatchDeletionKey, + // } testObjectBatchDeletePermissions(t, validParams) + testObjectBatchDeleteCreatesExpectedRecords(t, validParams) // START HERE @@ -121,22 +116,108 @@ func TestObjectBatchDelete(t *testing.T) { func testObjectBatchDeletePermissions(t *testing.T, params admin_api.ObjectBatchDeleteParams) { - tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). - WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). - WithHeader(constants.APIKeyHeader, "password"). - WithJSON(params). - Expect().Status(http.StatusCreated) - + // No institutional admin can create a bulk deletion request. Period. tu.Inst1AdminClient.POST("/admin-api/v3/objects/init_batch_delete"). WithHeader(constants.APIUserHeader, tu.Inst2Admin.Email). WithHeader(constants.APIKeyHeader, "password"). WithJSON(params). Expect().Status(http.StatusForbidden) + // No institutional user can create a bulk deletion request. Period. tu.Inst1UserClient.POST("/admin-api/v3/objects/init_batch_delete"). WithHeader(constants.APIUserHeader, tu.Inst2User.Email). WithJSON(params). WithHeader(constants.APIKeyHeader, "password"). Expect().Status(http.StatusForbidden) + // APTrust SysAdmin can create bulk deletion requests on behalf + // of a local institutional admin. + tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect().Status(http.StatusCreated) + +} + +// Run this test with validaParams. All other param sets are invalid +// and will fail. +func testObjectBatchDeleteCreatesExpectedRecords(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + + // Make sure we got the expected status. + resp.Status(http.StatusCreated) + respData, err := io.ReadAll(resp.Raw().Body) + resp.Raw().Body.Close() + require.NoError(t, err) + + deletionRequest := &pgmodels.DeletionRequest{} + err = json.Unmarshal(respData, deletionRequest) + require.NoError(t, err) + + // Make sure the deletion request has the right info. + // We're not testing RequestedByID here because we don't + // serialize that value to JSON. I can't remember why not, + // but I think it's to keep from exposing user IDs. We + // will test RequestedByID below, when we retrieve the + // database record. + assert.Equal(t, params.InstitutionID, deletionRequest.InstitutionID) + assert.Equal(t, len(params.ObjectIDs), len(deletionRequest.IntellectualObjects)) + for _, objId := range params.ObjectIDs { + found := false + for _, obj := range deletionRequest.IntellectualObjects { + if obj.ID == objId { + found = true + break + } + } + assert.True(t, found, objId) + } + + // Make sure this deletion request was saved to the DB as well. + savedDelRequest, err := pgmodels.DeletionRequestByID(deletionRequest.ID) + require.NoError(t, err) + require.NotNil(t, savedDelRequest) + assert.Equal(t, params.InstitutionID, savedDelRequest.InstitutionID) + assert.Equal(t, params.RequestorID, savedDelRequest.RequestedByID) + assert.Equal(t, len(params.ObjectIDs), len(savedDelRequest.IntellectualObjects)) + for _, objId := range params.ObjectIDs { + found := false + for _, obj := range savedDelRequest.IntellectualObjects { + if obj.ID == objId { + found = true + break + } + } + assert.True(t, found, objId) + } + + // Because this deletion request has not yet been approved, + // there should be no associated WorkItem. We create the WorkItem + // only after the deletion request has been approved by the + // local institutional admin. + assert.Empty(t, deletionRequest.WorkItemID) + + // Make sure the deletion request alert was created in the DB. + query := pgmodels.NewQuery().Where("deletion_request_id", "=", deletionRequest.ID) + alerts, err := pgmodels.AlertSelect(query) + require.NoError(t, err) + assert.Equal(t, 1, len(alerts)) + alert := alerts[0] + assert.Equal(t, params.InstitutionID, alert.InstitutionID) + assert.Equal(t, "Deletion Requested", alert.Subject) + assert.Equal(t, "Deletion Requested", alert.Type) + + // The link to review the deletion request should also contain a token, + // but we don't know offhand what it is. Other tests dig into this. + // Here, we just want to be sure we're sending the right email message. + expectedReviewLink := fmt.Sprintf("http://localhost/deletions/review/%d?token=", deletionRequest.ID) + assert.Contains(t, alert.Content, expectedReviewLink) + + // Should be no work items because request has not yet been approved. + assert.Empty(t, alert.WorkItems) } From 7fc354a3c18fa0653b32ae1e4ecd023bd0bd834e Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 08:35:05 -0500 Subject: [PATCH 07/27] Working on batch deletion tests --- .../intellectual_objects_batch_delete_test.go | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 0a6f7c04..df38799c 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "os" "testing" "github.com/APTrust/registry/common" @@ -19,7 +20,7 @@ import ( func TestObjectBatchDelete(t *testing.T) { - //os.Setenv("APT_ENV", "test") + os.Setenv("APT_ENV", "test") err := db.ForceFixtureReload() require.Nil(t, err) @@ -27,7 +28,7 @@ func TestObjectBatchDelete(t *testing.T) { idsThatCanBeDeleted := []int64{5, 6, 12, 13} // idAlreadyDeleted := int64(14) - // idWithPendingWorkItems := int64(4) + idWithPendingWorkItems := int64(4) // idBelongingToOtherInst := int64(1) // These params are valid and will create a bulk deletion request @@ -64,13 +65,13 @@ func TestObjectBatchDelete(t *testing.T) { // SecretKey: common.Context().Config.BatchDeletionKey, // } - // // This batch contains one id that has pending work items. - // paramsBadPendingWorkItem := admin_api.ObjectBatchDeleteParams{ - // InstitutionID: tu.Inst2Admin.InstitutionID, - // RequestorID: tu.Inst2Admin.ID, - // ObjectIDs: append(idsThatCanBeDeleted, idWithPendingWorkItems), - // SecretKey: common.Context().Config.BatchDeletionKey, - // } + // This batch contains one id that has pending work items. + paramsBadPendingWorkItem := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst2Admin.ID, + ObjectIDs: append(idsThatCanBeDeleted, idWithPendingWorkItems), + SecretKey: common.Context().Config.BatchDeletionKey, + } // // This batch contains one object that belongs to another institution. // paramsBadOtherInstItem := admin_api.ObjectBatchDeleteParams{ @@ -80,15 +81,15 @@ func TestObjectBatchDelete(t *testing.T) { // SecretKey: common.Context().Config.BatchDeletionKey, // } + // Test permissions. Only APTrust admin should be allowed to do this. testObjectBatchDeletePermissions(t, validParams) - testObjectBatchDeleteCreatesExpectedRecords(t, validParams) - // START HERE - - // Test permissions. Only APTrust admin should be allowed to do this. + // Make sure a successful request creates the expected records in the DB. + testObjectBatchDeleteCreatesExpectedRecords(t, validParams) // Ensure that we get failure if we include an object // with a pending WorkItem. + testObjectBatchDeleteWithPendingWorkItem(t, paramsBadPendingWorkItem) // Ensure that we get failure if we include an object // that belongs to another institution. @@ -179,6 +180,8 @@ func testObjectBatchDeleteCreatesExpectedRecords(t *testing.T, params admin_api. } // Make sure this deletion request was saved to the DB as well. + // Also ensure that the request is linked to all four requested + // objects, and no other objects. savedDelRequest, err := pgmodels.DeletionRequestByID(deletionRequest.ID) require.NoError(t, err) require.NotNil(t, savedDelRequest) @@ -221,3 +224,13 @@ func testObjectBatchDeleteCreatesExpectedRecords(t *testing.T, params admin_api. // Should be no work items because request has not yet been approved. assert.Empty(t, alert.WorkItems) } + +func testObjectBatchDeleteWithPendingWorkItem(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusConflict) + assert.Equal(t, `{"StatusCode":409,"Error":"task cannot be completed because this object has pending work items"}`, resp.Body().Raw()) +} From c5368f3d879656d1c0c65c3b4e30c009bccbee04 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 08:46:19 -0500 Subject: [PATCH 08/27] More batch deletion testing --- common/errors.go | 8 ++++++ .../intellectual_objects_batch_delete_test.go | 25 +++++++++++++------ web/api/errors.go | 3 ++- web/webui/deletion.go | 4 +-- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/common/errors.go b/common/errors.go index bfc3dda6..fbf35031 100644 --- a/common/errors.go +++ b/common/errors.go @@ -144,6 +144,14 @@ var ErrCountTypeNotSupported = errors.New("type is not supported for view count" // but they shouldn't be doing it in this context. var ErrMustCompleteReset = errors.New("you must complete your own password reset") +// ErrInvalidObjectID occurs when requesting a batch deletion that +// contains one or more invalid object ids. +var ErrInvalidObjectID = errors.New("one or more object ids is invalid") + +// ErrInvalidRequestorID occurs when APTrust admin submits batch delete +// request on behalf of a user who is not allowed to initiate a batch deletion. +var ErrInvalidRequestorID = errors.New("invalid requestor id") + type ValidationError struct { Errors map[string]string } diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index df38799c..6446cac6 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -29,7 +29,7 @@ func TestObjectBatchDelete(t *testing.T) { idsThatCanBeDeleted := []int64{5, 6, 12, 13} // idAlreadyDeleted := int64(14) idWithPendingWorkItems := int64(4) - // idBelongingToOtherInst := int64(1) + idBelongingToOtherInst := int64(1) // These params are valid and will create a bulk deletion request // if submitted by APTrust admin user. @@ -74,12 +74,12 @@ func TestObjectBatchDelete(t *testing.T) { } // // This batch contains one object that belongs to another institution. - // paramsBadOtherInstItem := admin_api.ObjectBatchDeleteParams{ - // InstitutionID: tu.Inst2Admin.InstitutionID, - // RequestorID: tu.Inst2Admin.ID, - // ObjectIDs: append(idsThatCanBeDeleted, idBelongingToOtherInst), - // SecretKey: common.Context().Config.BatchDeletionKey, - // } + paramsBadOtherInstItem := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst2Admin.ID, + ObjectIDs: append(idsThatCanBeDeleted, idBelongingToOtherInst), + SecretKey: common.Context().Config.BatchDeletionKey, + } // Test permissions. Only APTrust admin should be allowed to do this. testObjectBatchDeletePermissions(t, validParams) @@ -93,6 +93,7 @@ func TestObjectBatchDelete(t *testing.T) { // Ensure that we get failure if we include an object // that belongs to another institution. + testObjectBatchDeleteWithOtherInstItem(t, paramsBadOtherInstItem) // Ensure that we get failure if requestorID belongs // to an inst user rather than inst admin. @@ -234,3 +235,13 @@ func testObjectBatchDeleteWithPendingWorkItem(t *testing.T, params admin_api.Obj resp.Status(http.StatusConflict) assert.Equal(t, `{"StatusCode":409,"Error":"task cannot be completed because this object has pending work items"}`, resp.Body().Raw()) } + +func testObjectBatchDeleteWithOtherInstItem(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusBadRequest) + assert.Equal(t, `{"StatusCode":400,"Error":"one or more object ids is invalid"}`, resp.Body().Raw()) +} diff --git a/web/api/errors.go b/web/api/errors.go index 8635c5ec..cff33502 100644 --- a/web/api/errors.go +++ b/web/api/errors.go @@ -61,7 +61,8 @@ func StatusCodeForError(err error) (status int) { status = http.StatusInternalServerError case common.ErrPendingWorkItems: status = http.StatusConflict - case common.ErrWrongDataType, common.ErrIDMismatch, common.ErrInstIDChange, common.ErrIdentifierChange, common.ErrStorageOptionChange, common.ErrDecodeCookie: + case common.ErrWrongDataType, common.ErrIDMismatch, common.ErrInstIDChange, common.ErrIdentifierChange, + common.ErrStorageOptionChange, common.ErrDecodeCookie, common.ErrInvalidObjectID, common.ErrInvalidRequestorID: status = http.StatusBadRequest default: status = http.StatusInternalServerError diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 8f79a648..623839b7 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -96,7 +96,7 @@ func NewDeletionForObjectBatch(requestorID, institutionID int64, objIDs []int64, } if requestingUser.InstitutionID != institutionID || requestingUser.Role != constants.RoleInstAdmin { common.Context().Log.Error().Msgf("Requesting user %d is not admin at institution %d. Rejecting bulk deletion request.", requestorID, institutionID) - return nil, fmt.Errorf("invalid requestor id") + return nil, common.ErrInvalidRequestorID } // Make sure that all objects belong to the specified institution. @@ -107,7 +107,7 @@ func NewDeletionForObjectBatch(requestorID, institutionID int64, objIDs []int64, if validObjectCount != len(objIDs) { common.Context().Log.Error().Msgf("Batch deletion requested for %d objects, of which only %d are valid. InstitutionID = %d. Current user = %s. IDs: %v", len(objIDs), validObjectCount, institutionID, requestingUser.Email, objIDs) - return nil, fmt.Errorf("one or more object ids is invalid") + return nil, common.ErrInvalidObjectID } // Make sure there are no pending work items for these objects. From f127876d2f817bff73372ade4115b284698f251e Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 09:03:10 -0500 Subject: [PATCH 09/27] More batch deletion tests --- .../intellectual_objects_batch_delete_test.go | 65 +++++++++++++------ 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 6446cac6..6729e77b 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -40,21 +40,23 @@ func TestObjectBatchDelete(t *testing.T) { SecretKey: common.Context().Config.BatchDeletionKey, } - // // Inst admin can request batch deletion, but inst user cannot. - // paramsBadRequestorRole := admin_api.ObjectBatchDeleteParams{ - // InstitutionID: tu.Inst2User.InstitutionID, - // RequestorID: tu.Inst2User.ID, - // ObjectIDs: idsThatCanBeDeleted, - // SecretKey: common.Context().Config.BatchDeletionKey, - // } + // Inst admin can request batch deletion, but inst user cannot. + paramsBadRequestorRole := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2User.InstitutionID, + RequestorID: tu.Inst2User.ID, + ObjectIDs: idsThatCanBeDeleted, + SecretKey: common.Context().Config.BatchDeletionKey, + } - // // This inst admin is at the wrong institution - // paramsBadRequestorInst := admin_api.ObjectBatchDeleteParams{ - // InstitutionID: tu.Inst1Admin.InstitutionID, - // RequestorID: tu.Inst1Admin.ID, - // ObjectIDs: idsThatCanBeDeleted, - // SecretKey: common.Context().Config.BatchDeletionKey, - // } + // This inst admin is at the wrong institution. + // He's requesting deletion of items belonging to Inst2, + // but he belongs to Inst1 + paramsBadRequestorInst := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst1Admin.ID, + ObjectIDs: idsThatCanBeDeleted, + SecretKey: common.Context().Config.BatchDeletionKey, + } // // This batch contains one id referring to an object that // // has already been deleted. @@ -84,7 +86,9 @@ func TestObjectBatchDelete(t *testing.T) { // Test permissions. Only APTrust admin should be allowed to do this. testObjectBatchDeletePermissions(t, validParams) - // Make sure a successful request creates the expected records in the DB. + // Ensure we get success with valid params and + // make sure a successful request creates the expected + // records in the DB. testObjectBatchDeleteCreatesExpectedRecords(t, validParams) // Ensure that we get failure if we include an object @@ -97,13 +101,12 @@ func TestObjectBatchDelete(t *testing.T) { // Ensure that we get failure if requestorID belongs // to an inst user rather than inst admin. + testObjectBatchDeleteWithWrongRole(t, paramsBadRequestorRole) - // Ensure we get success with valid params: - // inst id, user id, object ids. - - // Check post conditions. There should be a deletion - // request with all expected properties and with the - // right list of object IDs. + // Ensure failure if requestor id belongs to someone at + // a different institution. An institution that does not own + // the objects. + testObjectBatchDeleteWrongRequestorInst(t, paramsBadRequestorInst) // Check the text of the alert. It should include // all of the object identifiers. @@ -245,3 +248,23 @@ func testObjectBatchDeleteWithOtherInstItem(t *testing.T, params admin_api.Objec resp.Status(http.StatusBadRequest) assert.Equal(t, `{"StatusCode":400,"Error":"one or more object ids is invalid"}`, resp.Body().Raw()) } + +func testObjectBatchDeleteWithWrongRole(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusBadRequest) + assert.Equal(t, `{"StatusCode":400,"Error":"invalid requestor id"}`, resp.Body().Raw()) +} + +func testObjectBatchDeleteWrongRequestorInst(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusBadRequest) + assert.Equal(t, `{"StatusCode":400,"Error":"invalid requestor id"}`, resp.Body().Raw()) +} From 746ae2db4b465720104935753c3582fcc0a2148b Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 09:09:55 -0500 Subject: [PATCH 10/27] More batch deletion tests --- .../intellectual_objects_batch_delete_test.go | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 6729e77b..b14f79b4 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -27,7 +27,7 @@ func TestObjectBatchDelete(t *testing.T) { tu.InitHTTPTests(t) idsThatCanBeDeleted := []int64{5, 6, 12, 13} - // idAlreadyDeleted := int64(14) + idAlreadyDeleted := int64(14) idWithPendingWorkItems := int64(4) idBelongingToOtherInst := int64(1) @@ -58,14 +58,14 @@ func TestObjectBatchDelete(t *testing.T) { SecretKey: common.Context().Config.BatchDeletionKey, } - // // This batch contains one id referring to an object that - // // has already been deleted. - // paramsBadIdAlreadyDeleted := admin_api.ObjectBatchDeleteParams{ - // InstitutionID: tu.Inst2Admin.InstitutionID, - // RequestorID: tu.Inst2Admin.ID, - // ObjectIDs: append(idsThatCanBeDeleted, idAlreadyDeleted), - // SecretKey: common.Context().Config.BatchDeletionKey, - // } + // This batch contains one id referring to an object that + // has already been deleted. + paramsBadIdAlreadyDeleted := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst2Admin.ID, + ObjectIDs: append(idsThatCanBeDeleted, idAlreadyDeleted), + SecretKey: common.Context().Config.BatchDeletionKey, + } // This batch contains one id that has pending work items. paramsBadPendingWorkItem := admin_api.ObjectBatchDeleteParams{ @@ -108,15 +108,9 @@ func TestObjectBatchDelete(t *testing.T) { // the objects. testObjectBatchDeleteWrongRequestorInst(t, paramsBadRequestorInst) - // Check the text of the alert. It should include - // all of the object identifiers. - - // Confirm the alert, and then test that the correct - // WorkItems were created and that no spurious work - // items were created. - - // TODO: Create & test bulk delete ENV token from - // parameter store? + // This should fail because one of the objects in the list + // has already been deleted. + testObjectBatchDeleteAlreadyDeleted(t, paramsBadIdAlreadyDeleted) } func testObjectBatchDeletePermissions(t *testing.T, params admin_api.ObjectBatchDeleteParams) { @@ -268,3 +262,13 @@ func testObjectBatchDeleteWrongRequestorInst(t *testing.T, params admin_api.Obje resp.Status(http.StatusBadRequest) assert.Equal(t, `{"StatusCode":400,"Error":"invalid requestor id"}`, resp.Body().Raw()) } + +func testObjectBatchDeleteAlreadyDeleted(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusBadRequest) + assert.Equal(t, `{"StatusCode":400,"Error":"one or more object ids is invalid"}`, resp.Body().Raw()) +} From e1a090b5733f73250dacf282e84406ef572290c8 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 12:40:55 -0500 Subject: [PATCH 11/27] More bulk deletion tests --- .../intellectual_objects_batch_delete_test.go | 41 ++++++++++++++++++- .../admin/intellectual_objects_controller.go | 2 +- web/api/errors.go | 3 +- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index b14f79b4..f4c9e41d 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -40,6 +40,15 @@ func TestObjectBatchDelete(t *testing.T) { SecretKey: common.Context().Config.BatchDeletionKey, } + // These params are valid, except for the batch deletion + // secret key. + paramsBadKey := admin_api.ObjectBatchDeleteParams{ + InstitutionID: tu.Inst2Admin.InstitutionID, + RequestorID: tu.Inst2Admin.ID, + ObjectIDs: idsThatCanBeDeleted, + SecretKey: "this-key-is-incorrect", + } + // Inst admin can request batch deletion, but inst user cannot. paramsBadRequestorRole := admin_api.ObjectBatchDeleteParams{ InstitutionID: tu.Inst2User.InstitutionID, @@ -75,7 +84,7 @@ func TestObjectBatchDelete(t *testing.T) { SecretKey: common.Context().Config.BatchDeletionKey, } - // // This batch contains one object that belongs to another institution. + // This batch contains one object that belongs to another institution. paramsBadOtherInstItem := admin_api.ObjectBatchDeleteParams{ InstitutionID: tu.Inst2Admin.InstitutionID, RequestorID: tu.Inst2Admin.ID, @@ -111,6 +120,13 @@ func TestObjectBatchDelete(t *testing.T) { // This should fail because one of the objects in the list // has already been deleted. testObjectBatchDeleteAlreadyDeleted(t, paramsBadIdAlreadyDeleted) + + // Sending a bad deletion key should cause failure + testObjectBatchDeleteWithBadKey(t, paramsBadKey) + + // If deletion key is not set in config, request should fail even if + // params are valid. + testObjectBatchDeleteSystemKeyMissing(t, validParams) } func testObjectBatchDeletePermissions(t *testing.T, params admin_api.ObjectBatchDeleteParams) { @@ -272,3 +288,26 @@ func testObjectBatchDeleteAlreadyDeleted(t *testing.T, params admin_api.ObjectBa resp.Status(http.StatusBadRequest) assert.Equal(t, `{"StatusCode":400,"Error":"one or more object ids is invalid"}`, resp.Body().Raw()) } + +func testObjectBatchDeleteWithBadKey(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusBadRequest) + assert.Equal(t, `{"StatusCode":400,"Error":"invalid token"}`, resp.Body().Raw()) +} + +func testObjectBatchDeleteSystemKeyMissing(t *testing.T, params admin_api.ObjectBatchDeleteParams) { + deletionKey := common.Context().Config.BatchDeletionKey + defer func() { common.Context().Config.BatchDeletionKey = deletionKey }() + common.Context().Config.BatchDeletionKey = "" + resp := tu.SysAdminClient.POST("/admin-api/v3/objects/init_batch_delete"). + WithHeader(constants.APIUserHeader, tu.SysAdmin.Email). + WithHeader(constants.APIKeyHeader, "password"). + WithJSON(params). + Expect() + resp.Status(http.StatusInternalServerError) + assert.Equal(t, `{"StatusCode":500,"Error":"Configuration setting for BatchDeletionKey is missing or invalid"}`, resp.Body().Raw()) +} diff --git a/web/api/admin/intellectual_objects_controller.go b/web/api/admin/intellectual_objects_controller.go index a871b9ba..6c0ed93a 100644 --- a/web/api/admin/intellectual_objects_controller.go +++ b/web/api/admin/intellectual_objects_controller.go @@ -127,7 +127,7 @@ func IntellectualObjectInitBatchDelete(c *gin.Context) { if params.SecretKey != common.Context().Config.BatchDeletionKey { message := "Request secret key does not match configuration's BatchDeletionKey" common.Context().Log.Error().Msgf("IntellectualObjectInitBatchDelete: Rejecting object batch deletion request: %s", message) - api.AbortIfError(c, errors.New(message)) + api.AbortIfError(c, common.ErrInvalidToken) return } diff --git a/web/api/errors.go b/web/api/errors.go index cff33502..0c37fd8f 100644 --- a/web/api/errors.go +++ b/web/api/errors.go @@ -62,7 +62,8 @@ func StatusCodeForError(err error) (status int) { case common.ErrPendingWorkItems: status = http.StatusConflict case common.ErrWrongDataType, common.ErrIDMismatch, common.ErrInstIDChange, common.ErrIdentifierChange, - common.ErrStorageOptionChange, common.ErrDecodeCookie, common.ErrInvalidObjectID, common.ErrInvalidRequestorID: + common.ErrStorageOptionChange, common.ErrDecodeCookie, common.ErrInvalidObjectID, + common.ErrInvalidRequestorID, common.ErrInvalidToken: status = http.StatusBadRequest default: status = http.StatusInternalServerError From a0b881d43b6af43d71e75f9cd4f54f21a1ebff99 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 15:38:47 -0500 Subject: [PATCH 12/27] Interim commit. Working on bulk delete. --- db/fixtures/deletion_requests.csv | 8 +- db/fixtures/work_items.csv | 66 +++++++-------- .../011_batch_deletion_work_items.sql | 78 ++++++++++++++++++ member_api_v3.yml | 28 +------ pgmodels/deletion_request.go | 18 ++--- pgmodels/deletion_request_view.go | 16 ++-- pgmodels/deletion_request_view_test.go | 18 +++-- pgmodels/generic_file_test.go | 5 +- pgmodels/json_types.go | 2 - pgmodels/json_types_test.go | 5 +- pgmodels/work_item.go | 1 + .../intellectual_objects_batch_delete_test.go | 4 - web/webui/deletion.go | 81 +++++++++++-------- web/webui/deletion_requests_controller.go | 2 +- .../deletion_requests_controller_test.go | 4 +- web/webui/deletion_test.go | 5 +- 16 files changed, 202 insertions(+), 139 deletions(-) create mode 100644 db/migrations/011_batch_deletion_work_items.sql diff --git a/db/fixtures/deletion_requests.csv b/db/fixtures/deletion_requests.csv index 85f89411..5a8392a8 100644 --- a/db/fixtures/deletion_requests.csv +++ b/db/fixtures/deletion_requests.csv @@ -1,4 +1,4 @@ -"id","institution_id","requested_by_id","requested_at","encrypted_confirmation_token","confirmed_by_id","confirmed_at","cancelled_by_id","cancelled_at","work_item_id" -1,2,3,2021-05-13 11:10:32,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,,,,, -2,2,3,2021-05-13 11:10:33,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,2,2021-05-13 11:10:33,,, -3,2,3,2021-05-13 11:10:34,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,,,2,2021-05-13 11:10:34, +"id","institution_id","requested_by_id","requested_at","encrypted_confirmation_token","confirmed_by_id","confirmed_at","cancelled_by_id","cancelled_at" +1,2,3,2021-05-13 11:10:32,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,,,, +2,2,3,2021-05-13 11:10:33,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,2,2021-05-13 11:10:33,, +3,2,3,2021-05-13 11:10:34,$2a$10$TK8s1XnmWulSUdze8GN5uOgGmDDsnndQKF5/Rz1j0xaHT7AwXRVma,,,2,2021-05-13 11:10:34 diff --git a/db/fixtures/work_items.csv b/db/fixtures/work_items.csv index 2769ccb4..61a9dcf4 100644 --- a/db/fixtures/work_items.csv +++ b/db/fixtures/work_items.csv @@ -1,33 +1,33 @@ -id,created_at,updated_at,intellectual_object_id,generic_file_id,name,etag,bucket,user,note,action,stage,status,outcome,bag_date,date_processed,retry,node,pid,needs_admin_review,institution_id,queued_at,size,stage_started_at,aptrust_approver,inst_approver -1,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_01.tar,1.01010101010101E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -2,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_02.tar,2.02020202020202E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -3,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_03.tar,3.03030303030303E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -4,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_04.tar,4.04040404040404E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -5,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_05.tar,5.05050505050505E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -6,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_06.tar,6.06060606060606E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -7,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_07.tar,7.07070707070707E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -8,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_08.tar,8.08080808080808E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -9,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_09.tar,9.09090909090909E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -10,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_10.tar,1.01010101010101E+019,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,, -11,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_11.tar,1.11111111111111E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -12,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_12.tar,1.21212121212121E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -13,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_13.tar,1.31313131313131E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -14,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_14.tar,1.41414141414141E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -15,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_15.tar,1.51515151515152E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -16,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_16.tar,1.61616161616162E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -17,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_17.tar,1.71717171717172E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -18,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_18.tar,1.81818181818182E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -19,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_19.tar,1.91919191919192E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -20,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_20.tar,2.02020202020202E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,, -21,2016-08-20 16:01:59,2016-08-20 16:01:59,6,,toads.tar,25a4546c865e4073aa17b31b0708f75d,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -22,2016-08-20 16:01:59,2016-08-20 16:01:59,3,,glass_shards.tar,e24527590f31492b8f362c3d3c02ec80,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, -23,2016-08-20 16:01:59,2016-08-20 16:01:59,2,,pdfs.tar,6b5b18c999dc4fdc9c47a2db90782cee,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, -24,2016-08-20 16:01:59,2016-08-20 16:01:59,5,,coal.tar,0940747b1a8a42e78d2639e1093753ff,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -25,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, -26,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,chocolate.tar,6a40fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -27,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,record.tar,8880fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending record,Ingest,Record,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -28,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,cleanup.tar,9990fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending cleanup,Ingest,Cleanup,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -29,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,format.tar,9990fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending format identification,Ingest,Format Identification,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,, -30,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item deleted successfuly,Delete,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, -31,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item restored to https://s3.example.com/photos.tar.,Restore Object,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, -32,2016-08-20 16:01:59,2016-08-20 16:01:59,1,49,glass.tar,dba34a4e5bb944e59c74aefe781a0891,aptrust.receiving.institution1.edu,system@aptrust.org,Restoration requested,Restore Object,Requested,Pending,Ned Flanders? You're the devil?,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,, +id,created_at,updated_at,intellectual_object_id,generic_file_id,name,etag,bucket,user,note,action,stage,status,outcome,bag_date,date_processed,retry,node,pid,needs_admin_review,institution_id,queued_at,size,stage_started_at,aptrust_approver,inst_approver,deletion_request_id +1,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_01.tar,1.01010101010101E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +2,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_02.tar,2.02020202020202E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +3,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_03.tar,3.03030303030303E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +4,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_04.tar,4.04040404040404E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +5,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_05.tar,5.05050505050505E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +6,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_06.tar,6.06060606060606E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +7,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_07.tar,7.07070707070707E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +8,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_08.tar,8.08080808080808E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +9,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_09.tar,9.09090909090909E+018,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +10,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_10.tar,1.01010101010101E+019,aptrust.receiving.institution1.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,2,2016-08-30 15:41:45,,,,, +11,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_11.tar,1.11111111111111E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +12,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_12.tar,1.21212121212121E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +13,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_13.tar,1.31313131313131E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +14,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_14.tar,1.41414141414141E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +15,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_15.tar,1.51515151515152E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +16,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_16.tar,1.61616161616162E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +17,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_17.tar,1.71717171717172E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +18,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_18.tar,1.81818181818182E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +19,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_19.tar,1.91919191919192E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +20,2016-08-30 15:41:45,2016-08-30 15:41:45,,,fake_bag_20.tar,2.02020202020202E+019,aptrust.receiving.institution2.edu,system@aptrust.org,Item is in receiving bucket,Ingest,Receive,Pending,Item is awaiting ingest,2016-08-25 20:04:06,2016-08-25 20:04:26,true,,0,false,3,2016-08-30 15:41:45,,,,, +21,2016-08-20 16:01:59,2016-08-20 16:01:59,6,,toads.tar,25a4546c865e4073aa17b31b0708f75d,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +22,2016-08-20 16:01:59,2016-08-20 16:01:59,3,,glass_shards.tar,e24527590f31492b8f362c3d3c02ec80,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, +23,2016-08-20 16:01:59,2016-08-20 16:01:59,2,,pdfs.tar,6b5b18c999dc4fdc9c47a2db90782cee,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, +24,2016-08-20 16:01:59,2016-08-20 16:01:59,5,,coal.tar,0940747b1a8a42e78d2639e1093753ff,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +25,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, +26,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,chocolate.tar,6a40fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item ingested successfully,Ingest,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +27,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,record.tar,8880fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending record,Ingest,Record,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +28,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,cleanup.tar,9990fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending cleanup,Ingest,Cleanup,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +29,2016-08-20 16:01:59,2016-08-20 16:01:59,4,,format.tar,9990fff84939474aa6f5f77bf56fb8c8,aptrust.receiving.institution2.edu,system@aptrust.org,Item is pending format identification,Ingest,Format Identification,Pending,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,3,2016-08-30 15:41:45,,,,, +30,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item deleted successfuly,Delete,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, +31,2016-08-20 16:01:59,2016-08-20 16:01:59,1,,photos.tar,1f594a4e5bb944e59c74aefe781a3726,aptrust.receiving.institution1.edu,system@aptrust.org,Item restored to https://s3.example.com/photos.tar.,Restore Object,Cleanup,Success,No problems,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, +32,2016-08-20 16:01:59,2016-08-20 16:01:59,1,49,glass.tar,dba34a4e5bb944e59c74aefe781a0891,aptrust.receiving.institution1.edu,system@aptrust.org,Restoration requested,Restore Object,Requested,Pending,Ned Flanders? You're the devil?,2016-08-24 10:04:06,2016-08-24 10:12:26,false,,0,false,2,2016-08-30 15:41:45,,,,, diff --git a/db/migrations/011_batch_deletion_work_items.sql b/db/migrations/011_batch_deletion_work_items.sql new file mode 100644 index 00000000..772c2cbb --- /dev/null +++ b/db/migrations/011_batch_deletion_work_items.sql @@ -0,0 +1,78 @@ +-- 011_batch_deletion_work_items.sql +-- +-- Allow a single deletion request to map to multiple WorkItems. +-- This supports batch deletions. +-- + +-- Note that we're starting the migration. +insert into schema_migrations ("version", started_at) values ('011_batch_deletion_work_items', now()) +on conflict ("version") do update set started_at = now(); + + +do +$$ +begin + if exists( + select 1 from information_schema.columns + where table_schema = 'public' + and table_name = 'deletion_requests' + and column_name = 'work_item_id') + then + + -- We need to rebuild this view, removing all references + -- to the WorkItems table. + drop view if exists deletion_requests_view; + + create or replace view deletion_requests_view + AS SELECT dr.id, + dr.institution_id, + i.name AS institution_name, + i.identifier AS institution_identifier, + dr.requested_by_id, + req.name AS requested_by_name, + req.email AS requested_by_email, + dr.requested_at, + dr.confirmed_by_id, + conf.name AS confirmed_by_name, + conf.email AS confirmed_by_email, + dr.confirmed_at, + dr.cancelled_by_id, + can.name AS cancelled_by_name, + can.email AS cancelled_by_email, + dr.cancelled_at, + ( SELECT count(*) AS count + FROM deletion_requests_generic_files drgf + WHERE drgf.deletion_request_id = dr.id) AS file_count, + ( SELECT count(*) AS count + FROM deletion_requests_intellectual_objects drio + WHERE drio.deletion_request_id = dr.id) AS object_count + FROM deletion_requests dr + LEFT JOIN institutions i ON dr.institution_id = i.id + LEFT JOIN users req ON dr.requested_by_id = req.id + LEFT JOIN users conf ON dr.confirmed_by_id = conf.id + LEFT JOIN users can ON dr.confirmed_by_id = can.id; + + + + -- Add deletion_request_id to work_items as a nullable + -- foreign key to deletion_requests. + alter table work_items add column deletion_request_id bigint null; + alter table work_items add constraint fk_work_items_deletion_request_id + foreign key (deletion_request_id) references deletion_requests (id); + + -- Copy deletion request ids from legacy requests into the work_items table. + update work_items + set deletion_request_id = dr.id + from work_items wi inner join deletion_requests dr on dr.work_item_id = wi.id + where dr.work_item_id = wi.id; + + -- Now remove the work_item_id column from deletion requests + alter table deletion_requests drop column work_item_id; + + end if; +end +$$; + + +-- Now note that the migration is complete. +update schema_migrations set finished_at = now() where "version" = '011_batch_deletion_work_items'; diff --git a/member_api_v3.yml b/member_api_v3.yml index dc15f14a..7f118555 100644 --- a/member_api_v3.yml +++ b/member_api_v3.yml @@ -275,34 +275,10 @@ components: format: int64 description: The total number of objects to be deleted in this request. If zero, this is a file deletion request. minimum: 0 - work_item_id: - type: integer - format: int64 - description: The ID of the WorkItem for this deletion request. The WorkItem will not exist until the deletion request is approved and will never be created if the deletion request is rejected. - nullable: true - stage: - type: string - description: The stage of the deletion WorkItem, if it exists. This describes how far along the deletion worker is in the deletion process. - enum: ["Requested", "Resolve"] - nullable: true status: type: string - description: The status of the deletion WorkItem, if it exists. This describes whether the deletion operation has started, completed, etc. - enum: ["Cancelled", "Failed", "Pending", "Started", "Success"] - nullable: true - date_processed: - type: string - format: date-time - description: Timestamp of the last activity on this deletion's WorkItem (assuming the deletion was approved). - nullable: true - size: - type: integer - format: int64 - description: Total size, in bytes, of items to be deleted. This will be empty if the deletion request was not approved. - nullable: true - note: - type: string - description: A brief, human-readable summary of the current state of the deletion WorkItem. This will be null if the deletion request was not approved. + description: The status of the deletion request. + enum: ["Approved", "Awaiting Approval", "Rejected"] nullable: true DeletionRequestViewList: properties: diff --git a/pgmodels/deletion_request.go b/pgmodels/deletion_request.go index bd15e21c..b4731435 100644 --- a/pgmodels/deletion_request.go +++ b/pgmodels/deletion_request.go @@ -44,10 +44,9 @@ type DeletionRequest struct { RequestedBy *User `json:"requested_by" pg:"rel:has-one"` ConfirmedBy *User `json:"confirmed_by" pg:"rel:has-one"` CancelledBy *User `json:"cancelled_by" pg:"rel:has-one"` - WorkItemID int64 `json:"work_item_id"` GenericFiles []*GenericFile `json:"generic_files" pg:"many2many:deletion_requests_generic_files"` IntellectualObjects []*IntellectualObject `json:"intellectual_objects" pg:"many2many:deletion_requests_intellectual_objects"` - WorkItem *WorkItem `json:"work_item" pg:"rel:has-one"` + WorkItems []*WorkItem `json:"work_item" pg:"rel:has-many"` } type DeletionRequestsGenericFiles struct { @@ -247,7 +246,7 @@ func (request *DeletionRequest) saveRelations(tx *pg.Tx) error { if err != nil { return err } - return request.saveWorkItem(tx) + return request.saveWorkItems(tx) } func (request *DeletionRequest) saveFiles(tx *pg.Tx) error { @@ -274,18 +273,13 @@ func (request *DeletionRequest) saveObjects(tx *pg.Tx) error { return nil } -func (request *DeletionRequest) saveWorkItem(tx *pg.Tx) error { - if request.WorkItem != nil { - err := request.WorkItem.Save() +func (request *DeletionRequest) saveWorkItems(tx *pg.Tx) error { + for _, item := range request.WorkItems { + item.DeletionRequestID = request.ID + err := item.Save() if err != nil { return err } - if request.WorkItemID == 0 { - request.WorkItemID = request.WorkItem.ID - sql := "update deletion_requests set work_item_id = ? where id = ?" - _, err = tx.Exec(sql, request.WorkItem.ID, request.ID) - } - return err } return nil } diff --git a/pgmodels/deletion_request_view.go b/pgmodels/deletion_request_view.go index 404e95e5..38ff0a43 100644 --- a/pgmodels/deletion_request_view.go +++ b/pgmodels/deletion_request_view.go @@ -34,12 +34,11 @@ type DeletionRequestView struct { CancelledAt time.Time `json:"cancelled_at"` FileCount int64 `json:"file_count"` ObjectCount int64 `json:"object_count"` - WorkItemID int64 `json:"work_item_id"` - Stage string `json:"stage"` - Status string `json:"status"` - DateProcessed time.Time `json:"date_processed"` - Size int64 `json:"size"` - Note string `json:"note"` + // Stage string `json:"stage"` + // Status string `json:"status"` + // DateProcessed time.Time `json:"date_processed"` + // Size int64 `json:"size"` + // Note string `json:"note"` } // DeletionRequestViewByID returns the DeletionRequestView record @@ -72,9 +71,8 @@ func DeletionRequestViewGet(query *Query) (*DeletionRequestView, error) { func (request *DeletionRequestView) DisplayStatus() string { if request.CancelledByID > 0 { return "Rejected" - } - if request.Status != "" { - return request.Status + } else if request.ConfirmedByID > 0 { + return "Approved" } return "Awaiting Approval" } diff --git a/pgmodels/deletion_request_view_test.go b/pgmodels/deletion_request_view_test.go index 06bb5544..dfbacde3 100644 --- a/pgmodels/deletion_request_view_test.go +++ b/pgmodels/deletion_request_view_test.go @@ -3,7 +3,6 @@ package pgmodels_test import ( "testing" - "github.com/APTrust/registry/constants" "github.com/APTrust/registry/db" "github.com/APTrust/registry/pgmodels" @@ -27,12 +26,15 @@ func TestDeletionRequestView(t *testing.T) { func TestDeletionRequestViewDisplayStatus(t *testing.T) { req := &pgmodels.DeletionRequestView{} - for _, status := range constants.Statuses { - req.CancelledByID = 0 - req.Status = status - assert.Equal(t, status, req.DisplayStatus()) + req.CancelledByID = 0 + req.ConfirmedByID = 0 + assert.Equal(t, "Awaiting Approval", req.DisplayStatus()) - req.CancelledByID = 1000 - assert.Equal(t, "Rejected", req.DisplayStatus()) - } + req.CancelledByID = 0 + req.ConfirmedByID = 100 + assert.Equal(t, "Approved", req.DisplayStatus()) + + req.ConfirmedByID = 0 + req.CancelledByID = 1000 + assert.Equal(t, "Rejected", req.DisplayStatus()) } diff --git a/pgmodels/generic_file_test.go b/pgmodels/generic_file_test.go index 060c544f..ce8a095f 100644 --- a/pgmodels/generic_file_test.go +++ b/pgmodels/generic_file_test.go @@ -276,7 +276,7 @@ func TestFileDeletionPreConditions(t *testing.T) { assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item")) testGenericFileDeleteError(t, gf) - testFileDeletionRequest(t, gf, workItem.ID) + testFileDeletionRequest(t, gf) // Request not yet approved err = gf.AssertDeletionPreconditions() @@ -304,7 +304,7 @@ func TestFileDeletionPreConditions(t *testing.T) { testGenericFileDeleteSuccess(t, gf) } -func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItemID int64) { +func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile int64) { // Request doesn't exist yet. reqView, err := gf.DeletionRequest(workItemID) require.NotNil(t, err) @@ -316,7 +316,6 @@ func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItemID req, err := pgmodels.CreateDeletionRequest(nil, files) require.Nil(t, err) require.NotNil(t, req) - req.WorkItemID = workItemID req.ConfirmedByID = 0 require.Nil(t, req.Save()) diff --git a/pgmodels/json_types.go b/pgmodels/json_types.go index 6b902606..d8d63c03 100644 --- a/pgmodels/json_types.go +++ b/pgmodels/json_types.go @@ -56,7 +56,6 @@ type DeletionRequestMin struct { CancelledBy *UserMin `json:"cancelled_by"` GenericFiles []*GenericFile `json:"generic_files"` IntellectualObjects []*IntellectualObject `json:"intellectual_objects"` - WorkItem *WorkItem `json:"work_item"` } // ToMin returns DeletionRequestMin object suitable for serialization @@ -81,6 +80,5 @@ func (r *DeletionRequest) ToMin() *DeletionRequestMin { CancelledBy: cancelledBy, GenericFiles: r.GenericFiles, IntellectualObjects: r.IntellectualObjects, - WorkItem: r.WorkItem, } } diff --git a/pgmodels/json_types_test.go b/pgmodels/json_types_test.go index 81b30fcb..c40f3dbc 100644 --- a/pgmodels/json_types_test.go +++ b/pgmodels/json_types_test.go @@ -50,7 +50,10 @@ func TestDeletionRequestMin(t *testing.T) { pgmodels.RandomObject(), pgmodels.RandomObject(), }, - WorkItem: pgmodels.RandomWorkItem("object/id", constants.ActionDelete, 999, 222), + WorkItems: []*pgmodels.WorkItem{ + pgmodels.RandomWorkItem("object/id1", constants.ActionDelete, 888, 111), + pgmodels.RandomWorkItem("object/id2", constants.ActionDelete, 999, 222), + }, } reqMin := req.ToMin() diff --git a/pgmodels/work_item.go b/pgmodels/work_item.go index d90431bc..d4d588c5 100644 --- a/pgmodels/work_item.go +++ b/pgmodels/work_item.go @@ -67,6 +67,7 @@ type WorkItem struct { StageStartedAt time.Time `json:"stage_started_at"` APTrustApprover string `json:"aptrust_approver" pg:"aptrust_approver"` InstApprover string `json:"inst_approver"` + DeletionRequestID int64 `json:"deletion_request_id"` } // WorkItemByID returns the work item with the specified id. diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index f4c9e41d..880194d4 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "net/http" - "os" "testing" "github.com/APTrust/registry/common" @@ -19,9 +18,6 @@ import ( ) func TestObjectBatchDelete(t *testing.T) { - - os.Setenv("APT_ENV", "test") - err := db.ForceFixtureReload() require.Nil(t, err) tu.InitHTTPTests(t) diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 623839b7..3ec331c2 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -242,55 +242,72 @@ func (del *Deletion) initObjectDeletionRequest(institutionID int64, objIDs []int // CreateWorkItem creates a WorkItem describing this deletion. We call // this only if the admin approves the deletion. -func (del *Deletion) CreateWorkItem() (*pgmodels.WorkItem, error) { - // Create the deletion WorkItem - obj := del.DeletionRequest.FirstObject() - gf := del.DeletionRequest.FirstFile() +func (del *Deletion) CreateObjDeletionWorkItem(obj *pgmodels.IntellectualObject) error { + workItem, err := pgmodels.NewDeletionItem(obj, nil, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy) + if err != nil { + return err + } + del.DeletionRequest.WorkItems = append(del.DeletionRequest.WorkItems, workItem) + return nil +} - // Deletion may be file only, no object. - var err error - if obj == nil && gf != nil { - obj, err = pgmodels.IntellectualObjectByID(gf.IntellectualObjectID) - if err != nil { - return nil, err - } +func (del *Deletion) CreateFileDeletionWorkItem(gf *pgmodels.GenericFile) error { + obj, err := pgmodels.IntellectualObjectByID(gf.IntellectualObjectID) + if err != nil { + return err } workItem, err := pgmodels.NewDeletionItem(obj, gf, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy) if err != nil { - return nil, err + return err } - del.DeletionRequest.WorkItem = workItem - err = del.DeletionRequest.Save() - - return workItem, err + del.DeletionRequest.WorkItems = append(del.DeletionRequest.WorkItems, workItem) + return nil } // QueueWorkItem sends the WorkItem.ID into the appropriate NSQ topic. // We call this after calling CreateWorkItem, and only if the admin // approves the deletion. -func (del *Deletion) QueueWorkItem() error { - workItem := del.DeletionRequest.WorkItem - if workItem == nil { - return common.ErrInternal - } - topic, err := constants.TopicFor(workItem.Action, workItem.Stage) - if err != nil { - return err +func (del *Deletion) QueueWorkItems() error { + for _, item := range del.DeletionRequest.WorkItems { + if item == nil { + return common.ErrInternal + } + topic, err := constants.TopicFor(item.Action, item.Stage) + if err != nil { + return err + } + ctx := common.Context() + ctx.Log.Info().Msgf("Queueing WorkItem %d to topic %s", item.ID, topic) + err = ctx.NSQClient.Enqueue(topic, item.ID) + if err != nil { + return err + } } - ctx := common.Context() - ctx.Log.Info().Msgf("Queueing WorkItem %d to topic %s", workItem.ID, topic) - return ctx.NSQClient.Enqueue(topic, workItem.ID) + return nil } // CreateAndQueueWorkItem creates and queues a deletion WorkItem. // We call this only if the admin approves the DeletionRequest. -func (del *Deletion) CreateAndQueueWorkItem() (*pgmodels.WorkItem, error) { - workItem, err := del.CreateWorkItem() - if err == nil { - err = del.QueueWorkItem() +func (del *Deletion) CreateAndQueueWorkItems() error { + var err error + for _, gf := range del.DeletionRequest.GenericFiles { + err = del.CreateFileDeletionWorkItem(gf) + if err != nil { + return err + } + } + for _, obj := range del.DeletionRequest.IntellectualObjects { + err = del.CreateObjDeletionWorkItem(obj) + if err != nil { + return err + } + } + err = del.DeletionRequest.Save() + if err != nil { + return err } - return workItem, err + return del.QueueWorkItems() } // CreateRequestAlert creates an alert saying that a user has requested diff --git a/web/webui/deletion_requests_controller.go b/web/webui/deletion_requests_controller.go index 02675efa..5d13c879 100644 --- a/web/webui/deletion_requests_controller.go +++ b/web/webui/deletion_requests_controller.go @@ -121,7 +121,7 @@ func DeletionRequestApprove(c *gin.Context) { if AbortIfError(c, err) { return } - _, err = del.CreateAndQueueWorkItem() + err = del.CreateAndQueueWorkItems() if AbortIfError(c, err) { return } diff --git a/web/webui/deletion_requests_controller_test.go b/web/webui/deletion_requests_controller_test.go index 9930c198..4a1495d0 100644 --- a/web/webui/deletion_requests_controller_test.go +++ b/web/webui/deletion_requests_controller_test.go @@ -143,7 +143,7 @@ func TestDeletionRequestApprove(t *testing.T) { assert.Equal(t, req.ConfirmedByID, testutil.Inst1Admin.ID) assert.Equal(t, req.ConfirmedBy.ID, testutil.Inst1Admin.ID) assert.False(t, req.ConfirmedAt.IsZero()) - assert.NotNil(t, req.WorkItem) + assert.NotEmpty(t, req.WorkItems) // We also should have an alert for this deletion confirmation query := pgmodels.NewQuery(). @@ -178,7 +178,7 @@ func TestDeletionRequestCancel(t *testing.T) { assert.False(t, req.CancelledAt.IsZero()) // There should be no work item because the deletion was cancelled. - assert.Nil(t, req.WorkItem) + assert.Empty(t, req.WorkItems) // We also should have an alert for this deletion confirmation query := pgmodels.NewQuery(). diff --git a/web/webui/deletion_test.go b/web/webui/deletion_test.go index 1514168e..f5c25852 100644 --- a/web/webui/deletion_test.go +++ b/web/webui/deletion_test.go @@ -128,9 +128,10 @@ func TestNewDeletionForReview(t *testing.T) { } func testCreateAndQueueWorkItem(t *testing.T, del *webui.Deletion) { - item, err := del.CreateAndQueueWorkItem() + err := del.CreateAndQueueWorkItems() require.Nil(t, err) - require.NotNil(t, item) + require.Equal(t, 1, len(del.DeletionRequest.WorkItems)) + item := del.DeletionRequest.WorkItems[0] assert.True(t, item.ID > 0) assert.Equal(t, del.DeletionRequest.GenericFiles[0].ID, item.GenericFileID) assert.Equal(t, constants.ActionDelete, item.Action) From 9cc523318abcf73d8be1672ba8a610e4e3f8f500 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 15:59:56 -0500 Subject: [PATCH 13/27] More bulk deletion work --- alert_templates/deletion_confirmed.txt | 6 ++++- .../intellectual_objects_batch_delete_test.go | 2 +- web/webui/deletion.go | 23 +++++++++++-------- web/webui/deletion_requests_controller.go | 10 ++++---- web/webui/deletion_test.go | 10 ++++---- 5 files changed, 31 insertions(+), 20 deletions(-) diff --git a/alert_templates/deletion_confirmed.txt b/alert_templates/deletion_confirmed.txt index 8cfab6c2..d0352bfb 100644 --- a/alert_templates/deletion_confirmed.txt +++ b/alert_templates/deletion_confirmed.txt @@ -6,7 +6,11 @@ For your reference, the link below has information about the request. {{ .deletionReadOnlyURL }} -The Work Item showing the status of this deletion is at {{ .workItemURL }} +The Work Items showing the status of this deletion are at + +{{ range $index, $itemUrl := .workItemURLs }} +{{ $itemURL }} +{{ end }} If you have questions, please contact us at help@aptrust.org. diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 880194d4..20a369c7 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -213,7 +213,7 @@ func testObjectBatchDeleteCreatesExpectedRecords(t *testing.T, params admin_api. // there should be no associated WorkItem. We create the WorkItem // only after the deletion request has been approved by the // local institutional admin. - assert.Empty(t, deletionRequest.WorkItemID) + assert.Empty(t, deletionRequest.WorkItems) // Make sure the deletion request alert was created in the DB. query := pgmodels.NewQuery().Where("deletion_request_id", "=", deletionRequest.ID) diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 3ec331c2..3de6c744 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -338,13 +338,13 @@ func (del *Deletion) CreateRequestAlert() (*pgmodels.Alert, error) { func (del *Deletion) CreateApprovalAlert() (*pgmodels.Alert, error) { templateName := "alerts/deletion_confirmed.txt" alertType := constants.AlertDeletionConfirmed - workItemURL, err := del.WorkItemURL() + workItemURLs, err := del.WorkItemURLs() if err != nil { return nil, err } alertData := map[string]interface{}{ "deletionRequest": del.DeletionRequest, - "workItemURL": workItemURL, + "workItemURLs": workItemURLs, "deletionReadOnlyURL": del.ReadOnlyURL(), } return del.createDeletionAlert(templateName, alertType, alertData) @@ -397,16 +397,21 @@ func (del *Deletion) ReviewURL() (string, error) { del.DeletionRequest.ConfirmationToken), nil } -// WorkItemURL returns the URL for the WorkItem for this DeletionRequest. +// WorkItemURLs returns the URL for the WorkItem for this DeletionRequest. // If you call this on a cancelled or not-yet-approved request, there is // no WorkItem and you'll get common.ErrNotSupported. -func (del *Deletion) WorkItemURL() (string, error) { - if del.DeletionRequest.WorkItemID == 0 { - return "", common.ErrNotSupported +func (del *Deletion) WorkItemURLs() ([]string, error) { + urls := make([]string, 0) + if len(del.DeletionRequest.WorkItems) == 0 { + return urls, common.ErrNotSupported } - return fmt.Sprintf("%s/work_items/show/%d", - del.baseURL, - del.DeletionRequest.WorkItemID), nil + for _, item := range del.DeletionRequest.WorkItems { + itemUrl := fmt.Sprintf("%s/work_items/show/%d", + del.baseURL, + item.ID) + urls = append(urls, itemUrl) + } + return urls, nil } // ReadOnlyURL returns a URL that displays info about the deletion request diff --git a/web/webui/deletion_requests_controller.go b/web/webui/deletion_requests_controller.go index 5d13c879..803a07fd 100644 --- a/web/webui/deletion_requests_controller.go +++ b/web/webui/deletion_requests_controller.go @@ -51,10 +51,12 @@ func deletionRequestLoad(req *Request) error { } req.TemplateData["deletionRequest"] = deletionRequest - if deletionRequest.WorkItemID > 0 { - req.TemplateData["workItemURL"] = fmt.Sprintf("%s/work_items/show/%d", - req.BaseURL(), - deletionRequest.WorkItemID) + if len(deletionRequest.WorkItems) > 0 { + urls := make([]string, 0) + for _, item := range deletionRequest.WorkItems { + urls = append(urls, fmt.Sprintf("%s/work_items/show/%d", req.BaseURL(), item.ID)) + } + req.TemplateData["workItemURLs"] = urls } return nil } diff --git a/web/webui/deletion_test.go b/web/webui/deletion_test.go index f5c25852..450d27fd 100644 --- a/web/webui/deletion_test.go +++ b/web/webui/deletion_test.go @@ -121,10 +121,10 @@ func TestNewDeletionForReview(t *testing.T) { readOnlyURL := fmt.Sprintf("https://example.com/deletions/show/%d", del.DeletionRequest.ID) assert.Equal(t, readOnlyURL, del.ReadOnlyURL()) - expectedWorkItemURL := fmt.Sprintf("https://example.com/work_items/show/%d", del.DeletionRequest.WorkItemID) - actualWorkItemURL, err := del.WorkItemURL() + expectedWorkItemURL := fmt.Sprintf("https://example.com/work_items/show/%d", del.DeletionRequest.WorkItems[0].ID) + actualWorkItemURLs, err := del.WorkItemURLs() require.Nil(t, err) - assert.Equal(t, expectedWorkItemURL, actualWorkItemURL) + assert.Equal(t, expectedWorkItemURL, actualWorkItemURLs[0]) } func testCreateAndQueueWorkItem(t *testing.T, del *webui.Deletion) { @@ -170,9 +170,9 @@ func testCreateApprovalAlert(t *testing.T, del *webui.Deletion) { assert.Equal(t, del.DeletionRequest.InstitutionID, recipient.InstitutionID) } - workItemURL, err := del.WorkItemURL() + workItemURLs, err := del.WorkItemURLs() require.Nil(t, err) - assert.Contains(t, alert.Content, workItemURL) + assert.Contains(t, alert.Content, workItemURLs[0]) } func testCreateCancellationAlert(t *testing.T, del *webui.Deletion) { From e7a4d40f2ddf7886b2624936f5c4957570d8d2a9 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Thu, 25 Jan 2024 16:46:26 -0500 Subject: [PATCH 14/27] More bulk deletion work --- alert_templates/deletion_confirmed.txt | 2 +- pgmodels/deletion_request.go | 2 +- pgmodels/generic_file.go | 7 +++++-- pgmodels/generic_file_test.go | 4 ++-- pgmodels/intellectual_object_test.go | 3 --- web/api/admin/integration_test_controller.go | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/alert_templates/deletion_confirmed.txt b/alert_templates/deletion_confirmed.txt index d0352bfb..81810b8b 100644 --- a/alert_templates/deletion_confirmed.txt +++ b/alert_templates/deletion_confirmed.txt @@ -9,7 +9,7 @@ For your reference, the link below has information about the request. The Work Items showing the status of this deletion are at {{ range $index, $itemUrl := .workItemURLs }} -{{ $itemURL }} +{{ $itemUrl }} {{ end }} If you have questions, please contact us at help@aptrust.org. diff --git a/pgmodels/deletion_request.go b/pgmodels/deletion_request.go index b4731435..b3081f8f 100644 --- a/pgmodels/deletion_request.go +++ b/pgmodels/deletion_request.go @@ -78,7 +78,7 @@ func NewDeletionRequest() (*DeletionRequest, error) { // DeletionRequestByID returns the institution with the specified id. // Returns pg.ErrNoRows if there is no match. func DeletionRequestByID(id int64) (*DeletionRequest, error) { - query := NewQuery().Relations("RequestedBy", "ConfirmedBy", "CancelledBy", "GenericFiles", "IntellectualObjects", "WorkItem").Where(`"deletion_request"."id"`, "=", id) + query := NewQuery().Relations("RequestedBy", "ConfirmedBy", "CancelledBy", "GenericFiles", "IntellectualObjects", "WorkItems").Where(`"deletion_request"."id"`, "=", id) return DeletionRequestGet(query) } diff --git a/pgmodels/generic_file.go b/pgmodels/generic_file.go index 448fe612..14e7985b 100644 --- a/pgmodels/generic_file.go +++ b/pgmodels/generic_file.go @@ -474,8 +474,11 @@ func (gf *GenericFile) ActiveDeletionWorkItem() (*WorkItem, error) { } func (gf *GenericFile) DeletionRequest(workItemID int64) (*DeletionRequestView, error) { - query := NewQuery().Where("work_item_id", "=", workItemID) - return DeletionRequestViewGet(query) + workItem, err := WorkItemByID(workItemID) + if err != nil { + return nil, err + } + return DeletionRequestViewByID(workItem.DeletionRequestID) } func (gf *GenericFile) AssertDeletionPreconditions() error { diff --git a/pgmodels/generic_file_test.go b/pgmodels/generic_file_test.go index ce8a095f..2f3968b8 100644 --- a/pgmodels/generic_file_test.go +++ b/pgmodels/generic_file_test.go @@ -276,7 +276,7 @@ func TestFileDeletionPreConditions(t *testing.T) { assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item")) testGenericFileDeleteError(t, gf) - testFileDeletionRequest(t, gf) + testFileDeletionRequest(t, gf, workItem.ID) // Request not yet approved err = gf.AssertDeletionPreconditions() @@ -304,7 +304,7 @@ func TestFileDeletionPreConditions(t *testing.T) { testGenericFileDeleteSuccess(t, gf) } -func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile int64) { +func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItemID int64) { // Request doesn't exist yet. reqView, err := gf.DeletionRequest(workItemID) require.NotNil(t, err) diff --git a/pgmodels/intellectual_object_test.go b/pgmodels/intellectual_object_test.go index cb530f40..5e52a15e 100644 --- a/pgmodels/intellectual_object_test.go +++ b/pgmodels/intellectual_object_test.go @@ -295,7 +295,6 @@ func TestAssertObjDeletionPreconditions(t *testing.T) { req, err := pgmodels.CreateDeletionRequest(objects, nil) require.Nil(t, err) require.NotNil(t, req) - req.WorkItemID = workItem.ID require.Nil(t, req.Save()) err = obj.AssertDeletionPreconditions() @@ -364,7 +363,6 @@ func testObjectDeletionRequest(t *testing.T, obj *pgmodels.IntellectualObject) { req, err := pgmodels.CreateDeletionRequest(objects, nil) require.Nil(t, err) require.NotNil(t, req) - req.WorkItemID = item.ID require.Nil(t, req.Save()) deletionReqView, err := obj.DeletionRequest(item.ID) @@ -412,7 +410,6 @@ func TestNewObjDeletionEvent(t *testing.T) { req, err := pgmodels.CreateDeletionRequest(objects, nil) require.Nil(t, err) require.NotNil(t, req) - req.WorkItemID = workItem.ID require.Nil(t, req.Save()) event, err = obj.NewDeletionEvent() diff --git a/web/api/admin/integration_test_controller.go b/web/api/admin/integration_test_controller.go index 31d7b9a6..d596b0d0 100644 --- a/web/api/admin/integration_test_controller.go +++ b/web/api/admin/integration_test_controller.go @@ -95,7 +95,7 @@ func prepareFileDeletionPreconditions(gfID int64) (*pgmodels.WorkItem, error) { request.RequestedAt = now request.ConfirmedByID = instAdmin.ID request.ConfirmedAt = now - request.WorkItemID = item.ID + //request.WorkItemID = item.ID err = request.Save() return item, err } @@ -151,7 +151,7 @@ func prepareObjectDeletionPreconditions(objID int64) (*pgmodels.WorkItem, error) request.RequestedAt = now request.ConfirmedByID = instAdmin.ID request.ConfirmedAt = now - request.WorkItemID = item.ID + //request.WorkItemID = item.ID err = request.Save() return item, err } From e9e204b2dd14c203ae0bb8362c2a56d5868e5869 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 10:41:07 -0500 Subject: [PATCH 15/27] Fixed deletion test --- web/webui/deletion_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/webui/deletion_test.go b/web/webui/deletion_test.go index 450d27fd..27cde2bc 100644 --- a/web/webui/deletion_test.go +++ b/web/webui/deletion_test.go @@ -130,7 +130,8 @@ func TestNewDeletionForReview(t *testing.T) { func testCreateAndQueueWorkItem(t *testing.T, del *webui.Deletion) { err := del.CreateAndQueueWorkItems() require.Nil(t, err) - require.Equal(t, 1, len(del.DeletionRequest.WorkItems)) + // This request includes three files, so there should be three work items + require.Equal(t, 3, len(del.DeletionRequest.WorkItems)) item := del.DeletionRequest.WorkItems[0] assert.True(t, item.ID > 0) assert.Equal(t, del.DeletionRequest.GenericFiles[0].ID, item.GenericFileID) From a7c23d54705af152a44cea60848a37966d3d40fe Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 10:54:17 -0500 Subject: [PATCH 16/27] Fixed another deletion test --- pgmodels/intellectual_object.go | 14 +- pgmodels/intellectual_object_test.go | 194 ++++++++++++--------------- 2 files changed, 88 insertions(+), 120 deletions(-) diff --git a/pgmodels/intellectual_object.go b/pgmodels/intellectual_object.go index 99b4ff61..79febd36 100644 --- a/pgmodels/intellectual_object.go +++ b/pgmodels/intellectual_object.go @@ -264,18 +264,6 @@ func (obj *IntellectualObject) ActiveDeletionWorkItem() (*WorkItem, error) { return item, err } -func (obj *IntellectualObject) DeletionRequest(workItemID int64) (*DeletionRequestView, error) { - // web/webui/deletion_request_controller.go method DeletionRequestApprove - // creates a WorkItem when deletion request is approved. This is a - // one-to-one relationship. If we can't find the deletion request view - // for a valid deletion WorkItem, something is wrong! - query := NewQuery(). - Where("work_item_id", "=", workItemID). - Where("object_count", ">", 0). - Where("file_count", "=", 0) - return DeletionRequestViewGet(query) -} - func (obj *IntellectualObject) AssertDeletionPreconditions() error { err := obj.assertNoActiveFiles() if err == nil { @@ -342,7 +330,7 @@ func (obj *IntellectualObject) assertDeletionApproved() (*WorkItem, *DeletionReq if common.IsEmptyString(workItem.InstApprover) { return workItem, nil, fmt.Errorf("Deletion work item is missing institutional approver") } - deletionRequest, err := obj.DeletionRequest(workItem.ID) + deletionRequest, err := DeletionRequestViewByID(workItem.DeletionRequestID) if deletionRequest == nil || IsNoRowError(err) { return workItem, nil, fmt.Errorf("No deletion request for work item %d", workItem.ID) } diff --git a/pgmodels/intellectual_object_test.go b/pgmodels/intellectual_object_test.go index 5e52a15e..a9e283ef 100644 --- a/pgmodels/intellectual_object_test.go +++ b/pgmodels/intellectual_object_test.go @@ -234,88 +234,88 @@ func TestObjInsertAndUpdate(t *testing.T) { assert.True(t, obj.UpdatedAt.After(origUpdatedAt)) } -func TestAssertObjDeletionPreconditions(t *testing.T) { - defer db.ForceFixtureReload() - obj, err := pgmodels.CreateObjectWithRelations() - require.Nil(t, err) - require.NotNil(t, obj) - - testLastObjDeletionWorkItem(t, obj) - testObjectDeletionRequest(t, obj) - - // Hit the following underlying methods: - // assertNoActiveFiles - // assertNotAlreadyDeleted - // assertDeletionApproved - - // First pre-condition: No active files. - err = obj.AssertDeletionPreconditions() - require.NotNil(t, err) - assert.Equal(t, common.ErrActiveFiles, err) - - // Mark files deleted... - for _, gf := range obj.GenericFiles { - gf.State = constants.StateDeleted - err = gf.Save() - require.Nil(t, err) - } - - // Next pre-condition: Object isn't already deleted. - obj.State = constants.StateDeleted - err = obj.Save() - require.Nil(t, err) - - err = obj.AssertDeletionPreconditions() - require.NotNil(t, err) - assert.Equal(t, "Object is already in deleted state", err.Error()) - - obj.State = constants.StateActive - err = obj.Save() - require.Nil(t, err) - - // Create a deletion work item for this object - workItem := pgmodels.RandomWorkItem(obj.BagName, constants.ActionDelete, obj.ID, 0) - workItem.Status = constants.StatusStarted - require.Nil(t, workItem.Save()) - - err = obj.AssertDeletionPreconditions() - require.NotNil(t, err) - assert.True(t, strings.HasPrefix(err.Error(), "Deletion work item is missing institutional approver"), err.Error()) - - // Approve it - workItem.InstApprover = "someone@example.com" - require.Nil(t, workItem.Save()) - - err = obj.AssertDeletionPreconditions() - require.NotNil(t, err) - assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item"), err.Error()) - - // Last pre-condition: Object requires an approved deletion request - objects := []*pgmodels.IntellectualObject{obj} - req, err := pgmodels.CreateDeletionRequest(objects, nil) - require.Nil(t, err) - require.NotNil(t, req) - require.Nil(t, req.Save()) - - err = obj.AssertDeletionPreconditions() - require.NotNil(t, err) - assert.True(t, strings.Contains(err.Error(), "Deletion request"), err.Error()) - assert.True(t, strings.Contains(err.Error(), "has no approver"), err.Error()) - - // Add an approver - testEduAdmin, err := pgmodels.UserByEmail("admin@test.edu") - require.Nil(t, err) - req.ConfirmedByID = testEduAdmin.ID - req.ConfirmedAt = time.Now().UTC() - require.Nil(t, req.Save()) - - // Now we should be OK - err = obj.AssertDeletionPreconditions() - assert.Nil(t, err) - - // Now test the actual deletion - testObjectDelete(t, obj) -} +// func TestAssertObjDeletionPreconditions(t *testing.T) { +// defer db.ForceFixtureReload() +// obj, err := pgmodels.CreateObjectWithRelations() +// require.Nil(t, err) +// require.NotNil(t, obj) + +// testLastObjDeletionWorkItem(t, obj) +//////// testObjectDeletionRequest(t, obj) + +// // Hit the following underlying methods: +// // assertNoActiveFiles +// // assertNotAlreadyDeleted +// // assertDeletionApproved + +// // First pre-condition: No active files. +// err = obj.AssertDeletionPreconditions() +// require.NotNil(t, err) +// assert.Equal(t, common.ErrActiveFiles, err) + +// // Mark files deleted... +// for _, gf := range obj.GenericFiles { +// gf.State = constants.StateDeleted +// err = gf.Save() +// require.Nil(t, err) +// } + +// // Next pre-condition: Object isn't already deleted. +// obj.State = constants.StateDeleted +// err = obj.Save() +// require.Nil(t, err) + +// err = obj.AssertDeletionPreconditions() +// require.NotNil(t, err) +// assert.Equal(t, "Object is already in deleted state", err.Error()) + +// obj.State = constants.StateActive +// err = obj.Save() +// require.Nil(t, err) + +// // Create a deletion work item for this object +// workItem := pgmodels.RandomWorkItem(obj.BagName, constants.ActionDelete, obj.ID, 0) +// workItem.Status = constants.StatusStarted +// require.Nil(t, workItem.Save()) + +// err = obj.AssertDeletionPreconditions() +// require.NotNil(t, err) +// assert.True(t, strings.HasPrefix(err.Error(), "Deletion work item is missing institutional approver"), err.Error()) + +// // Approve it +// workItem.InstApprover = "someone@example.com" +// require.Nil(t, workItem.Save()) + +// err = obj.AssertDeletionPreconditions() +// require.NotNil(t, err) +// assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item"), err.Error()) + +// // Last pre-condition: Object requires an approved deletion request +// objects := []*pgmodels.IntellectualObject{obj} +// req, err := pgmodels.CreateDeletionRequest(objects, nil) +// require.Nil(t, err) +// require.NotNil(t, req) +// require.Nil(t, req.Save()) + +// err = obj.AssertDeletionPreconditions() +// require.NotNil(t, err) +// assert.True(t, strings.Contains(err.Error(), "Deletion request"), err.Error()) +// assert.True(t, strings.Contains(err.Error(), "has no approver"), err.Error()) + +// // Add an approver +// testEduAdmin, err := pgmodels.UserByEmail("admin@test.edu") +// require.Nil(t, err) +// req.ConfirmedByID = testEduAdmin.ID +// req.ConfirmedAt = time.Now().UTC() +// require.Nil(t, req.Save()) + +// // Now we should be OK +// err = obj.AssertDeletionPreconditions() +// assert.Nil(t, err) + +// // Now test the actual deletion +// testObjectDelete(t, obj) +// } func testLastObjDeletionWorkItem(t *testing.T, obj *pgmodels.IntellectualObject) { item, err := obj.ActiveDeletionWorkItem() @@ -347,31 +347,6 @@ func testLastObjDeletionWorkItem(t *testing.T, obj *pgmodels.IntellectualObject) } -func testObjectDeletionRequest(t *testing.T, obj *pgmodels.IntellectualObject) { - // Initially, there's no deletion request for this object - reqView, err := obj.DeletionRequest(99999999999) - require.NotNil(t, err) - assert.Nil(t, reqView) - - // Figure out the work item id. That will lead us back to - // the original deletion request. - item, err := obj.ActiveDeletionWorkItem() - require.Nil(t, err) - require.NotNil(t, item) - - objects := []*pgmodels.IntellectualObject{obj} - req, err := pgmodels.CreateDeletionRequest(objects, nil) - require.Nil(t, err) - require.NotNil(t, req) - require.Nil(t, req.Save()) - - deletionReqView, err := obj.DeletionRequest(item.ID) - require.Nil(t, err) - require.NotNil(t, deletionReqView) - - assert.Equal(t, req.ID, deletionReqView.ID) -} - func TestNewObjDeletionEvent(t *testing.T) { defer db.ForceFixtureReload() obj, err := pgmodels.CreateObjectWithRelations() @@ -412,6 +387,11 @@ func TestNewObjDeletionEvent(t *testing.T) { require.NotNil(t, req) require.Nil(t, req.Save()) + // Attach the deletion request to the work item, + // but remember, it's still not approved. + workItem.DeletionRequestID = req.ID + require.NoError(t, workItem.Save()) + event, err = obj.NewDeletionEvent() assert.Nil(t, event) require.NotNil(t, err) From 7f21c0c6cb2ccf61627bf81c101f10d61c95c63f Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 10:58:58 -0500 Subject: [PATCH 17/27] Fixed another deletion test --- pgmodels/intellectual_object_test.go | 169 ++++++++++++++------------- 1 file changed, 87 insertions(+), 82 deletions(-) diff --git a/pgmodels/intellectual_object_test.go b/pgmodels/intellectual_object_test.go index a9e283ef..ba2ee009 100644 --- a/pgmodels/intellectual_object_test.go +++ b/pgmodels/intellectual_object_test.go @@ -2,6 +2,7 @@ package pgmodels_test import ( "fmt" + "os" "strings" "testing" "time" @@ -234,88 +235,92 @@ func TestObjInsertAndUpdate(t *testing.T) { assert.True(t, obj.UpdatedAt.After(origUpdatedAt)) } -// func TestAssertObjDeletionPreconditions(t *testing.T) { -// defer db.ForceFixtureReload() -// obj, err := pgmodels.CreateObjectWithRelations() -// require.Nil(t, err) -// require.NotNil(t, obj) - -// testLastObjDeletionWorkItem(t, obj) -//////// testObjectDeletionRequest(t, obj) - -// // Hit the following underlying methods: -// // assertNoActiveFiles -// // assertNotAlreadyDeleted -// // assertDeletionApproved - -// // First pre-condition: No active files. -// err = obj.AssertDeletionPreconditions() -// require.NotNil(t, err) -// assert.Equal(t, common.ErrActiveFiles, err) - -// // Mark files deleted... -// for _, gf := range obj.GenericFiles { -// gf.State = constants.StateDeleted -// err = gf.Save() -// require.Nil(t, err) -// } - -// // Next pre-condition: Object isn't already deleted. -// obj.State = constants.StateDeleted -// err = obj.Save() -// require.Nil(t, err) - -// err = obj.AssertDeletionPreconditions() -// require.NotNil(t, err) -// assert.Equal(t, "Object is already in deleted state", err.Error()) - -// obj.State = constants.StateActive -// err = obj.Save() -// require.Nil(t, err) - -// // Create a deletion work item for this object -// workItem := pgmodels.RandomWorkItem(obj.BagName, constants.ActionDelete, obj.ID, 0) -// workItem.Status = constants.StatusStarted -// require.Nil(t, workItem.Save()) - -// err = obj.AssertDeletionPreconditions() -// require.NotNil(t, err) -// assert.True(t, strings.HasPrefix(err.Error(), "Deletion work item is missing institutional approver"), err.Error()) - -// // Approve it -// workItem.InstApprover = "someone@example.com" -// require.Nil(t, workItem.Save()) - -// err = obj.AssertDeletionPreconditions() -// require.NotNil(t, err) -// assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item"), err.Error()) - -// // Last pre-condition: Object requires an approved deletion request -// objects := []*pgmodels.IntellectualObject{obj} -// req, err := pgmodels.CreateDeletionRequest(objects, nil) -// require.Nil(t, err) -// require.NotNil(t, req) -// require.Nil(t, req.Save()) - -// err = obj.AssertDeletionPreconditions() -// require.NotNil(t, err) -// assert.True(t, strings.Contains(err.Error(), "Deletion request"), err.Error()) -// assert.True(t, strings.Contains(err.Error(), "has no approver"), err.Error()) - -// // Add an approver -// testEduAdmin, err := pgmodels.UserByEmail("admin@test.edu") -// require.Nil(t, err) -// req.ConfirmedByID = testEduAdmin.ID -// req.ConfirmedAt = time.Now().UTC() -// require.Nil(t, req.Save()) - -// // Now we should be OK -// err = obj.AssertDeletionPreconditions() -// assert.Nil(t, err) - -// // Now test the actual deletion -// testObjectDelete(t, obj) -// } +func TestAssertObjDeletionPreconditions(t *testing.T) { + os.Setenv("APT_ENV", "test") + defer db.ForceFixtureReload() + obj, err := pgmodels.CreateObjectWithRelations() + require.Nil(t, err) + require.NotNil(t, obj) + + testLastObjDeletionWorkItem(t, obj) + + // This hits the following underlying methods: + // assertNoActiveFiles + // assertNotAlreadyDeleted + // assertDeletionApproved + + // First pre-condition: No active files. + err = obj.AssertDeletionPreconditions() + require.NotNil(t, err) + assert.Equal(t, common.ErrActiveFiles, err) + + // Mark files deleted... + for _, gf := range obj.GenericFiles { + gf.State = constants.StateDeleted + err = gf.Save() + require.Nil(t, err) + } + + // Next pre-condition: Object isn't already deleted. + obj.State = constants.StateDeleted + err = obj.Save() + require.Nil(t, err) + + err = obj.AssertDeletionPreconditions() + require.NotNil(t, err) + assert.Equal(t, "Object is already in deleted state", err.Error()) + + obj.State = constants.StateActive + err = obj.Save() + require.Nil(t, err) + + // Create a deletion work item for this object + workItem := pgmodels.RandomWorkItem(obj.BagName, constants.ActionDelete, obj.ID, 0) + workItem.Status = constants.StatusStarted + require.Nil(t, workItem.Save()) + + err = obj.AssertDeletionPreconditions() + require.NotNil(t, err) + assert.True(t, strings.HasPrefix(err.Error(), "Deletion work item is missing institutional approver"), err.Error()) + + // Approve it + workItem.InstApprover = "someone@example.com" + require.Nil(t, workItem.Save()) + + err = obj.AssertDeletionPreconditions() + require.NotNil(t, err) + assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item"), err.Error()) + + // Last pre-condition: Object requires an approved deletion request + objects := []*pgmodels.IntellectualObject{obj} + req, err := pgmodels.CreateDeletionRequest(objects, nil) + require.Nil(t, err) + require.NotNil(t, req) + require.Nil(t, req.Save()) + + // Associate deletion request with work item + workItem.DeletionRequestID = req.ID + require.NoError(t, workItem.Save()) + + err = obj.AssertDeletionPreconditions() + require.NotNil(t, err) + assert.True(t, strings.Contains(err.Error(), "Deletion request"), err.Error()) + assert.True(t, strings.Contains(err.Error(), "has no approver"), err.Error()) + + // Add an approver + testEduAdmin, err := pgmodels.UserByEmail("admin@test.edu") + require.Nil(t, err) + req.ConfirmedByID = testEduAdmin.ID + req.ConfirmedAt = time.Now().UTC() + require.Nil(t, req.Save()) + + // Now we should be OK + err = obj.AssertDeletionPreconditions() + assert.Nil(t, err) + + // Now test the actual deletion + testObjectDelete(t, obj) +} func testLastObjDeletionWorkItem(t *testing.T, obj *pgmodels.IntellectualObject) { item, err := obj.ActiveDeletionWorkItem() From 5fbd8ab985fc687158f357334b76cf6a14e4c606 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 11:58:46 -0500 Subject: [PATCH 18/27] More deletion test fixes --- web/api/admin/integration_test_controller.go | 62 ++++++++++--------- .../intellectual_objects_controller_test.go | 9 ++- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/web/api/admin/integration_test_controller.go b/web/api/admin/integration_test_controller.go index d596b0d0..c3e6193c 100644 --- a/web/api/admin/integration_test_controller.go +++ b/web/api/admin/integration_test_controller.go @@ -68,20 +68,6 @@ func prepareFileDeletionPreconditions(gfID int64) (*pgmodels.WorkItem, error) { return nil, err } - // Also requires an approved Deletion work item - item := pgmodels.RandomWorkItem( - gf.IntellectualObject.BagName, - constants.ActionDelete, - gf.IntellectualObjectID, - gf.ID) - item.User = instAdmin.Email - item.InstApprover = instAdmin.Email - item.Status = constants.StatusStarted - err = item.Save() - if err != nil { - return nil, err - } - // Requires approved deletion request now := time.Now().UTC() request, err := pgmodels.NewDeletionRequest() @@ -95,8 +81,23 @@ func prepareFileDeletionPreconditions(gfID int64) (*pgmodels.WorkItem, error) { request.RequestedAt = now request.ConfirmedByID = instAdmin.ID request.ConfirmedAt = now - //request.WorkItemID = item.ID err = request.Save() + if err != nil { + return nil, err + } + + // Also requires an approved Deletion work item + item := pgmodels.RandomWorkItem( + gf.IntellectualObject.BagName, + constants.ActionDelete, + gf.IntellectualObjectID, + gf.ID) + item.User = instAdmin.Email + item.InstApprover = instAdmin.Email + item.Status = constants.StatusStarted + item.DeletionRequestID = request.ID + err = item.Save() + return item, err } @@ -124,20 +125,6 @@ func prepareObjectDeletionPreconditions(objID int64) (*pgmodels.WorkItem, error) return nil, err } - // Also requires an approved Deletion work item - item := pgmodels.RandomWorkItem( - obj.BagName, - constants.ActionDelete, - obj.ID, - 0) - item.User = instAdmin.Email - item.InstApprover = instAdmin.Email - item.Status = constants.StatusStarted - err = item.Save() - if err != nil { - return nil, err - } - // Requires approved deletion request now := time.Now().UTC() request, err := pgmodels.NewDeletionRequest() @@ -151,8 +138,23 @@ func prepareObjectDeletionPreconditions(objID int64) (*pgmodels.WorkItem, error) request.RequestedAt = now request.ConfirmedByID = instAdmin.ID request.ConfirmedAt = now - //request.WorkItemID = item.ID err = request.Save() + if err != nil { + return nil, err + } + + // Also requires an approved Deletion work item + item := pgmodels.RandomWorkItem( + obj.BagName, + constants.ActionDelete, + obj.ID, + 0) + item.User = instAdmin.Email + item.InstApprover = instAdmin.Email + item.Status = constants.StatusStarted + item.DeletionRequestID = request.ID + err = item.Save() + return item, err } diff --git a/web/api/admin/intellectual_objects_controller_test.go b/web/api/admin/intellectual_objects_controller_test.go index 009d5096..d6ac1c22 100644 --- a/web/api/admin/intellectual_objects_controller_test.go +++ b/web/api/admin/intellectual_objects_controller_test.go @@ -84,7 +84,6 @@ func TestObjectIndex(t *testing.T) { } func TestObjectCreateUpdateDelete(t *testing.T) { - if common.Context().Config.EnvName != "test" { // For security reasons, the deletion setup endpoint works // only in the test env. In all others, it returns an error. @@ -101,7 +100,7 @@ func TestObjectCreateUpdateDelete(t *testing.T) { obj := testObjectCreate(t) updatedObj := testObjectUpdate(t, obj) - createObjectDeletionPreConditions(t, obj) + createObjectDeletionPreConditions(t, updatedObj) testObjectDelete(t, updatedObj) } @@ -121,6 +120,12 @@ func createObjectDeletionPreConditions(t *testing.T, obj *pgmodels.IntellectualO WithHeader(constants.APIKeyHeader, "password"). Expect() resp.Status(http.StatusOK) + + workItem := &pgmodels.WorkItem{} + err := json.Unmarshal([]byte(resp.Body().Raw()), workItem) + require.NoError(t, err) + assert.NotEmpty(t, workItem.DeletionRequestID) + require.NoError(t, obj.AssertDeletionPreconditions()) } func testObjectCreate(t *testing.T) *pgmodels.IntellectualObject { From 76aef55f49160cdda54a5d4d66ca255e15ccc8da Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 12:13:30 -0500 Subject: [PATCH 19/27] More deletion test fixes --- pgmodels/generic_file.go | 10 +--------- pgmodels/generic_file_test.go | 16 +++++++++------- pgmodels/intellectual_object.go | 1 + pgmodels/intellectual_object_test.go | 2 -- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/pgmodels/generic_file.go b/pgmodels/generic_file.go index 14e7985b..423d0d73 100644 --- a/pgmodels/generic_file.go +++ b/pgmodels/generic_file.go @@ -473,14 +473,6 @@ func (gf *GenericFile) ActiveDeletionWorkItem() (*WorkItem, error) { return item, err } -func (gf *GenericFile) DeletionRequest(workItemID int64) (*DeletionRequestView, error) { - workItem, err := WorkItemByID(workItemID) - if err != nil { - return nil, err - } - return DeletionRequestViewByID(workItem.DeletionRequestID) -} - func (gf *GenericFile) AssertDeletionPreconditions() error { if gf.State == constants.StateDeleted { return fmt.Errorf("File is already in deleted state") @@ -500,7 +492,7 @@ func (gf *GenericFile) assertDeletionApproved() (*WorkItem, *DeletionRequestView if common.IsEmptyString(workItem.InstApprover) { return workItem, nil, fmt.Errorf("Deletion work item is missing institutional approver") } - deletionRequest, err := gf.DeletionRequest(workItem.ID) + deletionRequest, err := DeletionRequestViewByID(workItem.DeletionRequestID) if deletionRequest == nil || IsNoRowError(err) { return workItem, nil, fmt.Errorf("No deletion request for work item %d", workItem.ID) } diff --git a/pgmodels/generic_file_test.go b/pgmodels/generic_file_test.go index 2f3968b8..4fa0a511 100644 --- a/pgmodels/generic_file_test.go +++ b/pgmodels/generic_file_test.go @@ -276,7 +276,7 @@ func TestFileDeletionPreConditions(t *testing.T) { assert.True(t, strings.HasPrefix(err.Error(), "No deletion request for work item")) testGenericFileDeleteError(t, gf) - testFileDeletionRequest(t, gf, workItem.ID) + testFileDeletionRequest(t, gf, workItem) // Request not yet approved err = gf.AssertDeletionPreconditions() @@ -285,9 +285,7 @@ func TestFileDeletionPreConditions(t *testing.T) { testGenericFileDeleteError(t, gf) // Add approver & test - query := pgmodels.NewQuery(). - Where("work_item_id", "=", workItem.ID) - req, err := pgmodels.DeletionRequestGet(query) + req, err := pgmodels.DeletionRequestByID(workItem.DeletionRequestID) require.Nil(t, err) require.NotNil(t, req) require.NotEqual(t, int64(0), req.ID) @@ -304,9 +302,9 @@ func TestFileDeletionPreConditions(t *testing.T) { testGenericFileDeleteSuccess(t, gf) } -func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItemID int64) { +func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItem *pgmodels.WorkItem) { // Request doesn't exist yet. - reqView, err := gf.DeletionRequest(workItemID) + reqView, err := pgmodels.DeletionRequestViewByID(workItem.DeletionRequestID) require.NotNil(t, err) assert.True(t, pgmodels.IsNoRowError(err)) require.Nil(t, reqView) @@ -319,7 +317,11 @@ func testFileDeletionRequest(t *testing.T, gf *pgmodels.GenericFile, workItemID req.ConfirmedByID = 0 require.Nil(t, req.Save()) - reqView, err = gf.DeletionRequest(workItemID) + // Add it to the work item for next test + workItem.DeletionRequestID = req.ID + require.NoError(t, workItem.Save()) + + reqView, err = pgmodels.DeletionRequestViewByID(req.ID) require.Nil(t, err) require.NotNil(t, reqView) assert.Equal(t, req.ID, reqView.ID) diff --git a/pgmodels/intellectual_object.go b/pgmodels/intellectual_object.go index 79febd36..054aa59c 100644 --- a/pgmodels/intellectual_object.go +++ b/pgmodels/intellectual_object.go @@ -332,6 +332,7 @@ func (obj *IntellectualObject) assertDeletionApproved() (*WorkItem, *DeletionReq } deletionRequest, err := DeletionRequestViewByID(workItem.DeletionRequestID) if deletionRequest == nil || IsNoRowError(err) { + fmt.Println(workItem.ID, workItem.DeletionRequestID) return workItem, nil, fmt.Errorf("No deletion request for work item %d", workItem.ID) } if err != nil { diff --git a/pgmodels/intellectual_object_test.go b/pgmodels/intellectual_object_test.go index ba2ee009..37187232 100644 --- a/pgmodels/intellectual_object_test.go +++ b/pgmodels/intellectual_object_test.go @@ -2,7 +2,6 @@ package pgmodels_test import ( "fmt" - "os" "strings" "testing" "time" @@ -236,7 +235,6 @@ func TestObjInsertAndUpdate(t *testing.T) { } func TestAssertObjDeletionPreconditions(t *testing.T) { - os.Setenv("APT_ENV", "test") defer db.ForceFixtureReload() obj, err := pgmodels.CreateObjectWithRelations() require.Nil(t, err) From c086737cb43b17f4e81b0847acd4dbe5d8551110 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 16:17:51 -0500 Subject: [PATCH 20/27] Added instructions for manual batch delete test --- .../intellectual_objects_batch_delete_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/web/api/admin/intellectual_objects_batch_delete_test.go b/web/api/admin/intellectual_objects_batch_delete_test.go index 20a369c7..57554bf1 100644 --- a/web/api/admin/intellectual_objects_batch_delete_test.go +++ b/web/api/admin/intellectual_objects_batch_delete_test.go @@ -17,6 +17,22 @@ import ( "github.com/stretchr/testify/require" ) +// To test manually: +/* + curl -H "Content-Type: application/json" \ + -H "X-Pharos-API-User: system@aptrust.org" \ + -H "X-Pharos-API-Key: password" \ + -X POST \ + -d '{"institutionId": 3, "requestorId": 5, "secretKey": "00000000-0000-0000-0000-000000000000", "objectIds": [5,6,12,13]}' \ + http://localhost:8080/admin-api/v3/objects/init_batch_delete + + * Then log in as admin@inst2.edu + * Click the alerts icon + * Click the "deletion request alert" + * Click the review link in that alert + * Approve the deletion on the review page (approval currently fails because objects have no ingest events) +*/ + func TestObjectBatchDelete(t *testing.T) { err := db.ForceFixtureReload() require.Nil(t, err) From 8ddcab309480ea7e398d628ed247b3b1f658ae3d Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Fri, 26 Jan 2024 16:20:06 -0500 Subject: [PATCH 21/27] Updated deletion templates --- views/deletions/approved_file.html | 12 +++++++----- views/deletions/approved_object.html | 18 ++++++++---------- views/deletions/cancelled_file.html | 13 ++++++++----- views/deletions/cancelled_object.html | 15 +++++++-------- views/deletions/review.html | 12 +++++++----- 5 files changed, 37 insertions(+), 33 deletions(-) diff --git a/views/deletions/approved_file.html b/views/deletions/approved_file.html index bf6a1097..67560740 100644 --- a/views/deletions/approved_file.html +++ b/views/deletions/approved_file.html @@ -7,13 +7,15 @@

Deletion Approved

-

You have approved deletion of the following file:

+

You have approved deletion of the following files:

+ +
    + {{ range $index, $gf := .deletionRequest.GenericFiles }} +
  1. {{ $gf.Identifier }}
  2. + {{ end }} +

- {{ $gf.Identifier }}
- Size: {{ humanSize $gf.Size }}
- Created: {{ dateUS $gf.CreatedAt }}
- Updated: {{ dateUS $gf.UpdatedAt }}
Requested: {{ .deletionRequest.RequestedBy.Name }} on {{ dateUS .deletionRequest.RequestedAt }}
Approved: {{ .deletionRequest.ConfirmedBy.Name }} on {{ dateUS .deletionRequest.ConfirmedAt }}

diff --git a/views/deletions/approved_object.html b/views/deletions/approved_object.html index 9c9335bb..260567aa 100644 --- a/views/deletions/approved_object.html +++ b/views/deletions/approved_object.html @@ -2,21 +2,19 @@ {{ template "shared/_header.html" .}} -{{ $obj := (index .deletionRequest.IntellectualObjects 0) }} -

Deletion Approved

-

You have approved deletion of the following object:

+

You have approved deletion of the following objects:

+ +
    + {{ range $index, $obj := .deletionRequest.IntellectualObjects }} +
  1. {{ $obj.Identifier }}
  2. + {{ end }} +
+

- {{ $obj.Identifier }}
- Alt Identifier: {{ $obj.AltIdentifier }}
- Internal Sender Identifier: {{ $obj.InternalSenderIdentifier }}
- Bag Group: {{ $obj.BagGroupIdentifier }}
- Storage Option: {{ $obj.StorageOption }}
- Created: {{ dateUS $obj.CreatedAt }}
- Updated: {{ dateUS $obj.UpdatedAt }}
Requested: {{ .deletionRequest.RequestedBy.Name }} on {{ dateUS .deletionRequest.RequestedAt }}
Approved: {{ .deletionRequest.ConfirmedBy.Name }} on {{ dateUS .deletionRequest.ConfirmedAt }}

diff --git a/views/deletions/cancelled_file.html b/views/deletions/cancelled_file.html index d4dd0a1c..a417f94b 100644 --- a/views/deletions/cancelled_file.html +++ b/views/deletions/cancelled_file.html @@ -7,13 +7,16 @@

Deletion Cancelled

-

You have cancelled deletion of the following file:

+

You have cancelled deletion of the following files:

+ + +
    + {{ range $index, $gf := .deletionRequest.GenericFiles }} +
  1. {{ $gf.Identifier }}
  2. + {{ end }} +

- {{ $gf.Identifier }}
- Size: {{ humanSize $gf.Size }}
- Created: {{ dateUS $gf.CreatedAt }}
- Updated: {{ dateUS $gf.UpdatedAt }}
Requested: {{ .deletionRequest.RequestedBy.Name }} on {{ dateUS .deletionRequest.RequestedAt }}
Cancelled: {{ .deletionRequest.CancelledBy.Name }} on {{ dateUS .deletionRequest.CancelledAt }}

diff --git a/views/deletions/cancelled_object.html b/views/deletions/cancelled_object.html index 5994a380..325bc993 100644 --- a/views/deletions/cancelled_object.html +++ b/views/deletions/cancelled_object.html @@ -7,16 +7,15 @@

Deletion Cancelled

-

You have cancelled deletion of the following object:

+

You have cancelled deletion of the following objects:

+ +
    + {{ range $index, $obj := .deletionRequest.IntellectualObjects }} +
  1. {{ $obj.Identifier }}
  2. + {{ end }} +

- {{ $obj.Identifier }}
- Alt Identifier: {{ $obj.AltIdentifier }}
- Internal Sender Identifier: {{ $obj.InternalSenderIdentifier }}
- Bag Group: {{ $obj.BagGroupIdentifier }}
- Storage Option: {{ $obj.StorageOption }}
- Created: {{ dateUS $obj.CreatedAt }}
- Updated: {{ dateUS $obj.UpdatedAt }}
Requested: {{ .deletionRequest.RequestedBy.Name }} on {{ dateUS .deletionRequest.RequestedAt }}
Cancelled: {{ .deletionRequest.CancelledBy.Name }} on {{ dateUS .deletionRequest.CancelledAt }}

diff --git a/views/deletions/review.html b/views/deletions/review.html index 0b40f22f..2b20d03f 100644 --- a/views/deletions/review.html +++ b/views/deletions/review.html @@ -5,7 +5,7 @@

Review Deletion Request

-

User {{ .deletionRequest.RequestedBy.Name }} ({{ .deletionRequest.RequestedBy.Email }}) wants to delete the following items:

+

User {{ .deletionRequest.RequestedBy.Name }} ({{ .deletionRequest.RequestedBy.Email }}) wants to delete the following items:

{{ if (eq .itemType "file") }} @@ -25,8 +25,8 @@

Generic File

Intellectual Object

-

- {{ .object.Identifier }}
+

+ {{ .object.Identifier }}
Alt Identifier: {{ .object.AltIdentifier }}
Internal Sender Identifier: {{ .object.InternalSenderIdentifier }}
Bag Group: {{ .object.BagGroupIdentifier }}
@@ -41,13 +41,15 @@

Intellectual Object

Intellectual Objects

+
    {{ range $index, $obj := .objectList }} -

    {{ $obj.Identifier }}

    +
  1. {{ $obj.Identifier }}
  2. {{ end }} +
{{ end }} -

Do you want to approve or cancel this request? If you approve, the items(s) will be deleted as soon as possible. Deletion cannot be undone. If you cancel, the file(s) will stay and no one else will be able to approve this request.

+

Do you want to approve or cancel this request? If you approve, the items(s) will be deleted as soon as possible. Deletion cannot be undone. If you cancel, the file(s) will stay and no one else will be able to approve this request.

From 7b383d0127f980c301847d7c73810f30dd00e4e7 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 29 Jan 2024 11:27:26 -0500 Subject: [PATCH 22/27] Added deletion request ID to deletion work item --- pgmodels/work_item.go | 3 ++- pgmodels/work_item_test.go | 31 ++++++++++++++++++++++------ views/deletions/approved_object.html | 2 +- web/webui/deletion.go | 4 ++-- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/pgmodels/work_item.go b/pgmodels/work_item.go index d4d588c5..e392c055 100644 --- a/pgmodels/work_item.go +++ b/pgmodels/work_item.go @@ -458,7 +458,7 @@ func NewRestorationItem(obj *IntellectualObject, gf *GenericFile, user *User) (* // Param requestedBy is the User who initially requested the deletion. // Param approvedBy is the User who approved the deletion request. // These two are required. -func NewDeletionItem(obj *IntellectualObject, gf *GenericFile, requestedBy, approvedBy *User) (*WorkItem, error) { +func NewDeletionItem(obj *IntellectualObject, gf *GenericFile, requestedBy, approvedBy *User, deletionRequestID int64) (*WorkItem, error) { if obj == nil || requestedBy == nil || approvedBy == nil { return nil, common.ErrInvalidParam } @@ -490,6 +490,7 @@ func NewDeletionItem(obj *IntellectualObject, gf *GenericFile, requestedBy, appr deletionItem.Action = constants.ActionDelete deletionItem.User = requestedBy.Email deletionItem.InstApprover = approvedBy.Email + deletionItem.DeletionRequestID = deletionRequestID err = deletionItem.Save() return deletionItem, err } diff --git a/pgmodels/work_item_test.go b/pgmodels/work_item_test.go index b517323a..fa762153 100644 --- a/pgmodels/work_item_test.go +++ b/pgmodels/work_item_test.go @@ -8,6 +8,7 @@ import ( "github.com/APTrust/registry/constants" "github.com/APTrust/registry/db" "github.com/APTrust/registry/pgmodels" + "github.com/APTrust/registry/web/testutil" "github.com/jinzhu/copier" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -373,6 +374,22 @@ func TestNewDeletionItem(t *testing.T) { require.Nil(t, err) require.NotNil(t, obj) + inst1Admin := testutil.InitUser(t, "admin@inst1.edu") + inst1ID := inst1Admin.InstitutionID + + deletionRequest, err := pgmodels.NewDeletionRequest() + require.NoError(t, err) + objects, err := pgmodels.IntellectualObjectSelect(pgmodels.NewQuery().Where("institution_id", "=", inst1ID)) + require.NoError(t, err) + require.NotEmpty(t, objects) + + deletionRequest.InstitutionID = inst1ID + deletionRequest.IntellectualObjects = objects + deletionRequest.RequestedByID = inst1Admin.ID + deletionRequest.RequestedBy = inst1Admin + deletionRequest.RequestedAt = time.Now().UTC() + require.NoError(t, deletionRequest.Save()) + query := pgmodels.NewQuery(). Where("institution_id", "=", obj.InstitutionID). Where("state", "=", constants.StateActive). @@ -391,7 +408,7 @@ func TestNewDeletionItem(t *testing.T) { require.Nil(t, err) require.NotNil(t, approver) - item1, err := pgmodels.NewDeletionItem(obj, nil, requestor, approver) + item1, err := pgmodels.NewDeletionItem(obj, nil, requestor, approver, deletionRequest.ID) require.Nil(t, err) require.NotNil(t, item1) assert.Equal(t, obj.ID, item1.IntellectualObjectID) @@ -399,6 +416,7 @@ func TestNewDeletionItem(t *testing.T) { assert.Equal(t, requestor.Email, item1.User) assert.Equal(t, approver.Email, item1.InstApprover) assert.Empty(t, item1.GenericFileID) + assert.Equal(t, deletionRequest.ID, item1.DeletionRequestID) query2 := pgmodels.NewQuery(). Where("intellectual_object_id", "=", obj.ID). @@ -408,7 +426,7 @@ func TestNewDeletionItem(t *testing.T) { require.Nil(t, err) require.NotNil(t, gf) - item2, err := pgmodels.NewDeletionItem(obj, gf, requestor, approver) + item2, err := pgmodels.NewDeletionItem(obj, gf, requestor, approver, deletionRequest.ID) require.Nil(t, err) require.NotNil(t, item2) assert.Equal(t, obj.ID, item2.IntellectualObjectID) @@ -417,24 +435,25 @@ func TestNewDeletionItem(t *testing.T) { assert.Equal(t, approver.Email, item2.InstApprover) assert.Equal(t, gf.ID, item2.GenericFileID) assert.Equal(t, gf.Size, item2.Size) + assert.Equal(t, deletionRequest.ID, item2.DeletionRequestID) // Missing object should cause an error - item3, err := pgmodels.NewDeletionItem(nil, gf, requestor, approver) + item3, err := pgmodels.NewDeletionItem(nil, gf, requestor, approver, 888) require.NotNil(t, err) require.Nil(t, item3) // Missing requestor and missing approver should cause errors - item4, err := pgmodels.NewDeletionItem(obj, gf, nil, approver) + item4, err := pgmodels.NewDeletionItem(obj, gf, nil, approver, 888) require.NotNil(t, err) require.Nil(t, item4) - item5, err := pgmodels.NewDeletionItem(obj, gf, requestor, nil) + item5, err := pgmodels.NewDeletionItem(obj, gf, requestor, nil, 888) require.NotNil(t, err) require.Nil(t, item5) // Object that has never been ingested should cause error randomObj := pgmodels.RandomObject() - item6, err := pgmodels.NewDeletionItem(randomObj, nil, requestor, approver) + item6, err := pgmodels.NewDeletionItem(randomObj, nil, requestor, approver, 888) require.NotNil(t, err) require.Nil(t, item6) } diff --git a/views/deletions/approved_object.html b/views/deletions/approved_object.html index 260567aa..f7c006d6 100644 --- a/views/deletions/approved_object.html +++ b/views/deletions/approved_object.html @@ -19,7 +19,7 @@ Approved: {{ .deletionRequest.ConfirmedBy.Name }} on {{ dateUS .deletionRequest.ConfirmedAt }}

-

All files belonging to this object will be deleted from preservation storage shortly. We'll retain a tombstone record of the object and its files along with PREMIS events recording when each item was deleted and at whose request.

+

All files belonging to thes objects will be deleted from preservation storage shortly. We'll retain a tombstone record of the object and its files along with PREMIS events recording when each item was deleted and at whose request.

Work Item #{{ .deletionRequest.WorkItemID }} shows the status of this deletion.

diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 3de6c744..813fead1 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -243,7 +243,7 @@ func (del *Deletion) initObjectDeletionRequest(institutionID int64, objIDs []int // CreateWorkItem creates a WorkItem describing this deletion. We call // this only if the admin approves the deletion. func (del *Deletion) CreateObjDeletionWorkItem(obj *pgmodels.IntellectualObject) error { - workItem, err := pgmodels.NewDeletionItem(obj, nil, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy) + workItem, err := pgmodels.NewDeletionItem(obj, nil, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy, del.DeletionRequest.ID) if err != nil { return err } @@ -257,7 +257,7 @@ func (del *Deletion) CreateFileDeletionWorkItem(gf *pgmodels.GenericFile) error return err } - workItem, err := pgmodels.NewDeletionItem(obj, gf, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy) + workItem, err := pgmodels.NewDeletionItem(obj, gf, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy, del.DeletionRequest.ID) if err != nil { return err } From 041a7c6f9b386e9491672229eda393146344e020 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 29 Jan 2024 11:27:49 -0500 Subject: [PATCH 23/27] Added some env vars to db metadata page --- views/internal_metadata/index.html | 44 +++++++++++++++++++++++++++ web/webui/internal_data_controller.go | 5 +++ 2 files changed, 49 insertions(+) diff --git a/views/internal_metadata/index.html b/views/internal_metadata/index.html index 60d80cd3..e009ae40 100644 --- a/views/internal_metadata/index.html +++ b/views/internal_metadata/index.html @@ -2,6 +2,50 @@ {{ template "shared/_header.html" .}} + + +
+
+

Environment

+
+ + + + + + + + + + + + + + + + + + + + + +
KeyValue
+ APT_ENV + + {{ .envName }} +
+ Redis URL + + {{ .redisUrl }} +
+ Database + + {{ .dbName }} +
+
+ + +

Internal Metadata

diff --git a/web/webui/internal_data_controller.go b/web/webui/internal_data_controller.go index 9195223d..b828df94 100644 --- a/web/webui/internal_data_controller.go +++ b/web/webui/internal_data_controller.go @@ -1,8 +1,10 @@ package webui import ( + "fmt" "net/http" + "github.com/APTrust/registry/common" "github.com/APTrust/registry/pgmodels" "github.com/gin-gonic/gin" ) @@ -25,6 +27,9 @@ func InternalMetadataIndex(c *gin.Context) { return } req.TemplateData["migrations"] = migrations + req.TemplateData["envName"] = common.Context().Config.EnvName + req.TemplateData["redisUrl"] = common.Context().Config.Redis.URL + req.TemplateData["dbName"] = fmt.Sprintf("%s@%s", common.Context().Config.DB.Name, common.Context().Config.DB.Host) c.HTML(http.StatusOK, "internal_metadata/index.html", req.TemplateData) } From 968e8779a59ebfc037fb4d53394a26c47596cac4 Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 29 Jan 2024 14:19:28 -0500 Subject: [PATCH 24/27] Include DeletionRequestID in WorkItemsView --- .../011_batch_deletion_work_items.sql | 49 ++++++++++++++++++- member_api_v3.yml | 7 ++- pgmodels/work_item_view.go | 1 + views/deletions/approved_object.html | 2 +- web/webui/deletion.go | 8 +++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/db/migrations/011_batch_deletion_work_items.sql b/db/migrations/011_batch_deletion_work_items.sql index 772c2cbb..98d45b8a 100644 --- a/db/migrations/011_batch_deletion_work_items.sql +++ b/db/migrations/011_batch_deletion_work_items.sql @@ -68,7 +68,54 @@ begin -- Now remove the work_item_id column from deletion requests alter table deletion_requests drop column work_item_id; - + + + drop view if exists work_items_view; + + CREATE OR REPLACE VIEW public.work_items_view + AS SELECT wi.id, + wi.institution_id, + i.name AS institution_name, + i.identifier AS institution_identifier, + wi.intellectual_object_id, + io.identifier AS object_identifier, + io.alt_identifier, + io.bag_group_identifier, + io.storage_option, + io.bagit_profile_identifier, + io.source_organization, + io.internal_sender_identifier, + wi.generic_file_id, + gf.identifier AS generic_file_identifier, + wi.name, + wi.etag, + wi.bucket, + wi."user", + wi.note, + wi.action, + wi.stage, + wi.status, + wi.outcome, + wi.bag_date, + wi.date_processed, + wi.retry, + wi.node, + wi.pid, + wi.needs_admin_review, + wi.size, + wi.queued_at, + wi.stage_started_at, + wi.aptrust_approver, + wi.inst_approver, + wi.deletion_request_id, + wi.created_at, + wi.updated_at + FROM work_items wi + LEFT JOIN institutions i ON wi.institution_id = i.id + LEFT JOIN intellectual_objects io ON wi.intellectual_object_id = io.id + LEFT JOIN generic_files gf ON wi.generic_file_id = gf.id; + + end if; end $$; diff --git a/member_api_v3.yml b/member_api_v3.yml index 7f118555..73df11b9 100644 --- a/member_api_v3.yml +++ b/member_api_v3.yml @@ -3,7 +3,7 @@ openapi: 3.0.0 info: title: APTrust Registry Member API description: Open API documentation for version 3 of the APTrust Member API. - version: '3.0' + version: '3.0.1' contact: email: "help@aptrust.org" license: @@ -800,6 +800,11 @@ components: type: string format: date-time description: The date and time of last known activity on this work item. This timestamp may change several times during multipart processes such as ingest. + deletion_request_id: + type: integer + format: int64 + description: The ID of the file or object deletion request related to this item. This will be null for all actions other than Delete. + nullable: false etag: type: string description: The etag of tar file uploaded for ingest. diff --git a/pgmodels/work_item_view.go b/pgmodels/work_item_view.go index e4ccbc78..f5b10410 100644 --- a/pgmodels/work_item_view.go +++ b/pgmodels/work_item_view.go @@ -86,6 +86,7 @@ type WorkItemView struct { StageStartedAt time.Time `json:"stage_started_at" pg:"stage_started_at"` APTrustApprover string `json:"aptrust_approver" pg:"aptrust_approver"` InstApprover string `json:"inst_approver" pg:"inst_approver"` + DeletionRequestID int64 `json:"deletion_request_id" pg:"deletion_request_id"` CreatedAt time.Time `json:"created_at" pg:"created_at"` UpdatedAt time.Time `json:"updated_at" pg:"updated_at"` } diff --git a/views/deletions/approved_object.html b/views/deletions/approved_object.html index f7c006d6..f12c0ec0 100644 --- a/views/deletions/approved_object.html +++ b/views/deletions/approved_object.html @@ -21,7 +21,7 @@

All files belonging to thes objects will be deleted from preservation storage shortly. We'll retain a tombstone record of the object and its files along with PREMIS events recording when each item was deleted and at whose request.

-

Work Item #{{ .deletionRequest.WorkItemID }} shows the status of this deletion.

+

Check the Work Items list to see the status of this deletion.

Back to Deletions List
diff --git a/web/webui/deletion.go b/web/webui/deletion.go index 813fead1..76ff74a8 100644 --- a/web/webui/deletion.go +++ b/web/webui/deletion.go @@ -243,10 +243,18 @@ func (del *Deletion) initObjectDeletionRequest(institutionID int64, objIDs []int // CreateWorkItem creates a WorkItem describing this deletion. We call // this only if the admin approves the deletion. func (del *Deletion) CreateObjDeletionWorkItem(obj *pgmodels.IntellectualObject) error { + if del.DeletionRequest == nil || del.DeletionRequest.ID == 0 { + errMsg := "Cannot create deletion work item because deletion request id is zero." + common.Context().Log.Error().Msgf(errMsg) + return fmt.Errorf(errMsg) + } + common.Context().Log.Warn().Msgf("Creating deletion work item for object %d - %s", obj.ID, obj.Identifier) workItem, err := pgmodels.NewDeletionItem(obj, nil, del.DeletionRequest.RequestedBy, del.DeletionRequest.ConfirmedBy, del.DeletionRequest.ID) if err != nil { + common.Context().Log.Error().Msgf(err.Error()) return err } + common.Context().Log.Warn().Msgf("Created deletion work item %d with deletion request id %d", workItem.ID, workItem.DeletionRequestID) del.DeletionRequest.WorkItems = append(del.DeletionRequest.WorkItems, workItem) return nil } From 910e0c1235d8f6983892aa061b35388afd7ec6ac Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Mon, 29 Jan 2024 15:07:46 -0500 Subject: [PATCH 25/27] Updated SQL schema file to support batch deletions --- db/schema.sql | 513 +++++++++++++++++++++----------------------------- 1 file changed, 214 insertions(+), 299 deletions(-) diff --git a/db/schema.sql b/db/schema.sql index 99757d81..c128884c 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -1,12 +1,7 @@ --- Registry Schema - 2023-04-10 +-- Registry Schema - 2024-01-29 --- public.ar_internal_metadata definition --- Drop table - --- DROP TABLE ar_internal_metadata; - -CREATE TABLE ar_internal_metadata ( +CREATE TABLE public.ar_internal_metadata ( "key" varchar NOT NULL, value varchar NULL, created_at timestamp NOT NULL, @@ -19,11 +14,7 @@ CREATE UNIQUE INDEX ix_ar_internal_metadata_uniq_key ON public.ar_internal_metad -- public.bulk_delete_jobs definition --- Drop table - --- DROP TABLE bulk_delete_jobs; - -CREATE TABLE bulk_delete_jobs ( +CREATE TABLE public.bulk_delete_jobs ( id bigserial NOT NULL, requested_by varchar NULL, institutional_approver varchar NULL, @@ -40,11 +31,7 @@ CREATE TABLE bulk_delete_jobs ( -- public.bulk_delete_jobs_emails definition --- Drop table - --- DROP TABLE bulk_delete_jobs_emails; - -CREATE TABLE bulk_delete_jobs_emails ( +CREATE TABLE public.bulk_delete_jobs_emails ( bulk_delete_job_id int8 NULL, email_id int8 NULL ); @@ -54,11 +41,7 @@ CREATE INDEX index_bulk_delete_jobs_emails_on_email_id ON public.bulk_delete_job -- public.bulk_delete_jobs_generic_files definition --- Drop table - --- DROP TABLE bulk_delete_jobs_generic_files; - -CREATE TABLE bulk_delete_jobs_generic_files ( +CREATE TABLE public.bulk_delete_jobs_generic_files ( bulk_delete_job_id int8 NULL, generic_file_id int8 NULL ); @@ -68,11 +51,7 @@ CREATE INDEX index_bulk_delete_jobs_generic_files_on_generic_file_id ON public.b -- public.bulk_delete_jobs_institutions definition --- Drop table - --- DROP TABLE bulk_delete_jobs_institutions; - -CREATE TABLE bulk_delete_jobs_institutions ( +CREATE TABLE public.bulk_delete_jobs_institutions ( bulk_delete_job_id int8 NULL, institution_id int8 NULL ); @@ -82,11 +61,7 @@ CREATE INDEX index_bulk_delete_jobs_institutions_on_institution_id ON public.bul -- public.bulk_delete_jobs_intellectual_objects definition --- Drop table - --- DROP TABLE bulk_delete_jobs_intellectual_objects; - -CREATE TABLE bulk_delete_jobs_intellectual_objects ( +CREATE TABLE public.bulk_delete_jobs_intellectual_objects ( bulk_delete_job_id int8 NULL, intellectual_object_id int8 NULL ); @@ -96,11 +71,7 @@ CREATE INDEX index_bulk_delete_jobs_intellectual_objects_on_object_id ON public. -- public.confirmation_tokens definition --- Drop table - --- DROP TABLE confirmation_tokens; - -CREATE TABLE confirmation_tokens ( +CREATE TABLE public.confirmation_tokens ( id bigserial NOT NULL, "token" varchar NULL, intellectual_object_id int4 NULL, @@ -113,11 +84,7 @@ CREATE TABLE confirmation_tokens ( -- public.emails definition --- Drop table - --- DROP TABLE emails; - -CREATE TABLE emails ( +CREATE TABLE public.emails ( id bigserial NOT NULL, email_type varchar NULL, event_identifier varchar NULL, @@ -135,11 +102,7 @@ CREATE TABLE emails ( -- public.emails_generic_files definition --- Drop table - --- DROP TABLE emails_generic_files; - -CREATE TABLE emails_generic_files ( +CREATE TABLE public.emails_generic_files ( generic_file_id int8 NULL, email_id int8 NULL ); @@ -149,11 +112,7 @@ CREATE INDEX index_emails_generic_files_on_generic_file_id ON public.emails_gene -- public.emails_intellectual_objects definition --- Drop table - --- DROP TABLE emails_intellectual_objects; - -CREATE TABLE emails_intellectual_objects ( +CREATE TABLE public.emails_intellectual_objects ( intellectual_object_id int8 NULL, email_id int8 NULL ); @@ -163,11 +122,7 @@ CREATE INDEX index_emails_intellectual_objects_on_intellectual_object_id ON publ -- public.emails_premis_events definition --- Drop table - --- DROP TABLE emails_premis_events; - -CREATE TABLE emails_premis_events ( +CREATE TABLE public.emails_premis_events ( premis_event_id int8 NULL, email_id int8 NULL ); @@ -177,11 +132,7 @@ CREATE INDEX index_emails_premis_events_on_premis_event_id ON public.emails_prem -- public.emails_work_items definition --- Drop table - --- DROP TABLE emails_work_items; - -CREATE TABLE emails_work_items ( +CREATE TABLE public.emails_work_items ( work_item_id int8 NULL, email_id int8 NULL ); @@ -191,11 +142,7 @@ CREATE INDEX index_emails_work_items_on_work_item_id ON public.emails_work_items -- public.generic_files definition --- Drop table - --- DROP TABLE generic_files; - -CREATE TABLE generic_files ( +CREATE TABLE public.generic_files ( id serial4 NOT NULL, file_format varchar NULL, "size" int8 NULL, @@ -207,7 +154,7 @@ CREATE TABLE generic_files ( last_fixity_check timestamp NOT NULL DEFAULT '2000-01-01 00:00:00'::timestamp without time zone, institution_id int4 NOT NULL, storage_option varchar NOT NULL DEFAULT 'Standard'::character varying, - uuid varchar NOT NULL, + "uuid" varchar NOT NULL, CONSTRAINT generic_files_pkey PRIMARY KEY (id) ); CREATE INDEX index_generic_files_on_created_at ON public.generic_files USING btree (created_at); @@ -224,11 +171,7 @@ CREATE INDEX ix_gf_last_fixity_check ON public.generic_files USING btree (last_f -- public.historical_deposit_stats definition --- Drop table - --- DROP TABLE historical_deposit_stats; - -CREATE TABLE historical_deposit_stats ( +CREATE TABLE public.historical_deposit_stats ( institution_id int8 NULL, institution_name varchar(80) NULL, storage_option varchar(40) NULL, @@ -252,11 +195,7 @@ CREATE UNIQUE INDEX ix_historical_inst_opt_date ON public.historical_deposit_sta -- public.intellectual_objects definition --- Drop table - --- DROP TABLE intellectual_objects; - -CREATE TABLE intellectual_objects ( +CREATE TABLE public.intellectual_objects ( id serial4 NOT NULL, title varchar NULL, description text NULL, @@ -286,11 +225,7 @@ CREATE INDEX index_intellectual_objects_on_updated_at ON public.intellectual_obj -- public.old_passwords definition --- Drop table - --- DROP TABLE old_passwords; - -CREATE TABLE old_passwords ( +CREATE TABLE public.old_passwords ( id bigserial NOT NULL, encrypted_password varchar NOT NULL, password_salt varchar NULL, @@ -304,11 +239,7 @@ CREATE INDEX index_password_archivable ON public.old_passwords USING btree (pass -- public.premis_events definition --- Drop table - --- DROP TABLE premis_events; - -CREATE TABLE premis_events ( +CREATE TABLE public.premis_events ( id serial4 NOT NULL, identifier varchar NULL, event_type varchar NULL, @@ -339,11 +270,7 @@ CREATE INDEX index_premis_events_on_outcome ON public.premis_events USING btree -- public.schema_migrations definition --- Drop table - --- DROP TABLE schema_migrations; - -CREATE TABLE schema_migrations ( +CREATE TABLE public.schema_migrations ( "version" varchar NOT NULL, started_at timestamp NULL, finished_at timestamp NULL, @@ -353,11 +280,7 @@ CREATE TABLE schema_migrations ( -- public.snapshots definition --- Drop table - --- DROP TABLE snapshots; - -CREATE TABLE snapshots ( +CREATE TABLE public.snapshots ( id bigserial NOT NULL, audit_date timestamp NULL, institution_id int4 NULL, @@ -374,11 +297,7 @@ CREATE TABLE snapshots ( -- public.storage_options definition --- Drop table - --- DROP TABLE storage_options; - -CREATE TABLE storage_options ( +CREATE TABLE public.storage_options ( id bigserial NOT NULL, provider varchar NOT NULL, service varchar NOT NULL, @@ -394,11 +313,7 @@ CREATE UNIQUE INDEX index_storage_options_name ON public.storage_options USING b -- public.usage_samples definition --- Drop table - --- DROP TABLE usage_samples; - -CREATE TABLE usage_samples ( +CREATE TABLE public.usage_samples ( id serial4 NOT NULL, created_at timestamp NOT NULL, updated_at timestamp NOT NULL, @@ -408,80 +323,121 @@ CREATE TABLE usage_samples ( ); --- public.work_items definition - --- Drop table - --- DROP TABLE work_items; +-- public.checksums definition -CREATE TABLE work_items ( +CREATE TABLE public.checksums ( id serial4 NOT NULL, + algorithm varchar NULL, + datetime timestamp NULL, + digest varchar NULL, + generic_file_id int4 NULL, created_at timestamp NOT NULL, updated_at timestamp NOT NULL, - intellectual_object_id int4 NULL, + CONSTRAINT checksums_pkey PRIMARY KEY (id), + CONSTRAINT fk_rails_89bb0866e7 FOREIGN KEY (generic_file_id) REFERENCES public.generic_files(id) +); +CREATE INDEX index_checksums_on_generic_file_id ON public.checksums USING btree (generic_file_id); + + +-- public.storage_records definition + +CREATE TABLE public.storage_records ( + id bigserial NOT NULL, generic_file_id int4 NULL, - "name" varchar NULL, - etag varchar NULL, - bucket varchar NULL, - "user" varchar NULL, - note text NULL, - "action" varchar NULL, - stage varchar NULL, - status varchar NULL, - outcome text NULL, - bag_date timestamp NULL, - date_processed timestamp NULL, - retry bool NOT NULL DEFAULT false, - node varchar(255) NULL, - pid int4 NULL DEFAULT 0, - needs_admin_review bool NOT NULL DEFAULT false, + url varchar NULL, + CONSTRAINT storage_records_pkey PRIMARY KEY (id), + CONSTRAINT fk_rails_a126ea6adc FOREIGN KEY (generic_file_id) REFERENCES public.generic_files(id) +); +CREATE INDEX index_storage_records_on_generic_file_id ON public.storage_records USING btree (generic_file_id); + + +-- public.alerts definition + +CREATE TABLE public.alerts ( + id bigserial NOT NULL, institution_id int4 NULL, - queued_at timestamp NULL, - "size" int8 NULL, - stage_started_at timestamp NULL, - aptrust_approver varchar NULL, - inst_approver varchar NULL, - CONSTRAINT work_items_pkey PRIMARY KEY (id) + "type" varchar NOT NULL, + subject varchar NOT NULL, + "content" text NOT NULL, + deletion_request_id int4 NULL, + created_at timestamp NOT NULL, + CONSTRAINT alerts_pkey PRIMARY KEY (id) ); -CREATE INDEX index_work_items_etag_instid_and_name ON public.work_items USING btree (etag, institution_id, name); -CREATE INDEX index_work_items_on_action ON public.work_items USING btree (action); -CREATE INDEX index_work_items_on_date_processed ON public.work_items USING btree (date_processed); -CREATE INDEX index_work_items_on_etag_and_name ON public.work_items USING btree (etag, name); -CREATE INDEX index_work_items_on_generic_file_id ON public.work_items USING btree (generic_file_id); -CREATE INDEX index_work_items_on_inst_id_and_date_processed ON public.work_items USING btree (institution_id, date_processed); -CREATE INDEX index_work_items_on_institution_id ON public.work_items USING btree (institution_id); -CREATE INDEX index_work_items_on_intellectual_object_id ON public.work_items USING btree (intellectual_object_id); -CREATE INDEX index_work_items_on_stage ON public.work_items USING btree (stage); -CREATE INDEX index_work_items_on_status ON public.work_items USING btree (status); +CREATE INDEX index_alerts_institution_id ON public.alerts USING btree (institution_id); +CREATE INDEX index_alerts_type ON public.alerts USING btree (type); --- public.checksums definition +-- public.alerts_premis_events definition --- Drop table +CREATE TABLE public.alerts_premis_events ( + alert_id int4 NOT NULL, + premis_event_id int4 NOT NULL +); +CREATE INDEX index_alerts_premis_events_alert_id ON public.alerts_premis_events USING btree (alert_id); +CREATE UNIQUE INDEX index_alerts_premis_events_unique ON public.alerts_premis_events USING btree (alert_id, premis_event_id); --- DROP TABLE checksums; -CREATE TABLE checksums ( - id serial4 NOT NULL, - algorithm varchar NULL, - datetime timestamp NULL, - digest varchar NULL, - generic_file_id int4 NULL, - created_at timestamp NOT NULL, - updated_at timestamp NOT NULL, - CONSTRAINT checksums_pkey PRIMARY KEY (id), - CONSTRAINT fk_rails_89bb0866e7 FOREIGN KEY (generic_file_id) REFERENCES generic_files(id) +-- public.alerts_users definition + +CREATE TABLE public.alerts_users ( + alert_id int4 NOT NULL, + user_id int4 NOT NULL, + sent_at timestamp NULL, + read_at timestamp NULL ); -CREATE INDEX index_checksums_on_generic_file_id ON public.checksums USING btree (generic_file_id); +CREATE INDEX index_alerts_users_alert_id ON public.alerts_users USING btree (alert_id); +CREATE UNIQUE INDEX index_alerts_users_unique ON public.alerts_users USING btree (alert_id, user_id); +CREATE INDEX index_alerts_users_user_id ON public.alerts_users USING btree (user_id); --- public.institutions definition +-- public.alerts_work_items definition --- Drop table +CREATE TABLE public.alerts_work_items ( + alert_id int4 NOT NULL, + work_item_id int4 NOT NULL +); +CREATE INDEX index_alerts_work_items_alert_id ON public.alerts_work_items USING btree (alert_id); +CREATE UNIQUE INDEX index_alerts_work_items_unique ON public.alerts_work_items USING btree (alert_id, work_item_id); --- DROP TABLE institutions; -CREATE TABLE institutions ( +-- public.deletion_requests definition + +CREATE TABLE public.deletion_requests ( + id bigserial NOT NULL, + institution_id int4 NOT NULL, + requested_by_id int4 NOT NULL, + requested_at timestamp NOT NULL, + encrypted_confirmation_token varchar NOT NULL, + confirmed_by_id int4 NULL, + confirmed_at timestamp NULL, + cancelled_by_id int4 NULL, + cancelled_at timestamp NULL, + CONSTRAINT deletion_requests_pkey PRIMARY KEY (id) +); +CREATE INDEX index_deletion_requests_institution_id ON public.deletion_requests USING btree (institution_id); + + +-- public.deletion_requests_generic_files definition + +CREATE TABLE public.deletion_requests_generic_files ( + deletion_request_id int4 NOT NULL, + generic_file_id int4 NOT NULL +); +CREATE UNIQUE INDEX index_drgf_unique ON public.deletion_requests_generic_files USING btree (deletion_request_id, generic_file_id); + + +-- public.deletion_requests_intellectual_objects definition + +CREATE TABLE public.deletion_requests_intellectual_objects ( + deletion_request_id int4 NOT NULL, + intellectual_object_id int4 NOT NULL +); +CREATE UNIQUE INDEX index_drio_unique ON public.deletion_requests_intellectual_objects USING btree (deletion_request_id, intellectual_object_id); + + +-- public.institutions definition + +CREATE TABLE public.institutions ( id serial4 NOT NULL, "name" varchar NULL, identifier varchar NULL, @@ -496,8 +452,7 @@ CREATE TABLE institutions ( restore_bucket varchar NOT NULL, spot_restore_frequency int4 NOT NULL DEFAULT 0, last_spot_restore_work_item_id int8 NULL, - CONSTRAINT institutions_pkey PRIMARY KEY (id), - CONSTRAINT fk_institutions_last_spot_restore FOREIGN KEY (last_spot_restore_work_item_id) REFERENCES work_items(id) + CONSTRAINT institutions_pkey PRIMARY KEY (id) ); CREATE UNIQUE INDEX index_institutions_identifier ON public.institutions USING btree (identifier); CREATE INDEX index_institutions_on_name ON public.institutions USING btree (name); @@ -505,29 +460,9 @@ CREATE UNIQUE INDEX index_institutions_receiving_bucket ON public.institutions U CREATE UNIQUE INDEX index_institutions_restore_bucket ON public.institutions USING btree (restore_bucket); --- public.storage_records definition - --- Drop table - --- DROP TABLE storage_records; - -CREATE TABLE storage_records ( - id bigserial NOT NULL, - generic_file_id int4 NULL, - url varchar NULL, - CONSTRAINT storage_records_pkey PRIMARY KEY (id), - CONSTRAINT fk_rails_a126ea6adc FOREIGN KEY (generic_file_id) REFERENCES generic_files(id) -); -CREATE INDEX index_storage_records_on_generic_file_id ON public.storage_records USING btree (generic_file_id); - - -- public.users definition --- Drop table - --- DROP TABLE users; - -CREATE TABLE users ( +CREATE TABLE public.users ( id serial4 NOT NULL, "name" varchar NULL, email varchar NULL, @@ -566,8 +501,7 @@ CREATE TABLE users ( grace_period timestamp NULL, awaiting_second_factor bool NOT NULL DEFAULT false, "role" varchar(50) NOT NULL DEFAULT 'none'::character varying, - CONSTRAINT users_pkey PRIMARY KEY (id), - CONSTRAINT fk_rails_7fcf39ca13 FOREIGN KEY (institution_id) REFERENCES institutions(id) + CONSTRAINT users_pkey PRIMARY KEY (id) ); CREATE INDEX index_users_on_authy_id ON public.users USING btree (authy_id); CREATE UNIQUE INDEX index_users_on_email ON public.users USING btree (email); @@ -576,134 +510,107 @@ CREATE INDEX index_users_on_password_changed_at ON public.users USING btree (pas CREATE UNIQUE INDEX index_users_on_reset_password_token ON public.users USING btree (reset_password_token); --- public.deletion_requests definition - --- Drop table - --- DROP TABLE deletion_requests; +-- public.work_items definition -CREATE TABLE deletion_requests ( - id bigserial NOT NULL, - institution_id int4 NOT NULL, - requested_by_id int4 NOT NULL, - requested_at timestamp NOT NULL, - encrypted_confirmation_token varchar NOT NULL, - confirmed_by_id int4 NULL, - confirmed_at timestamp NULL, - cancelled_by_id int4 NULL, - cancelled_at timestamp NULL, - work_item_id int4 NULL, - CONSTRAINT deletion_requests_pkey PRIMARY KEY (id), - CONSTRAINT deletion_requests_cancelled_by_id_fkey FOREIGN KEY (cancelled_by_id) REFERENCES users(id), - CONSTRAINT deletion_requests_confirmed_by_id_fkey FOREIGN KEY (confirmed_by_id) REFERENCES users(id), - CONSTRAINT deletion_requests_institution_id_fkey FOREIGN KEY (institution_id) REFERENCES institutions(id), - CONSTRAINT deletion_requests_requested_by_id_fkey FOREIGN KEY (requested_by_id) REFERENCES users(id), - CONSTRAINT deletion_requests_work_item_id_fkey FOREIGN KEY (work_item_id) REFERENCES work_items(id) +CREATE TABLE public.work_items ( + id serial4 NOT NULL, + created_at timestamp NOT NULL, + updated_at timestamp NOT NULL, + intellectual_object_id int4 NULL, + generic_file_id int4 NULL, + "name" varchar NULL, + etag varchar NULL, + bucket varchar NULL, + "user" varchar NULL, + note text NULL, + "action" varchar NULL, + stage varchar NULL, + status varchar NULL, + outcome text NULL, + bag_date timestamp NULL, + date_processed timestamp NULL, + retry bool NOT NULL DEFAULT false, + node varchar(255) NULL, + pid int4 NULL DEFAULT 0, + needs_admin_review bool NOT NULL DEFAULT false, + institution_id int4 NULL, + queued_at timestamp NULL, + "size" int8 NULL, + stage_started_at timestamp NULL, + aptrust_approver varchar NULL, + inst_approver varchar NULL, + deletion_request_id int8 NULL, + CONSTRAINT work_items_pkey PRIMARY KEY (id) ); -CREATE INDEX index_deletion_requests_institution_id ON public.deletion_requests USING btree (institution_id); +CREATE INDEX index_work_items_etag_instid_and_name ON public.work_items USING btree (etag, institution_id, name); +CREATE INDEX index_work_items_on_action ON public.work_items USING btree (action); +CREATE INDEX index_work_items_on_date_processed ON public.work_items USING btree (date_processed); +CREATE INDEX index_work_items_on_etag_and_name ON public.work_items USING btree (etag, name); +CREATE INDEX index_work_items_on_generic_file_id ON public.work_items USING btree (generic_file_id); +CREATE INDEX index_work_items_on_inst_id_and_date_processed ON public.work_items USING btree (institution_id, date_processed); +CREATE INDEX index_work_items_on_institution_id ON public.work_items USING btree (institution_id); +CREATE INDEX index_work_items_on_intellectual_object_id ON public.work_items USING btree (intellectual_object_id); +CREATE INDEX index_work_items_on_stage ON public.work_items USING btree (stage); +CREATE INDEX index_work_items_on_status ON public.work_items USING btree (status); --- public.deletion_requests_generic_files definition +-- public.alerts foreign keys --- Drop table +ALTER TABLE public.alerts ADD CONSTRAINT alerts_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES public.deletion_requests(id); +ALTER TABLE public.alerts ADD CONSTRAINT alerts_institution_id_fkey FOREIGN KEY (institution_id) REFERENCES public.institutions(id); --- DROP TABLE deletion_requests_generic_files; -CREATE TABLE deletion_requests_generic_files ( - deletion_request_id int4 NOT NULL, - generic_file_id int4 NOT NULL, - CONSTRAINT deletion_requests_generic_files_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES deletion_requests(id), - CONSTRAINT deletion_requests_generic_files_generic_file_id_fkey FOREIGN KEY (generic_file_id) REFERENCES generic_files(id) -); -CREATE UNIQUE INDEX index_drgf_unique ON public.deletion_requests_generic_files USING btree (deletion_request_id, generic_file_id); +-- public.alerts_premis_events foreign keys +ALTER TABLE public.alerts_premis_events ADD CONSTRAINT alerts_premis_events_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES public.alerts(id); +ALTER TABLE public.alerts_premis_events ADD CONSTRAINT alerts_premis_events_premis_event_id_fkey FOREIGN KEY (premis_event_id) REFERENCES public.premis_events(id); --- public.deletion_requests_intellectual_objects definition --- Drop table +-- public.alerts_users foreign keys --- DROP TABLE deletion_requests_intellectual_objects; +ALTER TABLE public.alerts_users ADD CONSTRAINT alerts_users_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES public.alerts(id); +ALTER TABLE public.alerts_users ADD CONSTRAINT alerts_users_user_id_fkey FOREIGN KEY (user_id) REFERENCES public.users(id); -CREATE TABLE deletion_requests_intellectual_objects ( - deletion_request_id int4 NOT NULL, - intellectual_object_id int4 NOT NULL, - CONSTRAINT deletion_requests_intellectual_obje_intellectual_object_id_fkey FOREIGN KEY (intellectual_object_id) REFERENCES intellectual_objects(id), - CONSTRAINT deletion_requests_intellectual_objects_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES deletion_requests(id) -); -CREATE UNIQUE INDEX index_drio_unique ON public.deletion_requests_intellectual_objects USING btree (deletion_request_id, intellectual_object_id); +-- public.alerts_work_items foreign keys --- public.alerts definition +ALTER TABLE public.alerts_work_items ADD CONSTRAINT alerts_work_items_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES public.alerts(id); +ALTER TABLE public.alerts_work_items ADD CONSTRAINT alerts_work_items_work_item_id_fkey FOREIGN KEY (work_item_id) REFERENCES public.work_items(id); --- Drop table --- DROP TABLE alerts; +-- public.deletion_requests foreign keys -CREATE TABLE alerts ( - id bigserial NOT NULL, - institution_id int4 NULL, - "type" varchar NOT NULL, - subject varchar NOT NULL, - "content" text NOT NULL, - deletion_request_id int4 NULL, - created_at timestamp NOT NULL, - CONSTRAINT alerts_pkey PRIMARY KEY (id), - CONSTRAINT alerts_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES deletion_requests(id), - CONSTRAINT alerts_institution_id_fkey FOREIGN KEY (institution_id) REFERENCES institutions(id) -); -CREATE INDEX index_alerts_institution_id ON public.alerts USING btree (institution_id); -CREATE INDEX index_alerts_type ON public.alerts USING btree (type); +ALTER TABLE public.deletion_requests ADD CONSTRAINT deletion_requests_cancelled_by_id_fkey FOREIGN KEY (cancelled_by_id) REFERENCES public.users(id); +ALTER TABLE public.deletion_requests ADD CONSTRAINT deletion_requests_confirmed_by_id_fkey FOREIGN KEY (confirmed_by_id) REFERENCES public.users(id); +ALTER TABLE public.deletion_requests ADD CONSTRAINT deletion_requests_institution_id_fkey FOREIGN KEY (institution_id) REFERENCES public.institutions(id); +ALTER TABLE public.deletion_requests ADD CONSTRAINT deletion_requests_requested_by_id_fkey FOREIGN KEY (requested_by_id) REFERENCES public.users(id); --- public.alerts_premis_events definition +-- public.deletion_requests_generic_files foreign keys --- Drop table +ALTER TABLE public.deletion_requests_generic_files ADD CONSTRAINT deletion_requests_generic_files_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES public.deletion_requests(id); +ALTER TABLE public.deletion_requests_generic_files ADD CONSTRAINT deletion_requests_generic_files_generic_file_id_fkey FOREIGN KEY (generic_file_id) REFERENCES public.generic_files(id); --- DROP TABLE alerts_premis_events; -CREATE TABLE alerts_premis_events ( - alert_id int4 NOT NULL, - premis_event_id int4 NOT NULL, - CONSTRAINT alerts_premis_events_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES alerts(id), - CONSTRAINT alerts_premis_events_premis_event_id_fkey FOREIGN KEY (premis_event_id) REFERENCES premis_events(id) -); -CREATE INDEX index_alerts_premis_events_alert_id ON public.alerts_premis_events USING btree (alert_id); -CREATE UNIQUE INDEX index_alerts_premis_events_unique ON public.alerts_premis_events USING btree (alert_id, premis_event_id); +-- public.deletion_requests_intellectual_objects foreign keys +ALTER TABLE public.deletion_requests_intellectual_objects ADD CONSTRAINT deletion_requests_intellectual_obje_intellectual_object_id_fkey FOREIGN KEY (intellectual_object_id) REFERENCES public.intellectual_objects(id); +ALTER TABLE public.deletion_requests_intellectual_objects ADD CONSTRAINT deletion_requests_intellectual_objects_deletion_request_id_fkey FOREIGN KEY (deletion_request_id) REFERENCES public.deletion_requests(id); --- public.alerts_users definition --- Drop table +-- public.institutions foreign keys --- DROP TABLE alerts_users; +ALTER TABLE public.institutions ADD CONSTRAINT fk_institutions_last_spot_restore FOREIGN KEY (last_spot_restore_work_item_id) REFERENCES public.work_items(id); -CREATE TABLE alerts_users ( - alert_id int4 NOT NULL, - user_id int4 NOT NULL, - sent_at timestamp NULL, - read_at timestamp NULL, - CONSTRAINT alerts_users_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES alerts(id), - CONSTRAINT alerts_users_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) -); -CREATE INDEX index_alerts_users_alert_id ON public.alerts_users USING btree (alert_id); -CREATE UNIQUE INDEX index_alerts_users_unique ON public.alerts_users USING btree (alert_id, user_id); -CREATE INDEX index_alerts_users_user_id ON public.alerts_users USING btree (user_id); +-- public.users foreign keys --- public.alerts_work_items definition +ALTER TABLE public.users ADD CONSTRAINT fk_rails_7fcf39ca13 FOREIGN KEY (institution_id) REFERENCES public.institutions(id); --- Drop table --- DROP TABLE alerts_work_items; +-- public.work_items foreign keys -CREATE TABLE alerts_work_items ( - alert_id int4 NOT NULL, - work_item_id int4 NOT NULL, - CONSTRAINT alerts_work_items_alert_id_fkey FOREIGN KEY (alert_id) REFERENCES alerts(id), - CONSTRAINT alerts_work_items_work_item_id_fkey FOREIGN KEY (work_item_id) REFERENCES work_items(id) -); -CREATE INDEX index_alerts_work_items_alert_id ON public.alerts_work_items USING btree (alert_id); -CREATE UNIQUE INDEX index_alerts_work_items_unique ON public.alerts_work_items USING btree (alert_id, work_item_id); +ALTER TABLE public.work_items ADD CONSTRAINT fk_work_items_deletion_request_id FOREIGN KEY (deletion_request_id) REFERENCES public.deletion_requests(id); -- public.alerts_view source @@ -720,7 +627,7 @@ AS SELECT a.id, a.created_at, au.user_id, u.name AS user_name, - u.email as user_email, + u.email AS user_email, au.sent_at, au.read_at FROM alerts a @@ -807,19 +714,12 @@ AS SELECT dr.id, WHERE drgf.deletion_request_id = dr.id) AS file_count, ( SELECT count(*) AS count FROM deletion_requests_intellectual_objects drio - WHERE drio.deletion_request_id = dr.id) AS object_count, - dr.work_item_id, - wi.stage, - wi.status, - wi.date_processed, - wi.size, - wi.note + WHERE drio.deletion_request_id = dr.id) AS object_count FROM deletion_requests dr LEFT JOIN institutions i ON dr.institution_id = i.id LEFT JOIN users req ON dr.requested_by_id = req.id LEFT JOIN users conf ON dr.confirmed_by_id = conf.id - LEFT JOIN users can ON dr.confirmed_by_id = can.id - LEFT JOIN work_items wi ON dr.work_item_id = wi.id; + LEFT JOIN users can ON dr.confirmed_by_id = can.id; -- public.generic_file_counts source @@ -1136,6 +1036,7 @@ AS SELECT wi.id, wi.stage_started_at, wi.aptrust_approver, wi.inst_approver, + wi.deletion_request_id, wi.created_at, wi.updated_at FROM work_items wi @@ -1145,6 +1046,8 @@ AS SELECT wi.id, +-- DROP FUNCTION public.create_constraint_if_not_exists(text, text, text); + CREATE OR REPLACE FUNCTION public.create_constraint_if_not_exists(t_name text, c_name text, constraint_sql text) RETURNS void LANGUAGE plpgsql @@ -1160,6 +1063,8 @@ end; $function$ ; +-- DROP FUNCTION public.populate_all_historical_deposit_stats(); + CREATE OR REPLACE FUNCTION public.populate_all_historical_deposit_stats() RETURNS void LANGUAGE plpgsql @@ -1199,6 +1104,8 @@ end; $function$ ; +-- DROP FUNCTION public.populate_current_deposit_stats(); + CREATE OR REPLACE FUNCTION public.populate_current_deposit_stats() RETURNS integer LANGUAGE plpgsql @@ -1238,6 +1145,8 @@ AS $function$ $function$ ; +-- DROP FUNCTION public.populate_deposit_stats(date); + CREATE OR REPLACE FUNCTION public.populate_deposit_stats(stop_date date) RETURNS void LANGUAGE plpgsql @@ -1276,6 +1185,8 @@ AS $function$ $function$ ; +-- DROP FUNCTION public.populate_empty_deposit_stats(); + CREATE OR REPLACE FUNCTION public.populate_empty_deposit_stats() RETURNS integer LANGUAGE plpgsql @@ -1314,10 +1225,8 @@ end; $function$ ; --- Note: This version of the function is no longer used. See the version --- below that takes a timestamp param. We have to keep this version in the --- schema for data loading and testing on dev machines. Delete this and --- the tests won't run. :( +-- DROP FUNCTION public.populate_historical_deposit_stats(date); + CREATE OR REPLACE FUNCTION public.populate_historical_deposit_stats(stop_date date) RETURNS integer LANGUAGE plpgsql @@ -1380,6 +1289,8 @@ AS $function$ $function$ ; +-- DROP FUNCTION public.populate_historical_deposit_stats(timestamp); + CREATE OR REPLACE FUNCTION public.populate_historical_deposit_stats(stop_date timestamp without time zone) RETURNS integer LANGUAGE plpgsql @@ -1440,6 +1351,8 @@ AS $function$ $function$ ; +-- DROP FUNCTION public.update_counts(); + CREATE OR REPLACE FUNCTION public.update_counts() RETURNS integer LANGUAGE plpgsql @@ -1484,6 +1397,8 @@ AS $function$ $function$ ; +-- DROP FUNCTION public.update_current_deposit_stats(); + CREATE OR REPLACE FUNCTION public.update_current_deposit_stats() RETURNS integer LANGUAGE plpgsql From 66c350d8f72bcc65c049486acca8b5ab1a42247e Mon Sep 17 00:00:00 2001 From: "A. Diamond" Date: Tue, 30 Jan 2024 09:05:55 -0500 Subject: [PATCH 26/27] Verify deletion request is valid for specified object/file before saving WorkItem. --- pgmodels/deletion_request.go | 20 +++++++++++ pgmodels/deletion_request_test.go | 56 +++++++++++++++++++++++++++++++ pgmodels/work_item.go | 31 +++++++++++++++++ 3 files changed, 107 insertions(+) diff --git a/pgmodels/deletion_request.go b/pgmodels/deletion_request.go index b3081f8f..95fafcbb 100644 --- a/pgmodels/deletion_request.go +++ b/pgmodels/deletion_request.go @@ -96,6 +96,26 @@ func DeletionRequestSelect(query *Query) ([]*DeletionRequest, error) { return requests, err } +// DeletionRequestIncludesFile returns true if the deletion request with the +// specified ID includes the generic file with the specified ID. +func DeletionRequestIncludesFile(requestID, gfID int64) (bool, error) { + db := common.Context().DB + var count int + query := `SELECT count(*) FROM deletion_requests_generic_files where deletion_request_id = ? and generic_file_id = ?` + _, err := db.Model((*DeletionRequestsGenericFiles)(nil)).QueryOne(pg.Scan(&count), query, requestID, gfID) + return count > 0, err +} + +// DeletionRequestIncludesObject returns true if the deletion request with the +// specified ID includes the intellectual object with the specified ID. +func DeletionRequestIncludesObject(requestID, objID int64) (bool, error) { + db := common.Context().DB + var count int + query := `SELECT count(*) FROM deletion_requests_intellectual_objects where deletion_request_id = ? and intellectual_object_id = ?` + _, err := db.Model((*DeletionRequestsGenericFiles)(nil)).QueryOne(pg.Scan(&count), query, requestID, objID) + return count > 0, err +} + // Save saves this requestitution to the database. This will peform an insert // if DeletionRequest.ID is zero. Otherwise, it updates. func (request *DeletionRequest) Save() error { diff --git a/pgmodels/deletion_request_test.go b/pgmodels/deletion_request_test.go index 72ea8045..51d326da 100644 --- a/pgmodels/deletion_request_test.go +++ b/pgmodels/deletion_request_test.go @@ -2,11 +2,13 @@ package pgmodels_test import ( "testing" + "time" "github.com/APTrust/registry/common" "github.com/APTrust/registry/constants" "github.com/APTrust/registry/db" "github.com/APTrust/registry/pgmodels" + "github.com/APTrust/registry/web/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -232,6 +234,60 @@ func TestDeletionRequestAddObject(t *testing.T) { assert.Equal(t, req.IntellectualObjects[0], req.FirstObject()) } +func TestDeletionRequestIncludesFilesAndObjects(t *testing.T) { + defer db.ForceFixtureReload() + + // Create a deletion request + req, err := pgmodels.NewDeletionRequest() + require.Nil(t, err) + require.NotNil(t, req) + + // Add the requestor, institution id, etc. + inst2Admin := testutil.InitUser(t, "admin@inst2.edu") + req.RequestedBy = inst2Admin + req.RequestedByID = inst2Admin.ID + req.InstitutionID = inst2Admin.InstitutionID + req.RequestedAt = time.Now().UTC() + + // Now add three objects and three files to the request + for i := 0; i < 3; i++ { + obj := pgmodels.RandomObject() + obj.InstitutionID = inst2Admin.InstitutionID + require.NoError(t, obj.Save()) + req.AddObject(obj) + + gf := pgmodels.RandomGenericFile(obj.ID, obj.Identifier) + gf.InstitutionID = inst2Admin.InstitutionID + require.NoError(t, gf.Save()) + req.AddFile(gf) + } + + // Save the request, with associated files and objects + require.NoError(t, req.Save()) + + // Now test the IncludesFile and IncludesObject functions + for _, obj := range req.IntellectualObjects { + isIncluded, err := pgmodels.DeletionRequestIncludesObject(req.ID, obj.ID) + require.NoError(t, err) + assert.True(t, isIncluded) + } + // Random, bogus object should not be included + isIncluded, err := pgmodels.DeletionRequestIncludesObject(req.ID, 99999999) + require.NoError(t, err) + assert.False(t, isIncluded) + + // Check the files + for _, gf := range req.GenericFiles { + isIncluded, err := pgmodels.DeletionRequestIncludesFile(req.ID, gf.ID) + require.NoError(t, err) + assert.True(t, isIncluded) + } + // Random, bogus file should not be included + isIncluded, err = pgmodels.DeletionRequestIncludesFile(req.ID, 99999999) + require.NoError(t, err) + assert.False(t, isIncluded) +} + func TestDeletionRequestConfirm(t *testing.T) { user, err := pgmodels.UserByID(5) require.Nil(t, err) diff --git a/pgmodels/work_item.go b/pgmodels/work_item.go index e392c055..11a9bda6 100644 --- a/pgmodels/work_item.go +++ b/pgmodels/work_item.go @@ -223,6 +223,37 @@ func (item *WorkItem) Validate() *common.ValidationError { if len(errors) > 0 { return &common.ValidationError{Errors: errors} } + + // Inst users and admins can't create or update work items, + // but let's say some smarty pants figures out a way to do this. + // Prevent malicious users from inserting a deletion request ID + // into this work item. For such an insertion to succeed, there + // would have to be an existing deletion request that already + // includes this file or object. + if item.Action == constants.ActionDelete && item.DeletionRequestID != 0 { + if item.GenericFileID != 0 { + isLegitFileDeletion, err := DeletionRequestIncludesFile(item.DeletionRequestID, item.GenericFileID) + if err != nil { + common.Context().Log.Error().Msgf("Error checking whether deletion request includes file: %v", err) + if !isLegitFileDeletion { + errors["DeletionRequestID"] = "Invalid deletion request ID / file" + } + } + } else if item.IntellectualObjectID != 0 { + isLegitObjectDeletion, err := DeletionRequestIncludesObject(item.DeletionRequestID, item.IntellectualObjectID) + if err != nil { + common.Context().Log.Error().Msgf("Error checking whether deletion request includes object: %v", err) + if !isLegitObjectDeletion { + errors["DeletionRequestID"] = "Invalid deletion request ID / object" + } + } + } + } + + if len(errors) > 0 { + return &common.ValidationError{Errors: errors} + } + return nil } From 13e2e799f9f8521a3244ce8095ac9cf8f4b47aaa Mon Sep 17 00:00:00 2001 From: Caesonia Date: Tue, 30 Jan 2024 13:18:28 -0500 Subject: [PATCH 27/27] Updating registry container. --- Dockerfile.multi | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile.multi b/Dockerfile.multi index 91b4c1fa..1c1099c0 100644 --- a/Dockerfile.multi +++ b/Dockerfile.multi @@ -78,6 +78,7 @@ ENV LOG_TO_CONSOLE=true ENV LOG_SQL=false ENV AWS_SES_PWD=password ENV AWS_SES_USER=system@user.org +ENV BATCH_DELETION_KEY=key # Making a note for a new dockerfile test. EXPOSE 8080