Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

登録メール送信時の警告 SC_Helper_Mail::sfSendRegistMail() #982

Closed
clicktx opened this issue Aug 27, 2024 · 12 comments · Fixed by #989
Closed

登録メール送信時の警告 SC_Helper_Mail::sfSendRegistMail() #982

clicktx opened this issue Aug 27, 2024 · 12 comments · Fixed by #989
Milestone

Comments

@clicktx
Copy link
Contributor

clicktx commented Aug 27, 2024

$objMailText->assignobj($arrCustomerData);

該当部分で警告が出ます。
Warning(E_WARNING): get_object_vars() expects parameter 1 to be object, array given

assignobj()ではなくassignarray()を使うのが正しいのでしょうか?

$objMailText->assignarray($arrCustomerData);

また、関連して、assignarray()で検索すると下記のコードが見た感じArrayを引数にしているのですが、実際にはObjectなので警告が出ません。
$objTplVarなどに変数名を変えておいた方が良いのかも知れません。

$objMailView->assignobj($arrTplVar);

@seasoftjapan
Copy link
Contributor

動作未検証ですが、以下が必要かも疑問に思いました。

$objMailText->assign('name01', $arrCustomerData['name01']);
$objMailText->assign('name02', $arrCustomerData['name02']);

@nanasess
Copy link
Contributor

会員登録メールは name01, name02 しか使ってないので assignobj は無視されてるようですね。
削除してしまうのが良いかも

@seasoftjapan
Copy link
Contributor

カスタマイズで email, company_name あたりを追加するとかは割合あるので、assignarray() で支障なく動作するのであれば、その方がロジック変更無しでカスタマイズできて便利とは思います。

本当は、SC_Helper_Mail::sfSendOrderMail() の $arrTplVar->arrCustomer みたいな方が分かりやすいですね。

$arrTplVar->arrCustomer = $arrCustomer;

現在の所持ポイント <!--{$arrCustomer.point|default:0|n2s}--> pt

@clicktx
Copy link
Contributor Author

clicktx commented Aug 28, 2024

本当は、SC_Helper_Mail::sfSendOrderMail() の $arrTplVar->arrCustomer みたいな方が分かりやすいですね。

現在の所持ポイント <!--{$arrCustomer.point|default:0|n2s}--> pt

分かりやすいし、使いやすそうです。
customerに関する事を出力したい時多いですし。
ただ、他の箇所も同じ仕様にしたくなり、メールテンプレートも修正する必要があるため互換性が無くなりそうですね。
メールテンプレートカスタマイズしている勢としては気づきにくい変更となりそうです。。。
前のassignも残しつつ、追加するなら問題なさそうですかね。

$objMailText = new SC_SiteView_Ex();
$objMailText->setPage($this->getPage());
$objMailText->assign('CONF', $CONF);
$objMailText->assign('name01', $arrCustomerData['name01']);
$objMailText->assign('name02', $arrCustomerData['name02']);
$objMailText->assign('uniqid', $arrCustomerData['secret_key']);
$objMailText->assign('arrCustomer', $arrCustomerData);

動作未検証ですが、以下が必要かも疑問に思いました。

$objMailText->assign('name01', $arrCustomerData['name01']);
$objMailText->assign('name02', $arrCustomerData['name02']);

assignarray()で動作するなら不要になりそうな気はします。

@nanasess
Copy link
Contributor

@clicktx @seasoftjapan
以下のような感じでいかがでしょう?

  • 実装は assignarray に修正する
  • name01, name02 の assign は削除
  • 拡張用として arrCustomer も使えるようにする(何故この変数があるかをコメントしておく)

@seasoftjapan
Copy link
Contributor

テンプレート側も標準で $arrCustomer を使うのはどうでしょうか?

-<!--{$name01}--> <!--{$name02}--> 様
+<!--{$arrCustomer.name01}--> <!--{$arrCustomer.name02}--> 様
-<!--{$smarty.const.HTTPS_URL}-->regist/<!--{$smarty.const.DIR_INDEX_PATH}-->?mode=regist&id=<!--{$uniqid}--><!--{$etc_value}-->
+<!--{$smarty.const.HTTPS_URL}-->regist/<!--{$smarty.const.DIR_INDEX_PATH}-->?mode=regist&id=<!--{$arrCustomer.secret_key}--><!--{$etc_value}-->

互換性を考慮すると、ロジックは以下のイメージでしょうか。(動作未検証)

$objMailText = new SC_SiteView_Ex();
$objMailText->setPage($this->getPage());
$objMailText->assign('CONF', $CONF);
$objMailText->assign('arrCustomer', $arrCustomerData);

// 旧テンプレート互換用 https://github.com/EC-CUBE/ec-cube2/issues/982
$objMailText->assignarray($arrCustomerData);
$objMailText->assign('uniqid', $arrCustomerData['secret_key']);

@nanasess
Copy link
Contributor

@seasoftjapan 過去のテンプレートもそのまま動くようでしたら、こちらがよさそうですね

@clicktx
Copy link
Contributor Author

clicktx commented Aug 30, 2024

  • customer_regist_mail.tpl
  • customer_mail.tpl

関係するテンプレート、2つあるのですね。

テスト書いたのですが、
62d44b0

両者のテンプレート、宛名部分が若干違っている(姓と名の間に半角スペースあり、なし)のですが、そのままにしています。

@clicktx
Copy link
Contributor Author

clicktx commented Aug 30, 2024

if (CUSTOMER_CONFIRM_MAIL == true and $arrCustomerData['status'] == 1 or $arrCustomerData['status'] == 1 and $resend_flg == true) {
$subject = $objHelperMail->sfMakeSubject('会員登録のご確認', $objMailText);
$toCustomerMail = $objMailText->fetch('mail_templates/customer_mail.tpl');
} else {
$subject = $objHelperMail->sfMakeSubject('会員登録のご完了', $objMailText);
$toCustomerMail = $objMailText->fetch('mail_templates/customer_regist_mail.tpl');

本件とは全く関係ないのですが、このメールタイトルの日本語が気になっています。
自分は「会員登録が完了しました」に直して使っていますが。。。

@clicktx
Copy link
Contributor Author

clicktx commented Aug 30, 2024

テンプレート側も標準で $arrCustomer を使うのはどうでしょうか?

-<!--{$name01}--> <!--{$name02}--> 様
+<!--{$arrCustomer.name01}--> <!--{$arrCustomer.name02}--> 様
-<!--{$smarty.const.HTTPS_URL}-->regist/<!--{$smarty.const.DIR_INDEX_PATH}-->?mode=regist&id=<!--{$uniqid}--><!--{$etc_value}-->
+<!--{$smarty.const.HTTPS_URL}-->regist/<!--{$smarty.const.DIR_INDEX_PATH}-->?mode=regist&id=<!--{$arrCustomer.secret_key}--><!--{$etc_value}-->

互換性を考慮すると、ロジックは以下のイメージでしょうか。(動作未検証)

$objMailText = new SC_SiteView_Ex();
$objMailText->setPage($this->getPage());
$objMailText->assign('CONF', $CONF);
$objMailText->assign('arrCustomer', $arrCustomerData);

// 旧テンプレート互換用 https://github.com/EC-CUBE/ec-cube2/issues/982
$objMailText->assignarray($arrCustomerData);
$objMailText->assign('uniqid', $arrCustomerData['secret_key']);

こちらに変更してテスト通りました。

@seasoftjapan
Copy link
Contributor

両者のテンプレート、宛名部分が若干違っている(姓と名の間に半角スペースあり、なし)のですが、そのままにしています。

関連した Issue を登録しました。#991

@clicktx
Copy link
Contributor Author

clicktx commented Sep 5, 2024

// 仮会員が有効の場合
if (CUSTOMER_CONFIRM_MAIL == true and $arrCustomerData['status'] == 1 or $arrCustomerData['status'] == 1 and $resend_flg == true) {

ここの条件式に関してコメント書いたのですが、こちらにも残しておきます。

#989 (comment)

nanasess added a commit that referenced this issue Sep 9, 2024
登録メール送信時の警告を修正 #982
@nanasess nanasess added this to the 2.18(仮) milestone Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants