Conversation
例外が発生時に、バリデーションエラーが正常に画面に反映されない問題が修正されます
|
早速のPRありがとうございます! まとまった公式ドキュメントがないのですが、こちらが参考になると思います |
|
@nanasess 既存のテストコードを編集するより、別ファイルで作った方が良いでしょうか? |
|
@eternalharvest ありがとうございます。一応、機能ごとにPHPファイルがわかれていますので、別ファイルにせず、今回修正した機能ごとにテストを追加していただければ嬉しいです。 また、既存の PHPUnit が落ちているようですので、そちらも合わせて修正していただけると大変助かります。 |
|
phpunit の件は修正させていただきました。 もともと、コードを書き換える段階で問い合わせフォームだけなぜかカナフィールドが必須ではないのが気になっていたのですが、他と統一した方が良いのではと思い勝手に仕様を変更していました。 なぜ、問い合わせフォームだけカナがもともと必須フィールドでなかったのかについては理由はわからないですが、設計者の意図があるようにも感じます。問い合わせフォームのテストでカナが必須ではないことを検証するようになっているので、ここの仕様は変えるべきではないと判断しました。私はカナフィールドいらない派なので、問い合わせフォームがカナを必須にできないことは私にとっては問題ではありません。 みなさんの意見を聞いて必要であれば、仕様の変更を検討したいと思います。 |
|
@eternalharvest ご対応ありがとうございます!お問い合わせフォームは、必要最小限の入力でお問い合わせできるようにという配慮から、必須が外れているようです |
d6d3c59 to
a078f8b
Compare
a078f8b to
72c3934
Compare
…ort_option_require_kana
72c3934 to
a137073
Compare
|
ソースコードを現最新Versionに更新した後、Conflictを解消しマージするように動きます。 |
d5d6037 to
35edeb9
Compare
|
一旦 4.3 ブランチをマージしました。 |
概要(Overview・Refs Issue)
#3872 で議論で議題に上がっているように、会員登録や受注編集等の際にカナの入力を強制することは、カゴ落ちの原因になったり、エントリ業務の効率化の妨げになるのでカナ入力を強制しないよう設定変更できるようにしました。
また、これに関連して #4636 で報告されているバグをついでに修正しています。
フリガナ欄のオプション化に際しては互換性を損なわないよう最善の注意を払っていますが、新しい機能をバグ修正パッチと一緒に取り込むことに問題がある場合は、バグ修正コミットを別に分けようと思いますのでご連絡ください。あるいは、チェリーピックしていただいても構いません。
方針(Policy)
互換性を損なわないよう、店舗設定の基本情報で設定できるようにしデフォルトはこれまで通りフリガナの入力を強制します。
オプションの設定内容に関わらず、カナ項目が必須であることを示すバッジ以外の要素が物理的になくなったりすることはありません。
実装に関する補足(Appendix)
テスト(Test)
私の書いたコードの範囲はほぼ UI に関わる部分なのでテストコードは書いていませんが、考えうるコードパスはほぼ全て手動でテストして正しいことを確認しています。客観的に証明出来なくてすいません。
相談(Discussion)
フリガナ項目の非表示化の是非
今回はオプション化のみの実装に留めていますが、そもそもオプションにするくらいならフリガナの項目事態を UI 上からは消し去ってしまったほうが良いのではないか?と悩みましたが、それ以上は本体でやるというよりカスタマイズでやるべき内容なのかなとも思っています。皆さんの意見が聞きたいです。
フォローアップ
この PR を作成したことで、いろんなフォームのコードに目を通すことができました。各フォームで結構、仕様に統一感がなかったりしたのでそういった点はおいおい改善していければ良いなと思っています。フォームはいろんなところに影響が及びそうなので互換性には気をつけないけませんが。
特に、非会員での購入時のショッピングページでお客様情報を編集するフォームだけが AJAX になっていて、他とは異質な存在になっているように思います。もちろん、AJAX ならではのメリットもあるのですが下記 Issue でも指摘されているような問題を抱えています。
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目