PHP8 対応漏れ？「受注管理＞受注登録」画面でシステムエラー #829、Warning 回避

[seasoftjapan]

No description provided.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 55.61%. Comparing base (fc5a36a) to head (c80bc7c).

Files with missing lines	Patch %	Lines
data/class/util/SC_Utils.php	50.00%	2 Missing ⚠️
data/class/SC_FormParam.php	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #836      +/-   ##
==========================================
- Coverage   55.63%   55.61%   -0.02%     
==========================================
  Files          75       75              
  Lines        8917     8920       +3     
==========================================
  Hits         4961     4961              
- Misses       3956     3959       +3     

Flag	Coverage Δ
tests	55.61% <80.00%> (-0.02%)	⬇️

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.

[nanasess]

LGTM

[bbkids]

PHP8 Warning 回避 （b366a0f)
の修正を入れましたところ、
数量欄に「あ」を入力し、[計算結果の確認]ボタンを押すと
当方の環境では、システムエラーが出るようになりました。一応お知らせ致します。

[/manager/order/edit.php] Fatal error(E_ERROR): Uncaught TypeError: Unsupported operand types: int + string in data/class/pages/admin/order/LC_Page_Admin_Order_Edit.php:1212

[bbkids]

すみません、今試してみましたところ、
当初の私の検証ミスの様でして、
PHP8 Warning 回避 （b366a0f)
の修正以前でも、
数量欄のみ改善させていない様で御座います。

具体的には、
数量欄に「あ」を入力し、[計算結果の確認]ボタンを押すと
当方の環境では、システムエラーで落ちます。

その場合のエラーログは、
[/admin/order/edit.php] Fatal error(E_ERROR): Uncaught TypeError: Unsupported operand types: string * string in data/class/util/SC_Utils.php:895

[nanasess]

@bbkids @seasoftjapan

数量欄のみ改善させていない

こちらは別途 issues で対応したほうが良いですかね?

[bbkids]

数量欄のみ改善させていない

すみません、「数量欄のみ改善されていない」の間違いでした。

別途 issues でOKです。

[seasoftjapan]

@bbkids 不具合報告ありがとうございます。
@nanasess #836 (comment) の再現を確認しましたが、同じ画面内で且つ性質は似ているので、Issue は #829 で良さそうな気がします。一旦 fixed #829 の記述を削除しました。マージに間に合わなかったら PR は追加します。

[seasoftjapan]

まずは、#836 (comment) への対応です。

lfCheckError() の前に setProductsQuantity() が呼ばれていたり、
lfCheckError() を通過せずに setProductsQuantity() が呼ばれていたりする状況でした。

setProductsQuantity() の前に lfCheckError() を呼び、エラーがあったら switch を break するよう調整しています。

変更箇所が多いですが、十分にデバッグできていないため、一旦別ブランチに push すると思います。

【追記】
push しました。
https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2

[bbkids]

この修正以降、
商品追加で一度　規格1と規格2がある商品を追加し、その追加した商品を規格がない商品に変更をすると

public function changeShipmentProducts(); の中の以下の箇所で、エラー落ちします。

$arrShipmentProducts['shipment_classcategory_name1'][$shipping_id][$no] = $arrProductInfo['classcategory_name1'];
$arrShipmentProducts['shipment_classcategory_name2'][$shipping_id][$no] = $arrProductInfo['classcategory_name2'];

Fatal error(E_ERROR): Cannot assign an empty string to a string offset が出ます。
また、その逆で、規格がない商品を一度追加して、その追加した商品を規格1と規格2がある商品に変更すると
同じ個所で、 Warning(E_WARNING): Only the first byte will be assigned to the string offset がでます。

[bbkids]

$arrShipmentProducts[]を辿ってみましたところ、
入れ子の配列のについて初期化されていなかったようで、NULLがセットされエラー落ちしていたものと考えられます。

$arrShipmentProducts['shipment_classcategory_name1'][$shipping_id] = [];
$arrShipmentProducts['shipment_classcategory_name2'][$shipping_id] = [];

changeShipmentProducts()の中で値をセットする直前に、配列として初期化したところエラー落ち解消されました。
LC_Page_Admin_Order_Edit全体を把握できていないので、ここでの初期化が適切かどうかは何とも言えません。

【追記】
その後、
https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2 に上記をマージしたものを一通り試して見ましたが、エラー落ちはなくなりました。
Warninngは少々残ります（かなり減りました）が、当方が試した限りは、おおむね良さそうな感じで御座います。

[seasoftjapan]

@bbkids ご報告ありがとうございます。
イレギュラーな操作をすると、他にもPHPエラーありますね。

当プルリクをドラフトに変更して、もう少しじっくりと調整しようかと思います。併せて、seasoft-829-2 ブランチに push していた内容を、プルリク対象ブランチに push し直す予定です。
(@nanasess それで、自動マージになっても master にマージされなくなると想定していますが、違いましたらご指摘ください。)

この画面は、PHPエラー以外の既存バグも多々あると思うので、どこで切りをつけるか難しくなりそうです。

要調整とのことですので一旦 approve を取り下げす

[seasoftjapan]

本件、再開しようと思いますが、経緯が分からなくなってしまいました。

特に https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2 (8fac1d1) の役割を思い出せず、一旦黙殺する予定です。

まずは、https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829 (a23d6b7) に、#836 (comment) で報告いただきました配列要素の初期化漏れを改修しようと思います。

[seasoftjapan]

特に https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2 (8fac1d1) の役割を思い出せず、一旦黙殺する予定です。

https://github.com/seasoftjapan/eccube-2_13/tree/seasoft-829-2 の内容は、本件 PR のブランチにチェリーピックしてありました。(fe9bdf7)

[bbkids]

data/class/pages/admin/order/LC_Page_Admin_Order_Edit.php　の 625行目

$arrShippings[$row['shipping_id']] = $row;
　　　　　　　　　　↓
$arrShippings[$row['shipping_id']] = array_intersect_key($row, array_flip($this->arrShippingKeys));

この修正を入れると、商品削除できなくなりませんか？
また商品変更で、既存商品と重複する商品を選ぶと落ちるようです。