ec-cube
ec-cube copied to clipboard
EC-CUBE
管理画面の[設定]-[店舗設定]-[メール設定]に新規テンプレート作成機能、テンプレート削除機能を追加
概要(Overview・Refs Issue)
管理画面の[設定]-[店舗設定]-[メール設定]において、今まではメールテンプレートの更新のみが可能で、テンプレートの新規作成や削除は不可能でした。 このプルリクエストはテンプレートの新規作成機能と削除機能を追加します。
方針(Policy)
既存の機能に変更を加えたり、DBのテーブルを変更せずに済む仕様であることです。 追加分のコードのみで対応できるようにしています。
実装に関する補足(Appendix)
4.0系をベースに4.2向けのコードを作成したため、バージョン違いによるデグレードがないかご確認をお願いします。 また後述しますが、4.2系で動作を確認していないためそちらもどうかよろしくお願いいたします。
テスト（Test)
4.0系において、以下を確認しています。
- テンプレートを「テキスト」「HTML」「テキスト＆HTML」の3パターンで新規作成できること
- テンプレートの更新機能がデグレードしていないこと
- テンプレートの更新時、Twigファイル名を更新できること
- テンプレートを削除できること
- 以下の4種類の入力チェックが動作すること
- Twigファイル名が「.twig」で終わっていない場合
- Twigファイル名が「Mail/」から始まっていない場合
- Twigファイル名に「/」を含む場合（先頭のMail/を除く）
- Twigファイル名を変更したとき、変更先に同じTwigファイル名が存在する場合
ただし、コードは4.2系向けに書き直しており、4.2系で動作を確認していません。 4.0系で動作させるにあたっては、本コードから一部を変更して確認しました。 どなたか、4.2系でどうかご確認いただきますようお願いいたします。
相談（Discussion）
Twigファイル名で実際のファイルを作成するため、スラッシュ(/)の入力を禁止していますが、他に入力を禁止すべき文字がありましたらご指摘をお願いいたします。
マイナーバージョン互換性保持のための制限事項チェックリスト
- [x] 既存機能の仕様変更はありません
- [x] フックポイントの呼び出しタイミングの変更はありません
- [x] フックポイントのパラメータの削除・データ型の変更はありません
- [x] twigファイルに渡しているパラメータの削除・データ型の変更はありません
- [x] Serviceクラスの公開関数の、引数の削除・データ型の変更はありません
- [x] 入出力ファイル(CSVなど)のフォーマット変更はありません
レビュワー確認項目
- [ ] 動作確認
- [ ] コードレビュー
- [ ] E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
- [ ] 互換性が保持されているか
- [ ] セキュリティ上の問題がないか
- [ ] 権限を超えた操作が可能にならないか
- [ ] 不要なファイルアップロードがないか
- [ ] 外部へ公開されるファイルや機能の追加ではないか
- [ ] テンプレートでのエスケープ漏れがないか
メールテンプレートの新規作成機能は求められることが多いのでこのprは是非取り込んで欲しいのですが、 こちらの修正も含んでもらえれば嬉しいです。
https://github.com/EC-CUBE/ec-cube/blob/4.3/src/Eccube/Form/Type/Admin/OrderMailType.php#L56 今だと注文メールしか選択できないため条件を外してもらえればと思います。
方針として「既存の機能に変更を加えたり、DBのテーブルを変更せずに済む仕様であることです。」とありますが、 https://github.com/EC-CUBE/ec-cube/pull/4440 こちらのprのようにedit_typeカラムを一つ用意して対応してもらう方法もあります。
@k-yamamura ご提案ありがとうございます。社内で仕様について検討しようと思いますが、ご教示いただいた https://github.com/EC-CUBE/ec-cube/pull/4440 とどちらの案がおすすめとかありしたら先にご教示いただけますでしょうか？
@k-yamamura こちら別チケットとして起票致しました。 詳細な要望などあれば、コメントよろしくお願い致します。 https://github.com/EC-CUBE/ec-cube/issues/6097
Codecov Report
Attention: 66 lines in your changes are missing coverage. Please review.
Comparison is base (
cc2ee13) 82.45% compared to head (0dea232) 82.30%.
| Files | Patch % | Lines |
|---|---|---|
| ...e/Controller/Admin/Setting/Shop/MailController.php | 24.13% | 66 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## 4.3 #5836 +/- ##
============================================
- Coverage 82.45% 82.30% -0.15%
- Complexity 6465 6477 +12
============================================
Files 477 477
Lines 25946 26007 +61
============================================
+ Hits 21394 21406 +12
- Misses 4552 4601 +49
| Flag | Coverage Δ | |
|---|---|---|
| E2E | 69.27% <34.00%> (-0.19%) |
:arrow_down: |
| Unit | 79.52% <34.00%> (-0.15%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@ushui こちらローカルで動作確認を行いました。 確認したいことが2点あります。
-
新規作成の仕方があっているかわかりませんでした。新規作成する方法をご教示ください。 →「Twigファイル名」に入力し、命名規則に則してテンプレートを作成したのですがテンプレートが増えませんでした。既存のテンプレート名を選択して別twig名を指定し行った場合、登録はされますがテンプレート名の登録方法がわかりませんでした。
-
「Twigファイル名」の入力欄ですが、先頭の
Mail/や末尾の.twigなどは必ず入力しないといけません。ここは思い切って、ファイル名の拡張子なし（例えば、注文受付メールなら「order」だけにする）でも良いのではと思いました。
他の方のご意見も伺いたいです。
