Feature/search filters pagination #58
Conversation
- Add CatalogFilters and CatalogPage Pydantic models for validation - Implement list_catalog() with 6 filters: language, tag, difficulty, min_rating, min_views, min_syncs - Add 5 sorting options: newest, most_viewed, most_synced, highest_rated, trending - Implement pagination with offset/limit and total_count tracking - Update GET /catalog endpoint to accept 9 query parameters - Add comprehensive test suite with 22 test cases covering filters, sorting, pagination, and edge cases - Fix all linting issues (black, isort, flake8) - Update documentation with implementation details Closes part of search filters feature - backend complete
- Add CatalogFilters type with 9 filter parameters (page, page_size, language, tag, difficulty, min_rating, min_views, min_syncs, sort) - Add CatalogPage type matching backend pagination structure - Update listCatalog() to build query string from filters and return CatalogPage - Update RoadmapCatalogProvider to handle CatalogPage.items response - Maintain backward compatibility with existing search page - Update documentation with frontend integration details Note: Advanced filter UI components (dropdowns, sliders, pagination controls) can be added as future enhancement
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async listCatalog( | ||
| page = 1, | ||
| pageSize = 50, | ||
| sort: | ||
| | "newest" | ||
| | "most_viewed" | ||
| | "most_synced" | ||
| | "highest_rated" | ||
| | "trending" = "newest" | ||
| ): Promise<ApiClientResponse<RoadmapCatalogPage>> { | ||
| filters?: CatalogFilters | ||
| ): Promise<ApiClientResponse<CatalogPage>> { | ||
| if (!env.apiBaseUrl) { | ||
| return { ok: false, status: 0, error: "API base URL missing" }; | ||
| } | ||
| return apiClient<RoadmapCatalogPage>(env.apiBaseUrl, { | ||
| path: `${API_ROUTES.catalog}?page=${page}&page_size=${pageSize}&sort=${sort}`, | ||
|
|
||
| // Build query string from filters | ||
| const params = new URLSearchParams() | ||
| if (filters?.page) params.set("page", filters.page.toString()) | ||
| if (filters?.page_size) params.set("page_size", filters.page_size.toString()) | ||
| if (filters?.language) params.set("language", filters.language) | ||
| if (filters?.tag) params.set("tag", filters.tag) | ||
| if (filters?.difficulty) params.set("difficulty", filters.difficulty) | ||
| if (filters?.min_rating !== undefined) params.set("min_rating", filters.min_rating.toString()) | ||
| if (filters?.min_views !== undefined) params.set("min_views", filters.min_views.toString()) | ||
| if (filters?.min_syncs !== undefined) params.set("min_syncs", filters.min_syncs.toString()) | ||
| if (filters?.sort) params.set("sort", filters.sort) |
There was a problem hiding this comment.
The signature of repoService.listCatalog now accepts a single CatalogFilters object, but existing callers still invoke it with three positional arguments (e.g., in app/search/page.tsx). TypeScript treats this as passing three arguments to a function that takes at most one, so tsc/Biome will fail with “Expected 0–1 arguments, but got 3”. Even if the type error were ignored, page, page size, and sort would silently be dropped because the numerical arguments become a non-object filters. Either keep the previous parameter list or update every call site to pass a CatalogFilters object.
Useful? React with 👍 / 👎.
| Args: | ||
| page: Page number (1-indexed) | ||
| page_size: Number of items per page | ||
| language: Filter by primary language | ||
| tag: Filter by topic tag | ||
| difficulty: Filter by difficulty level | ||
| min_rating: Minimum average rating | ||
| min_views: Minimum view count | ||
| min_syncs: Minimum sync count | ||
| sort: Sort option | ||
| Returns: |
There was a problem hiding this comment.
The catalog retrieval method was renamed from list_paginated to list_catalog without leaving a compatibility alias. Existing usages—including the established backend tests under tests/test_roadmap_views.py—still call list_paginated, which now raises AttributeError and breaks the test suite. Consider keeping a thin wrapper or updating all references to the new name before removing the old method.
Useful? React with 👍 / 👎.
No description provided.