fix(mail): メールテンプレートの更新時に削除可否を変更しないようにする (#7053) - #7079
Conversation
システム用テンプレート（注文受付メール等）を1回保存すると削除ボタンが出現し、 削除できてしまう。削除すると dtb_mail_template の行だけでなく Mail/order.twig 等の テンプレートファイルも消えるため、メールが送れなくなる。 原因は aa6f4f8「削除ボタン表示のタイミングを調整」が setDeletable(true) を MailType の POST_SUBMIT から MailController へ移した際に、元々あった `if (null === $data->getId())`（新規のみ）の条件を落としたこと。 保存パスで新規のときだけフラグを立てるようにする。判定は MailType が既に 2箇所（file_name 項目の追加・ファイル名重複チェック）で使っている作法に揃えた。 delete() には既に isDeletable() のガードがあるため変更不要。 なお次の形はいずれも成立しない。 - 行を単に削除する: MailTemplate::$deletable の既定が false なので、 ユーザーが作成したテンプレートも削除できなくなる - 生成時に立てる: 新規画面でもビューに Mail を渡しており、未保存のフォームに 削除ボタンが出る。モーダルの削除リンクは id が null で壊れた URL になる テストは「削除不可のテンプレートを更新しても削除不可のまま」を追加し、 新規側の退行ガードとして testCreate に isDeletable() のアサートを足した。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughメールテンプレートの保存時に、削除可能フラグを新規登録時だけ設定します。更新時は既存の削除可否を保持します。新規登録と更新のテストを追加します。 Changesメールテンプレートの削除可否制御
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change keeps existing system mail templates non-deletable when updated while preserving deletion for newly created templates. No actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #7079 +/- ##
==========================================
+ Coverage 77.67% 77.71% +0.03%
==========================================
Files 597 597
Lines 29333 29334 +1
==========================================
+ Hits 22785 22796 +11
+ Misses 6548 6538 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@ttokoro20240902 |
2 participants
概要(Overview・Refs Issue)
fixes #7053
システム用テンプレート（注文受付メール等）を 1 回保存すると削除ボタンが出現し、削除できてしまう不具合を修正します。
削除すると
dtb_mail_templateの行だけでなくMail/order.twigなどのテンプレートファイルも消えるため（MailController::delete()）、メールが送信できなくなり、復旧にはファイルの復元が必要になります。方針(Policy)
原因は
aa6f4f85be「削除ボタン表示のタイミングを調整」（2024-04-16）です。setDeletable(true)をMailTypeのPOST_SUBMITからコントローラへ移した際に、元々あったif (null === $data->getId())（新規のみ）の条件が落ちていました。そのため保存パスで新規のときだけフラグを立てる形に戻します。判定方法は
MailTypeが既に 2 箇所（file_name項目の追加・ファイル名重複チェック）で使っているnull === $data->getId()に揃えました。delete()には既にif (!$Mail->isDeletable())のガードがあるため、そちらは変更していません。検討して採らなかった案
setDeletable(true)の行を単に削除する —MailTemplate::$deletableは既定falseなので、ユーザーが作成したテンプレートも削除できなくなります。MailControllerTest::testDelete()とadmin-basicinfo.spec.tsの「テンプレート新規作成 → 削除」が落ちます$Mail ??= new MailTemplate()）に立てる / Repository に生成メソッドを新設する —index()は新規画面でもビューにMailを渡しており、mail.twigが{% if Mail.isDeletable %}で削除ボタンと削除モーダルを出し分けています。生成時に立てると未保存の新規フォームに削除ボタンが出ます。モーダルの削除リンクはurl('admin_setting_shop_mail_delete', {id: Mail.id})でMail.idがnullになり壊れた URL になります。これはaa6f4f85beが調整した「タイミング」そのものですなお同じ
deletable列を持つBlockではBlockRepository::newBlock()が生成時にだけフラグを立て、BlockControllerの保存処理はdeletableに一切触れません。本修正はこの作法と整合します。テスト（Test)
MailControllerTestに以下を追加しました。testEditKeepsNotDeletable()— 削除不可のテンプレートを更新しても削除不可のままであることを確認testCreate()に「新規登録したテンプレートは削除できる」アサートを追加（aa6f4f85beの意図に対する退行ガード）修正前は
testEditKeepsNotDeletableがFailed asserting that true is false.で落ちることを確認済みです（空振りでないことの担保）。MailControllerTest（10 本）vendor/bin/phpstan analyse srcvendor/bin/php-cs-fixer fix --dry-run（変更 2 ファイル）実機（
APP_ENV=dev）でも Issue の再現手順を踏んで確認しました。dtb_mail_templateのdeletableもfalseのまま実装に関する補足(Appendix)
4.3.0/4.3.0-rc/4.3.1/4.3.1-p1に出荷済みで、システム用テンプレートを 1 回でも保存した環境ではdeletableがtrueのまま残ります。補正が必要であれば、app/DoctrineMigrationsの既存の作法（Version20200303053716のように id と旧値でガードしたUPDATE）で対応できますが、Issue のスコープを超えるため本 PR には含めていません。別途ご判断くださいorigin/4.3にも同じ形で残っています（MailController.php:101）。4.3 系への対応が必要かはメンテナ判断にお任せしますe2e/tests/admin-basicinfo.spec.tsは出荷通知メール（id=8）を保存しており、修正前はこの spec を流すだけで id=8 のdeletableがtrueに書き換わっていました。 修正後は書き換わりませんマイナーバージョン互換性保持のための制限事項チェックリスト
新規登録時の挙動は従来どおり（削除可能）で、変わるのは「更新時に
deletableを書き換えない」点のみです。レビュワー確認項目
🤖 Generated with Claude Code
Summary by CodeRabbit
不具合修正
テスト