feat: Placement dashboard sharing via unique share links#66
Merged
Conversation
Extends dashboard share links with encrypted re-copy support, richer opportunity snapshots, Share Progress modal, per-internship sharing from the Internships tab, and a compact enterprise-style Shared Links table.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds end-to-end opportunity sharing: database support for share links, backend routes and crypto helpers, frontend creation/management/public pages, and documentation plus test updates. ChangesDashboard Share Links
Sequence Diagram(s)sequenceDiagram
participant Owner as Dashboard Owner
participant ShareModal as ShareProgressModal
participant API as shareLinkService
participant Backend as Express /api/share-links
participant DB as Supabase share_links
Owner->>ShareModal: submit fields, expiry, passcode
ShareModal->>API: create({ opportunityIds, fields, expiry, passcode })
API->>Backend: POST /api/share-links
Backend->>DB: SELECT opportunities
DB-->>Backend: opportunity rows
Backend->>Backend: buildShareSnapshot()
Backend->>Backend: generateShareToken() + encryptShareToken()
Backend->>DB: INSERT share_links row
DB-->>Backend: inserted row
Backend-->>API: token and share URL
API-->>ShareModal: created share
sequenceDiagram
participant Viewer as Public Viewer
participant PublicPage as PublicSharePage
participant API as shareLinkService
participant Backend as Express /api/public/share-links
participant DB as Supabase share_links
Viewer->>PublicPage: open /share/:token
PublicPage->>API: getPublic(token)
API->>Backend: GET /api/public/share-links/:token
Backend->>DB: lookup share_links by token_hash
DB-->>Backend: share row
alt passcode required
Backend-->>API: requiresPasscode
Viewer->>PublicPage: enter passcode
PublicPage->>API: verifyPasscode(token, passcode)
API->>Backend: POST /api/public/share-links/:token/verify
Backend->>DB: update view_count
Backend-->>API: public snapshot
else no passcode
Backend->>DB: update view_count
Backend-->>API: public snapshot
end
API-->>PublicPage: share data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.js (1)
47-56: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMask public share tokens before analytics tracking.
Line 55 currently sends full
/share/<token>totrackPageView(location.pathname), which leaks the share secret into analytics logs. Use the newisPublicSharePageflag to send a redacted path (or skip tracking) for share pages.Suggested fix
useEffect(() => { - trackPageView(location.pathname); - }, [location.pathname]); + const analyticsPath = isPublicSharePage ? '/share/:token' : location.pathname; + trackPageView(analyticsPath); + }, [location.pathname, isPublicSharePage]);🤖 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 `@src/App.js` around lines 47 - 56, The page-view tracking in App.js is sending the full public share pathname from trackPageView(location.pathname), which exposes the share token. Update the useEffect that tracks route changes to use the existing isPublicSharePage flag and either pass a redacted share path or skip tracking entirely for /share/* routes. Keep the change localized around isPublicSharePage and trackPageView so the analytics behavior is preserved for normal routes.
🧹 Nitpick comments (2)
backend/tests/integration/share-links.test.js (1)
119-174: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for specific opportunity selection.
The create test only covers the “all opportunities” path, so the new
.in('id', opportunityIds)branch is untested. Add a case withopportunityIdsthat asserts the query uses those IDs and the snapshot selection mode isspecific.🤖 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 `@backend/tests/integration/share-links.test.js` around lines 119 - 174, The share-link creation test only covers the default “all opportunities” path, so the new opportunity-specific branch in the share-link flow is unverified. Add a test around the share-link creation path that passes opportunityIds, then assert the opportunities query is filtered with those IDs and the resulting snapshot records selectionMode as specific. Use the existing share-link test setup and the share-link creation logic symbols to locate where the .in('id', opportunityIds) branch is exercised.supabase/migrations/20260624163000_create_share_links.sql (1)
79-83: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the extra
token_hashindexes or make the lookup query use them.
token_hashis already indexed by theUNIQUEconstraint on Line 18, and the public lookup shown inbackend/src/routes/public-share-links.js:31-46only filters bytoken_hash. As written,idx_share_links_token_hashduplicates the constraint-backed index, andidx_share_links_active_token_hashwill not be used unless the query also addsis_active = true.Suggested cleanup
-CREATE INDEX IF NOT EXISTS idx_share_links_token_hash ON share_links(token_hash); -CREATE INDEX IF NOT EXISTS idx_share_links_active_token_hash - ON share_links(token_hash) - WHERE is_active = TRUE; +-- `token_hash` is already indexed by the UNIQUE constraint above. +-- Keep an extra partial index only if the lookup query also filters by `is_active = TRUE`.🤖 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 `@supabase/migrations/20260624163000_create_share_links.sql` around lines 79 - 83, The share_links migration currently adds redundant token_hash indexes that duplicate the UNIQUE constraint-backed index. Update the migration to remove the extra idx_share_links_token_hash and either drop idx_share_links_active_token_hash or make the public-share-links lookup in backend/src/routes/public-share-links.js filter by is_active = true so that index can be used. Refer to the share_links index definitions in the migration and the public lookup handler to keep the schema and query aligned.
🤖 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 `@backend/src/app.js`:
- Around line 118-120: The current publicShareLimiter is applied to the entire
/api/public/share-links router, so POST /:token/verify shares the same generous
limit as public reads. Keep publicShareLimiter for GET /:token, but add a
separate stricter rate limiter for the verification flow in the public
share-links router, ideally keyed by token plus client IP, and apply it only to
the verify handler so passcode guessing is slowed down without affecting read
traffic.
In `@backend/src/lib/shareLinks.js`:
- Around line 133-177: normalizeFieldOptions() is incorrectly tying
rejectedRound to rounds, and toPublicOpportunity() only gates both round fields
behind fields.rounds. Update normalizeFieldOptions() so rounds and rejectedRound
are tracked independently, then adjust toPublicOpportunity() to use separate
checks so currentRoundNumber follows rounds while rejectedRoundNumber follows
rejectedRound, matching the share-links schema flags.
In `@backend/src/routes/public-share-links.js`:
- Around line 48-52: Update the view count logic in incrementViewCount so it
uses an atomic database operation on share_links instead of reading
share.view_count and writing back a computed value, and make the function return
the persisted row from Supabase. Adjust the caller(s) that currently rely on a
locally synthesized count so they consume the updated row returned by
incrementViewCount rather than assuming the increment succeeded. Use the
incrementViewCount helper and the share_links update path to locate the fix.
- Around line 76-92: The public share flow in findShareByToken,
incrementViewCount, and sendPublicShare still has a revoke/expiry TOCTOU window
because it authorizes on one read and serves on a later step. Update the
handlers so the final response is gated by a single conditional database
operation that only succeeds if the share is still active and unexpired, and
only send the returned row from that operation. Apply the same pattern to both
the normal public-share path and the passcode path so revocation/expiration is
checked at the moment of serving, not just during the initial lookup.
In `@backend/src/routes/share-links.js`:
- Around line 58-62: The owner share list query in the share-links route is
returning revoked rows because it only filters by user_id; update the Supabase
query in the owner-list handler to also require active shares by filtering on
is_active so only currently active links are returned. Keep the change scoped to
the existing share_links select flow that uses OWNER_SHARE_FIELDS and
req.auth.internalUserId.
- Around line 87-99: The specific-share path in the share link route currently
builds a snapshot even when some requested opportunity IDs do not resolve, which
allows partial or empty results. Update the handler around the existing
`query.in('id', opportunityIds)` and `buildShareSnapshot` flow to verify that
every ID in `opportunityIds` was returned for the authenticated user before
creating the snapshot. If any requested ID is missing, return a 400 error
instead of continuing; keep the normal behavior only when all requested IDs are
found.
In `@backend/src/validation/share-links-schemas.js`:
- Around line 23-28: The createShareLinkSchema currently allows an empty
opportunityIds array, which can be treated the same as omission by
buildShareSnapshot() and accidentally widen or misclassify the share scope.
Update the Joi validation in createShareLinkSchema so that when opportunityIds
is present it must contain at least one UUID, while still allowing the field to
be omitted entirely. Keep the behavior aligned with buildShareSnapshot() in
shareLinks.js by rejecting empty “specific selection” payloads at the API
boundary.
In `@README.md`:
- Around line 226-236: The SHARE_LINKS ERD is outdated and missing the new
encrypted token and passcode fields, so update the README schema reference to
match the current share_links table definition. Add token_ciphertext, token_iv,
token_auth_tag, and passcode_salt alongside the existing columns, and keep the
SHARE_LINKS block aligned with the create route and migration behavior so the
quick reference reflects the actual schema.
In `@src/components/opportunities/OpportunityCard.jsx`:
- Around line 71-81: The share button inside OpportunityCard is still affected
by the card’s Enter/Space keyboard handler, so keyboard activation can trigger
handleCardClick() or get suppressed on Space. Update the OpportunityCard wrapper
keyboard logic to ignore events originating from the share button (or otherwise
separate the share action from the card-level handler), and ensure the button
itself remains a normal accessible button via onClick/onKey handling without
bubbling into the card action.
In `@src/components/sharing/ManageSharesPanel.jsx`:
- Around line 81-85: The copy-flow in ManageSharesPanel is stacking multiple
timeout callbacks, which causes a previous timer to clear copied state for a
later link. Update the clipboard handler that calls
navigator.clipboard.writeText, setCopiedId, and setTimeout so it cancels any
existing pending timer before scheduling a new one, ideally by storing the
timeout id in a ref and clearing it first. This keeps the “Copied” state aligned
with the most recent share row.
In `@src/pages/PublicSharePage.jsx`:
- Around line 33-40: The date handling in PublicSharePage is timezone-shifting
date-only values because formatDate and the deadline-state logic are using new
Date(...) on YYYY-MM-DD strings, which can move the day for some viewers. Update
the date parsing/display path to treat date-only values as plain local dates
(not UTC timestamps) and make sure the deadline comparison logic uses the same
normalized date representation. Use the existing formatDate helper and the
deadline-state code near the other date parsing to keep both display and math
consistent.
---
Outside diff comments:
In `@src/App.js`:
- Around line 47-56: The page-view tracking in App.js is sending the full public
share pathname from trackPageView(location.pathname), which exposes the share
token. Update the useEffect that tracks route changes to use the existing
isPublicSharePage flag and either pass a redacted share path or skip tracking
entirely for /share/* routes. Keep the change localized around isPublicSharePage
and trackPageView so the analytics behavior is preserved for normal routes.
---
Nitpick comments:
In `@backend/tests/integration/share-links.test.js`:
- Around line 119-174: The share-link creation test only covers the default “all
opportunities” path, so the new opportunity-specific branch in the share-link
flow is unverified. Add a test around the share-link creation path that passes
opportunityIds, then assert the opportunities query is filtered with those IDs
and the resulting snapshot records selectionMode as specific. Use the existing
share-link test setup and the share-link creation logic symbols to locate where
the .in('id', opportunityIds) branch is exercised.
In `@supabase/migrations/20260624163000_create_share_links.sql`:
- Around line 79-83: The share_links migration currently adds redundant
token_hash indexes that duplicate the UNIQUE constraint-backed index. Update the
migration to remove the extra idx_share_links_token_hash and either drop
idx_share_links_active_token_hash or make the public-share-links lookup in
backend/src/routes/public-share-links.js filter by is_active = true so that
index can be used. Refer to the share_links index definitions in the migration
and the public lookup handler to keep the schema and query aligned.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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 Plus
Run ID: 99fa3f26-e029-4b93-a287-a9dd228b9075
📒 Files selected for processing (25)
README.mdbackend/.env.examplebackend/src/app.jsbackend/src/lib/shareLinks.jsbackend/src/routes/public-share-links.jsbackend/src/routes/share-links.jsbackend/src/validation/share-links-schemas.jsbackend/tests/integration/share-links.test.jsbackend/tests/mocks/supabase.jsbackend/tests/setup.jsdocs/CODEBASE_GUIDE.mddocs/TESTING.mddocs/share-links-migration.sqldocs/share-links.mdsrc/App.jssrc/components/opportunities/OpportunityCard.jsxsrc/components/opportunities/OpportunityList.jsxsrc/components/sharing/ManageSharesPanel.jsxsrc/components/sharing/ShareProgressModal.jsxsrc/pages/Dashboard.jsxsrc/pages/InternshipList.jsxsrc/pages/PublicSharePage.jsxsrc/services/api.jssupabase/migrations/20260624163000_create_share_links.sqlsupabase/migrations/20260624171000_add_recoverable_share_tokens.sql
Add atomic serve RPC, stricter passcode rate limits, independent round field toggles, active-only owner listing, opportunity ID validation, and small frontend fixes for analytics redaction, date parsing, keyboard share access, and copy state.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/routes/public-share-links.js`:
- Around line 34-40: The rate limiter key in publicShareVerifyLimiter should be
normalized so IPv6 subnet masking is preserved and bearer tokens are not stored
in limiter state. Update the keyGenerator in
backend/src/routes/public-share-links.js to use ipKeyGenerator(req.ip) for the
IP part, and hash the req.params.token before combining it into the key. Keep
the change localized to publicShareVerifyLimiter and its keyGenerator logic.
🪄 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 Plus
Run ID: 8e53b973-1a89-49d3-89dc-550be2229c86
📒 Files selected for processing (14)
README.mdbackend/src/app.jsbackend/src/lib/shareLinks.jsbackend/src/routes/public-share-links.jsbackend/src/routes/share-links.jsbackend/src/validation/share-links-schemas.jsbackend/tests/integration/share-links.test.jsdocs/share-links-migration.sqlsrc/App.jssrc/components/opportunities/OpportunityCard.jsxsrc/components/sharing/ManageSharesPanel.jsxsrc/pages/PublicSharePage.jsxsupabase/migrations/20260624163000_create_share_links.sqlsupabase/migrations/20260624180000_share_links_serve_rpc.sql
💤 Files with no reviewable changes (2)
- supabase/migrations/20260624163000_create_share_links.sql
- docs/share-links-migration.sql
✅ Files skipped from review due to trivial changes (2)
- supabase/migrations/20260624180000_share_links_serve_rpc.sql
- README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- src/App.js
- backend/src/validation/share-links-schemas.js
- src/components/sharing/ManageSharesPanel.jsx
- backend/src/routes/share-links.js
- backend/src/lib/shareLinks.js
- backend/tests/integration/share-links.test.js
- src/components/opportunities/OpportunityCard.jsx
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements opt-in placement dashboard sharing with secure, revocable read-only links at
/share/:token, covering the full #17 epic and sub-issues.share_linkstable, token hashing, frozen snapshots, authenticated CRUD + public read routes, rate limiting, and integration tests/share/:tokenview with opportunity details, deadlines, and apply CTAs (notes/docs/prep never exposed)Also included (follow-up polish):
Closes #17
Closes #18
Closes #19
Closes #20
Closes #21
Closes #22
Test plan
20260624163000_create_share_links.sqland20260624171000_add_recoverable_share_tokens.sqlSHARE_LINK_ENCRYPTION_KEYin backend.env(see.env.example)cd backend && npm test(share-links integration tests)npm testandnpm run build(frontend)Summary by CodeRabbit
/share/:tokenpage that renders shared opportunities, shows snapshot insights, and supports passcode unlock when required.