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

商品規格ごとの税率設定を削除 #301

Merged
merged 8 commits into from
Sep 17, 2019

Conversation

nanasess
Copy link
Contributor

@nanasess nanasess commented Aug 21, 2019

商品一覧及び商品詳細で商品別税率が適用されず、とり急ぎ応急処置したい場合は以下の SQL を実行することで改善可能

UPDATE dtb_tax_rule SET product_class_id = 0;

@nanasess
Copy link
Contributor Author

data/Smarty/templates/admin/products/product.tpl の差分がわかりづらいですが、フォームの位置を移動しているだけです

@nobuhiko
Copy link
Contributor

@nanasess 商品規格ごとの税率設定をなくすだけで #99 の問題解決するんですか?

@coveralls
Copy link

coveralls commented Aug 21, 2019

Coverage Status

Coverage remained the same at 43.127% when pulling deb171d on nanasess:improve/product-tax into 7c98671 on EC-CUBE:improve/php7.

@nanasess
Copy link
Contributor Author

@nobuhiko dtb_tax_rule.product_class_id は常に0が入るようになるので、 SC_Product::setIncTaxToProduct()SC_Helper_TaxRule_Ex::sfCalcIncTax() でも product_id を渡すのみで正常に税込価格が計算されます

@chihiro-adachi chihiro-adachi added this to the 2.17.0 milestone Aug 22, 2019
@nomoto-neo
Copy link

nomoto-neo commented Aug 22, 2019

admin/products/confirm.tpl
admin/products/product_class_confirm.tpl が漏れてるようです

@nanasess
Copy link
Contributor Author

@nomoto-neo 対応しました。ありがとうございます!

- dtb_tax_rule.product_class_id は常に 0 になるため
- 商品別税率 ON の場合に商品規格ごとに SQL がコールされるのを防止
@nanasess
Copy link
Contributor Author

@nomoto-neo setTaxRuleForProduct で、 dtb_tax_rule.product_class_id は常に0 を入れるようにしたので大丈夫だと思いますが、いかがでしょうか?

@nomoto-neo
Copy link

dtb_tax_rule の既存データは product_class_id に値が入ってしまっているので、読み取れなくなりますね。SC_Helper_TaxRule_Ex::setTaxRuleでもdtb_tax_rule.product_class_idは0に更新されないので、dtb_tax_ruleの既存データはDBのデータ更新が必要になります。

@nanasess
Copy link
Contributor Author

@nomoto-neo はい、既存のデータは更新が必要です

@nomoto-neo
Copy link

nomoto-neo commented Aug 23, 2019

product_class_idに意味がなくなったのでしたら、SC_Helper_TaxRule_Ex::getTaxRule をこうすれば既存データの修正も不要になりませんか?
// 商品税率有無設定により分岐
(
(product_id = 0 OR product_id = ?)
AND (product_class_id = 0 OR product_class_id = ?)
)

(product_id = 0 OR product_id = ?)
だけに。

@nanasess
Copy link
Contributor Author

@nomoto-neo
product_class_idに意味がなくなったのでしたら、SC_Helper_TaxRule_Ex::getTaxRule をこうすれば既存データの修正も不要になりませんか?

これをやると、SC_Helper_TaxRule::getTaxRule() の振舞いも変わってしまう(意味の無い引数が残存する)のと、カスタマイズ時の余地を残したいため、既存のデータ修正をする方針でいきたいです。

@nomoto-neo
Copy link

既存データが product_class_id > 0 で読み出せないという話なので =0 固定は意味ないですね。
やはりデータを修正するのがスマートかな? update dtb_tax_rule set product_class_id=0; 一発だし。

@nobuhiko
Copy link
Contributor

@nomoto-neo 失礼しました

AND (product_class_id = 0 OR product_class_id = ?) を product_class_id == 0 の時は無視するようにすれば振舞いは変わらないのではないでしょうか

@nanasess
Copy link
Contributor Author

@nobuhiko AND (product_class_id = 0 OR product_class_id = ?) を product_class_id == 0 の時は無視するようにすると既存のテストが落ちちゃうので、そこを修正してまで SC_Helper_TaxRule::getTaxRule() を修正しなくても良いかなと思ってます🤔

1) SC_Helper_TaxRule_getTaxRule_OptionProductTaxRuleTest::商品idを指定した場合商品に設定かつ適用日時内の最新の値が返される
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    'apply_date' => '2014-02-02 00:00:00'
-    'tax_rate' => '8'
+    'apply_date' => '2014-02-03 00:00:00'
+    'tax_rate' => '9'
     'product_id' => '1000'
-    'product_class_id' => '0'
+    'product_class_id' => '2000'
     'del_flg' => '0'
 )

@chihiro-adachi
Copy link
Contributor

@nobuhiko @nomoto-neo @nanasess
こちらのPRですが、データ修正の方針で合意ができているのかなと認識しています。
マージさせていただく予定ですが、もしその他ご意見あればコメントいただければと思います。

@chihiro-adachi chihiro-adachi merged commit 1aafcfc into EC-CUBE:improve/php7 Sep 17, 2019
@chihiro-adachi
Copy link
Contributor

@nanasess 遅くなりましてすみません。PRありがとうございます。

@nanasess nanasess deleted the improve/product-tax branch October 3, 2022 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants