Skip to content

feat:Add education plan project limit alert on org page#2963

Merged
HarshMN2345 merged 2 commits intomainfrom
fix-education-plan-project-limit-alert
Apr 8, 2026
Merged

feat:Add education plan project limit alert on org page#2963
HarshMN2345 merged 2 commits intomainfrom
fix-education-plan-project-limit-alert

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

What does this PR do?

image

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Greptile Summary

This PR adds an informational alert on the organization page for users on the GitHub Student Developer education plan, warning them when they've reached the 2-project limit and offering a link to upgrade.

Key changes:

  • Adds educationPlanAlertDismissed state and a dismissEducationPlanAlert handler that stores the dismissed state via hideNotification with a 24-hour cool-off period
  • Derives isEducationProgram by checking data.program?.$id against the hardcoded string 'github-student-developer'
  • Shows the alert when isCloud && isEducationProgram && data.projects.total >= 2
  • The alert coexists correctly with the existing free-plan alert (mutually exclusive via the !data.program guard on that alert)

Finding: The project limit is hardcoded as 2 in both the trigger condition and the alert message copy, rather than read from the plan's service-limit data (which projectsLimit already derives via getServiceLimit). If the education plan cap is ever adjusted, the condition and the displayed number will be stale.

Prior open threads (not repeated here): missing analytics tracking on dismiss, inline style attribute for the underline link.

Confidence Score: 4/5

Safe to merge with minor cleanup; the hardcoded limit is a maintenance concern but does not cause immediate breakage.

The off-by-one from the previous review is correctly fixed (>= 2). The only new finding is a P2 maintenance concern: the limit 2 is hardcoded in both the reactive condition and the alert text instead of being derived from projectsLimit. This won't break anything today but will silently drift if the education plan cap changes. Two threads from prior reviews (analytics tracking, inline style) remain open but were already flagged.

src/routes/(console)/organization-[organization]/+page.svelte — hardcoded project limit value

Vulnerabilities

No security concerns identified. The new code reads plan data from server-provided data props and uses an existing getChangePlanUrl helper for the upgrade link — no user-supplied input is interpolated unsafely.

Important Files Changed

Filename Overview
src/routes/(console)/organization-[organization]/+page.svelte Adds an education plan project limit alert that shows when an org has the github-student-developer program and reaches 2 projects; the threshold off-by-one from a prior review is fixed, but the limit value is hardcoded rather than read from the plan's service-limit data, and minor style issues from prior review threads remain open.

Reviews (2): Last reviewed commit: "Update src/routes/(console)/organization..." | Re-trigger Greptile

Comment on lines +122 to +126
function dismissEducationPlanAlert() {
educationPlanAlertDismissed = true;
const notificationId = `educationPlanAlert_${data.organization.$id}`;
hideNotification(notificationId, { coolOffPeriod: 24 });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Missing analytics tracking on dismiss

dismissFreePlanAlert calls trackEvent(Click.OrganizationClickUpgrade, ...) when dismissed, giving the team signal on user interactions. dismissEducationPlanAlert skips this entirely. If analytics coverage is intentional, a comment explaining why would help; otherwise, consider adding the equivalent tracking call here.

Comment on lines +199 to +202
href={getChangePlanUrl(data.organization.$id)}
style="text-decoration: underline;">
upgrade your plan
</a>.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Inline style instead of utility class

The style="text-decoration: underline;" inline style is inconsistent with the rest of the codebase, which relies on Pink design system utilities. Consider using the design system's link component or an existing CSS class instead of a one-off inline style.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Tip:

Greploops — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

@HarshMN2345 HarshMN2345 merged commit 14613f6 into main Apr 8, 2026
4 checks passed
@HarshMN2345 HarshMN2345 deleted the fix-education-plan-project-limit-alert branch April 8, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants