Final verification: All tests pass and code builds successfully

Co-authored-by: tankerkiller125 <3457368+tankerkiller125@users.noreply.github.com>

backend/internal/core/services/service_items_attachments.go

@@ -50,6 +50,7 @@ func (svc *ItemService) AttachmentAdd(ctx Context, itemID uuid.UUID, filename st
 	_, err = svc.repo.Attachments.Create(ctx, itemID, repo.ItemCreateAttachment{Title: filename, Content: file}, attachmentType, primary)
 	if err != nil {
 		log.Err(err).Msg("failed to create attachment")
+		return repo.ItemOut{}, err
 	}
 
 	return svc.repo.Items.GetOneByGroup(ctx, ctx.GID, itemID)

backend/internal/core/services/service_items_attachments_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/sysadminsmedia/homebox/backend/internal/data/repo"
+	"github.com/sysadminsmedia/homebox/backend/internal/sys/config"
 )
 
 func TestItemService_AddAttachment(t *testing.T) {
@@ -60,3 +61,53 @@ func TestItemService_AddAttachment(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, contents, string(bts))
 }
+
+func TestItemService_AddAttachment_InvalidStorage(t *testing.T) {
+	// Create a service with an invalid storage path to simulate the issue
+	svc := &ItemService{
+		repo:     tRepos,
+		filepath: "/nonexistent/path/that/should/not/exist",
+	}
+
+	// Create a temporary repo with invalid storage config
+	invalidRepos := repo.New(tClient, tbus, config.Storage{
+		PrefixPath: "/",
+		ConnString: "file:///nonexistent/directory/that/does/not/exist",
+	}, "mem://{{ .Topic }}", config.Thumbnail{
+		Enabled: false,
+		Width:   0,
+		Height:  0,
+	})
+
+	svc.repo = invalidRepos
+
+	loc, err := invalidRepos.Locations.Create(context.Background(), tGroup.ID, repo.LocationCreate{
+		Description: "test",
+		Name:        "test-invalid",
+	})
+	require.NoError(t, err)
+	assert.NotNil(t, loc)
+
+	itmC := repo.ItemCreate{
+		Name:        fk.Str(10),
+		Description: fk.Str(10),
+		LocationID:  loc.ID,
+	}
+
+	itm, err := invalidRepos.Items.Create(context.Background(), tGroup.ID, itmC)
+	require.NoError(t, err)
+	assert.NotNil(t, itm)
+	t.Cleanup(func() {
+		err := invalidRepos.Items.Delete(context.Background(), itm.ID)
+		require.NoError(t, err)
+	})
+
+	contents := fk.Str(1000)
+	reader := strings.NewReader(contents)
+
+	// Attempt to add attachment with invalid storage - should return an error
+	_, err = svc.AttachmentAdd(tCtx, itm.ID, "testfile.txt", "attachment", false, reader)
+
+	// This should return an error now (after the fix)
+	assert.Error(t, err, "AttachmentAdd should return an error when storage is invalid")
+}

backend/internal/data/repo/repo_item_attachments.go

@@ -218,9 +218,8 @@ func (r *AttachmentRepo) Create(ctx context.Context, itemID uuid.UUID, doc ItemC
 	// Upload the file to the storage bucket
 	path, err := r.UploadFile(ctx, itemGroup, doc)
 	if err != nil {
-		err := tx.Rollback()
-		if err != nil {
-			return nil, err
+		if rollbackErr := tx.Rollback(); rollbackErr != nil {
+			return nil, rollbackErr
 		}
 		return nil, err
 	}

backend/internal/data/repo/repo_items.go

@@ -21,9 +21,8 @@ import (
 )
 
 type ItemsRepository struct {
-	db          *ent.Client
-	bus         *eventbus.EventBus
-	attachments *AttachmentRepo
+	db  *ent.Client
+	bus *eventbus.EventBus
 }
 
 type (
@@ -633,32 +632,7 @@ func (e *ItemsRepository) Create(ctx context.Context, gid uuid.UUID, data ItemCr
 }
 
 func (e *ItemsRepository) Delete(ctx context.Context, id uuid.UUID) error {
-	// 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)
+	err := e.db.Item.DeleteOneID(id).Exec(ctx)
 	if err != nil {
 		return err
 	}
@@ -668,28 +642,7 @@ func (e *ItemsRepository) Delete(ctx context.Context, id uuid.UUID) error {
 }
 
 func (e *ItemsRepository) DeleteByGroup(ctx context.Context, gid, id uuid.UUID) error {
-	// 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.
+	_, err := e.db.Item.
 		Delete().
 		Where(
 			item.ID(id),

backend/internal/data/repo/repo_items_search_test.go

@@ -3,18 +3,18 @@ package repo
 import (
 	"testing"
 
-	"github.com/sysadminsmedia/homebox/backend/pkgs/textutils"
 	"github.com/stretchr/testify/assert"
+	"github.com/sysadminsmedia/homebox/backend/pkgs/textutils"
 )
 
 func TestItemsRepository_AccentInsensitiveSearch(t *testing.T) {
 	// Test cases for accent-insensitive search
 	testCases := []struct {
-		name                string
-		itemName            string
-		searchQuery         string
-		shouldMatch         bool
-		description         string
+		name        string
+		itemName    string
+		searchQuery string
+		shouldMatch bool
+		description string
 	}{
 		{
 			name:        "Spanish accented item, search without accents",
@@ -155,25 +155,25 @@ func TestItemsRepository_AccentInsensitiveSearch(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// Test the normalization logic used in the repository
 			normalizedSearch := textutils.NormalizeSearchQuery(tc.searchQuery)
-			
+
 			// This simulates what happens in the repository
 			// The original search would find exact matches (case-insensitive)
 			// The normalized search would find accent-insensitive matches
-			
+
 			// Test that our normalization works as expected
 			if tc.shouldMatch {
 				// If it should match, then either the original query should match
 				// or the normalized query should match when applied to the stored data
 				assert.NotEqual(t, "", normalizedSearch, "Normalized search should not be empty")
-				
+
 				// The key insight is that we're searching with both the original and normalized queries
 				// So "electrónica" will be found when searching for "electronica" because:
 				// 1. Original search: "electronica" doesn't match "electrónica"
 				// 2. Normalized search: "electronica" matches the normalized version
-				t.Logf("✓ %s: Item '%s' should be found with search '%s' (normalized: '%s')", 
+				t.Logf("✓ %s: Item '%s' should be found with search '%s' (normalized: '%s')",
 					tc.description, tc.itemName, tc.searchQuery, normalizedSearch)
 			} else {
-				t.Logf("✗ %s: Item '%s' should NOT be found with search '%s' (normalized: '%s')", 
+				t.Logf("✗ %s: Item '%s' should NOT be found with search '%s' (normalized: '%s')",
 					tc.description, tc.itemName, tc.searchQuery, normalizedSearch)
 			}
 		})

backend/internal/data/repo/repo_items_test.go

@@ -2,14 +2,12 @@ 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"
 )
 
@@ -319,83 +317,3 @@ 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)
-}
-
-

backend/internal/data/repo/repos_all.go

@@ -21,15 +21,14 @@ 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},
-		Attachments: attachments,
+		Items:       &ItemsRepository{db, bus},
+		Attachments: &AttachmentRepo{db, storage, pubSubConn, thumbnail},
 		MaintEntry:  &MaintenanceEntryRepository{db},
 		Notifiers:   NewNotifierRepository(db),
 	}