fix(favorites): address review follow-ups

commit: 2ddb1b29ccdfb802cf5cf20e4372867103e9cc89
author: brice · 3mo ago · 2026-06-28 13:09 UTC
parent: 7be5016
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

6 files · +100 -17

M docs/systemviews-wasm-crud-plan.md +3 -3


@@ -52,9 +52,9 @@ Preferred direction for new work:
52	52		\| Operation \| Old Svelte surface \| Backend message/API \| Current WASM status \| Gap \|
53	53		\| --- \| --- \| --- \| --- \| --- \|
54	54		\| Read favorites \| `FavoritesPanel.svelte`, `FavoriteGrid.svelte`, `FavoriteCard.svelte` \| `favorite_list` \| Present, but bare link list (`favorites_view.go`) with tags-first data. \| Add rich rows/cards, favicons, shortcut badges, and tag metadata. \|
55		-	\| Create favorite \| Old homepage hints imply adding/pinning from history; native omnibox toggle exists outside this page. \| `ManageFavoritesUseCase.Add` exists, but no homepage/systemview message. \| Missing from `port.HomepageFavorites`/`SystemviewFavoritesService` and handlers. \| Add `favorite_create` handler/bridge and UI. \|
56		-	\| Update favorite \| `FavoriteEditor.svelte` locally updates title, shortcut, and tags. \| `favorite_set_shortcut`, `tag_assign`, `tag_remove`; no title update handler. \| Partial bridge support. \| Add `favorite_update` for title/favicon/url policy; keep shortcut/tag specialized calls or fold into update after design. \|
57		-	\| Delete favorite \| `FavoriteEditor.svelte` local-only delete placeholder. \| `ManageFavoritesUseCase.Remove` exists, but no homepage/systemview message. \| Missing. \| Add `favorite_delete` handler/bridge/UI. \|
	55	+	\| Create favorite \| Old homepage hints imply adding/pinning from history; native omnibox toggle exists outside this page. \| `ManageFavoritesUseCase.AddFavorite`, `port.HomepageFavorites`, `SystemviewFavoritesService`, homepage handlers, and bridge expose `favorite_create`. \| Supported in WASM UI. \| Keep validation/error states covered. \|
	56	+	\| Update favorite \| `FavoriteEditor.svelte` locally updates title, shortcut, and tags. \| `ManageFavoritesUseCase.UpdateFavorite`, `favorite_update`, `favorite_set_shortcut`, `tag_assign`, and `tag_remove` are exposed through the ports, handlers, and bridge. \| Supported in WASM UI. \| Keep shortcut/tag semantics covered by regression tests. \|
	57	+	\| Delete favorite \| `FavoriteEditor.svelte` local-only delete placeholder. \| `ManageFavoritesUseCase.DeleteFavorite`/`Remove`, `port.HomepageFavorites`, `SystemviewFavoritesService`, homepage handlers, and bridge expose `favorite_delete`. \| Supported in WASM UI. \| Keep destructive confirmation and refresh behavior covered. \|
58	58		\| Read tags \| `TagCloud.svelte` \| `tag_list` \| Present as static list. \| Add color chips and selection. \|
59	59		\| Create tag \| `TagCloud.svelte` inline create \| `tag_create` \| Bridge and handler exist. \| Add UI with color picker/palette. \|
60	60		\| Update tag \| Handler exists (`tag_update`). \| `tag_update` \| Bridge exists. \| Add rename/color edit UI. \|


@@ -52,9 +52,9 @@ Preferred direction for new work:
52		\| Operation \| Old Svelte surface \| Backend message/API \| Current WASM status \| Gap \|	52		\| Operation \| Old Svelte surface \| Backend message/API \| Current WASM status \| Gap \|
53		\| --- \| --- \| --- \| --- \| --- \|	53		\| --- \| --- \| --- \| --- \| --- \|
54		\| Read favorites \| `FavoritesPanel.svelte`, `FavoriteGrid.svelte`, `FavoriteCard.svelte` \| `favorite_list` \| Present, but bare link list (`favorites_view.go`) with tags-first data. \| Add rich rows/cards, favicons, shortcut badges, and tag metadata. \|	54		\| Read favorites \| `FavoritesPanel.svelte`, `FavoriteGrid.svelte`, `FavoriteCard.svelte` \| `favorite_list` \| Present, but bare link list (`favorites_view.go`) with tags-first data. \| Add rich rows/cards, favicons, shortcut badges, and tag metadata. \|
55	-	\| Create favorite \| Old homepage hints imply adding/pinning from history; native omnibox toggle exists outside this page. \| `ManageFavoritesUseCase.Add` exists, but no homepage/systemview message. \| Missing from `port.HomepageFavorites`/`SystemviewFavoritesService` and handlers. \| Add `favorite_create` handler/bridge and UI. \|	55	+	\| Create favorite \| Old homepage hints imply adding/pinning from history; native omnibox toggle exists outside this page. \| `ManageFavoritesUseCase.AddFavorite`, `port.HomepageFavorites`, `SystemviewFavoritesService`, homepage handlers, and bridge expose `favorite_create`. \| Supported in WASM UI. \| Keep validation/error states covered. \|
56	-	\| Update favorite \| `FavoriteEditor.svelte` locally updates title, shortcut, and tags. \| `favorite_set_shortcut`, `tag_assign`, `tag_remove`; no title update handler. \| Partial bridge support. \| Add `favorite_update` for title/favicon/url policy; keep shortcut/tag specialized calls or fold into update after design. \|	56	+	\| Update favorite \| `FavoriteEditor.svelte` locally updates title, shortcut, and tags. \| `ManageFavoritesUseCase.UpdateFavorite`, `favorite_update`, `favorite_set_shortcut`, `tag_assign`, and `tag_remove` are exposed through the ports, handlers, and bridge. \| Supported in WASM UI. \| Keep shortcut/tag semantics covered by regression tests. \|
57	-	\| Delete favorite \| `FavoriteEditor.svelte` local-only delete placeholder. \| `ManageFavoritesUseCase.Remove` exists, but no homepage/systemview message. \| Missing. \| Add `favorite_delete` handler/bridge/UI. \|	57	+	\| Delete favorite \| `FavoriteEditor.svelte` local-only delete placeholder. \| `ManageFavoritesUseCase.DeleteFavorite`/`Remove`, `port.HomepageFavorites`, `SystemviewFavoritesService`, homepage handlers, and bridge expose `favorite_delete`. \| Supported in WASM UI. \| Keep destructive confirmation and refresh behavior covered. \|
58		\| Read tags \| `TagCloud.svelte` \| `tag_list` \| Present as static list. \| Add color chips and selection. \|	58		\| Read tags \| `TagCloud.svelte` \| `tag_list` \| Present as static list. \| Add color chips and selection. \|
59		\| Create tag \| `TagCloud.svelte` inline create \| `tag_create` \| Bridge and handler exist. \| Add UI with color picker/palette. \|	59		\| Create tag \| `TagCloud.svelte` inline create \| `tag_create` \| Bridge and handler exist. \| Add UI with color picker/palette. \|
60		\| Update tag \| Handler exists (`tag_update`). \| `tag_update` \| Bridge exists. \| Add rename/color edit UI. \|	60		\| Update tag \| Handler exists (`tag_update`). \| `tag_update` \| Bridge exists. \| Add rename/color edit UI. \|

M internal/application/usecase/manage_favorites.go +1 -5


@@ -82,11 +82,7 @@ func (uc *ManageFavoritesUseCase) addNewFavorite(ctx context.Context, input AddF
82	82		// Assign tags
83	83		for _, tagID := range input.Tags {
84	84		if err := uc.tagRepo.AssignToFavorite(ctx, tagID, fav.ID); err != nil {
85		-	log.Warn().
86		-	Int64("tag_id", int64(tagID)).
87		-	Int64("favorite_id", int64(fav.ID)).
88		-	Err(err).
89		-	Msg("failed to assign tag")
	85	+	return nil, fmt.Errorf("failed to assign tag %d to favorite %d: %w", tagID, fav.ID, err)
90	86		}
91	87		}
92	88

 	// Assign tags
 	for _, tagID := range input.Tags {
 		if err := uc.tagRepo.AssignToFavorite(ctx, tagID, fav.ID); err != nil {
-			log.Warn().
+			return nil, fmt.Errorf("failed to assign tag %d to favorite %d: %w", tagID, fav.ID, err)
-				Int64("tag_id", int64(tagID)).
-				Int64("favorite_id", int64(fav.ID)).
-				Err(err).
-				Msg("failed to assign tag")
 		}
 	}

M internal/application/usecase/manage_favorites_test.go +24 -0


@@ -429,6 +429,30 @@ func TestManageFavoritesUseCase_AddFavoriteAssignsTagsWithoutFolder(t *testing.T
429	429		assert.Equal(t, entity.FavoriteID(42), fav.ID)
430	430		}
431	431
	432	+	func TestManageFavoritesUseCase_AddFavoriteReturnsAssignTagError(t *testing.T) {
	433	+	ctx := testContext()
	434	+
	435	+	favoriteRepo := repomocks.NewMockFavoriteRepository(t)
	436	+	tagRepo := repomocks.NewMockTagRepository(t)
	437	+
	438	+	favoriteRepo.EXPECT().FindByURL(mock.Anything, "https://example.com").Return(nil, nil).Once()
	439	+	favoriteRepo.EXPECT().Save(mock.Anything, mock.Anything).Run(func(_ context.Context, fav *entity.Favorite) {
	440	+	fav.ID = entity.FavoriteID(42)
	441	+	}).Return(nil).Once()
	442	+	tagRepo.EXPECT().AssignToFavorite(mock.Anything, entity.TagID(7), entity.FavoriteID(42)).Return(errors.New("tag missing")).Once()
	443	+
	444	+	uc := usecase.NewManageFavoritesUseCase(favoriteRepo, tagRepo)
	445	+
	446	+	fav, err := uc.AddFavorite(ctx, dto.FavoriteCreateInput{
	447	+	URL: "https://example.com",
	448	+	Title: "Example",
	449	+	Tags: []entity.TagID{7},
	450	+	})
	451	+	require.Error(t, err)
	452	+	assert.Nil(t, fav)
	453	+	assert.Contains(t, err.Error(), "failed to assign tag 7 to favorite 42")
	454	+	}
	455	+
432	456		func TestManageFavoritesUseCase_AddFavoriteRejectsUnsafeOrMalformedURL(t *testing.T) {
433	457		ctx := testContext()
434	458


@@ -429,6 +429,30 @@ func TestManageFavoritesUseCase_AddFavoriteAssignsTagsWithoutFolder(t *testing.T
429	assert.Equal(t, entity.FavoriteID(42), fav.ID)	429		assert.Equal(t, entity.FavoriteID(42), fav.ID)
430	}	430		}
431		431
		432	+	func TestManageFavoritesUseCase_AddFavoriteReturnsAssignTagError(t *testing.T) {
		433	+	ctx := testContext()
		434	+
		435	+	favoriteRepo := repomocks.NewMockFavoriteRepository(t)
		436	+	tagRepo := repomocks.NewMockTagRepository(t)
		437	+
		438	+	favoriteRepo.EXPECT().FindByURL(mock.Anything, "https://example.com").Return(nil, nil).Once()
		439	+	favoriteRepo.EXPECT().Save(mock.Anything, mock.Anything).Run(func(_ context.Context, fav *entity.Favorite) {
		440	+	fav.ID = entity.FavoriteID(42)
		441	+	}).Return(nil).Once()
		442	+	tagRepo.EXPECT().AssignToFavorite(mock.Anything, entity.TagID(7), entity.FavoriteID(42)).Return(errors.New("tag missing")).Once()
		443	+
		444	+	uc := usecase.NewManageFavoritesUseCase(favoriteRepo, tagRepo)
		445	+
		446	+	fav, err := uc.AddFavorite(ctx, dto.FavoriteCreateInput{
		447	+	URL: "https://example.com",
		448	+	Title: "Example",
		449	+	Tags: []entity.TagID{7},
		450	+	})
		451	+	require.Error(t, err)
		452	+	assert.Nil(t, fav)
		453	+	assert.Contains(t, err.Error(), "failed to assign tag 7 to favorite 42")
		454	+	}
		455	+
432	func TestManageFavoritesUseCase_AddFavoriteRejectsUnsafeOrMalformedURL(t *testing.T) {	456		func TestManageFavoritesUseCase_AddFavoriteRejectsUnsafeOrMalformedURL(t *testing.T) {
433	ctx := testContext()	457		ctx := testContext()
434		458

M internal/infrastructure/handlers/homepage/favorites_test.go +22 -0


@@ -93,3 +93,25 @@ func TestHandleFavoriteUpdatePreservesShortcutWhenOmitted(t *testing.T) {
93	93		require.True(t, ok)
94	94		require.True(t, resp.Success)
95	95		}
	96	+	func TestHandleFavoriteUpdateClearsShortcutWhenExplicitNull(t *testing.T) {
	97	+	t.Parallel()
	98	+
	99	+	favorites := portmocks.NewMockHomepageFavorites(t)
	100	+	favorites.EXPECT().
	101	+	UpdateFavorite(mock.Anything, dto.FavoriteUpdateInput{
	102	+	ID: entity.FavoriteID(42),
	103	+	Title: "Updated",
	104	+	FaviconURL: "https://example.com/favicon.ico",
	105	+	ShortcutKeySet: true,
	106	+	}).
	107	+	Return(&entity.Favorite{ID: 42, URL: "https://example.com", Title: "Updated"}, nil).
	108	+	Once()
	109	+
	110	+	handler := NewFavoritesHandlers(favorites).HandleUpdate()
	111	+	got, err := handler.Handle(context.Background(), port.WebViewID(0), json.RawMessage(`{"requestId":"req-5","id":42,"title":"Updated","favicon_url":"https://example.com/favicon.ico","shortcut_key":null}`))
	112	+	require.NoError(t, err)
	113	+
	114	+	resp, ok := got.(Response)
	115	+	require.True(t, ok)
	116	+	require.True(t, resp.Success)
	117	+	}


@@ -93,3 +93,25 @@ func TestHandleFavoriteUpdatePreservesShortcutWhenOmitted(t *testing.T) {
93	require.True(t, ok)	93		require.True(t, ok)
94	require.True(t, resp.Success)	94		require.True(t, resp.Success)
95	}	95		}
		96	+	func TestHandleFavoriteUpdateClearsShortcutWhenExplicitNull(t *testing.T) {
		97	+	t.Parallel()
		98	+
		99	+	favorites := portmocks.NewMockHomepageFavorites(t)
		100	+	favorites.EXPECT().
		101	+	UpdateFavorite(mock.Anything, dto.FavoriteUpdateInput{
		102	+	ID: entity.FavoriteID(42),
		103	+	Title: "Updated",
		104	+	FaviconURL: "https://example.com/favicon.ico",
		105	+	ShortcutKeySet: true,
		106	+	}).
		107	+	Return(&entity.Favorite{ID: 42, URL: "https://example.com", Title: "Updated"}, nil).
		108	+	Once()
		109	+
		110	+	handler := NewFavoritesHandlers(favorites).HandleUpdate()
		111	+	got, err := handler.Handle(context.Background(), port.WebViewID(0), json.RawMessage(`{"requestId":"req-5","id":42,"title":"Updated","favicon_url":"https://example.com/favicon.ico","shortcut_key":null}`))
		112	+	require.NoError(t, err)
		113	+
		114	+	resp, ok := got.(Response)
		115	+	require.True(t, ok)
		116	+	require.True(t, resp.Success)
		117	+	}

M internal/infrastructure/persistence/sqlite/favorite_repo.go +17 -9


@@ -13,6 +13,8 @@ import (
13	13
14	14		// ==================== Favorite Repository ====================
15	15
	16	+	const favoriteTagHydrationBatchSize = 900
	17	+
16	18		type favoriteRepo struct {
17	19		queries *sqlc.Queries
18	20		}

@@ -146,16 +148,22 @@ func (r favoriteRepo) hydrateTags(ctx context.Context, favorites []entity.Favo
146	148		return nil
147	149		}
148	150
149		-	rows, err := r.queries.GetTagsForFavorites(ctx, ids)
150		-	if err != nil {
151		-	return err
152		-	}
153		-	for _, row := range rows {
154		-	fav := byID[entity.FavoriteID(row.FavoriteID)]
155		-	if fav == nil {
156		-	continue
	151	+	for start := 0; start < len(ids); start += favoriteTagHydrationBatchSize {
	152	+	end := start + favoriteTagHydrationBatchSize
	153	+	if end > len(ids) {
	154	+	end = len(ids)
	155	+	}
	156	+	rows, err := r.queries.GetTagsForFavorites(ctx, ids[start:end])
	157	+	if err != nil {
	158	+	return err
	159	+	}
	160	+	for _, row := range rows {
	161	+	fav := byID[entity.FavoriteID(row.FavoriteID)]
	162	+	if fav == nil {
	163	+	continue
	164	+	}
	165	+	fav.Tags = append(fav.Tags, *tagFromRow(row.FavoriteTag))
157	166		}
158		-	fav.Tags = append(fav.Tags, *tagFromRow(row.FavoriteTag))
159	167		}
160	168		return nil
161	169		}

 // ==================== Favorite Repository ====================
+const favoriteTagHydrationBatchSize = 900
 type favoriteRepo struct {
 	queries *sqlc.Queries
 }
 		return nil
 	}
-	rows, err := r.queries.GetTagsForFavorites(ctx, ids)
+	for start := 0; start < len(ids); start += favoriteTagHydrationBatchSize {
-	if err != nil {
+		end := start + favoriteTagHydrationBatchSize
-		return err
+		if end > len(ids) {
-	}
+			end = len(ids)
-	for _, row := range rows {
+		}
-		fav := byID[entity.FavoriteID(row.FavoriteID)]
+		rows, err := r.queries.GetTagsForFavorites(ctx, ids[start:end])
-		if fav == nil {
+		if err != nil {
-			continue
+			return err
+		}
+		for _, row := range rows {
+			fav := byID[entity.FavoriteID(row.FavoriteID)]
+			if fav == nil {
+				continue
+			}
+			fav.Tags = append(fav.Tags, *tagFromRow(row.FavoriteTag))
 		}
-		fav.Tags = append(fav.Tags, *tagFromRow(row.FavoriteTag))
 	}
 	return nil
 }

M internal/infrastructure/persistence/sqlite/favorite_repo_test.go +33 -0


@@ -3,6 +3,7 @@ package sqlite
3	3		import (
4	4		"context"
5	5		"database/sql"
	6	+	"fmt"
6	7		"testing"
7	8
8	9		"github.com/bnema/dumber/internal/domain/entity"

@@ -154,6 +155,38 @@ func TestFavoriteRepositorySaveUpdatesShortcutKey(t *testing.T) {
154	155		}
155	156		}
156	157
	158	+	func TestFavoriteRepositoryGetAllHydratesTagsInBatches(t *testing.T) {
	159	+	db := openFavoriteRepoTestDB(t)
	160	+	mustExec(t, db, `INSERT INTO favorite_tags(id, name, color) VALUES (1, 'batched', '#111111')`)
	161	+	for i := int64(1); i <= favoriteTagHydrationBatchSize+5; i++ {
	162	+	_, err := db.Exec(
	163	+	`INSERT INTO favorites(id, url, title, position) VALUES (?, ?, ?, ?)`,
	164	+	i,
	165	+	fmt.Sprintf("https://batch.example/%d", i),
	166	+	"Batch",
	167	+	i,
	168	+	)
	169	+	if err != nil {
	170	+	t.Fatalf("insert favorite %d: %v", i, err)
	171	+	}
	172	+	_, err = db.Exec(`INSERT INTO favorite_tag_assignments(favorite_id, tag_id) VALUES (?, 1)`, i)
	173	+	if err != nil {
	174	+	t.Fatalf("insert assignment %d: %v", i, err)
	175	+	}
	176	+	}
	177	+	repo := NewFavoriteRepository(db)
	178	+
	179	+	favorites, err := repo.GetAll(context.Background())
	180	+	if err != nil {
	181	+	t.Fatalf("GetAll: %v", err)
	182	+	}
	183	+	if len(favorites) != favoriteTagHydrationBatchSize+5 {
	184	+	t.Fatalf("expected %d favorites, got %d", favoriteTagHydrationBatchSize+5, len(favorites))
	185	+	}
	186	+	assertTagNames(t, favorites[0], "batched")
	187	+	assertTagNames(t, favorites[len(favorites)-1], "batched")
	188	+	}
	189	+
157	190		func assertTagNames(t testing.T, fav entity.Favorite, names ...string) {
158	191		t.Helper()
159	192		if fav == nil {


@@ -3,6 +3,7 @@ package sqlite
3	import (	3		import (
4	"context"	4		"context"
5	"database/sql"	5		"database/sql"
		6	+	"fmt"
6	"testing"	7		"testing"
7		8
8	"github.com/bnema/dumber/internal/domain/entity"	9		"github.com/bnema/dumber/internal/domain/entity"

@@ -154,6 +155,38 @@ func TestFavoriteRepositorySaveUpdatesShortcutKey(t *testing.T) {
154	}	155		}
155	}	156		}
156		157
		158	+	func TestFavoriteRepositoryGetAllHydratesTagsInBatches(t *testing.T) {
		159	+	db := openFavoriteRepoTestDB(t)
		160	+	mustExec(t, db, `INSERT INTO favorite_tags(id, name, color) VALUES (1, 'batched', '#111111')`)
		161	+	for i := int64(1); i <= favoriteTagHydrationBatchSize+5; i++ {
		162	+	_, err := db.Exec(
		163	+	`INSERT INTO favorites(id, url, title, position) VALUES (?, ?, ?, ?)`,
		164	+	i,
		165	+	fmt.Sprintf("https://batch.example/%d", i),
		166	+	"Batch",
		167	+	i,
		168	+	)
		169	+	if err != nil {
		170	+	t.Fatalf("insert favorite %d: %v", i, err)
		171	+	}
		172	+	_, err = db.Exec(`INSERT INTO favorite_tag_assignments(favorite_id, tag_id) VALUES (?, 1)`, i)
		173	+	if err != nil {
		174	+	t.Fatalf("insert assignment %d: %v", i, err)
		175	+	}
		176	+	}
		177	+	repo := NewFavoriteRepository(db)
		178	+
		179	+	favorites, err := repo.GetAll(context.Background())
		180	+	if err != nil {
		181	+	t.Fatalf("GetAll: %v", err)
		182	+	}
		183	+	if len(favorites) != favoriteTagHydrationBatchSize+5 {
		184	+	t.Fatalf("expected %d favorites, got %d", favoriteTagHydrationBatchSize+5, len(favorites))
		185	+	}
		186	+	assertTagNames(t, favorites[0], "batched")
		187	+	assertTagNames(t, favorites[len(favorites)-1], "batched")
		188	+	}
		189	+
157	func assertTagNames(t testing.T, fav entity.Favorite, names ...string) {	190		func assertTagNames(t testing.T, fav entity.Favorite, names ...string) {
158	t.Helper()	191		t.Helper()
159	if fav == nil {	192		if fav == nil {