登録メール送信時の警告を修正 #982

[clicktx]

fixed #982

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.29%. Comparing base (66445b3) to head (d989c3d).
Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
+ Coverage   55.63%   56.29%   +0.66%     
==========================================
  Files          75       75              
  Lines        8917     8917              
==========================================
+ Hits         4961     5020      +59     
+ Misses       3956     3897      -59     

Flag	Coverage Δ
tests	56.29% <100.00%> (+0.66%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[seasoftjapan]

フロントに不具合が出る様子です。

SC_Helper_Mail::sfSendRegistMail() は、LC_Page_Admin_Customer からのみ使われており、LC_Page_Entry 及び lfSendRegistMail は、各ページクラス内に類似した処理を抱えている様子です。一方で、メールテンプレートは共用しているようです。

LC_Page_Entry 及び LC_Page_Regist も、SC_Helper_Mail::sfSendRegistMail() を呼び出して済ませばスマートな予感がします。

現況は LC_Page_Regist のみ Bcc を送信していないので、その部分で差異がでそうです。ただ、その仕様も不自然なので、何れも送信で統一されて良い気がします。

[clicktx]

LC_Page_Admin_xxxクラスにメール送信処理が散らばっていて、テストしにくかったので後回しにしたのですが、影響ありましたね。。。

とりあえずテンプレートを戻すだけにしても良いかも知れませんが、回避案のようにSC_Helper_Mail::sfSendRegistMail() にまとめた方がスマートですね

[clicktx]

どっちに書くべきか迷ったのですが、こちらに。

        if ($arrCustomerData['status'] == 1
             && (CUSTOMER_CONFIRM_MAIL == true || $resend_flg == true)
         ) {...

PR頂いて、条件式のリファクタリング部分を見ていて思ったのですが、
$arrCustomerData['status'] == 1 = 「仮会員」の状態の時は「会員登録のご確認」メールを出すはずですよね？

条件を満たさないと「会員登録のご完了」メールが送信されるようになっている、と。
そうなると実は && (CUSTOMER_CONFIRM_MAIL == true || $resend_flg == true) も不要なのではないかと思いました。

• CUSTOMER_CONFIRM_MAIL == trueじゃないと仮会員にならないはず
• $resend_flg はそもそも不要なのでは説

ec-cube2/data/class/pages/entry/LC_Page_Entry.php

Lines 194 to 195 in fc5a36a

	// 仮会員 1 本会員 2
	$arrResults['status'] = (CUSTOMER_CONFIRM_MAIL == true) ? '1' : '2';

ec-cube2/data/class/pages/regist/LC_Page_Regist.php

Lines 71 to 72 in fc5a36a

	$registSecretKey = $this->lfRegistData($_GET); //本会員登録（フラグ変更）
	$this->lfSendRegistMail($registSecretKey); //本会員登録完了メール送信

$arrCustomerData['status'] == 1の状態の会員に「会員登録のご完了」メールを送る状況ってあるのでしょうか？？

[seasoftjapan]

@clicktx 今回は、あくまでリファクタリングの範囲に留めて処理内容は変更していませんが、 私も漠然と違和感はありました。

もしかすると、CUSTOMER_CONFIRM_MAIL を途中変更した場合の考慮でしょうか。(実際に目的を達成しているか未検証です。)

第1引数・第2引数の被りも違和感ありますし、第3引数も今となっては不要なはずですし、色々と改善の余地はありそうに思います。本 Issue でどこまで扱うか難しいところですが。

[nanasess]

@seasoftjapan @clicktx
単に考慮できていなかっただけのように見えますね。
別の issues で対応しましょうか。

[clicktx]

@seasoftjapan @clicktx 単に考慮できていなかっただけのように見えますね。 別の issues で対応しましょうか。

issueたてました
#1000 (comment)

[nanasess]

古いフォーマットのメールテンプレートの互換テストが欲しいので別途 issues 登録しますね

[nanasess]

LGTM