Fix: Return error to UI when attachment upload fails due to storage misconfiguration (#1045)

* Initial plan

* Fix attachment upload error handling to return errors to UI

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

* Final verification: All tests pass and code builds successfully

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/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 rollbackErr := tx.Rollback(); rollbackErr != nil {
-		if err != nil {
+			return nil, rollbackErr
-			return nil, err
 		}
 		return nil, err
 	}

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
+		name        string
-		itemName            string
+		itemName    string
-		searchQuery         string
+		searchQuery string
-		shouldMatch         bool
+		shouldMatch bool
-		description         string
+		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)
 			}
 		})