Quick apply via '/facilitated-courses'#2621
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 24 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a quick-apply banner feature for the facilitated-courses page. The implementation consists of three parts: a persisted Zustand store that tracks which banner dismissals the user has made, a Possibly related PRs
🚥 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 |
Greptile SummaryThis PR adds a dismissible
Confidence Score: 4/5Safe to merge; the change is additive and the banner gracefully renders nothing on loading/error states. The implementation is clean and well-guarded. The only notable concerns are the unconventional date-as-version in the persist store and the absence of unit tests for the new component and its dismissal logic. No files require special attention beyond the minor version convention in Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (facilitator)
participant FC as /facilitated-courses page
participant QB as QuickApplyBanner
participant TRPC as tRPC: eligibleRounds
participant ZS as Zustand persist store
U->>FC: visits /facilitated-courses
FC->>QB: renders QuickApplyBanner
QB->>TRPC: useQuery(facilitatorApplications.eligibleRounds)
TRPC-->>QB: EligibleRoundsCourse[]
QB->>QB: "derive dismissKey (sorted round IDs joined by '|')"
QB->>ZS: read dismissedKeys[dismissKey]
alt No eligible rounds OR already dismissed
QB-->>FC: render null (hidden)
else Has eligible rounds AND not dismissed
QB-->>U: render banner with Quick Apply + Hide buttons
alt User clicks Hide
U->>QB: onClick dismiss
QB->>ZS: "set dismissedKeys[dismissKey] = true (persisted to localStorage)"
QB-->>FC: render null (hidden)
else User clicks Quick Apply
U->>U: navigate to /facilitator-applications
end
end
Reviews (1): Last reviewed commit: "`QuickApplyBanner` stories" | Re-trigger Greptile |
| import { useQuickApplyBannerStore } from '../../stores/quickApplyBanner'; | ||
| import { trpc } from '../../utils/trpc'; | ||
|
|
||
| export const QuickApplyBanner = () => { |
There was a problem hiding this comment.
No tests for new component/store
The QuickApplyBanner component and the quickApplyBanner Zustand store introduce new logic (dismiss-key derivation, persistence keying by round IDs) but ship without unit tests. A test covering the case where eligibleRoundIds is empty (banner hidden), non-empty (banner visible), and a previously-dismissed key (banner hidden again) would guard against regressions in the dismissal logic.
Rule Used: Consider adding tests for any new functionality in... (source)
Learned From
bluedotimpact/bluedot#956
bluedotimpact/bluedot#969
bluedotimpact/bluedot#958
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/website/src/components/my-courses/QuickApplyBanner.tsx (1)
26-31: ⚡ Quick winUse
@bluedot/uitypography primitives for banner text.At Lines 26-31, use
H4/Pfrom@bluedot/uirather than raw<p>elements to stay consistent with the website typography contract.As per coding guidelines, “Use text components from `@bluedot/ui`: `H1`, `H2`, `H3`, `H4`, `P`, and `A`.”Suggested refactor
-import { CTALinkOrButton } from '`@bluedot/ui`'; +import { CTALinkOrButton, H4, P } from '`@bluedot/ui`'; ... - <p className="text-size-sm font-bold">Quick Apply (~2 min)</p> + <H4 className="text-size-sm font-bold">Quick Apply (~2 min)</H4> ... - <p className="text-size-xs mt-1 text-pretty"> + <P className="text-size-xs mt-1 text-pretty"> Thanks for facilitating with BlueDot. If you want to facilitate the same course again, as a return facilitator, quick applying only takes 2 min! - </p> + </P>🤖 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 `@apps/website/src/components/my-courses/QuickApplyBanner.tsx` around lines 26 - 31, Replace the raw <p> tags in the QuickApplyBanner component with the typography primitives from `@bluedot/ui`: use H4 for the title ("Quick Apply (~2 min)") and P for the descriptive paragraph, preserving existing className/spacing styles as props or className on those components; add the corresponding import { H4, P } from '`@bluedot/ui`' at the top of the file and remove the old <p> elements so the component uses H4 and P instead of plain HTML paragraphs.Source: Coding guidelines
🤖 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 `@apps/website/src/components/my-courses/QuickApplyBanner.stories.tsx`:
- Around line 26-31: The QuickApplyBanner stories are flaky because dismissals
persist across stories; add a Storybook decorator in this file that resets the
persisted state (e.g., clear the specific localStorage key or call the store
reset used by QuickApplyBanner) before each story renders so Default and other
stories are deterministic; update the meta object (where loggedInStory() and
component QuickApplyBanner are used) to include this decorator to clear the
banner dismissal state prior to rendering.
In `@apps/website/src/components/my-courses/QuickApplyBanner.tsx`:
- Line 37: The CTALinkOrButton in QuickApplyBanner is pointing to the wrong
route constant (ROUTES.facilitatorApplications.url); update the CTA to use the
quick-apply route constant used elsewhere (e.g., ROUTES.quickApply.url or the
app's dedicated quick-apply route) so the button navigates to the quick-apply
flow; change the url prop on the CTALinkOrButton in QuickApplyBanner to the
correct ROUTES.<quickApplyName>.url constant.
---
Nitpick comments:
In `@apps/website/src/components/my-courses/QuickApplyBanner.tsx`:
- Around line 26-31: Replace the raw <p> tags in the QuickApplyBanner component
with the typography primitives from `@bluedot/ui`: use H4 for the title ("Quick
Apply (~2 min)") and P for the descriptive paragraph, preserving existing
className/spacing styles as props or className on those components; add the
corresponding import { H4, P } from '`@bluedot/ui`' at the top of the file and
remove the old <p> elements so the component uses H4 and P instead of plain HTML
paragraphs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5216e26-cf9f-4290-8d53-4f38af750501
📒 Files selected for processing (4)
apps/website/src/components/my-courses/QuickApplyBanner.stories.tsxapps/website/src/components/my-courses/QuickApplyBanner.tsxapps/website/src/pages/facilitated-courses.tsxapps/website/src/stores/quickApplyBanner.ts
Description
Adds a new panel to the 'facilitated courses' route that redirects users to 'facilitated applications' where they can quickly reapply to facilitate again.
Issue
Fixes #2608
Developer checklist
Screenshot