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

商品画像アップロード時のリサイズ処理 #4349

Closed
wants to merge 6 commits into from
Closed

商品画像アップロード時のリサイズ処理 #4349

wants to merge 6 commits into from

Conversation

Rights1995
Copy link

概要(Overview・Refs Issue)

#4347

方針(Policy)

2系であった画像アップロード時のリサイズ処理について、
4系ではアップロード画像の幅、高さやサイズの指定が運用レベルで加工が必要なため
アップロード時に、パラメータで指定した画像ファイルに加工する。

実装に関する補足(Appendix)

アップロード後の画像サイズや、jpg、pngファイルの圧縮率の指示がベタ書きなっています。

テスト(Test)

既に実稼働の環境での要望があり、実稼働サイトのプレビューサイトで、各画像ファイルのアップロードテストを行い、実稼働の環境に適用しています。

相談(Discussion)

補足にあげていますが、アップロード後の画像サイズや、jpg、pngファイルの圧縮率の指示がベタ書きなっています。設定が変更できるようになると尚良いと考えています。

@Rights1995 Rights1995 changed the title Update ProductController.php 商品画像アップロード時のリサイズ処理 Oct 9, 2019
@chihiro-adachi
Copy link
Contributor

@Rights1995
さっそくのPullRequestありがとうございます🥇
確認してコメントさせていただきますね。

@chihiro-adachi
Copy link
Contributor

chihiro-adachi commented Oct 10, 2019

相談事項に書いていただいている通り、

  • 縦横の画像サイズ
  • 圧縮率

はパラメータ指定できたほうがよいかなと思いました。

また、リサイズが不要なケースもあるかと思いますので、リサイズのON/OFFができればと(デフォルトはOFF)

これくらいの仕様でどうでしょうか。

※ パラメータは、app/config/eccube/packages/eccube.yamlで定義することができます。

@Rights1995
Copy link
Author

ありがとうございます。
ご指摘の点をふまえて、手を加えてみました。

@@ -124,3 +124,6 @@ parameters:
eccube_result_cache_lifetime: 3600 # doctrineのresult cacheのlifetime.
eccube_result_cache_lifetime_short: 10 # doctrineのresult cacheのlifetime. 商品一覧画面など長期間キャッシュできない箇所で使用する.
eccube_content_maintenance_file_path: '%env(ECCUBE_MAINTENANCE_FILE_PATH)%'
eccube_product_image_resize: true
Copy link
Contributor

Choose a reason for hiding this comment

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

@Rights1995
互換性のため、デフォルトはfalseだとありがたいです。

break;

default:
throw new RuntimeException('対応していないファイル形式です。: ', $type);
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらのように、UnsupportedMediaTypeHttpExceptionのほうが適切かと思いました。
https://github.com/EC-CUBE/ec-cube/pull/4349/files#diff-f8468afcee7633478e8ef4896ea25c3fR330

Copy link
Author

@Rights1995 Rights1995 left a comment

Choose a reason for hiding this comment

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

ご指摘の点、修正いたしました。
ただ気になったのですが、リサイズ可否分岐のELSEの括りが必要ないように感じますが、いかがでしょうか?

@Rights1995 Rights1995 closed this Oct 21, 2019
@Rights1995 Rights1995 reopened this Oct 21, 2019
@Rights1995
Copy link
Author

失礼しました。
間違ってCloseしてしまいました。

@chihiro-adachi
Copy link
Contributor

@Rights1995

ご指摘の点、修正いたしました。

すみません、修正が確認できず。
push いただいてますでしょうか?

ただ気になったのですが、リサイズ可否分岐のELSEの括りが必要ないように感じますが、いかがでしょうか?

以下のelse以下が不要ということでしょうか?

https://github.com/EC-CUBE/ec-cube/pull/4349/files#diff-f8468afcee7633478e8ef4896ea25c3fR382

@Rights1995
Copy link
Author

はい、その通りです。

@Rights1995 Rights1995 closed this Oct 21, 2019
@Rights1995 Rights1995 reopened this Oct 21, 2019
@Rights1995
Copy link
Author

Rights1995 commented Nov 21, 2019

すみません。
Pushはしておりますが、確認できないでしょうか?

@Rights1995
Copy link
Author

github-actions で Build Failed となっています。

ERRORED 08:48:38Z

  • Workflows can't be executed on this repository. Please check your payment method or billing status.

@okazy
Copy link
Contributor

okazy commented Mar 2, 2020

@Rights1995

動作確認をして3点問題かと思われる箇所がありました。

  1. ProductController.php の340行目でエラー(334行目でmoveした後に $image を参照しているので)
  2. また、ProductController.php の334行目でmoveした後、370行目でも保存しているため2個ファイルができてしまうかと思います。
  3. 画像のクオリティではなくリサイズを変更した方が良いのではないでしょうか。

github-actionsについては、最新の4.0ブランチでは実行されない設定になっているので、4.0ブランチに追従することで解消されるかと思います。

@Rights1995
Copy link
Author

すみません。
しばらく間が開いてしまいました。
一旦クローズして再度、挙げさせていただきます。

@Rights1995 Rights1995 closed this Feb 4, 2021
@Rights1995 Rights1995 deleted the 4.0_rs branch February 4, 2021 03:11
@Rights1995 Rights1995 restored the 4.0_rs branch February 4, 2021 03:30
@Rights1995 Rights1995 deleted the 4.0_rs branch February 4, 2021 03:37
@okazy okazy modified the milestones: 4.0.x, Not release Mar 16, 2021
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.

3 participants