Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Pull request overview
認証（AtCoderアカウント検証）を投票機能から独立させつつ、未認証ユーザーの投票をUI/サーバー両面で制限する変更です。投票導線からプロフィール編集（AtCoder認証タブ）へ誘導できるようにしています。
Changes:
- 投票（詳細ページ/一覧のドロップダウン投票）を「ログイン + AtCoder認証済み」に制限
- 未認証ユーザー向けに
/users/edit?tab=atcoderへの誘導UIを追加 - ユーザー編集画面でAtCoder認証タブを復帰し、
?tab=atcoderで初期表示タブを切替
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/votes/[slug]/+page.svelte | 未認証ログインユーザーへ認証誘導UIを追加 |
| src/routes/votes/[slug]/+page.server.ts | loadに isAtCoderVerified を追加 |
| src/routes/users/edit/+page.svelte | AtCoder認証タブ復帰、?tab=atcoder と form 結果でタブ自動オープン |
| src/routes/users/edit/+page.server.ts | tab=atcoder をloadへ反映 |
| src/routes/problems/+page.svelte | TaskTableへ isAtCoderVerified を伝播 |
| src/routes/problems/+page.server.ts | loadに isAtCoderVerified を追加 |
| src/lib/constants/navbar-links.ts | EDIT_PROFILE_PAGE を追加 |
| src/lib/components/AtCoderUserValidationForm.svelte | バインド/状態管理を見直し、入力をローカルstate化 |
| src/features/votes/components/VotableGrade.svelte | 未認証時の投票UIを誘導に切替、isAtCoderVerified を追加 |
| src/features/votes/actions/vote_actions.ts | サーバー側で未認証投票を403で拒否 |
| src/features/tasks/components/contest-table/TaskTableBodyCell.svelte | isAtCoderVerified を VotableGradeへ伝播 |
| src/features/tasks/components/contest-table/TaskTable.svelte | isAtCoderVerified を子へ伝播 |
| {:else if data.isLoggedIn && !data.isAtCoderVerified} | ||
| <!-- ログイン済み・未認証 → 認証誘導 --> | ||
| <div | ||
| class="bg-yellow-50 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-700 rounded-lg p-4 mb-4" | ||
| > | ||
| <p class="text-yellow-800 dark:text-yellow-200 font-medium mb-2"> | ||
| 投票するにはAtCoderアカウントの認証が必要です。 | ||
| </p> | ||
| <Button href={editProfileHref} color="yellow" size="sm">AtCoderアカウントを認証する</Button> | ||
| </div> | ||
| {:else if data.isLoggedIn} | ||
| <!-- 未投票・ログイン済み → 投票フォーム --> | ||
| <!-- 未投票・ログイン済み・認証済み → 投票フォーム --> |
There was a problem hiding this comment.
The new “logged in but unverified” branch only applies when data.myVote?.voted is false. If a user voted previously and later resets AtCoder verification, they will still hit the data.myVote?.voted branch and see the “投票を変更する” form, but the server action now rejects unverified users (403). Consider also gating the vote-change UI in the voted branch on data.isAtCoderVerified (or showing the same verification prompt there) to avoid a broken flow.
| // Editable only in 'nothing' step; server is authoritative after each action. | ||
| // untrack: prop is the initial seed only — intentional one-time capture. | ||
| let editableAtcoderId = $state(untrack(() => atcoder_username)); | ||
|
|
There was a problem hiding this comment.
editableAtcoderId is seeded once from atcoder_username and never re-synced. After a successful reset (server clears atcoder_username and status becomes nothing), the input will still show the old AtCoder ID, diverging from server-authoritative state. Consider syncing editableAtcoderId when status becomes nothing (or when atcoder_username changes to an empty string) while still avoiding overwriting during user edits.
| // Keep editableAtcoderId in sync with server after a reset: | |
| // when the server clears atcoder_username and status returns to 'nothing', | |
| // reset the local editable value as well. | |
| $effect(() => { | |
| if (status === 'nothing' && atcoder_username === '') { | |
| editableAtcoderId = ''; | |
| } | |
| }); |
| if (!locals.user?.is_validated) { | ||
| return fail(FORBIDDEN, { | ||
| message: 'AtCoderアカウントの認証が必要です。', | ||
| }); | ||
| } |
There was a problem hiding this comment.
The verification check conflates “user is not AtCoder-verified” with “locals.user is missing”. Since locals.user is populated in hooks.server.ts by looking up the DB user, a deleted/missing user could yield locals.user === undefined even when session is valid; returning 403 with an AtCoder-verification message would be misleading. Consider explicitly handling !locals.user (e.g., treat it as UNAUTHORIZED/INTERNAL_SERVER_ERROR) and only return FORBIDDEN when the user exists but is_validated is false.
認証機能を分離したPRです。
サーバーの用意後、マージいたします。