Fix/issue 44 external apps filter#1424
Conversation
|
Hi @SnehalTanpure7, thanks for contributing to InternHack! 🎉 I have automatically:
Our workflows will now analyze your changes to classify:
Tip Ensure your PR description references the issue it resolves (e.g. Happy coding! 🚀 |
github-actions
Bot
added
scope:frontend
📝 WalkthroughWalkthroughThis PR adds three independent UI enhancements across the student module: filter-driven visibility control for external applications in MyApplicationsPage, an updated completion banner with reset functionality in FirstPRRoadmapPage, and an empty-state display for zero tracked contributions in OpenSourceAnalyticsPage. ChangesStudent UI Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
|
I have submitted a PR for this issue. #245 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
client/src/module/student/applications/MyApplicationsPage.tsx (1)
292-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
activeFilterfrom dependencies or implement status filtering.
activeFilteris included in the dependency array (line 299) but is never used in the memo body. This causes unnecessary recomputation without changing the result.Additionally, the internal applications are not filtered by status at all - they only filter by search term. This is inconsistent with the external applications logic (line 302) which respects
activeFilter. If status-based filtering is intended, it should be implemented for internal applications as well.♻️ Proposed fix to remove unused dependency
const filtered = useMemo(() => { const base = !debouncedSearch.trim() ? applications : applications.filter( (a) => a.job?.title?.toLowerCase().includes(debouncedSearch.toLowerCase()) || a.job?.company?.toLowerCase().includes(debouncedSearch.toLowerCase()) ); return sortApplications(base, sortOption); - }, [applications, debouncedSearch, sortOption, activeFilter]); + }, [applications, debouncedSearch, sortOption]);Note: If status filtering for internal applications is actually needed to complete this feature, the memo body should filter
basebyactiveFilterbefore sorting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/module/student/applications/MyApplicationsPage.tsx` around lines 292 - 299, The memo for filtered (the useMemo that builds base, uses applications, debouncedSearch and calls sortApplications) currently includes activeFilter in its dependency array but doesn't use it; either remove activeFilter from the dependency list to avoid unnecessary recomputations, or implement status filtering by applying activeFilter to base (e.g., filter base by application status before calling sortApplications) so that activeFilter is actually used; update the dependency array to match the actual inputs (applications, debouncedSearch, sortOption and activeFilter only if you implement the filter).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/module/student/applications/MyApplicationsPage.tsx`:
- Line 237: EXTERNAL_STATUS is declared in MyApplicationsPage.tsx but never
used; either remove the dead constant or wire it into the application-status
mapping/filtering logic. If external applications should map to "APPLIED",
update the appropriate function that derives or filters application.status
(e.g., wherever internal statuses are normalized or filters are applied) to
reference EXTERNAL_STATUS when source === "external"; otherwise delete the
EXTERNAL_STATUS declaration to eliminate dead code.
- Line 269: The activeFilter state in MyApplicationsPage is never updated, so
filteredExternal remains stuck on "ALL"; wire the setActiveFilter setter into
the UI that renders the external filter controls (or add controls if missing) so
user actions update activeFilter and re-compute filteredExternal. Locate the
activeFilter and setActiveFilter declarations in MyApplicationsPage, find the
component/markup that should present the filter options (buttons, tabs, or a
select) and add onClick/onChange handlers that call
setActiveFilter("ALL"|"IN_PROGRESS"|"APPLIED") as appropriate, or pass
setActiveFilter down to the existing filter component so it can update state.
Ensure the filtering logic that derives filteredExternal reads the activeFilter
state so the view updates when the setter is called.
In `@client/src/module/student/opensource/FirstPRRoadmapPage.tsx`:
- Around line 129-134: Replace the plain <Link> in FirstPRRoadmapPage.tsx with
the reusable Button component composed with the Link via the asChild prop:
import Button from the ui/button component, wrap the existing <Link
to="/student/opensource">Discover repos to contribute to</Link> inside <Button
asChild> so the Button provides the styling and the Link remains the navigation
element, and remove the custom className styling previously applied to the Link
to rely on Button's styles.
- Around line 135-145: The Button instance rendering "Start over" should stop
using a custom className and instead use the Button component's built-in props:
replace the className on the Button in FirstPRRoadmapPage.tsx with appropriate
variant/mode/size props (for example variant="ghost" or "secondary",
mode="button", size="sm" or "md") to match the design system, keep the existing
onClick handler that confirms and calls setCompleted(new Set()) and
localStorage.removeItem(STORAGE_KEY), and remove any inline styling so the
Button styling comes from the component props only.
In `@client/src/module/student/opensource/OpenSourceAnalyticsPage.tsx`:
- Around line 404-409: Replace the inline-styled Link with the shared Button
component using its asChild slot: import Button from the component
(client/src/components/ui/button.tsx), wrap the existing <Link
to="/student/opensource"> with <Button asChild variant="primary" size="md"> (or
the variant/size that matches the current styling) and remove the duplicated
className on the Link; ensure the final structure is Button asChild containing
the Link and that no inline bg/text/padding/rounded classes remain on the Link.
---
Outside diff comments:
In `@client/src/module/student/applications/MyApplicationsPage.tsx`:
- Around line 292-299: The memo for filtered (the useMemo that builds base, uses
applications, debouncedSearch and calls sortApplications) currently includes
activeFilter in its dependency array but doesn't use it; either remove
activeFilter from the dependency list to avoid unnecessary recomputations, or
implement status filtering by applying activeFilter to base (e.g., filter base
by application status before calling sortApplications) so that activeFilter is
actually used; update the dependency array to match the actual inputs
(applications, debouncedSearch, sortOption and activeFilter only if you
implement the filter).
🪄 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: 9582ce1d-e1bd-4da0-8ac1-ba78d050dd3c
📒 Files selected for processing (3)
client/src/module/student/applications/MyApplicationsPage.tsxclient/src/module/student/opensource/FirstPRRoadmapPage.tsxclient/src/module/student/opensource/OpenSourceAnalyticsPage.tsx
| REJECTED: 4, | ||
| WITHDRAWN: 5, | ||
| }; | ||
| const EXTERNAL_STATUS = "APPLIED"; |
There was a problem hiding this comment.
Remove unused constant or clarify its purpose.
EXTERNAL_STATUS is defined but never referenced. If external applications should always map to an "APPLIED" status for filtering purposes, this logic is missing. Otherwise, remove the dead code.
🧹 Proposed fix to remove unused constant
-const EXTERNAL_STATUS = "APPLIED";
const EXTERNAL_VISIBLE_FILTERS = ["ALL", "APPLIED", "IN_PROGRESS"] as const;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const EXTERNAL_STATUS = "APPLIED"; | |
| const EXTERNAL_VISIBLE_FILTERS = ["ALL", "APPLIED", "IN_PROGRESS"] as const; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/applications/MyApplicationsPage.tsx` at line 237,
EXTERNAL_STATUS is declared in MyApplicationsPage.tsx but never used; either
remove the dead constant or wire it into the application-status
mapping/filtering logic. If external applications should map to "APPLIED",
update the appropriate function that derives or filters application.status
(e.g., wherever internal statuses are normalized or filters are applied) to
reference EXTERNAL_STATUS when source === "external"; otherwise delete the
EXTERNAL_STATUS declaration to eliminate dead code.
| const [page, setPage] = useState(1); | ||
| const [pendingDelete, setPendingDelete] = useState<PendingDelete | null>(null); | ||
| const [sortOption, setSortOption] = useState<"newest" | "oldest" | "company" | "status">("newest"); | ||
| const [activeFilter, setActiveFilter] = useState("ALL"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for filter UI implementation that might set activeFilter
# Search for setActiveFilter calls
rg -n 'setActiveFilter' --type=tsx --type=ts
# Search for potential filter button/tab patterns
rg -n -C3 'IN_PROGRESS|APPLIED|ALL.*filter|filter.*button' client/src/module/student/applications/ --type=tsx --type=tsRepository: Sachinchaurasiya360/InternHack
Length of output: 130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/applications/MyApplicationsPage.tsx"
echo "== Existence checks =="
ls -l "$FILE"
echo
echo "== Find activeFilter usage =="
rg -n "activeFilter" "$FILE"
echo
echo "== Find setActiveFilter usage in repo (scoped) =="
rg -n "setActiveFilter" client/src --glob='*.ts' --glob='*.tsx' || true
echo
echo "== Find IN_PROGRESS/APPLIED mentions in that module dir =="
rg -n "IN_PROGRESS|APPLIED" client/src/module/student/applications --glob='*.ts' --glob='*.tsx' || true
echo
echo "== Show filter/search-related UI area around line ~430-560 =="
# show manageable window
sed -n '420,560p' "$FILE" | nl -ba | sed -n '1,200p'Repository: Sachinchaurasiya360/InternHack
Length of output: 1493
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/applications/MyApplicationsPage.tsx"
echo "== Existence checks =="
ls -l "$FILE"
echo
echo "== Find activeFilter usage =="
rg -n "activeFilter" "$FILE" || true
echo
echo "== Find setActiveFilter usage in repo (scoped) =="
rg -n "setActiveFilter" client/src --glob='*.ts' --glob='*.tsx' || true
echo
echo "== Find IN_PROGRESS/APPLIED mentions in that module dir =="
rg -n "IN_PROGRESS|APPLIED" client/src/module/student/applications --glob='*.ts' --glob='*.tsx' || true
echo
echo "== Show filter/search-related UI area around line ~430-560 =="
sed -n '420,560p' "$FILE" | nl -ba | sed -n '1,250p'Repository: Sachinchaurasiya360/InternHack
Length of output: 1493
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="client/src/module/student/applications/MyApplicationsPage.tsx"
echo "== setActiveFilter call sites (anywhere) =="
rg -n "setActiveFilter" "$FILE" --glob='*.tsx' --glob='*.ts' || true
echo
echo "== Where EXTERNAL_VISIBLE_FILTERS is used =="
rg -n "EXTERNAL_VISIBLE_FILTERS|isExternalVisibleFilter" "$FILE" || true
echo
echo "== Show surrounding code for those constants/helpers =="
# print around first occurrence(s)
for line in $(rg -n "EXTERNAL_VISIBLE_FILTERS|isExternalVisibleFilter" "$FILE" | cut -d: -f1 | head -n 10); do
echo "--- around line $line ---"
sed -n "$((line-40)),$((line+60))p" "$FILE" | cat
done
echo
echo "== Look for UI event handlers referencing activeFilter =="
rg -n "activeFilter|setActiveFilter|onClick|onChange|RadioGroup|Tabs|Select|Filter" "$FILE" --glob='*.tsx' | head -n 80 || true
echo
echo "== Print a likely UI region (420-620) without nl =="
sed -n '400,650p' "$FILE" | catRepository: Sachinchaurasiya360/InternHack
Length of output: 30211
Critical: External application filter is unreachable (activeFilter never changes).
In client/src/module/student/applications/MyApplicationsPage.tsx, activeFilter is set up as useState("ALL") but there are no setActiveFilter(...) call sites and no UI controls wired to update it. That means the external list filtering (filteredExternal) always stays on "ALL", so users can’t switch to "IN_PROGRESS"/"APPLIED" views.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/applications/MyApplicationsPage.tsx` at line 269,
The activeFilter state in MyApplicationsPage is never updated, so
filteredExternal remains stuck on "ALL"; wire the setActiveFilter setter into
the UI that renders the external filter controls (or add controls if missing) so
user actions update activeFilter and re-compute filteredExternal. Locate the
activeFilter and setActiveFilter declarations in MyApplicationsPage, find the
component/markup that should present the filter options (buttons, tabs, or a
select) and add onClick/onChange handlers that call
setActiveFilter("ALL"|"IN_PROGRESS"|"APPLIED") as appropriate, or pass
setActiveFilter down to the existing filter component so it can update state.
Ensure the filtering logic that derives filteredExternal reads the activeFilter
state so the view updates when the setter is called.
| <Link | ||
| to="/student/opensource" | ||
| className="text-sm text-lime-700 dark:text-lime-400 underline font-medium" | ||
| > | ||
| Discover repos to contribute to | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use Button component with asChild for navigation links.
This new navigation link should use the reusable Button component with the asChild prop to compose with <Link>, rather than applying custom className styling to a bare Link. This ensures consistent button styling across the codebase.
♻️ Refactor to use Button with asChild
- <Link
- to="/student/opensource"
- className="text-sm text-lime-700 dark:text-lime-400 underline font-medium"
- >
- Discover repos to contribute to
- </Link>
+ <Button
+ variant="ghost"
+ mode="link"
+ size="sm"
+ asChild
+ >
+ <Link to="/student/opensource">
+ Discover repos to contribute to
+ </Link>
+ </Button>As per coding guidelines, use the reusable Button component from client/src/components/ui/button.tsx for all new buttons; the asChild prop (Radix Slot) should be used when composing Button with <Link> or other elements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link | |
| to="/student/opensource" | |
| className="text-sm text-lime-700 dark:text-lime-400 underline font-medium" | |
| > | |
| Discover repos to contribute to | |
| </Link> | |
| <Button | |
| variant="ghost" | |
| mode="link" | |
| size="sm" | |
| asChild | |
| > | |
| <Link to="/student/opensource"> | |
| Discover repos to contribute to | |
| </Link> | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/opensource/FirstPRRoadmapPage.tsx` around lines 129
- 134, Replace the plain <Link> in FirstPRRoadmapPage.tsx with the reusable
Button component composed with the Link via the asChild prop: import Button from
the ui/button component, wrap the existing <Link
to="/student/opensource">Discover repos to contribute to</Link> inside <Button
asChild> so the Button provides the styling and the Link remains the navigation
element, and remove the custom className styling previously applied to the Link
to rely on Button's styles.
| <Button | ||
| onClick={() => { | ||
| if (window.confirm("Reset all steps?")) { | ||
| setCompleted(new Set()); | ||
| try { localStorage.removeItem(STORAGE_KEY); } catch { /**/ } | ||
| } | ||
| }} | ||
| className="text-sm text-lime-700 dark:text-lime-400 border border-lime-400 px-3 py-0.5 rounded-lg font-medium" | ||
| > | ||
| Start over | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use Button variant/mode/size props instead of custom className.
The Button component should use its built-in variant, mode, and size props rather than custom className overrides. Applying custom styles bypasses the design system and reduces consistency.
♻️ Refactor to use proper Button props
<Button
+ variant="secondary"
+ size="sm"
onClick={() => {
if (window.confirm("Reset all steps?")) {
setCompleted(new Set());
try { localStorage.removeItem(STORAGE_KEY); } catch { /**/ }
}
}}
- className="text-sm text-lime-700 dark:text-lime-400 border border-lime-400 px-3 py-0.5 rounded-lg font-medium"
>
Start over
</Button>As per coding guidelines, the Button component supports variants (primary, secondary, mono, ghost, danger), modes (button, icon, link), and sizes (sm, md, lg) that should be used instead of custom className styling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/opensource/FirstPRRoadmapPage.tsx` around lines 135
- 145, The Button instance rendering "Start over" should stop using a custom
className and instead use the Button component's built-in props: replace the
className on the Button in FirstPRRoadmapPage.tsx with appropriate
variant/mode/size props (for example variant="ghost" or "secondary",
mode="button", size="sm" or "md") to match the design system, keep the existing
onClick handler that confirms and calls setCompleted(new Set()) and
localStorage.removeItem(STORAGE_KEY), and remove any inline styling so the
Button styling comes from the component props only.
| <Link | ||
| to="/student/opensource" | ||
| className="bg-indigo-600 text-white px-6 py-3 rounded-xl font-medium hover:bg-indigo-700 transition-colors" | ||
| > | ||
| Discover Repositories → | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the shared Button component with asChild instead of inline Link styles.
The Link duplicates Button styling with inline classes. As per coding guidelines, use the reusable Button component which supports the asChild prop (Radix Slot) for composing with <Link> or other elements.
♻️ Refactor to use Button with asChild
- <Link
- to="/student/opensource"
- className="bg-indigo-600 text-white px-6 py-3 rounded-xl font-medium hover:bg-indigo-700 transition-colors"
- >
- Discover Repositories →
- </Link>
+ <Button variant="primary" size="lg" asChild>
+ <Link to="/student/opensource">
+ Discover Repositories →
+ </Link>
+ </Button>As per coding guidelines: "Use the reusable Button component from client/src/components/ui/button.tsx for all new buttons; supports variants (primary, secondary, mono, ghost, danger), modes (button, icon, link), and sizes (sm, md, lg). Use asChild prop (Radix Slot) when composing Button with <Link> or other elements."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link | |
| to="/student/opensource" | |
| className="bg-indigo-600 text-white px-6 py-3 rounded-xl font-medium hover:bg-indigo-700 transition-colors" | |
| > | |
| Discover Repositories → | |
| </Link> | |
| <Button variant="primary" size="lg" asChild> | |
| <Link to="/student/opensource"> | |
| Discover Repositories → | |
| </Link> | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/module/student/opensource/OpenSourceAnalyticsPage.tsx` around
lines 404 - 409, Replace the inline-styled Link with the shared Button component
using its asChild slot: import Button from the component
(client/src/components/ui/button.tsx), wrap the existing <Link
to="/student/opensource"> with <Button asChild variant="primary" size="md"> (or
the variant/size that matches the current styling) and remove the duplicated
className on the Link; ensure the final structure is Button asChild containing
the Link and that no inline bg/text/padding/rounded classes remain on the Link.
1 participant
Pull Request
Description
Describe your changes.
Related Issue
Fixes #
Type of Change
Testing
Explain how you tested it.
Screenshots / Video
No UI changes in this PR (delete this line if you are making UI changes)
Checklist
.env, credentials, ornode_modulescommittedSummary by CodeRabbit