From 05a392346f8ff38f0d9aafe1a1cf2825648d4c22 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Oct 2025 08:55:02 -0400 Subject: [PATCH] Fix item deletion to properly clean up attachment files from storage (#1046) * Initial plan * Fix item deletion to properly clean up attachment files Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com> --- backend/internal/data/repo/repo_items.go | 55 ++++++++++++- backend/internal/data/repo/repo_items_test.go | 82 +++++++++++++++++++ backend/internal/data/repo/repos_all.go | 5 +- 3 files changed, 136 insertions(+), 6 deletions(-) diff --git a/backend/internal/data/repo/repo_items.go b/backend/internal/data/repo/repo_items.go index 455fb21f..5caec061 100644 --- a/backend/internal/data/repo/repo_items.go +++ b/backend/internal/data/repo/repo_items.go @@ -21,8 +21,9 @@ import ( ) type ItemsRepository struct { - db *ent.Client - bus *eventbus.EventBus + db *ent.Client + bus *eventbus.EventBus + attachments *AttachmentRepo } type ( @@ -632,7 +633,32 @@ func (e *ItemsRepository) Create(ctx context.Context, gid uuid.UUID, data ItemCr } func (e *ItemsRepository) Delete(ctx context.Context, id uuid.UUID) error { - err := e.db.Item.DeleteOneID(id).Exec(ctx) + // Get the item with its group and attachments before deletion + itm, err := e.db.Item.Query(). + Where(item.ID(id)). + WithGroup(). + WithAttachments(). + Only(ctx) + if err != nil { + return err + } + + // Get the group ID for attachment deletion + var gid uuid.UUID + if itm.Edges.Group != nil { + gid = itm.Edges.Group.ID + } + + // Delete all attachments (and their files) before deleting the item + for _, att := range itm.Edges.Attachments { + err := e.attachments.Delete(ctx, gid, id, att.ID) + if err != nil { + log.Err(err).Str("attachment_id", att.ID.String()).Msg("failed to delete attachment during item deletion") + // Continue with other attachments even if one fails + } + } + + err = e.db.Item.DeleteOneID(id).Exec(ctx) if err != nil { return err } @@ -642,7 +668,28 @@ func (e *ItemsRepository) Delete(ctx context.Context, id uuid.UUID) error { } func (e *ItemsRepository) DeleteByGroup(ctx context.Context, gid, id uuid.UUID) error { - _, err := e.db.Item. + // Get the item with its attachments before deletion + itm, err := e.db.Item.Query(). + Where( + item.ID(id), + item.HasGroupWith(group.ID(gid)), + ). + WithAttachments(). + Only(ctx) + if err != nil { + return err + } + + // Delete all attachments (and their files) before deleting the item + for _, att := range itm.Edges.Attachments { + err := e.attachments.Delete(ctx, gid, id, att.ID) + if err != nil { + log.Err(err).Str("attachment_id", att.ID.String()).Msg("failed to delete attachment during item deletion") + // Continue with other attachments even if one fails + } + } + + _, err = e.db.Item. Delete(). Where( item.ID(id), diff --git a/backend/internal/data/repo/repo_items_test.go b/backend/internal/data/repo/repo_items_test.go index 870bcd0c..b2f29440 100644 --- a/backend/internal/data/repo/repo_items_test.go +++ b/backend/internal/data/repo/repo_items_test.go @@ -2,12 +2,14 @@ package repo import ( "context" + "strings" "testing" "time" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/sysadminsmedia/homebox/backend/internal/data/ent/attachment" "github.com/sysadminsmedia/homebox/backend/internal/data/types" ) @@ -317,3 +319,83 @@ func TestItemRepository_GetAllCustomFields(t *testing.T) { assert.ElementsMatch(t, values[:1], results) } } + +func TestItemsRepository_DeleteWithAttachments(t *testing.T) { + // Create an item with an attachment + item := useItems(t, 1)[0] + + // Add an attachment to the item + attachment, err := tRepos.Attachments.Create( + context.Background(), + item.ID, + ItemCreateAttachment{ + Title: "test-attachment.txt", + Content: strings.NewReader("test content for attachment deletion"), + }, + attachment.TypePhoto, + true, + ) + require.NoError(t, err) + assert.NotNil(t, attachment) + + // Verify the attachment exists + retrievedAttachment, err := tRepos.Attachments.Get(context.Background(), tGroup.ID, attachment.ID) + require.NoError(t, err) + assert.Equal(t, attachment.ID, retrievedAttachment.ID) + + // Verify the attachment is linked to the item + itemWithAttachments, err := tRepos.Items.GetOne(context.Background(), item.ID) + require.NoError(t, err) + assert.Len(t, itemWithAttachments.Attachments, 1) + assert.Equal(t, attachment.ID, itemWithAttachments.Attachments[0].ID) + + // Delete the item + err = tRepos.Items.Delete(context.Background(), item.ID) + require.NoError(t, err) + + // Verify the item is deleted + _, err = tRepos.Items.GetOne(context.Background(), item.ID) + require.Error(t, err) + + // Verify the attachment is also deleted + _, err = tRepos.Attachments.Get(context.Background(), tGroup.ID, attachment.ID) + require.Error(t, err) +} + +func TestItemsRepository_DeleteByGroupWithAttachments(t *testing.T) { + // Create an item with an attachment + item := useItems(t, 1)[0] + + // Add an attachment to the item + attachment, err := tRepos.Attachments.Create( + context.Background(), + item.ID, + ItemCreateAttachment{ + Title: "test-attachment-by-group.txt", + Content: strings.NewReader("test content for attachment deletion by group"), + }, + attachment.TypePhoto, + true, + ) + require.NoError(t, err) + assert.NotNil(t, attachment) + + // Verify the attachment exists + retrievedAttachment, err := tRepos.Attachments.Get(context.Background(), tGroup.ID, attachment.ID) + require.NoError(t, err) + assert.Equal(t, attachment.ID, retrievedAttachment.ID) + + // Delete the item using DeleteByGroup + err = tRepos.Items.DeleteByGroup(context.Background(), tGroup.ID, item.ID) + require.NoError(t, err) + + // Verify the item is deleted + _, err = tRepos.Items.GetOneByGroup(context.Background(), tGroup.ID, item.ID) + require.Error(t, err) + + // Verify the attachment is also deleted + _, err = tRepos.Attachments.Get(context.Background(), tGroup.ID, attachment.ID) + require.Error(t, err) +} + + diff --git a/backend/internal/data/repo/repos_all.go b/backend/internal/data/repo/repos_all.go index d75cd03a..a9670aaa 100644 --- a/backend/internal/data/repo/repos_all.go +++ b/backend/internal/data/repo/repos_all.go @@ -21,14 +21,15 @@ type AllRepos struct { } func New(db *ent.Client, bus *eventbus.EventBus, storage config.Storage, pubSubConn string, thumbnail config.Thumbnail) *AllRepos { + attachments := &AttachmentRepo{db, storage, pubSubConn, thumbnail} return &AllRepos{ Users: &UserRepository{db}, AuthTokens: &TokenRepository{db}, Groups: NewGroupRepository(db), Locations: &LocationRepository{db, bus}, Labels: &LabelRepository{db, bus}, - Items: &ItemsRepository{db, bus}, - Attachments: &AttachmentRepo{db, storage, pubSubConn, thumbnail}, + Items: &ItemsRepository{db, bus, attachments}, + Attachments: attachments, MaintEntry: &MaintenanceEntryRepository{db}, Notifiers: NewNotifierRepository(db), }