✨ お知らせをWebUIから作成・公開できるようにする - #284
Conversation
これまで app/news/news-data.ts にハードコードしていたお知らせを Firestore の news コレクションに移し、管理者ページから作成・編集・公開できるようにする。 - 本文は Markdown で保持し、表示は react-markdown に任せる。 react-markdown は既定で生 HTML を描画しないので、これまでの dangerouslySetInnerHTML + HTML 文字列と違い XSS にならない - 表示用の date (自由形式) と並べ替え用の publishedAt (ISO) を分ける。 既存データの "2025年5月21-23日" "2025年4月中" という表記を保ったまま、 日本語日付を正規表現でパースして並べ替える処理を消せる - 一覧は where + メモリ側 sort にして複合インデックスを不要にする (blogs と同じ) - エラーは NewsError + コード表に寄せる (blogs と同じ) - 書き込み権限は当面 ADMIN_ROLE_ID を流用する。lib/news-auth.ts の 1 関数だけ差し替えればプロジェクトリーダーに開放できる - Firestore はビルド環境から到達できないため、お知らせを読むルートは force-dynamic にする。ISR だとデプロイ直後にビルド時のスナップショットが revalidate まで居座ってしまう - 移行が終わるまでは Firestore が空のとき旧データを返す。 scripts/migrate-news.ts を本番で流したら lib/news-legacy.ts ごと消せる トップページはお知らせ取得のためサーバーコンポーネントにし、 カルーセルを components/news-carousel.tsx に切り出した (markup は変更なし)。 Refs #262 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RJiMXW4FTJCECgpf3pQEtj
「下書きとして保存」してから一覧で公開する2段階だったが、 公開前にMarkdownの崩れを確認できないままだった。 フォームの主ボタンをプレビューに変え、モーダルで公開ページと同じ表示を 確認してから公開・下書き保存を選べるようにする。 ページ遷移にすると未保存の内容を渡すために一度保存が必要になり、 「まず下書き保存」に逆戻りしてしまう。タブだとフォームの幅で描画されるので 実際の記事幅 (max-w-3xl) での崩れが見つけられない。 - 見出しと本文の描画を components/news/news-article-view.tsx に切り出し、 公開ページとプレビューで同じものを使う。proseのクラスが枝分かれして 「プレビューでは崩れていないのに公開したら崩れる」のを構造的に防ぐ - 一覧の「公開」も必ずプレビューを通す (ラベルは「確認して公開」)。 記事をフォームに読み込んでから開くので、気になれば編集に戻せる - 本文欄の中にあったプレビュー切り替えは、モーダルと二重になるので削除 - 画像が未設定のときの /placeholder.svg をやめ、画像ブロックごと出さない Refs #262 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RJiMXW4FTJCECgpf3pQEtj
|
Deployed on https://dev-pr-284-birbz3fm6q-an.a.run.app |
There was a problem hiding this comment.
c200e9f をレビューしました。マージ前に修正したい不具合が 3 件あります。
仕様・動作
[P2] 最後の公開記事を非公開にすると、旧記事が再び公開される
対象: lib/news.ts:239
フォールバックの条件が「移行前で Firestore が空」ではなく「公開済みの記事が 0 件」になっています。移行後に最後の公開記事を下書きへ戻すと、非公開にしたものを含めて旧記事 9 件が公開一覧へ復活します。詳細取得も同様です。逆に、移行前に新しい記事を 1 件公開すると、旧記事 9 件が一覧から消え、旧 URL が 404 になります。
移行完了を明示的に管理するなど、未移行と意図的な非公開・削除を区別する必要があります。公開件数をフォールバックの切り替え条件にしないでください。
[P2] 公開失敗後のリトライで別の記事が作成される
対象: components/news/news-manager.tsx:153
新規記事の保存成功後、返された ID を editingId に保持していません。その後の公開リクエストが失敗してプレビューから再度「公開する」を押すと、既存の下書きを再利用せず、別の記事を作成します。公開自体は成功してレスポンスだけ失われた場合には、公開記事の重複にもつながります。
作成済み ID を公開処理の前に保持し、再試行では同じ記事を更新・公開するようにしてください。ローカルのモック検証で、POST /api/news → POST /api/news/1/publish(失敗)→ POST /api/news → POST /api/news/2/publish の順になることを確認しました。
要件の持ち越し: プロジェクトリーダーは利用できない
Issue #262 では「プロジェクトリーダーのロールを持った人が記事作成、公開までを WebUI で行う」ことを求めていますが、API と管理ページはいずれも管理者限定です。ただし、現状の実装としては管理者権限を持つユーザーとしておいた上で、別PRでプロジェクトリーダーロールを管理する機能をつけることが適切です。
セキュリティ
[P2] 退会済み管理者がニュース API を操作できる
セッションの ID と管理者判定は確認していますが、session.user.optedOut を確認していません。既存セッションに管理者フラグが残っている退会済みユーザーは、メンバー画面から拒否されても API を直接呼び、記事の作成・公開・削除・画像アップロードを行えます。
authorizeNewsWriter() で退会済みセッションを拒否してください。これは Discord 側のロール変更を既存セッションへ反映する方式とは別に、このヘルパーで防げる問題です。
- 公開件数を条件にした旧記事フォールバックを削除し、公開ページは Firestore のみを読む (デプロイ前に scripts/migrate-news.ts を本番で流す運用にする) - 新規記事の作成後に ID を editingId へ保持し、公開失敗後の再試行で別記事を作らない - authorizeNewsWriter() で退会済みセッションを 403 で弾く Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gtw14tqkjpEogwKBR6Bcd8
|
レビューありがとうございます。 [P2] 最後の公開記事を非公開にすると、旧記事が再び公開されるフォールバック自体を削除しました( [P2] 公開失敗後のリトライで別の記事が作成される
[P2] 退会済み管理者がニュース API を操作できる
要件の持ち越し: プロジェクトリーダーご提案どおり、このPRでは管理者限定のままにして、プロジェクトリーダーロールの管理は別PRで対応します。 🤖 Generated with Claude Code |
|
Deployed on https://dev-pr-284-birbz3fm6q-an.a.run.app |
|
@Tivo0921 再レビュー依頼する時は再レビュー依頼してください
|
Shion1305
left a comment
There was a problem hiding this comment.
レビューしました (Review 100% Authored Manually by @Shion1305 )
| const authorized = await authorizeNewsWriter(); | ||
| if ("response" in authorized) return authorized.response; |
There was a problem hiding this comment.
ここの書き方を改善したい (同様の実装について全て)
現状の整理
authorizeNewsWriterは、リクエストを送った人が権限があるかどうか(adminであるか)を確認する- 認可された場合は著者ID
authorIdを返す (==session.user.id) - 拒否がない場合は
NextResponseを返す
- 認可された場合は著者ID
- このロジックの依存部分では、返り値が
NextResponseであるかどうかで認可されたかを判断する
問題点の提起
- 呼び出し部分を見た時に、
authorizeNewsWriterの実装を確認しないとif ("response" in authorized) return authorized.response;の実装の意味を理解できない - 必ず
authorIdはsession.user.idと同値なのに、あえて別のデータとして返すことで実装が冗長
提案する改善
- 認可を確認する実装は
isAuthorizedNewsEditorなどとしてbooleanを返す形にする- true → 認可されたとして続行
- false → 権限がないとしてReject
- Rejectされた際に返すデータは定数化し、現状の
return aurhoized.responseがreturn RESPONSE_UNAUTHORIZEDなどとする (どのような命名が適当であるかは適宜検討してください)
There was a problem hiding this comment.
lib/news-response.ts みたいなところに記述しても良さそう Permission Deniedみたいなレスポンスて定義
それ以外のレスポンスを返す部分も定数化できたら見やすくなりそう
There was a problem hiding this comment.
d69dbed で整理しました。
authorizeNewsWriter()→isNewsEditor(session): boolean(lib/news-auth.ts)。セッションを引数に取る純粋関数にしたので、authorIdを別に返す必要も無くなり、作成ルートはsession.user.idをそのまま使っています- 呼び出し側は 4 ルート 6 ハンドラすべて次の形に揃えました
未ログイン (401) と権限なし (403) は分けて返したかったので 2 行になっています
const session = await auth(); if (!session?.user?.id) return NEWS_API_RESPONSES.unauthorized(); if (!isNewsEditor(session)) return NEWS_API_RESPONSES.forbidden();
isNewsEditorのテストをlib/news-auth.test.tsに追加(退会済みで admin フラグが残っているケースを含む)
There was a problem hiding this comment.
lib/news-response.ts に NEWS_API_RESPONSES としてまとめました(unauthorized / forbidden / success / imageRequired / imageUploadFailed)。
Response は一度しか送れないので、定数ではなく毎回作る関数の形にしています(NEWS_API_RESPONSES.forbidden())。
- authorizeNewsWriter() を isNewsEditor(session) に置き換え、呼び出し側で 未ログイン / 権限なしを読める形にする - 401 / 403 / success などの定型レスポンスを NEWS_API_RESPONSES にまとめる Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gtw14tqkjpEogwKBR6Bcd8
|
Deployed on https://dev-pr-284-birbz3fm6q-an.a.run.app |
#284 のレビュー指摘に合わせて、authorizeAdminApi() を isAdminSession(session) に置き換え、 401 / 403 / success などの定型レスポンスを EVENT_API_RESPONSES にまとめる Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gtw14tqkjpEogwKBR6Bcd8
Shion1305
left a comment
There was a problem hiding this comment.
d69dbed を再レビューしました。前回指摘した認可判定と定型レスポンスの整理が反映されており、既存の不具合修正にも回帰はありません。承認します。
確認結果:
- Vitest: 211件 pass
- ESLint: 変更箇所の error なし
- TypeScript: tsc --noEmit pass
- Prettier: pass
- CI の CodeQL / build-and-deploy / lint: pass
非ブロッキングの改善候補:
- お知らせ編集フォームを、リポジトリ方針に合わせて react-hook-form + Zod に寄せる
- カルーセルの aria-label を日本語にする
- ステータス表示を共通 Badge とデザイントークンに寄せる
管理画面のプレビューモーダルは、この環境ではログイン済みブラウザを利用できなかったため、目視確認は未実施です。

Closes #262
お知らせ (
/news) をapp/news/news-data.tsへのハードコードから Firestore に移し、管理者ページから作成・プレビュー・公開できるようにしました。使い方
管理者ページ → 「お知らせを書く」(
/internal/admin/news)一覧の「確認して公開」も同じプレビューを通ります。プレビューを経ずに公開できる経路はありません。
設計メモ
本文は Markdown。表示は
react-markdownに任せています。react-markdownは既定で生 HTML を描画しない (rehype-rawを入れていない) ので、これまでのdangerouslySetInnerHTML+ HTML 文字列と違い XSS になりません。sanitize ライブラリの追加は不要でした。本文に<script>を入れてもエスケープされることをローカルで確認済みです。表示用の
dateと並べ替え用のpublishedAtを分離。既存データに"2025年5月21-23日""2025年4月中"という自由形式の表記があり、これまでは正規表現でパースして並べ替えていました ("4月中"を 15 日と決め打ち)。ISO のpublishedAtを別に持たせることで、表記を保ったままこの脆いパースを消せます。プレビューと公開ページで同じコンポーネントを使う。
components/news/news-article-view.tsxを公開ページとプレビューの両方から使うことで、proseのクラスが枝分かれして「プレビューでは崩れていないのに公開したら崩れる」が起きないようにしています。一覧は
where+ メモリ側sortにして複合インデックスを不要にしました (lib/blogs.tsと同じ方針)。エラー処理もNewsError+ コード表に寄せています。お知らせを読むルートは
force-dynamic。Firestore はビルド環境から到達できないため、ここを静的プリレンダリングの対象にすると Docker ビルドが落ちます。ISR にしなかったのは、デプロイ直後にビルド時のスナップショットが revalidate まで居座ってしまうためです。権限について
書き込み権限は当面
ADMIN_ROLE_IDを流用しています。プロジェクトリーダーに開放する場合はlib/news-auth.tsの 1 関数だけ差し替えれば API ルート側は触らずに済みます (Discord のロール作成 +infra/variables.tf+ terraform apply が別途必要)。マージ後にやること
既存9件の移行が残っています。
--dry-runで確認してから流してください。ドキュメント ID には元の id をそのまま使うので
/news/9などの既存 URL は変わりません。何度流しても同じ結果になります (既にある ID はスキップ)。移行が終わるまでは Firestore が空のとき旧データを返すフォールバックが効きます。移行完了後は
lib/news-legacy.tsとapp/news/news-data.tsを削除できます。動作確認
just+ エミュレータで確認したこと:<script>がエスケープされるjust test207件 pass / lint エラー0 /tscエラー0 /pnpm build成功。レビューしてほしいところ
ADMIN_ROLE_ID流用のままにしてよいか🤖 Generated with Claude Code
https://claude.ai/code/session_01RJiMXW4FTJCECgpf3pQEtj