Repository navigation
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: defaults Review profile: CHILL Plan: Advanced 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 |
YosukeIida
left a comment
There was a problem hiding this comment.
確認しました。現在稼働中の gm3/develop とマージ後のソースツリーが同一であることを確認済みです。デプロイ時は .env の引き継ぎと、本番 DB で migration 20260625000001 まで適用済みであることを確認してください。
複数物品の貸出場所更新をPromise.allの並列実行から直列実行に変更し、途中で失敗した際に不整合な書き込みが広がらないようにした。またAPIに渡していたrentalItemId/stockerPlaceIdは実際には常にundefinedで送られておらず実害のない死んだコードだったため削除し、rental_place_idのみを送るよう簡略化した
- updateAssignmentsPlaceを先にローカル反映してから直列でPUTするようにし、体感速度を確保しつつ、途中失敗時は既にサーバーへ反映済みの分も含めて元の場所へ補償的にロールバックするようにした - assignments配列を団体ID・貸出場所IDで振り分けたMapをcomputedで持たせ、団体×物品×場所のネストしたループの中で全件走査を繰り返さないようにした - targetDeleteGroupId/targetDeletePlaceIdのfalsy判定をnullチェックに修正し、IDが0の場合の誤動作を防止 - 未使用になったgetAssignmentsByを削除
- 同一団体への操作が処理中に重複しないよう updatingGroupIds で多重実行を防止 - removeGroupFromPlace の失敗時にも fetchDataFromDB() で再同期するようにした - getGroupsInPlace が場所を跨いで団体の有効物品合計を見ていたため、無効化物品しかその場所に無い団体まで表示されていたのを、場所ごとの合計で判定するよう修正
物品貸出場所調整で物品ごとに異なる貸出場所を割り当て可能に
CodeRabbitの指摘に対応し、Group新規作成時のsecret生成(24文字)、 regenerate_secretによる値の更新、NOT NULL/UNIQUE制約違反時に 例外が発生することを検証するテストを追加。groups fixtureにも 必須となったsecretを設定。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rD1PnY2E7E8hpuFXuhDdY
TextBoxのinputに固定幅(w-[400px])があり、Modal/LoginModalを レスポンシブ化してもなお狭い画面でオーバーフローしていたため、 可変幅(w-full max-w-[400px])に変更。あわせてModalに縦スクロール と高さ上限を、LoginModalのpadding/幅にモバイル用のブレークポイント を追加。
Modal/LoginModalの各コンテナにw-full/min-w-0を明示し、フレックス アイテムのデフォルトmin-width:autoによる縮小阻害を回避。Modalに 左右ガター(px-4)を追加し、狭い画面でモーダルが画面端に密着しない ようにした。LoginModalの狭い画面向けpaddingも縮小。TextBoxのlabel をblock w-fullにし、input幅が親幅を確実に参照するよう明示化。
送信に失敗したときは登録画面に留まってエラーを出すため、画面が切り替わること 自体が成功の合図になっている。ただ「送ったつもりで送れていない」と取り違えると 当日の記録が抜けるため、戻った先で明示する。 登録画面は成功時だけ ?registered=1&group=<団体名> を付けて画面②へ戻り、 画面②がそれを見て「〇〇 の登録が完了しました」を出す。×で閉じられる。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
提供元の団体が持つ割当から候補を出していたため、その団体が持っていない物品や 在庫場所が選べなかった。当日は予定に無い物品を渡すこともあるため、物品・在庫場所・ 団体のいずれもマスタの全件から選べるようにする。 - API に get_rental_items_for_rental_view / get_stocker_places_for_rental_view を追加し、BFF 経由で全件返す - 例外対応シートは 物品 → 在庫場所 → 元の貸出先団体 の順(Figma どおり)に戻し、 互いに絞り込まない - 選んだ組み合わせを提供元が持っていなければ未貸出数は0になり、その旨を出して 送信を止める(回せる物が無いため) 作業場所の「すべての場所」は、貸出場所が登録されていない団体を選べなくなるため そのまま残す。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
#2184 で追加された assign_rental_items.remark を確定情報APIに載せ、 確定画面に表示する。 ## 列は増やさずセル内に添える 物品貸出表と同じ方針にした。output_rental_items_pdf.css の .remark-note に 「備考は列を増やさず物品名セル内に小さめの文字で添える」と明記されている。 ただし置き場所は物品名ではなく在庫場所のセルにしている。物品貸出表は 1行=1割り当てなので物品名のセルと備考が同じ行にあるが、確定画面は (物品, 貸出場所) ごとにまとめており物品名は見出しとして1回しか出ない。 備考は割り当てごとに付くため、見出しの下に置くと在庫場所が複数あるときに どの割り当ての備考か分からなくなる。在庫場所のセルなら行と対応が付く。 結果として列数は2列のまま変わらないので、横スクロールは不要になった。 375px幅の実測でも scrollWidth と innerWidth が一致しており、はみ出しはない。 ## 未入力は null のまま返す 空文字に潰すと「空欄が入力されている」と「未入力」が区別できなくなる。 project_name と同じ扱いにした。画面側は値があるときだけ要素を描画するので、 備考の無い行は高さも変わらない。 ## テスト 認証なしで露出するため、group と同じく stocks のキーを固定する契約テストを 追加した。assign_rental_items に列が追加されても勝手に公開されないようにする。 備考が返ること、未入力なら null になることもあわせて固定した。 なお make openapi は routes から src を再生成する docs タスクを含み、本PRと 無関係なファイルの書き換えと、このエンドポイントに存在しない422レスポンスの 追加を行うため使っていない。src を手で更新し、dist には該当スキーマ分のみ反映した。 iPhone 16e のシミュレータで表示を確認し、備考あり・なしの混在と長文の折り返し、 375px幅でのはみ出しが無いことをコンテナ内のPlaywrightで確認している。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKHBgcaSSC6wFtCDj7uLBr
CodeRabbitの指摘への対応。assign_rental_items.remark には空文字を禁じる
制約もバリデーションも無く、assign_rental_items_controller は受け取った値を
そのまま保存するため、備考なしが nil と空文字の2通りになる。
実際に remark: "" を保存するとAPIが "" を返すことを確認した。
前のコミットでは「空欄が入力されている」と「未入力」を区別できるよう
nilのまま返すと書いたが、その区別を使う側が存在しなかった。
出力側は既に present? で同一に扱っている。
output_csv_controller.rb:39 .select { |ari| ari.remark.present? }
output_all_groups_rental_items.pdf.erb:70 <% if ... .remark.present? %>
同じ「備考なし」が2通りのJSONで返るのは、利用側に無意味な分岐を強いるだけ
だったので、既存の扱いに合わせて nil に寄せる。
画面の表示は変わらない。フロントは元から stock.remark && で判定しており
空文字も falsy のため何も描画していなかった。API契約の一貫性の修正になる。
空文字を保存した状態で null が返ることをテストで固定した。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CKHBgcaSSC6wFtCDj7uLBr
- 超過貸出が記録できたら Slack に投稿する。物品・在庫場所・数量に加えて、 貸出元と貸出先の団体名、それぞれの割当数が「元の数 → 変更後の数」で どう変わったかを出す。既存の Slack::Web::Client と同じ作法に揃え、 同じ uid の再送(記録が動かない)では投稿しない。通知に失敗しても記録は 成立させる - 貸出残を「実効割当数 − 貸出中(貸出済 − 返却済)」に変える。返ってきた物は 同じ物なのでまた貸し出せる。5個渡して5個返ったら、また5個渡せる。 これに伴い rental_absolute の上限も 実効割当数+返却済 に広げる。 返却残(貸出中)の定義は変えないので、返しすぎは従来どおり弾く Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
レビュー指摘への対応。stocks の React key に stock_place_name を使っていたが、
同一 stocks 配列内で重複し得るため、割り当てのidを返してキーにする。
## 指摘より到達しやすい経路があった
指摘は「stocker_place_id が NULL の行は UNIQUE 制約が効かず複数作れる」
というものだった。これは成立するが、新規作成APIが exists? で IS NULL 込みの
重複チェックをしているため、通常の作成経路では作れない(PATCH には同じ
チェックが無いので、そこからは作れる)。
それより単純な経路がある。stocker_places.name には UNIQUE も NOT NULL も
モデルのバリデーションも無く、同名の在庫場所を作れる。管理者が同じ部屋を
二重登録し、両方から割り当てるだけで stock_place_name が重複する。
NULL も異常データも要らない。
## 実測で確認した
同名の在庫場所「AL1」を2つ作って両方から割り当てた状態で、ブラウザの
コンソールを比較した。
修正前 key={stock.stockPlaceName} 警告1件
Encountered two children with the same key
修正後 key={stock.id} 警告0件
重複キー自体は #2183 からあったが、行が持つのは在庫場所名と数量だけだった。
今回 remark が加わって行ごとの情報量が増えたため、Reactが行を取り違えた
ときの実害が大きくなり、顕在化した。
## idの公開について
認証なしで露出する項目が増えるため、何が可能になるか確認した。
assign_rental_items は既に読み取りも書き込みも認証なしで全件公開されており
(GET/PATCH/DELETE いずれも401にならない)、idもそこから取得できる。
確定画面がidを返しても、得られる情報も可能な操作も増えない。
この既存の問題自体は #2136 の範囲なので触らず、PRで申し送る。
同名の在庫場所でもidが一意になることをテストで固定した。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CKHBgcaSSC6wFtCDj7uLBr
レビュー指摘: - 再送でカードの編集が反映されるようにする。中身を変えずに送り直すときだけ 1回目と同じ uid を使い、編集されたら別の操作として uid を取り直す。 これまでは常に1回目の内容を送っていたため、上限超過のような本物のエラーだと 同じ内容を送り続けて抜け出せなかった - 409 を成功扱いしない。同じ内容の再送は 200 になるので、409 は 「同じ操作が別の内容で記録済み」を意味する。入れ直しを促す文言にする - Slack通知の「元の数」を行ロック取得後に読む。先に読むと、ほぼ同時の超過貸出で 古い値を変更前として通知していた - 割当数の集計を合算にする。find_by で1行だけ見ていたため、同一(団体×物品× 在庫場所)に割当が複数ある構成では通知の数字が実態と食い違いえた あわせて上限超過のエラー文言を「貸出残(3)を超えています。最新の状況を 確認してください」の形にし、属性名の前置を外して当日そのまま読めるようにする。 ロゴは admin_view と同じシンボルマークを暫定で使う(後で差し替え)。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
feat: 貸出・返却記録アプリ(rental/)のPoC実装
feat: 確定画面に物品割り当ての備考を表示する
…-update fix: 物品の部分更新で英語名が消える問題と保存の成否を直す
取得に失敗するとセレクタが無効になるだけで、Access の設定漏れなのか、 トークン不一致なのか、通信なのかが現地で切り分けられなかった。 APIが返したステータスとメッセージをそのまま添える。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT (cherry picked from commit d3cd7a1)
prod.Dockerfile の runner ステージが APP_ENV を ENV に持っていなかったため、 env_file に APP_ENV が無い環境では実行時に未設定になり、serverEnv.ts が "development" とみなしていた。その場合 Cloudflare Access の検証を飛ばして Cf-Access-Authenticated-User-Email を無検証で信用するため、本番で記録者を 偽装できる状態になる。SSR_API_URL と同じくビルド引数を runner の ENV にする。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT (cherry picked from commit d139010)
本番で読み取り(GET)は通るのに記録(POST)だけ401になっていた。記録系だけが require_rental_recorder_email! を通るため、記録者メールのヘッダーが 届いていないことが原因。 SSR_API_URL が公開URLの構成では BFF から API への区間が Cloudflare を通り、 クライアント由来の Cf- ヘッダーは偽装防止のため削除される。ローカルは http://api:3000 で直接繋ぐため剥がされず、気づけていなかった。 ヘッダー名を X-Rental-Recorder-Email に変え、経路に依存しないようにする。 値は BFF が Access の JWT を検証して取り出したもので、区間はこれまでどおり サービストークン(X-Rental-Api-Token)で守る。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT (cherry picked from commit 21d3ccd)
レビュー指摘: - 設定エラーの内部文言を画面に出さない。BFF は短いコード(access_not_configured など)だけを返し、設定名を含む詳細はサーバー側のログに残す - Cf-Access-* を転送しない理由が4箇所に重複していたので、設計書6章を正とし コード側は一行+参照に縮める - prod.Dockerfile の ARG/ENV を builder ステージと同じくまとめる - compose.prod.yml の rental / user のビルド引数にも :? ガードを付ける。 Compose は未設定の変数を空文字として明示的に渡すため ARG の既定値では埋まらず、 実行時 APP_ENV が空になって Access の検証が飛ぶ状態になりえた - select-place のキャストを1回にまとめる - 旧ヘッダー名を指していたテスト名を直す あわせて、BFFとAPIを同時に入れ替えられなかった場合に備え、新ヘッダーが無いときだけ 旧 Cf-Access-Authenticated-User-Email も見るフォールバックを API 側に入れる。 移行用なので、BFFの入れ替えが行き渡ったら消してよい。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
fix: 本番でrentalの記録が401になる問題を直す
「余り」のように全場所の余り物品をまとめて持つ団体では、同じ団体に同じ物品の 割当が在庫場所ちがいで複数ぶら下がる(割当の一意キーは 団体×在庫場所×物品)。 物品名だけだと「机」「机」と並んでどれか分からず、特に訂正モーダルの候補は 物品名しか出ないため選びようがなかった。 同じ物品が複数あるときだけ `机(講義棟104)` のように在庫場所を添える。 1件しか無い物品には添えない(通常の団体の画面が冗長になるため)。 src/lib/itemLabel.ts に集約し、処理対象アイテムのカード見出し・団体全体の 貸出予定一覧・訂正モーダルの候補・進捗確認の内訳で使う。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
団体一覧は常に「割当がある団体」に絞られていたため、割当を1件も持たない団体は 手動選択にも例外対応の候補にも出てこなかった。予定に無い物品を渡す相手は割当を 持たないことがあり、選べないと記録に辿り着けない。 作業場所を指定したときは従来どおりその場所に割当がある団体だけ。指定しないとき (「すべての場所」)は今年度の全団体を返す。貸出場所が未設定の割当しか持たない 団体も出る。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
feat: 同じ物品が複数あるとき物品名に在庫場所を添える
団体カードが混在していて「まだ終わっていないところ」を拾うのに一覧を 上から見ていく必要があった。未着手 → 進行中 → 完了 の順にまとめ、 終わっていないものを先頭に置く。 見出しには件数を出し、1件も無いステータスは見出しごと出さない。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT (cherry picked from commit 5d6bc72)
…ping feat: 進捗確認の一覧をステータスごとにまとめる
Access の無料枠は月あたりの席数に上限があり、当日のスタッフ数で使い切って しまった。Access を前段に置けない構成でも動かせるようにする。 - RENTAL_PASSCODE を設定すると src/proxy.ts(Next 16 では middleware ではなく proxy)が入口を守る。合言葉が合えば Cookie を発行し以降は素通し。Cookie には 合言葉そのものではなく、そこから決まる固定値を入れる。BFF への呼び出しには HTML ではなく JSON の 401 を返す(fetch がログイン画面を受け取らないため) - この構成では人の識別ができないため、記録者は担当者名の自己申告にする。 画面①で入力させ端末に保持し、毎回ヘッダーで送る。名前が要るのは記録のときだけで、 名前を決める前に通る読み取りでは要求しない - 担当者名は日本語のことがありHTTPヘッダーに載せられないため、BFF が URL エンコードして送り API 側で戻す。+ を含むメールも壊れない - Access と合言葉の両方があれば Access が優先。どちらも無ければローカル開発 以外は従来どおり閉じる Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
CF_ACCESS_* を設定したまま Access のポリシーを Bypass にすると、トークンが 来ないため access_token_missing で閉じてしまい、合言葉を設定していても 動かせなかった。 Access が設定されていてもトークンが来ない場合は合言葉の構成として扱う。 席数が戻ったときにポリシーを戻すだけでよく、CF_ACCESS_* を消したり入れ直したり しなくて済む。合言葉が無い状態でトークンも来なければ従来どおり閉じる。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
担当者名だけだと同姓の人を区別できないため、局名を足す。局は実行委員会の 7局+その他から選ぶ。記録には「局名 担当者名」の形で残す。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
局名と担当者名の間を空白からアンダースコアに変える(局名_担当者名)。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
CGI.unescapeがレガシーヘッダーの'+'を壊す件、JSONエラー組み立ての重複、 STAFF_NAME_HEADERの重複定義、decode/re-encodeの往復、staffHeadersの 再パース、「合言葉」表記の変更希望をそれぞれTODO/FIXMEコメントとして指摘。 挙動は変更していない。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- 旧ヘッダーをURLデコードしない。Cloudflare が付ける生の値なので、 CGI.unescape が生の + を空白に変えて staff+rental@example.com を壊していた - 合言葉の説明文が改行で分断され「局名 担当者名」と表示されていた。 保存する形(局名_担当者名)と一致させる - Access のトークンが来ていれば proxy も合言葉を求めずに通す。Access と合言葉の 両方を設定した移行期間に、Access で認証済みの人まで合言葉を求められていた。 トークンの検証は後段の BFF が行うため守りは緩まない Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
- ヘッダー名とエラー応答の形を src/lib/apiContract.ts に集約する。
X-Rental-Staff-Name が access.ts と apiClient.ts に別々の文字列で
書かれており、片方だけ変えても型エラーにならない状態だった。
proxy.ts と unlock の Route Handler が手組みしていた
{ status: { code, message } } も同じ形に揃える
- 作業セッションのパースを、生の文字列が変わらないあいだは省く。
記録者を添えるために呼び出しのたびに JSON.parse と検証が走っていた
- 画面とドキュメントの「合言葉」を「パスワード」に統一する。
識別子(RENTAL_PASSCODE / passcode)は英語のまま揃える
記録者の decode → re-encode は残している。アプリの中では平文で扱い、
ホップごとに必要な形へ包む設計で、Access のメールと担当者名を同じ型で
扱うために必要なため。その旨をコメントに書いた。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
- テストコメントに残っていた「合言葉」を「パスワード」に揃える - apiClient の宙に浮いていたコメントを staffHeaders の直前に寄せる - apiErrorBody を apiStatusBody に改名する。成功(200)にも使っており 名前と実態が合っていなかった - proxy がヘッダーの有無しか見ないことと、そのために必要な前提 (/api/rental/* は必ず forwardToApi を通す)をコメントに明記する Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcmQ5MoWg4c1wCB46mNAcT
feat: Cloudflare Access の代わりに合言葉で守れるようにする
対応Issue
resolve #0
概要
実装詳細
画面スクリーンショット等
テスト項目
備考