Added category filter and unified sorting to Projects search component#4168
Open
anurag2787 wants to merge 11 commits intoOWASP:mainfrom
Open
Added category filter and unified sorting to Projects search component#4168anurag2787 wants to merge 11 commits intoOWASP:mainfrom
anurag2787 wants to merge 11 commits intoOWASP:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a unified frontend search UI with category and sorting controls, backend GraphQL and REST extensions for filtering, ordering, pagination, and counts, Algolia query plumbing for forwarded filters, and numerous tests and utilities to support Algolia and GraphQL search backends across project listing and health dashboards. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
170-177:⚠️ Potential issue | 🟠 MajorURL-driven state sync is incomplete for search/filter.
Line 170-Line 177 rehydrates only ordering from
searchParams. On back/forward navigation,searchQueryandfilterscan diverge from URL, so fetched results no longer match visible URL state.💡 Proposed fix
useEffect(() => { const { field: f, direction: d } = parseOrderParam(searchParams.get('order')) const nextOrdering = buildGraphQLOrdering(f, d) if (JSON.stringify(nextOrdering) !== JSON.stringify(ordering)) { setOrdering(nextOrdering) } + + const nextSearch = searchParams.get('search') || '' + if (nextSearch !== searchQuery) { + setSearchQuery(nextSearch) + } + + const nextHealth = searchParams.get('health') + const nextLevel = searchParams.get('level') + let nextFilters = {} + if (nextHealth && nextHealth in healthFiltersMapping) { + nextFilters = healthFiltersMapping[nextHealth as keyof typeof healthFiltersMapping] + } else if (nextLevel && nextLevel in levelFiltersMapping) { + nextFilters = levelFiltersMapping[nextLevel as keyof typeof levelFiltersMapping] + } + if (JSON.stringify(nextFilters) !== JSON.stringify(filters)) { + setFilters(nextFilters) + } // eslint-disable-next-line react-hooks/exhaustive-deps }, [searchParams])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/projects/dashboard/metrics/page.tsx` around lines 170 - 177, The effect that currently only rehydrates ordering (inside the useEffect using parseOrderParam/buildGraphQLOrdering and setOrdering) must also parse and apply URL-driven search and filter state: read the relevant query param keys from searchParams (e.g. the search query param like 'q' and the filters param(s)), deserialize them into the same shapes used by the component, compare to current searchQuery and filters (deep-equal or JSON.stringify) and call setSearchQuery and setFilters when they differ; update the useEffect that references parseOrderParam/buildGraphQLOrdering to include this logic so back/forward navigation keeps searchQuery, filters, and ordering in sync with the URL.
🧹 Nitpick comments (4)
frontend/src/components/Search.tsx (1)
16-16:testIdprop is currently dead API surface.
testIdis added to props but never applied to the DOM, so consumers can pass it without effect. Either wire it (for example asdata-testid) or remove it to avoid confusion.Proposed fix
<input ref={inputRef} id={inputId} + data-testid={testId} type="text" value={searchQuery} onChange={handleSearchChange} placeholder={placeholder} className={inputClassName} />Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Search.tsx` at line 16, The testId prop is declared on the Search component but never applied to the DOM; either remove testId from the props interface/type or wire it into the rendered element(s) so it is useful. To fix, update the Search component (the functional component named Search and its props type that includes testId?: string) to add data-testid={testId} on the root DOM element (e.g., the top-level div/Container returned by Search) and guard it so it only renders when testId is provided, or alternatively remove testId from the props type and consumers; make the change consistently where the prop is declared/used in Search.tsx.backend/apps/owasp/api/internal/queries/project_health_metrics.py (1)
61-64: Extract shared query-filtering logic to keep list/count behavior locked together.The same normalization +
icontainsblock is duplicated in both resolvers. A shared helper reduces drift risk.Refactor sketch
class ProjectHealthMetricsQuery: @@ + `@staticmethod` + def _apply_query_filter(queryset, query: str): + cleaned_query = query.strip() if query else "" + if cleaned_query: + return queryset.filter(project__name__icontains=cleaned_query) + return queryset @@ - cleaned_query = query.strip() if query else "" - if cleaned_query: - queryset = queryset.filter(project__name__icontains=cleaned_query) + queryset = self._apply_query_filter(queryset, query) @@ - cleaned_query = query.strip() if query else "" - if cleaned_query: - queryset = queryset.filter(project__name__icontains=cleaned_query) + queryset = self._apply_query_filter(queryset, query)Also applies to: 102-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/api/internal/queries/project_health_metrics.py` around lines 61 - 64, Extract the duplicated normalization + icontains filtering into a single helper function (e.g., apply_project_name_query_filter(queryset, query)) that performs cleaned_query = query.strip() if query else "" and, if len(cleaned_query) >= MIN_SEARCH_QUERY_LENGTH, returns queryset.filter(project__name__icontains=cleaned_query) else returns the original queryset; replace the duplicated blocks in both resolver functions (the list resolver and the count resolver in project_health_metrics.py) to call this helper so list/count behavior stays identical and DRY.frontend/src/utils/sortingOptions.ts (1)
11-17: TypetypeOptionsProjectwith the shared option type for stronger safety.Using
CategoryOption[]here prevents shape drift across search option sources.Suggested refactor
+import type { CategoryOption } from 'types/category' ... -export const typeOptionsProject = [ +export const typeOptionsProject: CategoryOption[] = [ { label: 'All Types', key: '' }, { label: 'Code', key: 'idx_type:code' }, { label: 'Tool', key: 'idx_type:tool' }, { label: 'Documentation', key: 'idx_type:documentation' }, { label: 'Other', key: 'idx_type:other' }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/utils/sortingOptions.ts` around lines 11 - 17, The array typeOptionsProject is untyped and should use the shared CategoryOption type to prevent shape drift; update the declaration of typeOptionsProject to have the type CategoryOption[] and ensure you import or reference the existing CategoryOption type used by other search option sources so each item matches that interface (e.g., keep objects with label and key) and adjust any mismatched properties to conform.frontend/src/hooks/useSearchProjectsGraphQL.ts (1)
99-100: Counting viaprojectsTotal.lengthis expensive at scale.Fetching all matching IDs just to compute total count adds unnecessary payload and memory pressure. Prefer a dedicated count field (e.g.,
searchProjectsCount) from GraphQL and return an integer directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useSearchProjectsGraphQL.ts` around lines 99 - 100, The code currently computes totalProjects by counting data?.projectsTotal?.length which fetches all matching IDs and is costly; update the GraphQL schema/query to expose a dedicated integer count (e.g., searchProjectsCount), update the useSearchProjectsGraphQL query to request that field instead of projectsTotal, and replace the usage of data?.projectsTotal?.length with data?.searchProjectsCount || 0; also update any related TypeScript types/interfaces and server resolver to return the integer count to avoid loading the full ID list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/api/rest/v0/project.py`:
- Around line 109-124: The public REST ordering Literal named ordering is
missing the level_raw option while fallback ordering and existing sort keys
still reference "level_raw" and "-level_raw"; add both "level_raw" and
"-level_raw" to the ordering Literal so the public contract matches the fallback
and existing sort keys (ensure the symbol ordering in project.py includes
"level_raw" and "-level_raw" and that any fallback or sortBy handling code that
mentions "-level_raw" remains consistent).
In `@backend/apps/owasp/api/internal/queries/project_health_metrics.py`:
- Around line 62-64: The current guard (if cleaned_query and len(cleaned_query)
>= MIN_SEARCH_QUERY_LENGTH) skips filtering for non-empty short queries so
results remain unfiltered; change the logic to apply the icontains filter
whenever cleaned_query is non-empty (i.e., if cleaned_query: queryset =
queryset.filter(project__name__icontains=cleaned_query)) and remove the length
check, and make the same change at the second occurrence (the other resolver
block using cleaned_query, MIN_SEARCH_QUERY_LENGTH, and queryset) so 1–2
character inputs produce filtered results immediately.
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 121-122: The page currently computes currentCategory as
healthFilter || levelFilter which lets an old health filter mask a newly
selected level because the level selection code (select level handler around the
block that retains both filters) doesn't clear the other filter; change the
selection logic so the UI behaves as a single-select: when setting levelFilter
(e.g., in the level selection handler referenced in the 240-247 block)
explicitly clear healthFilter, and when setting healthFilter explicitly clear
levelFilter; also update currentCategory to use levelFilter || healthFilter ||
'' (or otherwise rely on the most recently set filter) so the visible dropdown
reflects the newly selected single category. Ensure you update the functions
that modify filters (the level selection handler and the health selection
handler) rather than only changing the computed value.
In `@frontend/src/hooks/useSearchPage.ts`:
- Around line 159-160: The computed total pages for GraphQL results forces a
minimum of 1 which diverges from Algolia's behavior; change the calculation in
the block using calculatedTotalPages, graphqlTotalCount, hitsPerPage and
setTotalPages so that when graphqlTotalCount is 0 calculatedTotalPages becomes 0
(e.g., remove the "|| 1" behavior) and then update setTotalPages((prev) => (prev
=== calculatedTotalPages ? prev : calculatedTotalPages)) accordingly so
empty-result pagination matches Algolia.
- Around line 60-67: The GraphQL hook useSearchProjectsGraphQL is being invoked
unconditionally in useSearchPage, causing GraphQL requests even when Algolia is
selected; change useSearchPage to only call useSearchProjectsGraphQL when
GraphQL backend is active (e.g., guard the call with a backend check like
backend === 'graphql' or !isAlgoliaBackend) or pass an enabled flag into the
hook call, and then update useSearchProjectsGraphQL.ts so the hook accepts an
options.enabled (or maps to Apollo's skip) and does not execute the query when
enabled is false; reference the hook invocation
useSearchProjectsGraphQL(searchQuery, category, sortBy, order, currentPage,
hitsPerPage, { pageSize: hitsPerPage }) and the hook implementation in
useSearchProjectsGraphQL.ts to add/consume the enabled/skip behavior.
In `@frontend/src/hooks/useSearchProjectsGraphQL.ts`:
- Around line 62-75: The fieldMapping in useSearchProjectsGraphQL.ts is missing
a mapping for the UI's "level_raw" key causing sortBy to send level_raw to
GraphQL; update the fieldMapping object to include ['level_raw', 'level'] (so
that when sortBy === 'level_raw' graphQLField becomes 'level') and keep the
existing fallback behavior using graphQLField/ sortBy and return [{
[graphQLField]: orderDirection }] unchanged; ensure any other UI-specific raw
keys are similarly mapped to their GraphQL ProjectOrder equivalents.
In `@frontend/src/server/fetchAlgoliaData.ts`:
- Around line 14-16: The code in fetchAlgoliaData (around variables facetFilters
and indexName) mutates the incoming facetFilters array with
facetFilters.push('idx_is_active:true'), which can cause duplicate filters if
the same array is reused; instead, do not mutate the caller's array—create a new
array when adding the active filter (e.g., use a spread/concat to produce a
newFacetFilters or check for existing 'idx_is_active:true' before adding) and
then use that new array for the Algolia query so fetchAlgoliaData never mutates
its input.
In `@frontend/src/server/queries/projectQueries.ts`:
- Around line 207-209: The current projectsTotal field calls searchProjects and
returns all matching ids, causing a full search payload; change the query to use
a dedicated count endpoint/field (e.g., searchProjectsCount) instead of
searchProjects to compute totals. Update the client-side query where
projectsTotal is defined to call searchProjectsCount(query: $query, filters:
$filters) and remove the id selection, and ensure any resolver or GraphQL schema
that currently backs projectsTotal maps to or exposes the new
searchProjectsCount field so the server returns only an integer count rather
than full nodes. Also update any usages of projectsTotal in components to expect
a number instead of an array.
In `@frontend/src/utils/backendConfig.ts`:
- Around line 24-25: The check "envBackend in SEARCH_BACKENDS" can match
inherited properties; update the validation to use an own-property check such as
Object.prototype.hasOwnProperty.call(SEARCH_BACKENDS, envBackend) (or use
Object.keys/values and .includes) so only real keys in SEARCH_BACKENDS are
accepted; locate the conditional that references envBackend and SEARCH_BACKENDS
and replace the "in" check with the hasOwnProperty-style check, keeping the
return envBackend as SearchBackend behavior unchanged.
---
Outside diff comments:
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 170-177: The effect that currently only rehydrates ordering
(inside the useEffect using parseOrderParam/buildGraphQLOrdering and
setOrdering) must also parse and apply URL-driven search and filter state: read
the relevant query param keys from searchParams (e.g. the search query param
like 'q' and the filters param(s)), deserialize them into the same shapes used
by the component, compare to current searchQuery and filters (deep-equal or
JSON.stringify) and call setSearchQuery and setFilters when they differ; update
the useEffect that references parseOrderParam/buildGraphQLOrdering to include
this logic so back/forward navigation keeps searchQuery, filters, and ordering
in sync with the URL.
---
Nitpick comments:
In `@backend/apps/owasp/api/internal/queries/project_health_metrics.py`:
- Around line 61-64: Extract the duplicated normalization + icontains filtering
into a single helper function (e.g., apply_project_name_query_filter(queryset,
query)) that performs cleaned_query = query.strip() if query else "" and, if
len(cleaned_query) >= MIN_SEARCH_QUERY_LENGTH, returns
queryset.filter(project__name__icontains=cleaned_query) else returns the
original queryset; replace the duplicated blocks in both resolver functions (the
list resolver and the count resolver in project_health_metrics.py) to call this
helper so list/count behavior stays identical and DRY.
In `@frontend/src/components/Search.tsx`:
- Line 16: The testId prop is declared on the Search component but never applied
to the DOM; either remove testId from the props interface/type or wire it into
the rendered element(s) so it is useful. To fix, update the Search component
(the functional component named Search and its props type that includes testId?:
string) to add data-testid={testId} on the root DOM element (e.g., the top-level
div/Container returned by Search) and guard it so it only renders when testId is
provided, or alternatively remove testId from the props type and consumers; make
the change consistently where the prop is declared/used in Search.tsx.
In `@frontend/src/hooks/useSearchProjectsGraphQL.ts`:
- Around line 99-100: The code currently computes totalProjects by counting
data?.projectsTotal?.length which fetches all matching IDs and is costly; update
the GraphQL schema/query to expose a dedicated integer count (e.g.,
searchProjectsCount), update the useSearchProjectsGraphQL query to request that
field instead of projectsTotal, and replace the usage of
data?.projectsTotal?.length with data?.searchProjectsCount || 0; also update any
related TypeScript types/interfaces and server resolver to return the integer
count to avoid loading the full ID list.
In `@frontend/src/utils/sortingOptions.ts`:
- Around line 11-17: The array typeOptionsProject is untyped and should use the
shared CategoryOption type to prevent shape drift; update the declaration of
typeOptionsProject to have the type CategoryOption[] and ensure you import or
reference the existing CategoryOption type used by other search option sources
so each item matches that interface (e.g., keep objects with label and key) and
adjust any mismatched properties to conform.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectsHealthDashboardQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (23)
backend/apps/api/rest/v0/project.pybackend/apps/owasp/api/internal/filters/project.pybackend/apps/owasp/api/internal/ordering/project.pybackend/apps/owasp/api/internal/queries/project.pybackend/apps/owasp/api/internal/queries/project_health_metrics.pybackend/apps/owasp/index/registry/project.pybackend/apps/owasp/index/search/project.pyfrontend/.env.e2e.examplefrontend/.env.examplefrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/app/projects/page.tsxfrontend/src/components/Search.tsxfrontend/src/components/SearchPageLayout.tsxfrontend/src/components/UnifiedSearchBar.tsxfrontend/src/hooks/useSearchPage.tsfrontend/src/hooks/useSearchProjectsGraphQL.tsfrontend/src/server/fetchAlgoliaData.tsfrontend/src/server/queries/projectQueries.tsfrontend/src/server/queries/projectsHealthDashboardQueries.tsfrontend/src/types/category.tsfrontend/src/types/unifiedSearchBar.tsfrontend/src/utils/backendConfig.tsfrontend/src/utils/sortingOptions.ts
backend/apps/owasp/api/internal/queries/project_health_metrics.py
Outdated
Show resolved
Hide resolved
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
10 issues found across 26 files
Confidence score: 3/5
- Several medium-severity, user-facing state sync issues in
frontend/src/app/projects/dashboard/metrics/page.tsxandfrontend/src/hooks/useSearchPage.tscan leave filters/pagination stale after navigation or show inconsistent empty states. - Sorting and ordering concerns in
frontend/src/hooks/useSearchProjectsGraphQL.tsandbackend/apps/api/rest/v0/project.pycould send invalid sort fields or produce incorrect maturity ordering. - Score reflects multiple concrete behavior inconsistencies that could affect search/metrics UX, though no clear hard failures are reported.
- Pay close attention to
frontend/src/app/projects/dashboard/metrics/page.tsx,frontend/src/hooks/useSearchPage.ts,frontend/src/hooks/useSearchProjectsGraphQL.ts,backend/apps/api/rest/v0/project.py- filter/URL sync and sorting correctness.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/projects/dashboard/metrics/page.tsx">
<violation number="1" location="frontend/src/app/projects/dashboard/metrics/page.tsx:142">
P2: Search and filter state are not kept in sync with URL param changes, causing stale GraphQL query variables after navigation/back-forward.</violation>
<violation number="2" location="frontend/src/app/projects/dashboard/metrics/page.tsx:240">
P2: Selecting a level preserves existing health filter, causing hidden combined filters while category UI shows only one selected value.</violation>
</file>
<file name="frontend/src/hooks/useSearchPage.ts">
<violation number="1" location="frontend/src/hooks/useSearchPage.ts:55">
P2: Search state is initialized from URL only once and is not re-synced when query params change, so back/forward navigation can leave UI/data stale vs URL.</violation>
<violation number="2" location="frontend/src/hooks/useSearchPage.ts:65">
P2: GraphQL query hook is called unconditionally, causing unnecessary GraphQL requests even when Algolia backend is selected.</violation>
<violation number="3" location="frontend/src/hooks/useSearchPage.ts:159">
P2: GraphQL pagination incorrectly forces at least one page for zero results, causing inconsistent and broken empty-state behavior.</violation>
</file>
<file name="frontend/src/server/queries/projectQueries.ts">
<violation number="1" location="frontend/src/server/queries/projectQueries.ts:207">
P2: `projectsTotal` is computed by a second unpaginated `searchProjects` call, causing avoidable extra query/load overhead and brittle total calculation semantics.</violation>
</file>
<file name="frontend/src/hooks/useSearchProjectsGraphQL.ts">
<violation number="1" location="frontend/src/hooks/useSearchProjectsGraphQL.ts:73">
P2: Sorting whitelist is bypassed by falling back to raw `sortBy`, allowing invalid ordering fields from URL/state to be sent to GraphQL.</violation>
</file>
<file name="frontend/src/components/UnifiedSearchBar.tsx">
<violation number="1" location="frontend/src/components/UnifiedSearchBar.tsx:66">
P2: Controlled Select uses an invalid fallback key (`''`) instead of empty selection when category is unset.</violation>
</file>
<file name="backend/apps/api/rest/v0/project.py">
<violation number="1" location="backend/apps/api/rest/v0/project.py:122">
P2: `ordering=level` sorts by text enum value instead of rank, causing incorrect project maturity ordering compared with the intended `level_raw` semantics.</violation>
</file>
<file name="frontend/src/utils/backendConfig.ts">
<violation number="1" location="frontend/src/utils/backendConfig.ts:24">
P2: `in`-based backend validation accepts prototype-chain keys, allowing invalid env values to be returned as `SearchBackend`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…-filter-implementation
Contributor
There was a problem hiding this comment.
1 issue found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/projects/dashboard/metrics/page.tsx">
<violation number="1" location="frontend/src/app/projects/dashboard/metrics/page.tsx:121">
P2: Initial URL parsing still applies both `health` and `level` filters, causing hidden active constraints while the UI shows only one category in the new single-select flow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
121-135:⚠️ Potential issue | 🟡 MinorURL bootstrap can still create hidden dual-category filtering.
If both
healthandlevelare present in the URL, both filters are merged, while the UI shows one selected category.💡 Suggested fix
- if (healthFilter && healthFilter in healthFiltersMapping) { + if (currentCategory && currentCategory in healthFiltersMapping) { currentFilters = { ...currentFilters, - ...healthFiltersMapping[healthFilter as keyof typeof healthFiltersMapping], + ...healthFiltersMapping[currentCategory as keyof typeof healthFiltersMapping], } - } - if (levelFilter && levelFilter in levelFiltersMapping) { + } else if (currentCategory && currentCategory in levelFiltersMapping) { currentFilters = { ...currentFilters, - ...levelFiltersMapping[levelFilter as keyof typeof levelFiltersMapping], + ...levelFiltersMapping[currentCategory as keyof typeof levelFiltersMapping], } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/projects/dashboard/metrics/page.tsx` around lines 121 - 135, The code merges both health and level mappings into currentFilters when both URL params exist, causing a hidden dual-category filter; change the logic to derive currentCategory (already set via currentCategory = levelFilter || healthFilter || '') and then apply only the mapping that corresponds to currentCategory (use an if/else or switch) instead of independently merging healthFiltersMapping and levelFiltersMapping; update the blocks around currentFilters, healthFilter, levelFilter, healthFiltersMapping and levelFiltersMapping so only the mapping for the chosen currentCategory (and not both) is spread into currentFilters.
🧹 Nitpick comments (1)
backend/apps/owasp/api/internal/queries/project_health_metrics.py (1)
60-62: Consider extracting shared query-filter logic to avoid drift.The same
cleaned_query+icontainsblock appears twice; a tiny helper would reduce maintenance risk.♻️ Suggested refactor
class ProjectHealthMetricsQuery: """Project health metrics queries.""" + + `@staticmethod` + def _apply_query_filter(queryset, query: str): + cleaned_query = query.strip() if query else "" + if cleaned_query: + return queryset.filter(project__name__icontains=cleaned_query) + return queryset @@ - cleaned_query = query.strip() if query else "" - if cleaned_query: - queryset = queryset.filter(project__name__icontains=cleaned_query) + queryset = self._apply_query_filter(queryset, query) @@ - cleaned_query = query.strip() if query else "" - if cleaned_query: - queryset = queryset.filter(project__name__icontains=cleaned_query) + queryset = self._apply_query_filter(queryset, query)Also applies to: 101-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/owasp/api/internal/queries/project_health_metrics.py` around lines 60 - 62, The query cleaning and icontains filter logic is duplicated; extract a small helper (e.g. _apply_project_name_filter(queryset, query) or apply_project_name_filter) that trims the incoming query (handle None -> "") and, if non-empty, applies queryset.filter(project__name__icontains=cleaned_query); replace both occurrences that currently set cleaned_query and call queryset.filter(...) with calls to this helper to centralize behavior and avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/owasp/api/internal/queries/project.py`:
- Around line 126-127: The current condition skips applying name__icontains for
short queries (checked via cleaned_query and MIN_SEARCH_QUERY_LENGTH), causing
1–2 char searches to bypass filtering; change the condition to always apply the
filter when cleaned_query is present (e.g., replace "if cleaned_query and
len(cleaned_query) >= MIN_SEARCH_QUERY_LENGTH:" with "if cleaned_query:") for
both occurrences that use base_queryset.filter(name__icontains=cleaned_query) so
short queries are properly filtered.
In `@frontend/src/hooks/useSearchProjectsGraphQL.ts`:
- Around line 85-90: The code currently rewrites short non-empty queries to ''
causing GraphQL to return broad results; in useSearchProjectsGraphQL
(searchParam, useQuery, GET_PROJECTS_LIST, searchQuery) stop masking valid short
inputs—use the trimmed searchQuery directly (e.g., const searchParam =
searchQuery.trim()) and pass that value (or undefined only when truly empty)
into the variables so the server receives the actual user input instead of an
empty string.
---
Duplicate comments:
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 121-135: The code merges both health and level mappings into
currentFilters when both URL params exist, causing a hidden dual-category
filter; change the logic to derive currentCategory (already set via
currentCategory = levelFilter || healthFilter || '') and then apply only the
mapping that corresponds to currentCategory (use an if/else or switch) instead
of independently merging healthFiltersMapping and levelFiltersMapping; update
the blocks around currentFilters, healthFilter, levelFilter,
healthFiltersMapping and levelFiltersMapping so only the mapping for the chosen
currentCategory (and not both) is spread into currentFilters.
---
Nitpick comments:
In `@backend/apps/owasp/api/internal/queries/project_health_metrics.py`:
- Around line 60-62: The query cleaning and icontains filter logic is
duplicated; extract a small helper (e.g. _apply_project_name_filter(queryset,
query) or apply_project_name_filter) that trims the incoming query (handle None
-> "") and, if non-empty, applies
queryset.filter(project__name__icontains=cleaned_query); replace both
occurrences that currently set cleaned_query and call queryset.filter(...) with
calls to this helper to centralize behavior and avoid drift.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/projectQueries.generated.tsis excluded by!**/__generated__/**
📒 Files selected for processing (13)
.pre-commit-config.yamlbackend/apps/api/rest/v0/project.pybackend/apps/owasp/api/internal/filters/project.pybackend/apps/owasp/api/internal/queries/project.pybackend/apps/owasp/api/internal/queries/project_health_metrics.pyfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/Search.tsxfrontend/src/components/UnifiedSearchBar.tsxfrontend/src/hooks/useSearchPage.tsfrontend/src/hooks/useSearchProjectsGraphQL.tsfrontend/src/server/fetchAlgoliaData.tsfrontend/src/server/queries/projectQueries.tsfrontend/src/utils/backendConfig.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/utils/backendConfig.ts
- frontend/src/server/fetchAlgoliaData.ts
- frontend/src/components/Search.tsx
- frontend/src/components/UnifiedSearchBar.tsx
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/apps/owasp/api/internal/queries/project.py">
<violation number="1" location="backend/apps/owasp/api/internal/queries/project.py:126">
P2: Minimum query-length protection was removed, causing broad `icontains` filtering for 1–2 character inputs in both search and count paths, which can significantly increase DB load.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/hooks/useSearchProjectsGraphQL.ts (1)
9-12:options.pageSizeis declared but never applied.This creates a confusing API surface: callers can pass
options.pageSize, but pagination always uses the positionalpageSizeargument.Proposed fix
export function useSearchProjectsGraphQL( searchQuery = '', category = '', sortBy = '', order = '', currentPage = 1, pageSize = 25, options?: UseSearchProjectsGraphQLOptions ): UseSearchProjectsGraphQLReturn { + const effectivePageSize = options?.pageSize ?? pageSize ... - const offset = (currentPage - 1) * pageSize + const offset = (currentPage - 1) * effectivePageSize ... pagination: { offset, - limit: pageSize, + limit: effectivePageSize, },Also applies to: 40-41, 83-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/useSearchProjectsGraphQL.ts` around lines 9 - 12, The options interface UseSearchProjectsGraphQLOptions exposes pageSize but the hook never uses it; update the useSearchProjectsGraphQL implementation to read options.pageSize and apply it to all pagination logic (replace hardcoded/positional pageSize usage with options.pageSize ?? pageSizeParam), including where page queries and cursor/offset calculations occur (searchProjectsQuery handler, pagination helpers, and any loadMore/hasMore logic). Ensure the default behavior remains the same by falling back to the existing pageSize parameter when options.pageSize is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/owasp/api/internal/queries/project.py`:
- Around line 91-94: The pagination.limit checks currently treat
strawberry.UNSET like a valid value and perform numeric comparisons, causing
TypeError; update the guard around pagination.limit in the functions that use it
(the blocks referencing pagination.limit and MAX_PROJECTS_LIMIT) to ensure the
value is a real number before comparing — e.g., only proceed if pagination.limit
is not None and pagination.limit is not strawberry.UNSET (or alternatively
verify type is int) before doing <= 0 or min(..., MAX_PROJECTS_LIMIT); apply the
same change to both places that currently read pagination.limit so UNSET is
skipped safely.
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 133-136: The component currently only re-syncs ordering from
searchParams, causing searchQuery and filters to drift; update the effect that
derives ordering (the effect around currentOrdering/currentOrdering
dependencies) to also call setSearchQuery(searchQueryParam) and
setFilters(currentFilters) whenever the relevant searchParams change (or when
currentFilters/currentOrdering/searchQueryParam values update). Ensure you
reference and update the state setters setSearchQuery, setFilters, and
setOrdering so the UI state (filters, ordering, searchQuery) is consistently
derived from the URL parameters rather than drifting.
In `@frontend/src/hooks/useSearchProjectsGraphQL.ts`:
- Line 3: Replace the removed ApolloError type with ErrorLike from Apollo Client
v4: update the import (bring ErrorLike in from '@apollo/client') and change any
type references to ApolloError in this file—especially the return interface that
currently uses ApolloError—so it uses ErrorLike instead; also verify imports for
useQuery are from '@apollo/client' if necessary and update any other occurrences
of ApolloError in functions like useQuery usage.
---
Nitpick comments:
In `@frontend/src/hooks/useSearchProjectsGraphQL.ts`:
- Around line 9-12: The options interface UseSearchProjectsGraphQLOptions
exposes pageSize but the hook never uses it; update the useSearchProjectsGraphQL
implementation to read options.pageSize and apply it to all pagination logic
(replace hardcoded/positional pageSize usage with options.pageSize ??
pageSizeParam), including where page queries and cursor/offset calculations
occur (searchProjectsQuery handler, pagination helpers, and any loadMore/hasMore
logic). Ensure the default behavior remains the same by falling back to the
existing pageSize parameter when options.pageSize is undefined.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/apps/owasp/api/internal/queries/project.pyfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/hooks/useSearchProjectsGraphQL.tsfrontend/src/utils/backendConfig.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/utils/backendConfig.ts
Contributor
Author
|
Hi @arkid15r pr is ready for review let me know if any changes are required or should i start writing test for the changes? |
Contributor
There was a problem hiding this comment.
7 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx">
<violation number="1" location="frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx:90">
P2: Loading/error/empty-state tests were weakened to only check the mocked search bar or header, so they no longer validate the actual loading spinner or empty/error UI and can pass even if those states regress.</violation>
<violation number="2" location="frontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsx:254">
P3: The “no-op” test ends with `expect(true).toBe(true)`, so it never verifies that URL updates were skipped. This leaves the edge case untested and could hide regressions in order handling.</violation>
</file>
<file name="frontend/src/wrappers/testUtil.tsx">
<violation number="1" location="frontend/src/wrappers/testUtil.tsx:11">
P2: ApolloProvider is initialized asynchronously at module load, so render() can run before it is set and return content without Apollo context, leading to flaky tests when Apollo hooks are used.</violation>
</file>
<file name="frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx">
<violation number="1" location="frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx:180">
P3: Category-change test does not assert that onCategoryChange is called, so it can pass even if the callback wiring breaks.</violation>
</file>
<file name="frontend/src/components/SearchPageLayout.tsx">
<violation number="1" location="frontend/src/components/SearchPageLayout.tsx:61">
P2: Removing `w-full` from the loaded-content wrapper inside a `flex-col items-center` container allows the wrapper to shrink to content width, which can break right alignment for the sort row and narrow full-width child layouts.</violation>
</file>
<file name="backend/apps/owasp/api/internal/queries/project.py">
<violation number="1" location="backend/apps/owasp/api/internal/queries/project.py:124">
P2: search_projects now rejects short/overlong queries, but search_projects_count still counts them, leading to inconsistent result vs count behavior for the same query.</violation>
<violation number="2" location="backend/apps/owasp/api/internal/queries/project.py:148">
P2: search_projects now materializes and slices the queryset to 3 items before returning, which prevents strawberry-django from applying its automatic filtering/ordering/pagination and hard-caps results to 3 regardless of pagination inputs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/api/rest/v0/project.py (1)
89-100:⚠️ Potential issue | 🟠 Major
ProjectFilter.typeis declared twice.The second
typefield overrides the first one, so the first declaration is dead code and the filter contract is ambiguous. Keep only onetypedeclaration and align it with the intended query semantics.Suggested cleanup
class ProjectFilter(FilterSchema): @@ - type: ProjectType | None = Field( - None, - description="Type (category) of the project", - ) @@ type: list[ProjectType] | None = Field( None, description="Type of the project", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/api/rest/v0/project.py` around lines 89 - 100, ProjectFilter currently declares the attribute "type" twice (once as ProjectType | None and once as list[ProjectType] | None); remove the duplicate and keep the correct shape for the filter (choose either a single ProjectType or a list of ProjectType). Edit the ProjectFilter class to have a single "type" Field, remove the other declaration, and update the Field's type annotation and description to match the intended query semantics (e.g., use list[ProjectType] | None if callers should be able to filter by multiple types, or ProjectType | None for a single value); ensure only the remaining "type" symbol is present and its description is accurate.
🧹 Nitpick comments (1)
backend/tests/apps/owasp/api/internal/filters/project_test.py (1)
45-46: Remove the duplicated assertion in this loop test.
assert result == Q(type=project_type)is repeated twice; the second assertion adds no extra signal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/apps/owasp/api/internal/filters/project_test.py` around lines 45 - 46, The test contains a duplicated assertion checking equality between result and Q(type=project_type); remove the redundant second assertion so the loop only asserts once. Locate the duplicate line referencing result and Q(type=project_type) in the test (variables: result, project_type, and Q) and delete the repeated assertion to avoid pointless duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/owasp/api/internal/queries/project.py`:
- Around line 98-103: The resolver search_projects currently materializes and
hard-caps results with list(base_queryset[:SEARCH_PROJECTS_LIMIT]) which
prevents Strawberry-Django from applying filters/ordering/pagination; change
search_projects to return the QuerySet itself (base_queryset or
base_queryset.all()) instead of a list slice so the framework can apply
filters/ordering/pagination, and if you must enforce a maximum cap use a
QuerySet-level limit only after framework pagination (or implement cap via
pagination settings) and avoid using SEARCH_PROJECTS_LIMIT to materialize
results inside search_projects.
In `@frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx`:
- Around line 180-188: The test "calls onCategoryChange when category is
selected" currently only fires a change on the select in UnifiedSearchBar;
update it to assert that the mock callback mockOnCategoryChange was invoked with
the selected value (e.g., 'tech') using the appropriate Jest matcher
(toHaveBeenCalledWith or toHaveBeenCalledTimes + toHaveBeenCalledWith). Apply
the same change to the other similar test (the one around lines 428-435) so both
verify the callback behavior on change.
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 137-147: handleSearchChange currently treats whitespace-only input
as an active query; trim the incoming query first (e.g., const trimmed =
query.trim()), then use trimmed for setSearchQuery and for deciding
pagination/URL updates so that empty or whitespace-only strings clear the search
state and remove the 'search' param (use newParams.set only when trimmed is
non-empty, otherwise newParams.delete). Apply the same trimming logic to the
other search handler that updates searchParams (the similar handler around line
156) so both UI state and URL are normalized to backend-trimmed behavior.
In `@frontend/src/components/UnifiedSearchBar.tsx`:
- Around line 63-65: The Select is controlled with selectedKeys but uses a DOM
onChange; replace the onChange handler in UnifiedSearchBar with HeroUI's
onSelectionChange to receive a Set<React.Key>, convert that set to the single
key/string you expect, and call onCategoryChange with that value (e.g.,
Array.from(selectedKeys)[0] or selectedKeys.values().next().value). Update the
handler tied to the Select component (where selectedKeys is used) to use
onSelectionChange and ensure the value passed into onCategoryChange matches the
expected string type.
In `@frontend/src/wrappers/testUtil.tsx`:
- Around line 11-20: The silent catch in the async IIFE that imports
'@apollo/client/react' masks real import failures for ApolloProvider; either
remove the try/catch so the dynamic import error propagates, or change the catch
to accept an error (e.g., catch (err)) and explicitly rethrow or log and throw a
new error so test initialization fails loudly; update the IIFE that assigns
ApolloProvider to use the chosen option so import issues for ApolloProvider are
visible during test setup.
---
Outside diff comments:
In `@backend/apps/api/rest/v0/project.py`:
- Around line 89-100: ProjectFilter currently declares the attribute "type"
twice (once as ProjectType | None and once as list[ProjectType] | None); remove
the duplicate and keep the correct shape for the filter (choose either a single
ProjectType or a list of ProjectType). Edit the ProjectFilter class to have a
single "type" Field, remove the other declaration, and update the Field's type
annotation and description to match the intended query semantics (e.g., use
list[ProjectType] | None if callers should be able to filter by multiple types,
or ProjectType | None for a single value); ensure only the remaining "type"
symbol is present and its description is accurate.
---
Nitpick comments:
In `@backend/tests/apps/owasp/api/internal/filters/project_test.py`:
- Around line 45-46: The test contains a duplicated assertion checking equality
between result and Q(type=project_type); remove the redundant second assertion
so the loop only asserts once. Locate the duplicate line referencing result and
Q(type=project_type) in the test (variables: result, project_type, and Q) and
delete the repeated assertion to avoid pointless duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 157df032-0ade-4970-968c-131f3b8b20a9
📒 Files selected for processing (14)
backend/apps/api/rest/v0/project.pybackend/apps/owasp/api/internal/queries/project.pybackend/tests/apps/owasp/api/internal/filters/project_test.pybackend/tests/apps/owasp/api/internal/queries/project_test.pybackend/tests/apps/owasp/index/search/project_test.pyfrontend/__tests__/unit/components/UnifiedSearchBar.test.tsxfrontend/__tests__/unit/pages/Contribute.test.tsxfrontend/__tests__/unit/pages/ProjectsHealthDashboardMetrics.test.tsxfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/SearchPageLayout.tsxfrontend/src/components/UnifiedSearchBar.tsxfrontend/src/hooks/useSearchProjectsGraphQL.tsfrontend/src/utils/helpers/mockApolloClient.tsfrontend/src/wrappers/testUtil.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/SearchPageLayout.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx (1)
277-286: Consider verifying selected category value.This test verifies the category select exists but doesn't assert the selected value is
'science'. While the component'sselectedKeysprop is tested implicitly through the mock, explicitly checking the select's value would strengthen the test.Optional enhancement
const categorySelect = screen.getByLabelText('Filter by category') expect(categorySelect).toBeInTheDocument() + expect(categorySelect).toHaveValue('science')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx` around lines 277 - 286, The test currently only asserts the category select exists but not that it reflects the passed-in category; update the 'renders with correct selected category' test for UnifiedSearchBar to also assert the select's value is 'science' by retrieving the element (categorySelect via screen.getByLabelText('Filter by category')) and adding an expectation like checking categorySelect.value or using expect(categorySelect).toHaveValue('science') so the component's selectedKeys behavior is explicitly verified.frontend/src/app/projects/dashboard/metrics/page.tsx (1)
307-318: Redundant empty state handling.The
emptyprop is passed to UnifiedSearchBar (line 307), but the component also renders its own empty state message (lines 314-318). Based on the SearchPageLayout implementation (context snippet 2),emptyis used by the layout when no children content exists. However, since children always renders either the loading spinner or the grid (with its own empty message), theemptyprop may never be displayed.Consider removing the inline empty state handling and relying on the
emptyprop for consistency, or remove theemptyprop if the inline handling is preferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/projects/dashboard/metrics/page.tsx` around lines 307 - 318, The component is rendering its own empty-message inside the children while also passing an empty prop to UnifiedSearchBar; remove the redundant inline empty state so the layout's empty prop is used consistently. Concretely, in page.tsx inside the UnifiedSearchBar children (the block that conditionally renders LoadingSpinner or the grid), stop rendering the fallback div when metrics.length === 0 — either render nothing (null) or only render the grid when metrics.length > 0 and still render LoadingSpinner when loading is true; keep the empty prop as-is. Update references to the metrics array and searchQuery only for conditional rendering (do not change the empty prop or UnifiedSearchBar invocation), and ensure LoadingSpinner remains displayed while loading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/tests/apps/owasp/api/internal/queries/project_test.py`:
- Around line 498-513: Update the test to reflect the production behavior where
overly-long queries (>100 chars) return 0 instead of truncating: in
test_search_projects_count_with_long_query_bounded, change the expected result
assertion from 3 to 0 and replace the
mock_queryset.filter.assert_called_with(...) check with an assertion that
Project.objects.filter was not called (e.g., mock_filter.assert_not_called()),
referencing the tested methods search_projects_count and search_projects to
locate the logic to align with.
---
Nitpick comments:
In `@frontend/__tests__/unit/components/UnifiedSearchBar.test.tsx`:
- Around line 277-286: The test currently only asserts the category select
exists but not that it reflects the passed-in category; update the 'renders with
correct selected category' test for UnifiedSearchBar to also assert the select's
value is 'science' by retrieving the element (categorySelect via
screen.getByLabelText('Filter by category')) and adding an expectation like
checking categorySelect.value or using
expect(categorySelect).toHaveValue('science') so the component's selectedKeys
behavior is explicitly verified.
In `@frontend/src/app/projects/dashboard/metrics/page.tsx`:
- Around line 307-318: The component is rendering its own empty-message inside
the children while also passing an empty prop to UnifiedSearchBar; remove the
redundant inline empty state so the layout's empty prop is used consistently.
Concretely, in page.tsx inside the UnifiedSearchBar children (the block that
conditionally renders LoadingSpinner or the grid), stop rendering the fallback
div when metrics.length === 0 — either render nothing (null) or only render the
grid when metrics.length > 0 and still render LoadingSpinner when loading is
true; keep the empty prop as-is. Update references to the metrics array and
searchQuery only for conditional rendering (do not change the empty prop or
UnifiedSearchBar invocation), and ensure LoadingSpinner remains displayed while
loading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff9c02ce-8cf4-4943-992c-2372ebfbd01a
📒 Files selected for processing (9)
backend/apps/api/rest/v0/project.pybackend/apps/owasp/api/internal/queries/project.pybackend/tests/apps/owasp/api/internal/filters/project_test.pybackend/tests/apps/owasp/api/internal/queries/project_test.pyfrontend/__tests__/unit/components/ProjectTypeDashboardCard.test.tsxfrontend/__tests__/unit/components/UnifiedSearchBar.test.tsxfrontend/src/app/projects/dashboard/metrics/page.tsxfrontend/src/components/UnifiedSearchBar.tsxfrontend/src/wrappers/testUtil.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/tests/apps/owasp/api/internal/filters/project_test.py
- backend/apps/api/rest/v0/project.py
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/tests/apps/owasp/api/internal/queries/project_test.py">
<violation number="1" location="backend/tests/apps/owasp/api/internal/queries/project_test.py:134">
P2: Short-query tests are too weak: `assert result != []` does not verify that query filtering behavior actually occurs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Contributor
Author
|
Hi @arkid15r the pr is ready for review Thanks! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
This PR extends the Projects search functionality by introducing a category filter and a unified sorting dropdown within a reusable search component.
The search UI has been refactored into a shared component used across both the Projects list page and the Project Health dashboard to ensure consistent behavior and UX.
Resolves #4086
Screencast.from.2026-03-04.06-18-20.webm
Checklist
make check-testlocally: all warnings addressed, tests passed