🔥 Remove legacy adminUrlId admin token#2313
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRefactors poll identification and access control: API and client now use Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client (Browser)
participant Page as Web Page (React)
participant API as TRPC Router
participant DB as Database
Browser->>Page: User submits update (form data)
Page->>API: updatePollMutation({ pollId, ...data })
API->>DB: Update poll where id = pollId
DB-->>API: Update result
API-->>Page: { ok: true, ... }
Page-->>Browser: Redirect / show success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
- Remove adminToken fallback from polls.get (unauthenticated admin access via URL token) - Remove adminUrlId from polls.get response; add canManage boolean instead - Change polls.modify input from urlId (adminUrlId) to pollId - Remove getPollIdFromAdminUrlId helper - Update edit pages to pass poll.id instead of poll.adminUrlId - Update layout guard to use canManage instead of adminUrlId presence Closes RAL-1134 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lukevella
force-pushed
the
luke/ral-1134-remove-legacy-adminurlid-admin-token
branch
from
404ec24 to
46fa87f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(optional-space)/poll/[urlId]/layout.tsx (1)
19-31: Defer the manager-only prefetches until after Line 30.Now that
canManageis the real gate, non-managers still pay forparticipants.listandcomments.liston every redirected/poll/[urlId]hit. Fetch the poll first, redirect when!poll.canManage, then prefetch the manager-only queries.♻️ Suggested restructuring
- const [poll] = await Promise.all([ - trpc.polls.get.fetch({ urlId: params.urlId }).catch((e) => { - if (e instanceof TRPCError && e.code === "NOT_FOUND") { - notFound(); - } - throw e; - }), - trpc.polls.participants.list.prefetch({ pollId: params.urlId }), - trpc.polls.comments.list.prefetch({ pollId: params.urlId }), - ]); + const poll = await trpc.polls.get.fetch({ urlId: params.urlId }).catch((e) => { + if (e instanceof TRPCError && e.code === "NOT_FOUND") { + notFound(); + } + throw e; + }); if (!poll.canManage) { redirect(`/invite/${params.urlId}`); } + + await Promise.all([ + trpc.polls.participants.list.prefetch({ pollId: params.urlId }), + trpc.polls.comments.list.prefetch({ pollId: params.urlId }), + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(optional-space)/poll/[urlId]/layout.tsx around lines 19 - 31, The current Promise.all runs manager-only prefetches (trpc.polls.participants.list.prefetch and trpc.polls.comments.list.prefetch) before checking poll.canManage, causing non-managers to pay for them; change the flow to first await trpc.polls.get.fetch({ urlId: params.urlId }) (keeping the existing catch that calls notFound()), then if (!poll.canManage) call redirect(`/invite/${params.urlId}`), and only after that run the two prefetches (trpc.polls.participants.list.prefetch and trpc.polls.comments.list.prefetch) when poll.canManage is true so manager-only queries are deferred until access is confirmed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/app/`[locale]/(optional-space)/poll/[urlId]/layout.tsx:
- Around line 19-31: The current Promise.all runs manager-only prefetches
(trpc.polls.participants.list.prefetch and trpc.polls.comments.list.prefetch)
before checking poll.canManage, causing non-managers to pay for them; change the
flow to first await trpc.polls.get.fetch({ urlId: params.urlId }) (keeping the
existing catch that calls notFound()), then if (!poll.canManage) call
redirect(`/invite/${params.urlId}`), and only after that run the two prefetches
(trpc.polls.participants.list.prefetch and trpc.polls.comments.list.prefetch)
when poll.canManage is true so manager-only queries are deferred until access is
confirmed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bb991bd9-f64d-4815-a879-58dd2289da6a
📒 Files selected for processing (6)
apps/web/src/app/[locale]/(optional-space)/poll/[urlId]/edit-details/page.tsxapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/edit-options/page.tsxapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/edit-settings/page.tsxapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/layout.tsxapps/web/src/trpc/client/types.tsapps/web/src/trpc/routers/polls.ts
lukevella
force-pushed
the
luke/ral-1134-remove-legacy-adminurlid-admin-token
branch
from
1f89bec to
5325495
Compare
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.
Closes RAL-1134
Summary by CodeRabbit
Refactor