Skip package search when query is empty

An empty search returns hundreds of 100% popularity packages which
is not useful. Only call FindAll when the user has provided an
explicit search query.

Author: pierres

internal/ui/packagepage/handler.go

@@ -37,10 +37,16 @@ func (h *Handler) HandlePackages(w http.ResponseWriter, r *http.Request) {
 
 	currentMonth := web.GetLastCompleteMonth()
 
-	list, err := h.repo.FindAll(r.Context(), query, currentMonth, currentMonth, limit, offset)
-	if err != nil {
-		layout.ServerError(w, "failed to fetch packages", err)
-		return
+	var list *packages.PackagePopularityList
+	if query != "" {
+		var err error
+		list, err = h.repo.FindAll(r.Context(), query, currentMonth, currentMonth, limit, offset)
+		if err != nil {
+			layout.ServerError(w, "failed to fetch packages", err)
+			return
+		}
+	} else {
+		list = &packages.PackagePopularityList{}
 	}
 
 	selectedPackages, err := h.fetchComparePackages(r, compare, currentMonth)

internal/ui/packagepage/handler_test.go

@@ -25,14 +25,42 @@ func (m *mockRepo) FindByName(ctx context.Context, name string, startMonth, endM
 }
 
 func (m *mockRepo) FindAll(ctx context.Context, query string, startMonth, endMonth, limit, offset int) (*packages.PackagePopularityList, error) {
-	return m.findAllFunc(ctx, query, startMonth, endMonth, limit, offset)
+	if m.findAllFunc != nil {
+		return m.findAllFunc(ctx, query, startMonth, endMonth, limit, offset)
+	}
+	return &packages.PackagePopularityList{Total: 0}, nil
 }
 
 func (m *mockRepo) FindSeriesByName(ctx context.Context, name string, startMonth, endMonth, limit, offset int) (*packages.PackagePopularityList, error) {
 	return nil, nil
 }
 
 func TestHandlePackages(t *testing.T) {
+	manifest, _ := layout.NewManifest([]byte(`{}`))
+	repo := &mockRepo{
+		findAllFunc: func(ctx context.Context, query string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
+			t.Error("FindAll should not be called without a query")
+			return &packages.PackagePopularityList{Total: 0}, nil
+		},
+	}
+	handler := NewHandler(repo, manifest)
+
+	req := httptest.NewRequest(http.MethodGet, "/packages", nil)
+	rr := httptest.NewRecorder()
+
+	handler.HandlePackages(rr, req)
+
+	if rr.Code != http.StatusOK {
+		t.Errorf("expected status 200, got %d", rr.Code)
+	}
+
+	body := rr.Body.String()
+	if !strings.Contains(body, "Package statistics") {
+		t.Error("expected body to contain title")
+	}
+}
+
+func TestHandlePackages_WithQuery(t *testing.T) {
 	manifest, _ := layout.NewManifest([]byte(`{}`))
 	repo := &mockRepo{
 		findAllFunc: func(ctx context.Context, query string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
@@ -46,7 +74,7 @@ func TestHandlePackages(t *testing.T) {
 	}
 	handler := NewHandler(repo, manifest)
 
-	req := httptest.NewRequest(http.MethodGet, "/packages", nil)
+	req := httptest.NewRequest(http.MethodGet, "/packages?query=pacman", nil)
 	rr := httptest.NewRecorder()
 
 	handler.HandlePackages(rr, req)
@@ -56,9 +84,6 @@ func TestHandlePackages(t *testing.T) {
 	}
 
 	body := rr.Body.String()
-	if !strings.Contains(body, "Package statistics") {
-		t.Error("expected body to contain title")
-	}
 	if !strings.Contains(body, "pacman") {
 		t.Error("expected body to contain package name")
 	}
@@ -68,6 +93,7 @@ func TestHandlePackages_WithCompare(t *testing.T) {
 	manifest, _ := layout.NewManifest([]byte(`{}`))
 	repo := &mockRepo{
 		findAllFunc: func(ctx context.Context, query string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
+			t.Error("FindAll should not be called when compare is set without query")
 			return &packages.PackagePopularityList{Total: 0}, nil
 		},
 	}
@@ -88,6 +114,41 @@ func TestHandlePackages_WithCompare(t *testing.T) {
 	}
 }
 
+func TestHandlePackages_WithCompareAndQuery(t *testing.T) {
+	manifest, _ := layout.NewManifest([]byte(`{}`))
+	findAllCalled := false
+	repo := &mockRepo{
+		findAllFunc: func(ctx context.Context, query string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
+			findAllCalled = true
+			return &packages.PackagePopularityList{
+				Total: 1,
+				PackagePopularities: []packages.PackagePopularity{
+					{Name: "pacman", Popularity: 10.5},
+				},
+			}, nil
+		},
+	}
+	handler := NewHandler(repo, manifest)
+
+	req := httptest.NewRequest(http.MethodGet, "/packages?query=pac&compare=glibc", nil)
+	rr := httptest.NewRecorder()
+
+	handler.HandlePackages(rr, req)
+
+	if rr.Code != http.StatusOK {
+		t.Errorf("expected status 200, got %d", rr.Code)
+	}
+
+	if !findAllCalled {
+		t.Error("FindAll should be called when both query and compare are set")
+	}
+
+	body := rr.Body.String()
+	if !strings.Contains(body, "pacman") {
+		t.Error("expected body to contain search result")
+	}
+}
+
 func TestHandlePackages_CompareExceedsSelectLimit(t *testing.T) {
 	manifest, _ := layout.NewManifest([]byte(`{}`))
 
@@ -154,11 +215,7 @@ func TestHandlePackages_CompareExceedsChartLimit(t *testing.T) {
 		names[i] = "pkg" + strings.Repeat("x", i)
 	}
 
-	repo := &mockRepo{
-		findAllFunc: func(_ context.Context, _ string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
-			return &packages.PackagePopularityList{Total: 0}, nil
-		},
-	}
+	repo := &mockRepo{}
 	handler := NewHandler(repo, manifest)
 
 	req := httptest.NewRequest(http.MethodGet, "/packages?compare="+strings.Join(names, ","), nil)
@@ -191,9 +248,6 @@ func TestHandlePackages_CompareExceedsChartLimit(t *testing.T) {
 func TestHandlePackages_CompareError(t *testing.T) {
 	manifest, _ := layout.NewManifest([]byte(`{}`))
 	repo := &mockRepo{
-		findAllFunc: func(_ context.Context, _ string, _, _, _, _ int) (*packages.PackagePopularityList, error) {
-			return &packages.PackagePopularityList{Total: 0}, nil
-		},
 		findByNameFunc: func(_ context.Context, _ string, _, _ int) (*packages.PackagePopularity, error) {
 			return nil, errors.New("db error")
 		},