[codex] ユーザー登録のトランザクション化とE2E追加#1106
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an E2E testing environment using Playwright and Docker, and refactors the signup flow to create a user, mail authentication, and session within a single database transaction. The frontend has been updated to support this unified signup API. The reviewer feedback highlights three key areas for improvement: a security concern regarding sensitive data (email and password) being passed as query parameters, a performance optimization to move password hashing outside of the database transaction, and the removal of a leftover debug print statement.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Deploying finansu with
|
| Latest commit: |
4efc548
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cbb078f0.finansu.pages.dev |
| Branch Preview URL: | https://codex-transactional-signup-e.finansu.pages.dev |
There was a problem hiding this comment.
Pull request overview
新規登録まわりの整合性(ユーザー・メール認証・セッション)を担保するために、バックエンドの登録/削除処理をトランザクション化し、/mail_auth/signup を query params から JSON body へ移行、フロントも生成 API クライアント(orval)に寄せつつ、APIテストとDocker完結のPlaywright E2E・PR実行用Workflowを追加するPR。
Changes:
- 新規登録/ユーザー削除のDB更新をトランザクション化し、部分成功の残骸を防止
/mail_auth/signupを JSON request body 化し、OpenAPI/生成コード/フロント実装を追従- APIテスト+composeベースのPlaywright E2E+PR実行用 GitHub Actions を追加
Reviewed changes
Copilot reviewed 22 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| view/next-project/tsconfig.json | E2EディレクトリをTSの対象外にして型チェック範囲を整理 |
| view/next-project/src/utils/api/signUp.ts | 旧・手書きsignup API呼び出しを削除 |
| view/next-project/src/generated/model/postMailAuthSignupBody.ts | signupのリクエスト型をJSON body(name/bureau_id/role_id含む)へ更新 |
| view/next-project/src/generated/model/postMailAuthSignup200.ts | signupレスポンス型に accessToken/userID を明示 |
| view/next-project/src/generated/model/index.ts | 生成model export を Params→Body に差し替え |
| view/next-project/src/generated/hooks.ts | /mail_auth/signup をクエリなしPOST+JSON bodyの生成関数に更新 |
| view/next-project/src/components/auth/SignUpView.tsx | 手書きfetch/ユーザー事前作成を廃止し postMailAuthSignup を利用、遷移先を /my_page に修正 |
| view/next-project/next.config.js | e2e環境向けAPIエンドポイント設定を追加 |
| view/next-project/e2e/tests/signup.spec.ts | signup後の遷移・/current_user・signupリクエスト形式を検証するE2E追加 |
| view/next-project/e2e/playwright.config.ts | E2E用Playwright設定を追加 |
| view/next-project/e2e/package.json | E2E用依存(@playwright/test)を追加 |
| view/next-project/.gitignore | Playwright成果物(report/test-results)をignoreへ追加 |
| openapi/openapi.yaml | /mail_auth/signup を query params→JSON requestBody に変更、200レスポンススキーマも明確化 |
| mysql/e2e_seed.sql | E2E用の最小シードを追加 |
| Makefile | make run-e2e でcompose完結のE2E実行手順を追加 |
| compose.e2e.yml | DB/API/View/Playwright をDocker内で完結させるE2E用composeを追加 |
| api/test/signup_transaction_test.go | signupのトランザクション性(成功/ロールバック)を検証するテスト追加 |
| api/internals/usecase/user_usecase.go | user作成後の取得を「新規レコード検索」から「insert idで取得」へ変更 |
| api/internals/usecase/mail_auth_usecase.go | signupをusers/mail_auth/sessionの単一トランザクションで作成するよう変更 |
| api/internals/domain/mail_auth.go | signupレスポンスに userID を追加 |
| api/internals/di/wire_gen.go | signupトランザクション化に伴うDI構成(UserRepo/TransactionRepo注入)を更新 |
| api/generated/openapi_gen.go | signupのrequestBody化に合わせて生成サーバーI/Fを更新 |
| api/externals/repository/user_repository.go | user関連クエリをgoqu化+tx対応メソッド追加、削除処理をtx化 |
| api/externals/repository/session_repository.go | session作成をgoqu化+tx対応メソッド追加 |
| api/externals/repository/mail_auth_repository.go | mail_auth作成をgoqu化+tx対応メソッド追加 |
| api/externals/handler/mail_auth_handler.go | signupハンドラをJSON bodyバインド方式へ更新 |
| api/drivers/server/server.go | E2Eコンテナからのアクセスを想定してCORS originに http://view:3000 を追加 |
| .github/workflows/e2e.yml | PRで make run-e2e を回すE2Eワークフローを追加 |
Files not reviewed (1)
- api/internals/di/wire_gen.go: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 28 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- api/internals/di/wire_gen.go: Language not supported
Comments suppressed due to low confidence (1)
api/internals/usecase/mail_auth_usecase.go:158
IsSignInでrow.Scanの戻り値を捨てているため、sql.ErrNoRows以外の DB/Scan エラーも「未ログイン」として握りつぶされます。結果が誤るだけでなく、障害の検知も遅れやすいので、Scan エラーはErrNoRowsのみ未ログイン扱いにし、それ以外は error を返す形にしてください。
_ = row.Scan(
&session.ID,
&session.AuthID,
&session.UserID,
&session.AccessToken,
&session.CreatedAt,
&session.UpdatedAt,
)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| alert( | ||
| '新規登録に失敗しました。メールアドレスもしくはパスワードがすでに登録されている可能性があります', | ||
| ); |
There was a problem hiding this comment.
[imo]
今回の実装範囲じゃないけど「新規登録に失敗しました。このメールアドレスは既に登録されている可能性があります。」とかでいいと思う。
今回の修正範囲じゃないかもだけど、名前が空欄で登録できないようにしたい。
名前もreact-hook-formの管理対象にして登録できないようにしたい。
| "extends": "./tsconfig.paths.json", | ||
| "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", "src/**/*.ts", "src/**/*.tsx"], | ||
| "exclude": ["node_modules"] | ||
| "exclude": ["node_modules", "e2e"] |
There was a problem hiding this comment.
[ask]
VSCodeでview/next-project/e2e/tests/signup.spec.tsを開くとめっちゃエラー出てくる
ESLintからでてて、tsconfig.jsonでe2eディレクトリを除外しているため、ESLintがこのファイルを解析できていないらしい
これは気にしなくていいのか知りたい。
AIはE2E用tsconfigを新しく作ったら?って言ってくる
| expect(currentUserStatuses).not.toContain(404); | ||
| expect(page404Responses).toEqual([]); |
There was a problem hiding this comment.
[ask]
signupと関係のない画像やAPIなどが404になった場合もE2Eが失敗する認識。
/current_userの404は検証されているけど、ページ内すべての404を禁止している意図を知りたい。
どこまでをテストで保証する品質要件にするかってところで。
| email := "signup-missing-fields@example.com" | ||
| beforeUsers := countRows(t, "SELECT COUNT(*) FROM users") | ||
| r := postSignup(t, testServer.URL, map[string]any{ | ||
| "email": email, | ||
| "password": "password123", | ||
| "role_id": 1, | ||
| }) | ||
| defer r.Body.Close() | ||
|
|
||
| assert.Equal(t, http.StatusBadRequest, r.StatusCode) | ||
| assert.Equal(t, beforeUsers, countRows(t, "SELECT COUNT(*) FROM users")) | ||
| assert.Equal(t, 0, countRows(t, "SELECT COUNT(*) FROM mail_auth WHERE email = ?", email)) | ||
| assert.Equal(t, 0, countRows(t, "SELECT COUNT(*) FROM session")) |
There was a problem hiding this comment.
[nits]
インデント崩れてる
gofmt -w api/test/signup_transaction_test.go でフォーマットお願いします
これって勝手にならないのか
Chikuwa0141
left a comment
There was a problem hiding this comment.
対応あざす!
関係ないところコメントできないからここでします↓
E2E用tsconfigがESLint側のparserOptions.projectから参照されていないため、signup.spec.tsを開くと以前と同じParsing errorが表示された
./e2e/tsconfig.jsonも参照先へ追加する必要あり
| @@ -7,6 +7,8 @@ | |||
|
|
|||
There was a problem hiding this comment.
[imo]
E2Eの依存関係をローカルでインストールすると、e2e/node_modules/がGitの未追跡ファイルとして表示された。
誤ってコミットしないように、/e2e/node_modules/も.gitignoreへ追加した方がよさそう
対応Issue
resolve #0
概要
users、mail_auth、sessionの作成を同一トランザクションに変更usersだけが残らないよう rollback を追加users更新とmail_auth更新を同一トランザクション化/mail_auth/signupのemail/passwordなどを query params から JSON requestBody に変更postMailAuthSignup利用へ変更/purchaseordersから/my_pageに修正画面スクリーンショット等
/my_pageへ遷移し、/current_userが 404 にならないことを確認/mail_auth/signupのURLにqueryが付かず、JSON bodyで送信されることも確認テスト項目
go test ./...corepack pnpm run type-checkcorepack pnpm run lint(既存 warning 104件、error なし)make gendocker compose -f compose.e2e.yml config.github/workflows/e2e.ymlでPR時にmake run-e2eを実行2026-06-09.154846.mp4
備考
view/next-project/e2e配下に配置compose.e2e.ymlで DB/API/View/Playwright を Docker 内で起動e2e/test-results/e2e/playwright-reportはignore対象に追加/mail_auth/signinは既存どおり query params のままです。今回の対応範囲は signup に限定しています。