refactor(phase4): migrate account components to src/features/account#3324
Merged
refactor(phase4): migrate account components to src/features/account#3324
Conversation
Move AtCoderUserValidationForm, UserAccountDeletionForm, and
WarningMessageOnDeletingAccount from src/lib/components/ to
src/features/account/components/{settings,delete}/, and update
import paths in +page.svelte accordingly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAtCoder検証データを Changes
Sequence DiagramsequenceDiagram
actor User
participant Page as "Edit Page"
participant Service as "atcoder_verification\nService"
participant DB as "Database"
participant ExternalAPI as "External\nConfirmation API"
User->>Page: Generate validation code
Page->>Service: generate(username, handle)
Service->>Service: create SHA256(validation seed)
Service->>DB: upsert AtCoderAccount (handle, code, isValidated=false)
DB-->>Service: upsert result
Service-->>Page: return validation code
User->>Page: Submit validation
Page->>Service: validate(username)
Service->>DB: load user with atCoderAccount
DB-->>Service: user + atCoderAccount
Service->>ExternalAPI: confirmWithExternalApi(handle)
ExternalAPI-->>Service: contents list
alt confirmation matches
Service->>DB: update AtCoderAccount (isValidated=true, validationCode='')
DB-->>Service: updated
Service-->>Page: return true
else not confirmed
Service-->>Page: return false
end
User->>Page: Reset validation
Page->>Service: reset(username)
Service->>DB: delete AtCoderAccount rows
DB-->>Service: deleted
Service-->>Page: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors account-related code by moving UI/services into src/features/account and normalizing AtCoder verification data into a dedicated Prisma model/table, in the context of the refactoring work mentioned for #3263.
Changes:
- Split AtCoder-related fields out of
Userinto a newAtCoderAccountmodel + migration, and update server-side reads to use the relation. - Replace/relocate account deletion UI to
src/features/account/components/deleteand update the users edit page to use the new component. - Introduce a new AtCoder verification service under
src/features/account/services(and remove the legacyvalidateApiService).
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/lib/utils/test_cases/account_transfer.ts | Updates Prisma User test fixtures to match removed AtCoder columns. |
| src/routes/users/edit/+page.svelte | Switches account deletion form import to the new feature component. |
| src/routes/users/edit/+page.server.ts | Loads AtCoder data via atCoderAccount and switches to the new verification service. |
| src/routes/users/[username]/+page.server.ts | Reads AtCoder handle via user.atCoderAccount. |
| src/lib/services/validateApiService.ts | Removes legacy AtCoder validation service. |
| src/lib/services/users.ts | Always includes atCoderAccount when fetching a user. |
| src/hooks.server.ts | Populates event.locals.user using user.atCoderAccount. |
| src/features/account/services/atcoder_verification.ts | Adds new AtCoder verification service backed by AtCoderAccount. |
| src/features/account/components/settings/AtCoderVerificationForm.svelte | Adds new AtCoder verification UI (currently not wired in). |
| src/features/account/components/delete/WarningMessageOnDeletingAccount.svelte | Adds deletion warning message component. |
| src/features/account/components/delete/AccountDeletionForm.svelte | Adjusts import to use local warning component. |
| prisma/schema.prisma | Introduces AtCoderAccount model and removes AtCoder columns from User. |
| prisma/migrations/20260328002556_split_atcoder_account/migration.sql | Creates atcoder_account table, migrates data, drops old columns. |
Comments suppressed due to low confidence (1)
src/routes/users/edit/+page.server.ts:97
actions(especiallydelete) does not validate the current session nor authorize that the postedusernamematches the logged-in user. As written, a direct POST to this endpoint could delete arbitrary accounts. Please validatelocals.auth.validate()inside each action and enforce authorization (e.g., only allowsession.user.username === usernameor admin), and avoid trustingusernamefromformDatafor destructive operations.
delete: async ({ request, locals }) => {
const formData = await request.formData();
const username = formData.get('username')?.toString() as string;
const atcoder_username = formData.get('atcoder_username')?.toString() as string;
await userService.deleteUser(username);
locals.auth.setSession(null); // remove cookie
Comment on lines
93
to
104
| @@ -104,8 +98,8 @@ export const actions: Actions = { | |||
|
|
|||
| return { | |||
| success: true, | |||
| username: username, | |||
| atcoder_username: atcoder_username, | |||
| username, | |||
| atcoder_username, | |||
| atcoder_validationcode: false, | |||
| message_type: 'green', | |||
There was a problem hiding this comment.
In the delete action, atcoder_username is read from formData, but the current AccountDeletionForm only submits username, so this will be undefined at runtime. Also atcoder_validationcode is returned as false (boolean) even though elsewhere it is treated as a string. Consider removing these from the return payload or defaulting them to empty strings.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/routes/users/edit/+page.svelte (1)
13-25: 🧹 Nitpick | 🔵 Trivial
openAtCoderTabが未使用
+page.server.tsが返すopenAtCoderTabが Props に含まれておらず、タブ切り替えに活用されていない。AtCoder検証フォームが再有効化される際に対応が必要。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/users/edit/`+page.svelte around lines 13 - 25, Props interface and the component ignore the openAtCoderTab flag returned from +page.server.ts, so the AtCoder validation tab can't be programmatically activated; update the Props type to include openAtCoderTab: boolean (or the correct type returned by the server) and then pass that prop into the component state/initializer used to control the tab (look for the tab state or the function that toggles tabs, e.g., openAtCoderTab and any tab selection logic) so the component reads the server-provided value on mount and activates the AtCoder validation tab when true.src/routes/users/edit/+page.server.ts (1)
99-106:⚠️ Potential issue | 🟡 Minor
atcoder_validationcode: falseは型が不正
atcoder_validationcodeは文字列型であるべきだが、false(boolean) が設定されている。🐛 修正案
return { success: true, username, atcoder_username, - atcoder_validationcode: false, + atcoder_validationcode: '', message_type: 'green', message: 'Successfully deleted.', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/users/edit/`+page.server.ts around lines 99 - 106, The returned object sets atcoder_validationcode to the boolean false but the type expects a string; update the return in the function that builds the response (the object with keys success, username, atcoder_username, atcoder_validationcode, message_type, message) to provide a string value instead (e.g., an empty string or appropriate validation code string) so atcoder_validationcode is always a string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prisma/schema.prisma`:
- Line 56: Update ERD.md to reflect the new AtCoderAccount table and relation:
remove any mention of the old user.atcoder_* columns and add a new
AtCoderAccount entity with its fields (as defined in the Prisma model) and the
relationship to User via User.atCoderAccount (or AtCoderAccount.user). Ensure
the ER diagram and any textual schema summary show the correct cardinality and
foreign-key link between User and AtCoderAccount and update any example queries
or references that assumed the old columns.
In `@src/features/account/services/atcoder_verification.ts`:
- Line 4: The hardcoded external API URL stored in the constant CONFIRM_API_URL
in atcoder_verification.ts should be moved to an environment variable; change
usages of CONFIRM_API_URL to read from process.env.CONFIRM_API_URL (with an
optional safe default or throw if missing) and add a corresponding declaration
for CONFIRM_API_URL in src/app.d.ts so TypeScript recognizes
process.env.CONFIRM_API_URL; update any tests or startup docs to set
CONFIRM_API_URL in .env for dev/staging/prod.
- Around line 52-58: confirmWithExternalApi is being called with
user.atCoderAccount.validationCode even though validate() sets validationCode to
an empty string, causing an unnecessary external check; before calling
confirmWithExternalApi (in the block using user.atCoderAccount.handle and
validationCode) add a guard that ensures validationCode is non-empty (e.g., if
user.atCoderAccount.validationCode is falsy) and handle that case by skipping
the external call and setting confirmed to false or returning an appropriate
result/error from the AtCoder verification flow; reference
confirmWithExternalApi, validate(), validationCode and
user.atCoderAccount.handle to locate the code to change.
- Around line 7-17: confirmWithExternalApi currently uses fetch without a
timeout (risking indefinite hang) and calls response.json() without handling
parse errors; update confirmWithExternalApi to use an AbortController with a
configurable timeout when calling fetch(CONFIRM_API_URL...), cancel the request
on timeout, and wrap the response.json() call in try/catch to handle invalid
JSON and return false (or propagate a controlled error) instead of letting an
exception bubble; ensure you reference CONFIRM_API_URL, the fetch call, the
response.json() usage, and the function name confirmWithExternalApi when making
the changes.
In `@src/routes/users/edit/`+page.server.ts:
- Around line 54-71: The validate handler in +page.server.ts is extracting
unused fields (atcoder_username, atcoder_validationcode) from the
request.formData even though verificationService.validate(username) uses only
username; remove the unnecessary formData.get(...) calls for atcoder_username
and atcoder_validationcode and stop returning them in the returned user object
in the validate function so the handler only extracts and returns the data
actually used (username, success, message_type, message) — locate the validate
function and update the form extraction and returned user shape accordingly.
---
Outside diff comments:
In `@src/routes/users/edit/`+page.server.ts:
- Around line 99-106: The returned object sets atcoder_validationcode to the
boolean false but the type expects a string; update the return in the function
that builds the response (the object with keys success, username,
atcoder_username, atcoder_validationcode, message_type, message) to provide a
string value instead (e.g., an empty string or appropriate validation code
string) so atcoder_validationcode is always a string.
In `@src/routes/users/edit/`+page.svelte:
- Around line 13-25: Props interface and the component ignore the openAtCoderTab
flag returned from +page.server.ts, so the AtCoder validation tab can't be
programmatically activated; update the Props type to include openAtCoderTab:
boolean (or the correct type returned by the server) and then pass that prop
into the component state/initializer used to control the tab (look for the tab
state or the function that toggles tabs, e.g., openAtCoderTab and any tab
selection logic) so the component reads the server-provided value on mount and
activates the AtCoder validation tab when true.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5b160457-6f5e-4eee-8351-9e063f3ba982
📒 Files selected for processing (13)
prisma/migrations/20260328002556_split_atcoder_account/migration.sqlprisma/schema.prismasrc/features/account/components/delete/AccountDeletionForm.sveltesrc/features/account/components/delete/WarningMessageOnDeletingAccount.sveltesrc/features/account/components/settings/AtCoderVerificationForm.sveltesrc/features/account/services/atcoder_verification.tssrc/hooks.server.tssrc/lib/services/users.tssrc/lib/services/validateApiService.tssrc/routes/users/[username]/+page.server.tssrc/routes/users/edit/+page.server.tssrc/routes/users/edit/+page.sveltesrc/test/lib/utils/test_cases/account_transfer.ts
💤 Files with no reviewable changes (2)
- src/test/lib/utils/test_cases/account_transfer.ts
- src/lib/services/validateApiService.ts
KATO-Hiro
requested changes
Mar 28, 2026
Collaborator
There was a problem hiding this comment.
早速、リファクタリングしていただきありがとうございます。
影響範囲が広く大変な修正だったかと思いますが、今後の機能拡張・修正がしやすいものになっていると思います。
お手数ですが、以下の2点についてご確認・ご対応いただけますでしょうか?
・コードレビューツールの指摘事項を読んで、必要だと判断されたものに対応（対応しない場合は、その理由を該当の指摘事項に記載）。また、ローカル環境で Coderabbit で数回レビューを受けて事前に対応していただくと、よりスムーズに進むかと思います。
・テストの追加・補強（utils層 や service 層に単体テスト、主要なページに対してe2eテスト。今後の機能追加・修正などをしやすくするため）
これらは今後もレビューを依頼される前にも、ご確認いただけると幸いです。
お手数をお掛けしますが、手戻りをできるだけ減らすためにも、ご協力のほどよろしくお願いいたします。
- Move CONFIRM_API_URL to process.env (set via compose.yaml); throw if unset - Add AbortController timeout (5s) to confirmWithExternalApi - Wrap response.json() in try/catch; return false on invalid JSON - Guard validate() against empty validationCode before external API call - Remove unused atcoder_username/atcoder_validationcode from validate action - Fix atcoder_validationcode type: false -> '' in delete action - Add session validation and username authorization check to delete action - Add openAtCoderTab to +page.svelte Props interface - Update ERD.md to reflect AtCoderAccount table split Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/users/edit/+page.svelte (1)
13-25: 🧹 Nitpick | 🔵 Trivial
openAtCoderTabは未使用です。
+page.server.tsから値を受け取っていますが、このページは先頭タブをopen固定のままです。- 当面使わないなら
Propsから外し、使うならタブ再公開時にTabItemの open 制御までつなげた方が読みやすいです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/users/edit/`+page.svelte around lines 13 - 25, Props interface includes an unused openAtCoderTab property; either remove openAtCoderTab from the Props type (and stop sending it from +page.server.ts) if you won't use it, or wire it through to the UI by passing the prop into the tab component and controlling the TabItem open state (connect openAtCoderTab to TabItem's open prop/logic in +page.svelte) so the server-provided flag actually opens the AtCoder tab.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prisma/ERD.md`:
- Around line 119-127: The atcoder_account model uses camelCase fields (userId,
isValidated, validationCode, createdAt, updatedAt); rename them to snake_case
(user_id, is_validated, validation_code, created_at, updated_at) to match
existing auth entities (user, session, key), update any references/usages
(queries, relations, serializers) that use atcoder_account and its fields, and
generate the corresponding Prisma migration so the DB schema and application
code remain consistent.
In `@src/routes/users/edit/`+page.server.ts:
- Around line 39-45: The generate/validate/reset handlers (generate, validate,
reset) currently trust the form-provided username and call verificationService.*
directly; add the same authentication and ownership check used in delete: call
await locals.auth.validate() to get the session, then verify
session.user.username === username (the form value) before invoking
verificationService.generate/validate/reset; if validation fails, return the
appropriate unauthorized response (same behavior as delete). Repeat this change
for the blocks handling validate and reset as well.
- Around line 59-65: The current handler always returns a green success payload
even when verificationService.validate(username) returns false; check the result
stored in is_validated and branch: if true return the existing success object,
otherwise return an appropriate failure response (use fail(...) from SvelteKit
or return an object with message_type: 'red' and a failure message) so the
response and message_type/message are consistent; update the logic around
verificationService.validate and the returned object accordingly.
---
Outside diff comments:
In `@src/routes/users/edit/`+page.svelte:
- Around line 13-25: Props interface includes an unused openAtCoderTab property;
either remove openAtCoderTab from the Props type (and stop sending it from
+page.server.ts) if you won't use it, or wire it through to the UI by passing
the prop into the tab component and controlling the TabItem open state (connect
openAtCoderTab to TabItem's open prop/logic in +page.svelte) so the
server-provided flag actually opens the AtCoder tab.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 583ea6f5-4d6d-481d-a872-477899dfbd32
📒 Files selected for processing (5)
compose.yamlprisma/ERD.mdsrc/features/account/services/atcoder_verification.tssrc/routes/users/edit/+page.server.tssrc/routes/users/edit/+page.svelte
Comment on lines
39
to
45
| generate: async ({ request }) => { | ||
| console.log('users->actions->generate'); | ||
| const formData = await request.formData(); | ||
| const username = formData.get('username')?.toString() as string; | ||
| const atcoder_username = formData.get('atcoder_username')?.toString() as string; | ||
|
|
||
| //console.log('ここにvalidationCodeを作成してデータベースに登録するコードを書きます'); | ||
| const validationCode = await validationService.generate(username, atcoder_username); | ||
| const validationCode = await verificationService.generate(username, atcoder_username); | ||
|
|
Contributor
There was a problem hiding this comment.
generate / validate / reset にも本人確認が必要です。
delete にはある session / username の再検証が、ここにはありません。username を form からそのまま信頼しているので、直接 POST で他人の AtCoderAccount を生成・検証・初期化できます。3アクションとも locals.auth.validate() と session.user.username === username を通してから service を呼ぶべきです。
🔒 修正イメージ
- generate: async ({ request }) => {
+ generate: async ({ request, locals }) => {
+ const session = await locals.auth.validate();
+ if (!session) {
+ return fail(FORBIDDEN, { message: 'Not authenticated.' });
+ }
const formData = await request.formData();
const username = formData.get('username')?.toString() as string;
+ if (session.user.username !== username) {
+ return fail(FORBIDDEN, { message: 'Not authorized.' });
+ }
const atcoder_username = formData.get('atcoder_username')?.toString() as string;
const validationCode = await verificationService.generate(username, atcoder_username);validate と reset にも同じ前段チェックを入れてください。
Also applies to: 55-60, 68-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/users/edit/`+page.server.ts around lines 39 - 45, The
generate/validate/reset handlers (generate, validate, reset) currently trust the
form-provided username and call verificationService.* directly; add the same
authentication and ownership check used in delete: call await
locals.auth.validate() to get the session, then verify session.user.username ===
username (the form value) before invoking
verificationService.generate/validate/reset; if validation fails, return the
appropriate unauthorized response (same behavior as delete). Repeat this change
for the blocks handling validate and reset as well.
Comment on lines
+59
to
65
| const is_validated = await verificationService.validate(username); | ||
|
|
||
| return { | ||
| success: is_validated, | ||
| user: { | ||
| username: username, | ||
| atcoder_username: atcoder_username, | ||
| atcoder_validationcode: atcoder_validationcode, | ||
| message_type: 'green', | ||
| message: 'Successfully validated.', | ||
| }, | ||
| message_type: 'green', | ||
| message: 'Successfully validated.', | ||
| }; |
Contributor
There was a problem hiding this comment.
失敗時も成功メッセージを返しています。
verificationService.validate(username) が false でも、Line 63-64 は常に緑の成功文言です。返却データが自己矛盾するので、失敗時は fail(...) か別メッセージに分岐してください。
🧭 修正イメージ
const is_validated = await verificationService.validate(username);
+ if (!is_validated) {
+ return fail(400, {
+ success: false,
+ message_type: 'default',
+ message: 'Validation failed.',
+ });
+ }
+
return {
- success: is_validated,
+ success: true,
message_type: 'green',
message: 'Successfully validated.',
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const is_validated = await verificationService.validate(username); | |
| return { | |
| success: is_validated, | |
| user: { | |
| username: username, | |
| atcoder_username: atcoder_username, | |
| atcoder_validationcode: atcoder_validationcode, | |
| message_type: 'green', | |
| message: 'Successfully validated.', | |
| }, | |
| message_type: 'green', | |
| message: 'Successfully validated.', | |
| }; | |
| const is_validated = await verificationService.validate(username); | |
| if (!is_validated) { | |
| return fail(400, { | |
| success: false, | |
| message_type: 'default', | |
| message: 'Validation failed.', | |
| }); | |
| } | |
| return { | |
| success: true, | |
| message_type: 'green', | |
| message: 'Successfully validated.', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/users/edit/`+page.server.ts around lines 59 - 65, The current
handler always returns a green success payload even when
verificationService.validate(username) returns false; check the result stored in
is_validated and branch: if true return the existing success object, otherwise
return an appropriate failure response (use fail(...) from SvelteKit or return
an object with message_type: 'red' and a failure message) so the response and
message_type/message are consistent; update the logic around
verificationService.validate and the returned object accordingly.
KATO-Hiro
approved these changes
Mar 28, 2026
Collaborator
There was a problem hiding this comment.
修正していただき、ありがとうございます。
マージします。
また、重ね重ねで恐縮ですが、この PR の内容を #3319 に反映させていただけますでしょうか?
よろしくお願いいたします。
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
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.
#3263 に関するリファクタリングです。
認証に関するデータベースの整理などを行いました。
Summary by CodeRabbit
リリースノート