Skip to content

refactor: action 戻り値型統一・toast 通知・エラーハンドリング整備#230

Merged
coji merged 2 commits intomainfrom
refactor/unify-action-response-and-toast
Mar 22, 2026
Merged

refactor: action 戻り値型統一・toast 通知・エラーハンドリング整備#230
coji merged 2 commits intomainfrom
refactor/unify-action-response-and-toast

Conversation

@coji
Copy link
Copy Markdown
Owner

@coji coji commented Mar 22, 2026

Summary

  • 全 multi-intent action の戻り値を { ok: boolean, lastResult: SubmissionResult | null } に統一
  • 成功/エラー時の toast 通知が欠けていた箇所を補完
  • settings 系フォームのエラーハンドリング改善
  • コンポーネント側の型ガードを簡素化

変更内容

戻り値型の統一(5ルート)

teams._index, github-users._index, repositories._index, members, repositories/$repository/settings の全分岐で ok + lastResult を返すように統一。

toast 追加(成功 8箇所・エラー 4箇所)

  • 成功: teams add/update/delete, github-users add/update/delete/toggle-active, members changeRole/removeMember, repositories updateTeam/bulkUpdateTeam
  • エラー: settings/_index, export, integration, danger

エラーハンドリング改善

  • danger/index.tsx: deleteOrganization() に try/catch 追加
  • repositories.add/index.tsx: console.error 除去、実際のエラーメッセージ使用
  • settings 系: ハードコードされたエラー文字列を getErrorMessage(e) に置換

コンポーネント簡素化

  • member-row-actions.tsx: 'lastResult' in datadata?.lastResult'ok' in datadata?.ok === true
  • repositories/$repository/settings: 同上
  • data-table-floating-bar.tsx: useFetcher<{ ok: boolean }> ジェネリックで型キャスト除去

Closes #229

Test plan

  • pnpm validate 通過
  • 各コンポーネントで成功時に toast が表示される
  • エラー時に toast が表示される
  • ConfirmDialog のインラインエラー表示が維持される
  • EditableCell のエラーリセットが動作する

🤖 Generated with Claude Code

Summary by CodeRabbit

リリースノート

  • バグ修正

    • 組織設定、チーム管理、メンバー管理などの操作時に、より詳細なエラーメッセージを表示するように改善しました
    • フォーム送信時のエラーハンドリングを強化し、予期しない失敗時の対応を改善しました
  • ユーザー体験

    • 各種設定変更・削除操作成功時にトースト通知で確認メッセージを表示するようにしました
    • ダイアログと確認画面の表示・非表示をより確実に制御するようにしました

Closes #229

- 全 multi-intent action の戻り値を { ok, lastResult } に統一
- 成功パスに dataWithSuccess で toast 追加(8箇所)
- エラーパスに dataWithError で toast 追加(settings 系4箇所)
- danger route に try/catch 追加
- ConfirmDialogData 型を統一型に対応
- コンポーネントの型ガード簡素化
  - 'lastResult' in data → data?.lastResult
  - 'ok' in data → data?.ok === true
- repositories.add の console.error 除去
- data-table-floating-bar の型キャスト除去

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 22, 2026

Warning

Rate limit exceeded

@coji has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 23e2d40d-caef-4275-970a-302e2b22c4bd

📥 Commits

Reviewing files that changed from the base of the PR and between d07f1e1 and 9e8e4c6.

📒 Files selected for processing (1)
  • app/routes/$orgSlug/settings/repositories.add/index.tsx
📝 Walkthrough

Walkthrough

複数の設定ページのアクションハンドラを更新し、dataWithErrordataWithSuccessを使用した統一されたエラーハンドリングとトースト通知パターンを実装します。ConfirmDialogDataインターフェースを修正し、一貫したレスポンス形式(okフラグとlastResult)を適用します。

Changes

Cohort / File(s) Summary
Dialog Type Definition
app/components/confirm-dialog.tsx
ConfirmDialogDataインターフェースでokbooleanからオプショナル(ok?: boolean)に変更し、lastResultSubmissionResult | nullをサポートするようにアップグレード。
Settings Index & Export Routes
app/routes/$orgSlug/settings/_index/index.tsx, app/routes/$orgSlug/settings/export/index.tsx, app/routes/$orgSlug/settings/integration/index.tsx
アクションのエラーパスをgetErrorMessage(e)を使用してdataWithError(...)パターンに統一。ハードコードされたエラーメッセージをユーザー向けメッセージに置き換え。
Danger & Member Management Routes
app/routes/$orgSlug/settings/danger/index.tsx, app/routes/$orgSlug/settings/members/index.tsx
try/catchでエラーハンドリングを追加または改善。エラー時にok: falseフラグを返す。成功時にdataWithSuccessを使用してトースト通知を追加。
Github Users & Teams Routes
app/routes/$orgSlug/settings/github-users._index/index.tsx, app/routes/$orgSlug/settings/teams._index/index.tsx
複数の intent(add, update, delete, toggle-activeなど)に対して一貫した応答形式を実装。全パスにokフラグを追加し、成功時にdataWithSuccessでトースト通知を統合。
Repositories Routes
app/routes/$orgSlug/settings/repositories._index/index.tsx, app/routes/$orgSlug/settings/repositories.add/index.tsx, app/routes/$orgSlug/settings/repositories/$repository/settings/index.tsx
アクション応答にokフラグを追加。console.errorログを削除しgetErrorMessageに統一。成功/エラーでdataWithSuccess/dataWithErrorを使用してトースト通知を実装。
Client-side Data Extraction
app/routes/$orgSlug/settings/members/+components/member-row-actions.tsx, app/routes/$orgSlug/settings/repositories._index/+components/data-table-floating-bar.tsx
fetcher.dataの型チェックをオプショナルチェーン(fetcher.data?.ok)とnullish coalescing に簡素化。型キャストを削除しuseFetcherに型パラメータを追加。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 エラーなんて、ポポッと掴まえて
トーストに乗せりゃ、ふんわり通知♪
oklastResult、歩調そろえて
統一のお庭で、ぴょんぴょん跳ねるよ 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは実装内容(action戻り値型統一・toast通知・エラーハンドリング整備)を正確に反映しており、変更の主要な目的を明確に伝えている。
Linked Issues check ✅ Passed PRはIssue #229の全要件(型統一、toast通知補完、エラーハンドリング整備)をコード実装として達成している。
Out of Scope Changes check ✅ Passed 全ての変更はIssue #229の明示的スコープ内であり、スコープ外とされたルート(data-management等)への変更は含まれていない。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/unify-action-response-and-toast

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
app/routes/$orgSlug/settings/_index/index.tsx (1)

36-41: 統一されたレスポンス形式との整合性

バリデーション失敗時のレスポンスに ok: false が含まれていません。他のルート(repositories._index/index.tsx の Line 86、members/index.tsx の Line 92)では { ok: false, lastResult: submission.reply() } を返しています。

一貫性のため、このパスも同じ形式に更新することを検討してください。

♻️ 提案する修正
   if (submission.status !== 'success') {
-    return {
+    return data({
+      ok: false,
       intent: 'organization-settings' as const,
       lastResult: submission.reply(),
-    }
+    }, { status: 400 })
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/routes/`$orgSlug/settings/_index/index.tsx around lines 36 - 41, The
branch that handles non-success submission in the settings route returns an
object missing the unified ok flag; update the block that checks
submission.status !== 'success' so it returns { ok: false, intent:
'organization-settings' as const, lastResult: submission.reply() } (i.e., add
ok: false) to match the response shape used by repositories._index and
members/index handlers.
app/routes/$orgSlug/settings/members/index.tsx (1)

121-128: removeMember エラー時のトースト通知がありません

removeMember のエラーパスでは data() を使用しているため、トースト通知が表示されません。changeRole のエラーパスでは dataWithError を使用してトーストを表示しています。

shouldConfirm: true を維持しつつトースト通知も行う場合、dataWithErrorshouldConfirm を含める方法を検討してください。ただし、ダイアログ内にエラーが表示される設計であれば、現状のままでも問題ありません。

♻️ トースト通知を追加する場合の修正案
       } catch (e) {
+        const message = getErrorMessage(e)
-        return data(
+        return dataWithError(
           {
             ok: false,
-            lastResult: submission.reply({ formErrors: [getErrorMessage(e)] }),
+            lastResult: submission.reply({ formErrors: [message] }),
             shouldConfirm: true,
           },
-          { status: 400 },
+          { message },
         )
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/routes/`$orgSlug/settings/members/index.tsx around lines 121 - 128, The
removeMember error branch currently returns data(...) so no toast is shown;
change that return to use dataWithError(...) (the same helper used by
changeRole) and include shouldConfirm: true plus the existing lastResult payload
(submission.reply({ formErrors: [getErrorMessage(e)] })) so the toast/error UI
is triggered while preserving the confirmation dialog state; locate the error
return inside the removeMember handler and replace the data(...) call with
dataWithError(...) carrying the same object and status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/routes/`$orgSlug/settings/_index/index.tsx:
- Around line 36-41: The branch that handles non-success submission in the
settings route returns an object missing the unified ok flag; update the block
that checks submission.status !== 'success' so it returns { ok: false, intent:
'organization-settings' as const, lastResult: submission.reply() } (i.e., add
ok: false) to match the response shape used by repositories._index and
members/index handlers.

In `@app/routes/`$orgSlug/settings/members/index.tsx:
- Around line 121-128: The removeMember error branch currently returns data(...)
so no toast is shown; change that return to use dataWithError(...) (the same
helper used by changeRole) and include shouldConfirm: true plus the existing
lastResult payload (submission.reply({ formErrors: [getErrorMessage(e)] })) so
the toast/error UI is triggered while preserving the confirmation dialog state;
locate the error return inside the removeMember handler and replace the
data(...) call with dataWithError(...) carrying the same object and status.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 757a2059-3d9e-44a1-a482-2f9ab436d044

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd84a0 and d07f1e1.

📒 Files selected for processing (13)
  • app/components/confirm-dialog.tsx
  • app/routes/$orgSlug/settings/_index/index.tsx
  • app/routes/$orgSlug/settings/danger/index.tsx
  • app/routes/$orgSlug/settings/export/index.tsx
  • app/routes/$orgSlug/settings/github-users._index/index.tsx
  • app/routes/$orgSlug/settings/integration/index.tsx
  • app/routes/$orgSlug/settings/members/+components/member-row-actions.tsx
  • app/routes/$orgSlug/settings/members/index.tsx
  • app/routes/$orgSlug/settings/repositories._index/+components/data-table-floating-bar.tsx
  • app/routes/$orgSlug/settings/repositories._index/index.tsx
  • app/routes/$orgSlug/settings/repositories.add/index.tsx
  • app/routes/$orgSlug/settings/repositories/$repository/settings/index.tsx
  • app/routes/$orgSlug/settings/teams._index/index.tsx

@coji coji merged commit c438b8b into main Mar 22, 2026
6 checks passed
@coji coji deleted the refactor/unify-action-response-and-toast branch March 22, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: action の戻り値型統一・toast 通知・エラーハンドリング整備

1 participant