fix: address copilot review comments on PR #51
- commit
- 89aeee6bec2a08dbc416dea4827df332d540f982
- author
- brice · · 2025-12-28 08:41 UTC
- parent
- be07ce6
- signature
- unsigned
2 files · +93 -30
A internal/application/usecase/manage_favorites_test.go +77 -0
| @@ -0,0 +1,77 @@ | |||
| 1 | + | package usecase_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "errors" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 10 | + | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/mock" | |
| 12 | + | "github.com/stretchr/testify/require" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsURLSet(t *testing.T) { | |
| 16 | + | ctx := testContext() | |
| 17 | + | ||
| 18 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |
| 19 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |
| 20 | + | tagRepo := repomocks.NewMockTagRepository(t) | |
| 21 | + | ||
| 22 | + | favorites := []*entity.Favorite{ | |
| 23 | + | {ID: 1, URL: "https://example.com"}, | |
| 24 | + | {ID: 2, URL: "https://github.com"}, | |
| 25 | + | {ID: 3, URL: "https://google.com"}, | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(favorites, nil) | |
| 29 | + | ||
| 30 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |
| 31 | + | ||
| 32 | + | urls, err := uc.GetAllURLs(ctx) | |
| 33 | + | require.NoError(t, err) | |
| 34 | + | require.NotNil(t, urls) | |
| 35 | + | ||
| 36 | + | assert.Len(t, urls, 3) | |
| 37 | + | _, hasExample := urls["https://example.com"] | |
| 38 | + | _, hasGithub := urls["https://github.com"] | |
| 39 | + | _, hasGoogle := urls["https://google.com"] | |
| 40 | + | assert.True(t, hasExample) | |
| 41 | + | assert.True(t, hasGithub) | |
| 42 | + | assert.True(t, hasGoogle) | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsEmptySetWhenNoFavorites(t *testing.T) { | |
| 46 | + | ctx := testContext() | |
| 47 | + | ||
| 48 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |
| 49 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |
| 50 | + | tagRepo := repomocks.NewMockTagRepository(t) | |
| 51 | + | ||
| 52 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return([]*entity.Favorite{}, nil) | |
| 53 | + | ||
| 54 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |
| 55 | + | ||
| 56 | + | urls, err := uc.GetAllURLs(ctx) | |
| 57 | + | require.NoError(t, err) | |
| 58 | + | require.NotNil(t, urls) | |
| 59 | + | assert.Empty(t, urls) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsErrorOnRepoFailure(t *testing.T) { | |
| 63 | + | ctx := testContext() | |
| 64 | + | ||
| 65 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |
| 66 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |
| 67 | + | tagRepo := repomocks.NewMockTagRepository(t) | |
| 68 | + | ||
| 69 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(nil, errors.New("db connection failed")) | |
| 70 | + | ||
| 71 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |
| 72 | + | ||
| 73 | + | urls, err := uc.GetAllURLs(ctx) | |
| 74 | + | require.Error(t, err) | |
| 75 | + | assert.Nil(t, urls) | |
| 76 | + | assert.Contains(t, err.Error(), "failed to get favorites") | |
| 77 | + | } | |
| @@ -0,0 +1,77 @@ | |||||
| 1 | + | package usecase_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "errors" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |||
| 9 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |||
| 10 | + | "github.com/stretchr/testify/assert" | |||
| 11 | + | "github.com/stretchr/testify/mock" | |||
| 12 | + | "github.com/stretchr/testify/require" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsURLSet(t *testing.T) { | |||
| 16 | + | ctx := testContext() | |||
| 17 | + | ||||
| 18 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |||
| 19 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |||
| 20 | + | tagRepo := repomocks.NewMockTagRepository(t) | |||
| 21 | + | ||||
| 22 | + | favorites := []*entity.Favorite{ | |||
| 23 | + | {ID: 1, URL: "https://example.com"}, | |||
| 24 | + | {ID: 2, URL: "https://github.com"}, | |||
| 25 | + | {ID: 3, URL: "https://google.com"}, | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(favorites, nil) | |||
| 29 | + | ||||
| 30 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |||
| 31 | + | ||||
| 32 | + | urls, err := uc.GetAllURLs(ctx) | |||
| 33 | + | require.NoError(t, err) | |||
| 34 | + | require.NotNil(t, urls) | |||
| 35 | + | ||||
| 36 | + | assert.Len(t, urls, 3) | |||
| 37 | + | _, hasExample := urls["https://example.com"] | |||
| 38 | + | _, hasGithub := urls["https://github.com"] | |||
| 39 | + | _, hasGoogle := urls["https://google.com"] | |||
| 40 | + | assert.True(t, hasExample) | |||
| 41 | + | assert.True(t, hasGithub) | |||
| 42 | + | assert.True(t, hasGoogle) | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsEmptySetWhenNoFavorites(t *testing.T) { | |||
| 46 | + | ctx := testContext() | |||
| 47 | + | ||||
| 48 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |||
| 49 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |||
| 50 | + | tagRepo := repomocks.NewMockTagRepository(t) | |||
| 51 | + | ||||
| 52 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return([]*entity.Favorite{}, nil) | |||
| 53 | + | ||||
| 54 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |||
| 55 | + | ||||
| 56 | + | urls, err := uc.GetAllURLs(ctx) | |||
| 57 | + | require.NoError(t, err) | |||
| 58 | + | require.NotNil(t, urls) | |||
| 59 | + | assert.Empty(t, urls) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func TestManageFavoritesUseCase_GetAllURLs_ReturnsErrorOnRepoFailure(t *testing.T) { | |||
| 63 | + | ctx := testContext() | |||
| 64 | + | ||||
| 65 | + | favoriteRepo := repomocks.NewMockFavoriteRepository(t) | |||
| 66 | + | folderRepo := repomocks.NewMockFolderRepository(t) | |||
| 67 | + | tagRepo := repomocks.NewMockTagRepository(t) | |||
| 68 | + | ||||
| 69 | + | favoriteRepo.EXPECT().GetAll(mock.Anything).Return(nil, errors.New("db connection failed")) | |||
| 70 | + | ||||
| 71 | + | uc := usecase.NewManageFavoritesUseCase(favoriteRepo, folderRepo, tagRepo) | |||
| 72 | + | ||||
| 73 | + | urls, err := uc.GetAllURLs(ctx) | |||
| 74 | + | require.Error(t, err) | |||
| 75 | + | assert.Nil(t, urls) | |||
| 76 | + | assert.Contains(t, err.Error(), "failed to get favorites") | |||
| 77 | + | } | |||
M internal/ui/component/omnibox.go +16 -30
| @@ -606,26 +606,16 @@ func (o *Omnibox) performSearch() { | |||
| 606 | 606 | favCh := make(chan map[string]struct{}, 1) | |
| 607 | 607 | ||
| 608 | 608 | go func() { | |
| 609 | - | select { | |
| 610 | - | case <-ctx.Done(): | |
| 611 | - | searchCh <- searchResult{nil, ctx.Err()} | |
| 612 | - | default: | |
| 613 | - | searchInput := usecase.SearchInput{ | |
| 614 | - | Query: query, | |
| 615 | - | Limit: OmniboxListDefaults.MaxResults, | |
| 616 | - | } | |
| 617 | - | output, err := o.historyUC.Search(ctx, searchInput) | |
| 618 | - | searchCh <- searchResult{output, err} | |
| 609 | + | searchInput := usecase.SearchInput{ | |
| 610 | + | Query: query, | |
| 611 | + | Limit: OmniboxListDefaults.MaxResults, | |
| 619 | 612 | } | |
| 613 | + | output, err := o.historyUC.Search(ctx, searchInput) | |
| 614 | + | searchCh <- searchResult{output, err} | |
| 620 | 615 | }() | |
| 621 | 616 | ||
| 622 | 617 | go func() { | |
| 623 | - | select { | |
| 624 | - | case <-ctx.Done(): | |
| 625 | - | favCh <- nil | |
| 626 | - | default: | |
| 627 | - | favCh <- o.getFavoriteURLs(ctx) | |
| 628 | - | } | |
| 618 | + | favCh <- o.getFavoriteURLs(ctx) | |
| 629 | 619 | }() | |
| 630 | 620 | ||
| 631 | 621 | // Wait for both results | |
| @@ -636,6 +626,9 @@ func (o *Omnibox) performSearch() { | |||
| 636 | 626 | log.Error().Err(sr.err).Msg("history search failed") | |
| 637 | 627 | return | |
| 638 | 628 | } | |
| 629 | + | if sr.output == nil { | |
| 630 | + | return | |
| 631 | + | } | |
| 639 | 632 | ||
| 640 | 633 | suggestions := make([]Suggestion, 0, len(sr.output.Matches)) | |
| 641 | 634 | for _, r := range sr.output.Matches { | |
| @@ -678,23 +671,13 @@ func (o *Omnibox) loadInitialHistory() { | |||
| 678 | 671 | favCh := make(chan map[string]struct{}, 1) | |
| 679 | 672 | ||
| 680 | 673 | go func() { | |
| 681 | - | select { | |
| 682 | - | case <-ctx.Done(): | |
| 683 | - | historyCh <- historyResult{nil, ctx.Err()} | |
| 684 | - | default: | |
| 685 | - | // TODO: Implement GetMostVisited in use case if needed | |
| 686 | - | results, err := o.historyUC.GetRecent(ctx, OmniboxListDefaults.MaxResults, 0) | |
| 687 | - | historyCh <- historyResult{results, err} | |
| 688 | - | } | |
| 674 | + | // TODO: Implement GetMostVisited in use case if needed | |
| 675 | + | results, err := o.historyUC.GetRecent(ctx, OmniboxListDefaults.MaxResults, 0) | |
| 676 | + | historyCh <- historyResult{results, err} | |
| 689 | 677 | }() | |
| 690 | 678 | ||
| 691 | 679 | go func() { | |
| 692 | - | select { | |
| 693 | - | case <-ctx.Done(): | |
| 694 | - | favCh <- nil | |
| 695 | - | default: | |
| 696 | - | favCh <- o.getFavoriteURLs(ctx) | |
| 697 | - | } | |
| 680 | + | favCh <- o.getFavoriteURLs(ctx) | |
| 698 | 681 | }() | |
| 699 | 682 | ||
| 700 | 683 | // Wait for both results | |
| @@ -705,6 +688,9 @@ func (o *Omnibox) loadInitialHistory() { | |||
| 705 | 688 | log.Error().Err(hr.err).Msg("failed to load recent history") | |
| 706 | 689 | return | |
| 707 | 690 | } | |
| 691 | + | if hr.results == nil { | |
| 692 | + | return | |
| 693 | + | } | |
| 708 | 694 | ||
| 709 | 695 | suggestions = make([]Suggestion, 0, len(hr.results)) | |
| 710 | 696 | for _, r := range hr.results { | |
| @@ -606,26 +606,16 @@ func (o *Omnibox) performSearch() { | |||||
| 606 | favCh := make(chan map[string]struct{}, 1) | 606 | favCh := make(chan map[string]struct{}, 1) | ||
| 607 | 607 | ||||
| 608 | go func() { | 608 | go func() { | ||
| 609 | - | select { | 609 | + | searchInput := usecase.SearchInput{ |
| 610 | - | case <-ctx.Done(): | 610 | + | Query: query, |
| 611 | - | searchCh <- searchResult{nil, ctx.Err()} | 611 | + | Limit: OmniboxListDefaults.MaxResults, |
| 612 | - | default: | |||
| 613 | - | searchInput := usecase.SearchInput{ | |||
| 614 | - | Query: query, | |||
| 615 | - | Limit: OmniboxListDefaults.MaxResults, | |||
| 616 | - | } | |||
| 617 | - | output, err := o.historyUC.Search(ctx, searchInput) | |||
| 618 | - | searchCh <- searchResult{output, err} | |||
| 619 | } | 612 | } | ||
| 613 | + | output, err := o.historyUC.Search(ctx, searchInput) | |||
| 614 | + | searchCh <- searchResult{output, err} | |||
| 620 | }() | 615 | }() | ||
| 621 | 616 | ||||
| 622 | go func() { | 617 | go func() { | ||
| 623 | - | select { | 618 | + | favCh <- o.getFavoriteURLs(ctx) |
| 624 | - | case <-ctx.Done(): | |||
| 625 | - | favCh <- nil | |||
| 626 | - | default: | |||
| 627 | - | favCh <- o.getFavoriteURLs(ctx) | |||
| 628 | - | } | |||
| 629 | }() | 619 | }() | ||
| 630 | 620 | ||||
| 631 | // Wait for both results | 621 | // Wait for both results | ||
| @@ -636,6 +626,9 @@ func (o *Omnibox) performSearch() { | |||||
| 636 | log.Error().Err(sr.err).Msg("history search failed") | 626 | log.Error().Err(sr.err).Msg("history search failed") | ||
| 637 | return | 627 | return | ||
| 638 | } | 628 | } | ||
| 629 | + | if sr.output == nil { | |||
| 630 | + | return | |||
| 631 | + | } | |||
| 639 | 632 | ||||
| 640 | suggestions := make([]Suggestion, 0, len(sr.output.Matches)) | 633 | suggestions := make([]Suggestion, 0, len(sr.output.Matches)) | ||
| 641 | for _, r := range sr.output.Matches { | 634 | for _, r := range sr.output.Matches { | ||
| @@ -678,23 +671,13 @@ func (o *Omnibox) loadInitialHistory() { | |||||
| 678 | favCh := make(chan map[string]struct{}, 1) | 671 | favCh := make(chan map[string]struct{}, 1) | ||
| 679 | 672 | ||||
| 680 | go func() { | 673 | go func() { | ||
| 681 | - | select { | 674 | + | // TODO: Implement GetMostVisited in use case if needed |
| 682 | - | case <-ctx.Done(): | 675 | + | results, err := o.historyUC.GetRecent(ctx, OmniboxListDefaults.MaxResults, 0) |
| 683 | - | historyCh <- historyResult{nil, ctx.Err()} | 676 | + | historyCh <- historyResult{results, err} |
| 684 | - | default: | |||
| 685 | - | // TODO: Implement GetMostVisited in use case if needed | |||
| 686 | - | results, err := o.historyUC.GetRecent(ctx, OmniboxListDefaults.MaxResults, 0) | |||
| 687 | - | historyCh <- historyResult{results, err} | |||
| 688 | - | } | |||
| 689 | }() | 677 | }() | ||
| 690 | 678 | ||||
| 691 | go func() { | 679 | go func() { | ||
| 692 | - | select { | 680 | + | favCh <- o.getFavoriteURLs(ctx) |
| 693 | - | case <-ctx.Done(): | |||
| 694 | - | favCh <- nil | |||
| 695 | - | default: | |||
| 696 | - | favCh <- o.getFavoriteURLs(ctx) | |||
| 697 | - | } | |||
| 698 | }() | 681 | }() | ||
| 699 | 682 | ||||
| 700 | // Wait for both results | 683 | // Wait for both results | ||
| @@ -705,6 +688,9 @@ func (o *Omnibox) loadInitialHistory() { | |||||
| 705 | log.Error().Err(hr.err).Msg("failed to load recent history") | 688 | log.Error().Err(hr.err).Msg("failed to load recent history") | ||
| 706 | return | 689 | return | ||
| 707 | } | 690 | } | ||
| 691 | + | if hr.results == nil { | |||
| 692 | + | return | |||
| 693 | + | } | |||
| 708 | 694 | ||||
| 709 | suggestions = make([]Suggestion, 0, len(hr.results)) | 695 | suggestions = make([]Suggestion, 0, len(hr.results)) | ||
| 710 | for _, r := range hr.results { | 696 | for _, r := range hr.results { | ||