Skip to content

Fix/issue 44 external apps filter #1424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SnehalTanpure7 wants to merge 3 commits into from
+56 −5
Closed

Fix/issue 44 external apps filter #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ede133d
client/src/module/student/opensource/OpenSourceAnalyticsPage.tsx
SnehalTanpure7 May 30, 2026
615f096
feat/683-congratulations-state
SnehalTanpure7 May 31, 2026
4801e9c
fix: include external applications in IN_PROGRESS filter (#44)
SnehalTanpure7 Jun 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions client/src/module/student/applications/MyApplicationsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,13 @@ const STATUS_ORDER: Record<string, number> = {
REJECTED: 4,
WITHDRAWN: 5,
};
const EXTERNAL_STATUS = "APPLIED";
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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.

Suggested change
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 EXTERNAL_VISIBLE_FILTERS = ["ALL", "APPLIED", "IN_PROGRESS"] as const;
type ExternalVisibleFilter = (typeof EXTERNAL_VISIBLE_FILTERS)[number];

function isExternalVisibleFilter(filter: string): filter is ExternalVisibleFilter {
return (EXTERNAL_VISIBLE_FILTERS as readonly string[]).includes(filter);
}

function sortApplications(
apps: Application[],
Expand All @@ -259,6 +266,7 @@ export default function MyApplicationsPage() {
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");
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

🧩 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=ts

Repository: 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" | cat

Repository: 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.


useEffect(() => {
const t = setTimeout(() => setDebouncedSearch(search), 200);
Expand All @@ -268,7 +276,7 @@ export default function MyApplicationsPage() {
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setPage(1);
}, [debouncedSearch]);
}, [debouncedSearch, activeFilter]);

const { data, isLoading } = useQuery({
queryKey: queryKeys.applications.mine(),
Expand All @@ -288,9 +296,10 @@ export default function MyApplicationsPage() {
(a) => a.job?.title?.toLowerCase().includes(debouncedSearch.toLowerCase()) || a.job?.company?.toLowerCase().includes(debouncedSearch.toLowerCase())
);
return sortApplications(base, sortOption);
}, [applications, debouncedSearch, sortOption]);
}, [applications, debouncedSearch, sortOption, activeFilter]);

const filteredExternal = useMemo(() => {
if (!isExternalVisibleFilter(activeFilter)) return [];
const base = !debouncedSearch.trim()
? externalApplications
: externalApplications.filter(
Expand All @@ -304,7 +313,7 @@ export default function MyApplicationsPage() {
if (sortOption === "company") return (a.adminJob.company ?? "").localeCompare(b.adminJob.company ?? "");
return 0;
});
}, [externalApplications, debouncedSearch, sortOption]);
}, [externalApplications, debouncedSearch, sortOption, activeFilter]);

const totalAll = applications.length + externalApplications.length;
const totalFiltered = filtered.length + filteredExternal.length;
Expand Down
23 changes: 21 additions & 2 deletions client/src/module/student/opensource/FirstPRRoadmapPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,27 @@ export default function FirstPRRoadmapPage() {
>
<Trophy className="w-8 h-8 text-green-500 shrink-0" />
<div>
<p className="text-base font-bold text-green-900 dark:text-green-300">You're an open source contributor!</p>
<p className="text-sm text-green-700 dark:text-green-400 mt-0.5">You've completed all {totalSteps} steps. Time to find your next issue!</p>
<p className="text-base font-bold text-green-900 dark:text-green-300">You shipped your first PR!</p>
<p className="text-sm text-green-700 dark:text-green-400 mt-0.5">10 / 10 steps complete. Your open source journey has begun.</p>
<div className="flex gap-4 mt-3 flex-wrap items-center">
<Link
to="/student/opensource"
className="text-sm text-lime-700 dark:text-lime-400 underline font-medium"
>
Discover repos to contribute to
</Link>
Comment on lines +129 to +134
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
<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>
Comment on lines +135 to +145
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

</div>
</div>
</motion.div>
)}
Expand Down
23 changes: 23 additions & 0 deletions client/src/module/student/opensource/OpenSourceAnalyticsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,29 @@ export default function OpenSourceAnalyticsPage() {
);
}

// Empty state: student has zero contributions
if (!trendIsLoading && !trendIsError && contributionTotal === 0) {
return (
<div className="min-h-screen bg-gray-50 flex flex-col items-center justify-center px-4 text-center">
<div className="bg-indigo-50 rounded-full p-6 mb-5">
<BarChart3 className="w-12 h-12 text-indigo-400" />
</div>
<h2 className="text-2xl font-bold text-gray-900 mb-3">
No contributions tracked yet
</h2>
<p className="text-gray-500 max-w-md mb-8">
Submit a repo suggestion and get it approved to start tracking
your open source journey.
</p>
<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>
Comment on lines +404 to +409
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
<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.

</div>
);
}
return (
<div className="min-h-screen bg-gray-50 pb-16">
{/* Header */}
Expand Down
Loading