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

Fix #4209 商品の在庫数が足りないときに受注のステータスを「注文取消し」から「対応中」に変更しすると、システムエラーが発生する不具合を修正 #4400

Merged
merged 5 commits into from Mar 3, 2020

Conversation

yoshiharu-semachi
Copy link
Contributor

@yoshiharu-semachi yoshiharu-semachi commented Nov 29, 2019

概要(Overview・Refs Issue)

修正したIssue:#4209

このPull Requestは以前作成したPR #4379 の修正版となります。
okazyさんに上記PRのレビューをしていただき、その内容を踏まえて修正しました。

実装に関する補足(Appendix)

StockDiffProcessor

  • ステータスを「キャンセル」から「対応中」に変更した場合の条件分岐をvalidateメソッドに追加

EditController

  • try-catch文を追加しました。

参考

受注対応状況の流れ - EC-CUBE 4.0 開発者向けドキュメント
Service のカスタマイズ - EC-CUBE 4.0 開発者向けドキュメント

テスト(Test)

以下の手順で「<商品名>の在庫が足りません。」と表示されることを確認

  1. 新規受注を登録する。
  2. 受注管理より、対応状況:注文取消し数量:1に設定し商品を登録する。
  3. 商品管理より、受注管理で登録した商品を編集し在庫数:0で登録する。
  4. 受注管理で登録した受注情報の編集画面に移り、受注より対応状況:対応中に変更し登録する。
  5. <商品名>の在庫が足りません。」と表示される。
  6. 商品一覧より、商品の在庫数:0のままになっていることを確認。

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

Copy link
Contributor

@okazy okazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1点コメントしました。

落ちているテストについて、ここはテスト自体が間違っているので、テストを修正した方が良いかと思います。

落ちているテストは以下の関数です。

/**
* @dataProvider validateProvider
*
* @param $stock int 在庫数
* @param $beforeQuantity int 編集前の商品の数量
* @param $afterQuantity int 編集後の商品の数量
* @param $isError bool バリデーションエラーがある場合はtrue
* @param $beforeOrderStatus int 編集前の受注ステータス
* @param $afterOrderStatus int 編集後の受注ステータス
*/
public function testValidate($stock, $beforeQuantity, $afterQuantity, $isError, $beforeOrderStatus, $afterOrderStatus)

テストケースについては以下の関数で定義されています。

public function validateProvider()
{
return [
[10, 2, 12, false, OrderStatus::NEW, OrderStatus::NEW],
[1, 3, 2, false, OrderStatus::NEW, OrderStatus::PAID],
[1, 1, 2, false, OrderStatus::NEW, OrderStatus::IN_PROGRESS],

テストケースの中の以下のテストケースが落ちていました。

[1, 1, 2, false, OrderStatus::CANCEL, OrderStatus::IN_PROGRESS],

配列の要素はそれぞれ以下を表します。

  • 在庫数:1
  • 編集前の商品の数量:1
  • 編集後の商品の数量:2
  • バリデーションエラーの有無:なし
  • 編集前の受注ステータス:キャンセル
  • 編集後の受注ステータス:対応中

今回の修正でこのケースでは、エラーになるのが正しい挙動となりました。
このケースはエラーなしのケースとするのが良いかと思いますので、「編集後の商品の数量」を1個にする修正でいかがでしょうか。

@okazy okazy added the bug:Low label Nov 29, 2019
@okazy okazy added this to the 4.0.4 milestone Nov 29, 2019
@okazy okazy merged commit a0b9463 into EC-CUBE:4.0 Mar 3, 2020
@okazy
Copy link
Contributor

okazy commented Mar 3, 2020

@yoshiharu-semachi
ありがとうございます!取り込みました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants