fix: show actual filter values per project in OSS search results #364#367
fix: show actual filter values per project in OSS search results #364#367VivekArgSharma wants to merge 1 commit into
Conversation
|
@VivekArgSharma is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes add new repository metadata fields (star count, fork count, and timestamps) to the GitHub GraphQL query and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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 |
|
Hey can I get authorization! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/utils/converter.ts (2)
25-45: Deduplicate popularity/competition ranges withuserInputValues.The star/fork bucket boundaries are now defined twice — once inside
computePopularity/computeCompetition(lines 26–32, 37–43) and again inuserInputValues.Popularity/userInputValues.Competition(lines 134–147). They must stay in lockstep, or a user can filter for "Low" and see a result labeled "Very low" for the same value. Consider deriving the compute functions from the single source of truth inuserInputValues.♻️ Example: single source of truth
-const computePopularity = (stars: number): string => { - const ranges: { [key: string]: { min?: string; max?: string } } = { - "Very low": { min: "10", max: "500" }, - Low: { min: "501", max: "1000" }, - Moderate: { min: "1001", max: "2000" }, - High: { min: "2001", max: "5000" }, - "Very high": { min: "5001" }, - }; - return categorize(stars, ranges); -}; - -const computeCompetition = (forks: number): string => { - const ranges: { [key: string]: { min?: string; max?: string } } = { - "Very low": { min: "0", max: "200" }, - Low: { min: "201", max: "500" }, - Moderate: { min: "501", max: "1000" }, - High: { min: "1001", max: "2000" }, - "Very high": { min: "2001" }, - }; - return categorize(forks, ranges); -}; +const computePopularity = (stars: number): string => + categorize(stars, userInputValues.Popularity); + +const computeCompetition = (forks: number): string => + categorize(forks, userInputValues.Competition);This requires moving
userInputValuesabove the compute helpers (or hoisting the popularity/competition maps into named consts) and relaxingcategorize'srangesparameter type to accept the shared shape.As per coding guidelines: "Avoid unnecessary comments; code should be self-documenting when possible" and general DRY expectations for
.tsfiles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/utils/converter.ts` around lines 25 - 45, computePopularity and computeCompetition duplicate the same bucket maps that already exist on userInputValues, so remove the inline maps and derive ranges from userInputValues.Popularity and userInputValues.Competition (or hoist those maps into shared consts) and pass them into categorize; also relax the categorize parameter type to accept the shared shape used by userInputValues. Update computePopularity, computeCompetition, and categorize signatures to reference userInputValues.Popularity / userInputValues.Competition (or the hoisted const names) so there is a single source of truth for range boundaries.
71-89: Remove commented-out example blocks.These are inline usage examples, not explanations of "why". They're exactly the kind of dead/illustrative comments that belong in docs or tests, not in the source file.
✂️ Proposed cleanup
-// // USER INPUT EXAMPLE - -// const userInput = { -// "Tech stack": "Python", -// Popularity: "Low", -// Competition: "High", -// Stage: "Very early", -// Activity: "Normal", -// }; - -// API INPUT EXAMPLE - -// const apiInput = { -// language: "python", -// stars: { min: "100", max: "2000" }, -// forks: { min: "50", max: "1000" }, -// pushed: ">=2024-12-08", -// created: ">=2024-12-12", -// }; -As per coding guidelines: "Avoid unnecessary comments; code should be self-documenting when possible" and "Use comments to explain 'why', not 'what'".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/utils/converter.ts` around lines 71 - 89, Remove the dead example comments in apps/web/src/utils/converter.ts: delete the commented-out USER INPUT EXAMPLE and API INPUT EXAMPLE blocks (the commented userInput and apiInput example objects) so the file contains only live code and meaningful "why" comments; locate the examples by the identifiers userInput and apiInput in the converter.ts diff and remove those commented sections, then run a quick lint/format to ensure no leftover empty comment blocks remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/ui/FiltersContainer.tsx`:
- Line 49: The call to convertApiOutputToUserOutput(projects) is correct for the
new single-argument signature, but the async fetch flow leaves the loading state
stuck on error; update the fetch/error handling around getProjects so
setLoading(true) is balanced by a finally that calls setLoading(false) (i.e.,
add a finally { setLoading(false); } after the try/catch that surrounds
getProjects and setLoading calls) to ensure the UI always clears the loading
state even when an exception is thrown.
---
Nitpick comments:
In `@apps/web/src/utils/converter.ts`:
- Around line 25-45: computePopularity and computeCompetition duplicate the same
bucket maps that already exist on userInputValues, so remove the inline maps and
derive ranges from userInputValues.Popularity and userInputValues.Competition
(or hoist those maps into shared consts) and pass them into categorize; also
relax the categorize parameter type to accept the shared shape used by
userInputValues. Update computePopularity, computeCompetition, and categorize
signatures to reference userInputValues.Popularity / userInputValues.Competition
(or the hoisted const names) so there is a single source of truth for range
boundaries.
- Around line 71-89: Remove the dead example comments in
apps/web/src/utils/converter.ts: delete the commented-out USER INPUT EXAMPLE and
API INPUT EXAMPLE blocks (the commented userInput and apiInput example objects)
so the file contains only live code and meaningful "why" comments; locate the
examples by the identifiers userInput and apiInput in the converter.ts diff and
remove those commented sections, then run a quick lint/format to ensure no
leftover empty comment blocks remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ec750a4-dfb3-4265-8332-be84d6da7552
📒 Files selected for processing (4)
apps/api/src/services/project.service.tsapps/web/src/components/ui/FiltersContainer.tsxapps/web/src/utils/converter.tspackages/shared/types/projects.ts
| return; | ||
| } | ||
| const modifiedProjects = convertApiOutputToUserOutput(projects, filters); | ||
| const modifiedProjects = convertApiOutputToUserOutput(projects); |
There was a problem hiding this comment.
LGTM — call site aligned with the new single-arg converter.
Dropping filters here is consistent with the refactored convertApiOutputToUserOutput(response) signature and is the core of the fix: unselected filters will now reflect each project's actual values rather than being blank/filter-derived.
One small note unrelated to this line: if getProjects throws, setLoading(true) will remain set because the catch block on line 54–56 doesn't reset it. Worth considering a finally { setLoading(false); } so the UI doesn't get stuck on error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ui/FiltersContainer.tsx` at line 49, The call to
convertApiOutputToUserOutput(projects) is correct for the new single-argument
signature, but the async fetch flow leaves the loading state stuck on error;
update the fetch/error handling around getProjects so setLoading(true) is
balanced by a finally that calls setLoading(false) (i.e., add a finally {
setLoading(false); } after the try/catch that surrounds getProjects and
setLoading calls) to ensure the UI always clears the loading state even when an
exception is thrown.
1 participant
Description
Problem:
so if you select a specific filter for searching in the OSS Project Search bar and hit the search the result do show the projects according to the filter you use but the filter that you dont use goes totally blank .
Fixed this problem now you can see the values that were not selected in the filter
Summary by CodeRabbit