symfony7対応 - #95
Conversation
ec-cube/4.3-symfony7で動作するように改修しました。 Symfony7対応やPHP8.2以降の対応を含みます。 - 名前空間をProductReview42からProductReview44に変更 - アノテーションからAttributeへの移行 - @route → #[Route] - @template → #[Template] - Doctrineアノテーション → PHP 8属性 - コンストラクタのプロパティプロモーションを使用 - 型宣言の追加と整合性の修正 - 戻り値型、パラメータ型の追加 - エンティティのプロパティとgetter/setterの型を統一（nullable型の整合性） - EntityManager::flush()の呼び出しを引数なしに変更 - セッションキーの修正（CSVダウンロード機能） - 検索機能の修正 - 型アノテーションを追加 - !is_nullはissetで担保されるため削除 - ProductReviewEvent.phpでnull安全演算子を使用 - テンプレートパスをProductReview42からProductReview44に変更 - テストコードの更新 - declare(strict_types=1)の追加 - 型宣言の追加（プロパティをnullable型に変更） - アサーション方法の更新 - テンプレートクラス名の修正（heading02 → ec-rectHeading） - composer.jsonの更新（パッケージ名とバージョン）
EC-CUBE 4.3-symfony7ブランチを指定してテストが実行できるように設定を整理しました。 正式版リリース後、ブランチ名を修正してください。 - ワークフロー名をProductReview42からProductReview44に変更 - matrixのDB設定をmysqlとpgsqlに統一（mysql8を削除） - MySQLのポート番号を3308から3306に変更（標準ポートに統一） - servicesのデータベース名を固定値'eccube_db'に変更（matrix.dbnameの参照を削除） - MYSQL_DATABASEを削除（元から未設定のため） - eccube_versionに4.3-symfony7ブランチを追加 - GitHub Actionsのアクションをバージョンアップ - actions/cache: v2 → v4 - svenstaro/upload-release-action: v1-release → v2
- 本体の Symfony7 対応は 4.3-symfony7 ブランチから 4.4 へ統合され、 4.3-symfony7 ブランチは消滅したため checkout が解決できない Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Docker Hub からの pull タイムアウトで CI が不安定になるのを避けるため、 mysql/postgres/mailcatcher の services コンテナを廃止 - 本体 4.4 と同様に ankane/setup-mysql・ankane/setup-postgres で DB を用意 - メール送信を行わないため mailcatcher は不要 - runner を ubuntu-24.04 に統一 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- testReviewEditSuccess: find() が null の際にアサーションごと スキップされ緑になる問題を assertNotNull で防止 - getAvgAll: 返り値型を mixed から array へ(成功時の集計行も catch のフォールバックも配列、呼び出し側も配列前提) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- symfony/cache 7.4 が ext-redis >=6.1 を要求するが、ランナー同梱の redis が PHP 8.3 では 5.3.7 で composer install が conflict する - setup-php で redis を明示インストールし解消 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- nanasess/setup-php は PHP 8.3 に redis 5.3.7 を入れ、本体 lock の symfony/cache 7.4 が要求する ext-redis >=6.1 と conflict する - 本体 unit-test.yml と同じ shivammathur/setup-php + extensions: redis で 全 PHP に redis 6.x を入れる。github-token:'' も本体に合わせる Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 有効プラグインのルーティングはコンテナのコンパイル時に確定する。 phpunit の遅延コンパイル任せだと DB/タイミングで有効プラグイン一覧を 取りこぼし RouteNotFound になる flake があるため、warmup で確定させる - Setup Plugin を install→enable→cache:clear の順に整理 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- enable は ProductReviewConfig エンティティを参照するため、install 後に cache:clear でマッピングを登録してから enable する必要がある (enable→cache:clear 順だと MappingException で落ちる) - phpunit 前の cache:warmup は維持 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- APP_DEBUG=1 強制だと phpunit が debug コンテナを別途遅延ビルドし、 Run PHPUnit 前の cache:warmup(debug=0) が再利用されず RouteNotFound flake が残る。 debug 強制を外し warmup 済みコンテナを再利用させる - 廃止済み <listeners>/<filter> を PHPUnit 11 の <extensions>(DAMA)/<source> に更新 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- RouteNotFound フレークの発生源特定用。dtb_plugin の enabled と コンテナ再ビルド6回ごとの product_review ルート数を出力する。確認後 revert する
- ankane DB だと本体 configurePlugins のビルド時 DB 接続が空 enabled を返し RouteNotFound フレークが多発するため、緑実績のある coupon 同様 docker services を使う - redis は composer --ignore-platform-req=ext-redis で回避 (symfony/cache 7.4 の ext-redis>=6.1 制約。テストは redis 未使用) - enable が ProductReviewConfig を参照するため install→cache:clear→enable の順とする Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
4.4 環境（PHP 8.5.4 / Symfony 7.4.13 / Doctrine ORM 3.6.2 / DBAL 4.4.3）へ本 PR のブランチをインストールし、実機で動作確認したうえでレビューしました。
投稿 → 確認 → 完了のフロント導線、管理画面の一覧・編集・公開切り替え、公開レビューのフロント表示はいずれも正常に動作しました。一方で 3 件の機能停止を確認しています。
- CSV ダウンロードが HTTP 500（ORM 3 で削除された API の呼び出し残存）
- 必須項目に半角スペースだけ入力して保存すると 500（setter を非 nullable に狭めたため）
- ページ管理からのテンプレート編集が反映されない（テンプレート参照を名前空間形式に変更したため）
CI が緑である点について
CI は緑ですが、これは 2026-07-06 実行時点の 4.4 で取得された結果です。
$ gh run view 28764276392 --json createdAt,headSha,conclusion
createdAt=2026-07-06T02:40:14Z sha=e0c0dec5 success当時の 4.4 の composer.lock は doctrine/dbal 3.10.5 で、現在は doctrine/dbal 4.4.3（b978d3a72a / 2026-07-16 の #6855 で更新）です。Tests/Web/ReviewAdminControllerTest.php には CSV ダウンロードのテスト（$this->assertTrue($this->client->getResponse()->isSuccessful());）があるため、現行 4.4 で再実行すれば指摘 1 で落ちるはずです。マージ前に再実行をお願いできますでしょうか。
実機動作検証の結果
| 検証項目 | 結果 |
|---|---|
| 商品詳細のレビュー表示 | ✅ |
| レビュー投稿（入力 → 確認 → 完了） | ✅ 3 ステップ完走・DB 登録 |
| 管理画面のレビュー一覧 | ✅ |
| 管理画面の編集・公開切り替え | ✅ |
| 公開レビューのフロント表示 | ✅ |
| CSV ダウンロード | ❌ HTTP 500 |
| 必須項目に空白のみを入力して保存 | ❌ 500（title / comment / reviewer_name の 3 つで再現） |
| ページ管理からのテンプレート編集 | ❌ 反映されない |
参考: 移行手法について
同じ 4.4 対応 PR 群のうち、本 PR 以外の 6 本すべてが Resource/rector.php を追加しています（本 PR にはありません。また phpstan.neon.dist と static-analysis ジョブも本 PR だけ持っていません）。指摘 1 の setSQLLogger() 残存は、機械的な移行や静的解析があれば検出できた可能性が高いため、いずれかの導入をご検討いただけると今後の移行が楽になりそうです。
別 PR での対応をご検討ください（差分範囲外）
Resource/template/admin/index.twig ほかに Bootstrap 5 で廃止されたクラスが残っています（実機の /admin/product_review/ のレスポンスで form-group 6 件 / text-right 3 件を確認。本体 4.4 の管理画面テンプレートには form-group は 0 件）。移行前から存在するもので本 PR の差分にも含まれないため、別 PR での対応をご検討ください。
良い点
- フロントの投稿フロー（入力 → 確認 → 完了）が 3 ステップとも 4.4 上で完走する
- 管理画面の公開／非公開切り替えがフロント表示に正しく反映される
- 差分が -795 行と大きく、アノテーション由来の冗長な DocBlock が整理されている
- コメントアウトされていた
testProductReviewを復活させている
| // sql loggerを無効にする. | ||
| $em = $this->entityManager; | ||
| $em->getConfiguration()->setSQLLogger(null); | ||
| $em->getConfiguration()->setSQLLogger(); |
There was a problem hiding this comment.
[blocker] Configuration::setSQLLogger() は Doctrine ORM 3 で削除済みです — CSV 出力が必ず 500 になります
実機で再現（/admin/product_review/download へアクセス）:
admin.ERROR - システムエラーが発生しました。
["Call to undefined method Doctrine\\ORM\\Configuration::setSQLLogger()",
".../Controller/Admin/ProductReviewController.php", 207, ...]
[GET, /admin/product_review/download, ...]
$ php -r 'require "vendor/autoload.php";
var_dump(method_exists(new \Doctrine\ORM\Configuration(), "setSQLLogger"));'
bool(false)管理画面の全ルートを巡回しましたが、500 を返すのはこの 1 本のみでした。
移行前（origin/4.4）は setSQLLogger(null) と引数付きで呼んでいました。ORM 2 では引数省略も可能でしたが、ORM 3 ではメソッド自体が存在しません。
参考: 同じ 4.4 対応 PR 群の EC-CUBE/securitychecker-plugin#68 は同一箇所を正しく処理しています。
set_time_limit(0);
// Doctrine ORM 3.0 で Configuration::setSQLLogger() は削除された。
// ORM 3 では既定で SQL ログをメモリに蓄積しないため、無効化処理は不要。修正案: 該当行をコメントごと削除してください。ORM 3 では SQL ログの蓄積が既定で行われないため、無効化処理そのものが不要です。
There was a problem hiding this comment.
ご指摘のとおり、該当行をコメントごと削除いたしました。
ORM 3 は既定でSQLログをメモリに蓄積しないため、無効化処理そのものが不要でした。
修正前のコードに phpstan level 6 を当てて、この行が検出されることも確認いたしました。
207 Call to an undefined method Doctrine\ORM\Configuration::setSQLLogger().
🪪 method.notFound
| @@ -270,7 +226,7 @@ public function download(Request $request) | |||
| $session = $request->getSession(); | |||
There was a problem hiding this comment.
[blocker] 上の setSQLLogger() を直しても CSV が壊れます — 出力開始後にセッションを参照しているためです
setCallback() の中で exportHeader()（出力開始）の後に $request->getSession() を呼んでいるため、StreamedResponse のコールバック実行時には既にヘッダが送信済みで session_start() が失敗します。
検証方法: 上の指摘の該当行だけを取り除いた状態で /admin/product_review/download にアクセスしました。
結果、ダウンロードされる CSV は ヘッダ行の直後に HTML（システムエラー画面）が連結された壊れたファイルになります。
商品名,公開・非公開,投稿日,投稿者名,投稿者URL,性別,おすすめレベル,タイトル,コメント
<!doctype html>
<html lang="ja">
...
["Failed to start the session because headers have already been sent by
".../src/Eccube/Service/CsvExportService.php" at line 281.", ...]
#4 Controller/Admin/ProductReviewController.php(227)
#6 vendor/symfony/http-foundation/StreamedResponse.php(125)
コアおよび他プラグインとの比較（いずれも実機で正常出力を確認）
| 実装 | セッション参照の位置 | 結果 |
|---|---|---|
コア ProductController::export() |
exportHeader() より前（getProductQueryBuilder($request) 内） |
✅ |
SalesReport44 export() |
setCallback の 外 |
✅ |
Securitychecker44 download() |
セッションを使わない | ✅ |
| 本実装 | exportHeader() の 後 |
❌ HTML 混入 |
修正案: 検索条件の解決を setCallback の外に出してください。
public function download(Request $request): StreamedResponse
{
set_time_limit(0);
// セッション・フォームの解決は出力開始前に済ませる
$session = $request->getSession();
$searchForm = $this->createForm(ProductReviewSearchType::class);
$viewData = $session->get('product_review.admin.product_review.search', []);
$searchData = FormUtil::submitAndGetData($searchForm, $viewData);
$response = new StreamedResponse();
$response->setCallback(function () use ($searchData) {
// 以降は $searchData だけを使う
});なお処理順序自体は移行前と同一のため本 PR で新規に混入したものではない可能性がありますが、4.4 では確実に壊れるため、上の指摘とあわせて修正しないと CSV 機能は復旧しません。
There was a problem hiding this comment.
ご提案のとおり、検索条件の解決を setCallback の外へ移しました。
実 HTTP でも確認いたしました（ORM 3.6.2 / DBAL 4.4.4 / PHP 8.2）。
CSV のレスポンスが 200 で、本文に <!doctype html> / <html> が含まれないことを検証しています。
1点補足いたします。
コールバック内でセッションを参照する状態に戻して再現を試みたところ、こちらの環境（symfony server + php-cgi）では正常なCSVが返り、再現いたしませんでした。
出力バッファリングが効いている間は headers_sent() が false のままになるためと考えております。
SAPI や output_buffering に依存する失敗のようですので、環境によらず成立するようコールバックの外へ出す形にしております。
| * @return ProductReview | ||
| */ | ||
| public function setReviewerName($reviewer_name) | ||
| public function setReviewerName(string $reviewer_name): ProductReview |
There was a problem hiding this comment.
[blocker] setter を非 nullable に狭めたため、必須項目に空白のみを入力して保存すると 500 になります
setReviewerName(string) / setRecommendLevel(int) / setTitle(string) / setComment(string) の 4 つが、移行前の型なしから非 nullable に狭められています（プロパティ側は ?string / ?int のまま）。
Symfony のフォームはバリデーションの前にデータマッピングを行うため、TrimListener で空白のみの入力が空文字になり → empty_data で null → setter に null が渡り TypeError になります。
空文字は HTML の required 属性でブラウザが弾きますが、空白のみ（" "）は通過するため、管理者が誤って半角スペースだけ入力した場合に到達します。
実機で再現（/admin/product_review/2/edit で該当項目に " " を入力して保存）
| 入力したフィールド | 結果 |
|---|---|
title |
❌ システムエラー画面 |
comment |
❌ システムエラー画面 |
reviewer_name |
❌ システムエラー画面 |
admin.ERROR - ["Expected argument of type \"string\", \"null\" given at property path \"title\".", ...]
#7 Controller/Admin/ProductReviewController.php(155): Symfony\Component\Form\Form->handleRequest(...)
[POST, /admin/product_review/2/edit, ...]
移行前は型が無かったため null が代入され、その後のバリデーションで NotBlank エラーが表示されていました。
修正案: プロパティ側の型に合わせて setter も nullable にしてください。
public function setReviewerName(?string $reviewer_name): ProductReview
public function setRecommendLevel(?int $recommend_level): ProductReview
public function setTitle(?string $title): ProductReview
public function setComment(?string $comment): ProductReview（setReviewerUrl(?string $reviewer_url) は既に nullable なので、それに揃える形です。）
There was a problem hiding this comment.
ご指摘のとおり、4つの setter を nullable に戻しました。
回帰テストを追加しております（Tests/Web/ReviewAdminControllerTest.php の testReviewEditWithBlankOnlyRequiredValue）。
title / comment / reviewer_name に " " を入力して保存し、500 にならず入力エラーになることを検証しています。
このテストが実際に回帰を検出することも確認いたしました。
setTitle(string $title) に戻すと落ちます。
Failed asserting that false is true.
ReviewAdminControllerTest.php:206
null が渡る経路は、TrimListener で空文字になった後、FormType の既定 empty_data クロージャが非 compound で null を返すためでした（vendor/symfony/form/Extension/Core/Type/FormType.php）。
| } | ||
|
|
||
| return $this->render('ProductReview42/Resource/template/default/index.twig', [ | ||
| return $this->render('@ProductReview44/default/index.twig', [ |
There was a problem hiding this comment.
[major] テンプレート参照を @ProductReview44/... に変えたため、ページ管理での編集が反映されなくなっています
// 移行前
return $this->render('ProductReview42/Resource/template/default/index.twig', [...]);
// 移行後
return $this->render('@ProductReview44/default/index.twig', [...]);この 2 つは 解決先が異なります。
| 参照形式 | 解決先 |
|---|---|
@ProductReview44/default/index.twig |
app/Plugin/ProductReview44/Resource/template/default/index.twig（プラグイン本体） |
ProductReview44/Resource/template/default/index.twig（移行前の形式） |
app/template/default/ProductReview44/...（テーマ側） |
dtb_page には 名前空間なしのパスで 4 件登録されています。
$ sqlite3 var/eccube.db "select id, file_name, url from dtb_page where file_name like '%ProductReview%';"
94|ProductReview44/Resource/template/default/review|product_review_display
95|ProductReview44/Resource/template/default/index|product_review_index
96|ProductReview44/Resource/template/default/confirm|product_review_confirm
97|ProductReview44/Resource/template/default/complete|product_review_complete本体の PageController は $templatePath.'/'.$Page->getFileName().'.twig'（eccube_theme_front_dir = app/template/default）に dumpFile() するため、ページ管理で編集した内容はテーマ側に保存されるのに、コントローラはプラグイン本体を読みます。
実機で確認: app/template/default/ProductReview44/Resource/template/default/index.twig に目印を追記し cache:clear 後に /product_review/2/review を取得したところ、目印は一切出力されませんでした（0 件）。
PluginManager が app/template/default/ProductReview44/... へテンプレートをコピーする実装は残っておりファイル自体は存在するため、店舗側は「編集したのに反映されない」状態になります。
修正案: いずれかに統一してください。
- (a)
render()の参照を移行前と同じ名前空間なし形式に戻す - (b)
@ProductReview44/...を維持するなら、dtb_pageへの登録とPluginManagerのテンプレートコピーを廃止し、ページ管理の対象外であることを明示する
There was a problem hiding this comment.
(a) を採用し、フロントの render() と #[Template] を名前空間なし形式に戻しました。
実 HTTP で反映も確認いたしました。
app/template/default/ProductReview44/Resource/template/default/index.twig に目印を入れ、/product_review/2/review の出力に現れることを検証しています（修正前は現れないことも確認済みです）。
解決順は debug:twig でも確認できました。
$ bin/console debug:twig ProductReview44/Resource/template/default/index.twig
[OK] app/template/default/ProductReview44/Resource/template/default/index.twig
Overridden Files
* app/Plugin/ProductReview44/Resource/template/default/index.twigテーマ側のコピーが無い環境ではプラグイン本体へフォールバックします。
| public function index(Request $request, ?int $page_no = null): array | ||
| { | ||
| $CsvType = $this->productReviewConfigRepository | ||
| ->get() |
There was a problem hiding this comment.
[minor] get(): ?ProductReviewConfig にした一方、呼び出し側の null 対応が不揃いです
戻り値を ?ProductReviewConfig と明示したことで null 可能であることが型として表明されましたが、呼び出し側で null を考慮しているのは ProductReviewEvent だけです。
| 呼び出し元 | null 対応 |
|---|---|
ProductReviewEvent.php:44 |
✅ あり |
Controller/Admin/ProductReviewController.php:62（この行） |
❌ ->get()->getCsvType() と直接チェーン |
Controller/Admin/ProductReviewController.php:211 |
❌ 同様 |
型として null を認めるなら呼び出し側も揃えるか、get() が必ず値を返す設計なら戻り値型から ? を外すのが一貫します。
（なお ConfigController.php:40 は ProductReviewConfigType が data_class を設定しているため empty_data で新規インスタンスが生成され、実害はありません。）
There was a problem hiding this comment.
呼び出し側を揃える方向で対応いたしました。
index() と download() の両方を ?-> に統一し、null の場合の扱いを次のようにしています。
index(): CSV関連のボタンを表示しない（テンプレート側で{% if CsvType %}）download():NotFoundHttpException
画面からは理由が分からないため、両方で log_error を残しています。
find(1) が null を返し得るのは事実ですので、戻り値型は ?ProductReviewConfig のままにしております。
なお csv_type_id は nullable で uninstall() が setCsvType(null) を実行するため、設定行はあるが CsvType が NULL という状態も存在します。
| php: [ '7.4', '8.0', '8.1', '8.2', '8.3' ] | ||
| db: [ 'mysql', 'mysql8', 'pgsql' ] | ||
| plugin_code: [ 'ProductReview42' ] | ||
| eccube_version: [ '4.4' ] |
There was a problem hiding this comment.
[minor] CI は実行時点の 4.4 をチェックアウトするため、現行 4.4 での再実行が必要です
ref: ${{ matrix.eccube_version }} = 4.4 を都度チェックアウトする構成のため、実行時点の 4.4 の依存で結果が変わります。
最新 run は 2026-07-06 実行で、当時の 4.4 は doctrine/dbal 3.10.5 でした。現在の upstream/4.4 は doctrine/dbal 4.4.3 です（b978d3a72a / 2026-07-16 の #6855 で更新）。
Tests/Web/ReviewAdminControllerTest.php に CSV ダウンロードのテストがあるため、現行 4.4 で再実行すれば setSQLLogger() の指摘で落ちるはずです。
$this->client->request(Request::METHOD_POST,
$this->generateUrl('product_review_admin_product_review_download'));
$this->assertTrue($this->client->getResponse()->isSuccessful());There was a problem hiding this comment.
現行の 4.4 で再実行し、8ジョブすべて green になりました（PHP 8.2〜8.5 × MySQL 8 / PostgreSQL 14）。
https://github.com/EC-CUBE/ProductReview-plugin/actions/runs/30774121890
| ); | ||
|
|
||
| $codeStatus = $this->client->getResponse()->getStatusCode(); | ||
| $this->client->getResponse()->getStatusCode(); |
There was a problem hiding this comment.
[nit] 戻り値を使わない行が残っています
元のコメントアウト実装では $codeStatus = $this->client->getResponse()->getStatusCode(); でしたが、変数への代入を外した結果、副作用も検証もない行だけが残っています。
アサーションを足すか、行ごと削除するのが良さそうです。
There was a problem hiding this comment.
アサーションに置き換えました。
$this->assertTrue($this->client->getResponse()->isSuccessful());| $this->assertTrue($this->client->getResponse()->isRedirection()); | ||
|
|
||
| $this->assertNull($this->productReviewRepo->find($productReviewId)); | ||
| $this->assertNotInstanceOf(ProductReview::class, $this->productReviewRepo->find($productReviewId)); |
There was a problem hiding this comment.
[nit] assertNull → assertNotInstanceOf はアサーションが弱くなっています
find() の戻り値は ?ProductReview なので実質は等価ですが、削除確認としては「null であること」を直接検証する方が意図が明確で強い検査になります。
| #[ORM\Column(name: 'id', type: Types::INTEGER, options: ['unsigned' => true])] | ||
| #[ORM\Id] | ||
| #[ORM\GeneratedValue(strategy: 'IDENTITY')] | ||
| /** @phpstan-ignore-next-line */ |
There was a problem hiding this comment.
[nit] @phpstan-ignore-next-line の抑止対象が読み取れません
$id にだけ付いていますが、同じ形の ProductReviewConfig::$id には付いておらず不揃いです。またこのプラグインには phpstan の設定・CI ジョブがないため、何を抑止しているのかがコードから読み取れません。属性の直後・プロパティの直前という配置も読みづらさの一因になっています。
（Doctrine ORM 3 は AttributeDriver で読むため、この docblock 挿入自体がマッピングに影響しないことは doctrine:schema:update --dump-sql に差分が出ないことで確認済みです。）
There was a problem hiding this comment.
削除いたしました。
本体の phpstan.neon.dist と同じ定義で level 6 を実行し、この docblock が無くても指摘が出ないことを確認しております。
- ORM 3 で削除された Configuration::setSQLLogger() の呼び出しを削除する。 ORM 3 は既定でSQLログをメモリに蓄積しないため無効化処理そのものが不要 - 検索条件の復元をコールバックの外へ移す。 コールバックはヘッダ送出後に実行されるため、中でセッションを参照すると 出力バッファの状態次第で session_start() が失敗し、CSVにHTMLが混入する - CSV出力項目設定が無い場合のガードを追加する。 設定が無いのは plg_product_review_config そのものが無い場合と csv_type_id が NULL の場合があり、いずれも出力対象を決定できない - 一覧画面のCSV関連ボタンは出力項目設定がある場合のみ表示する Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- ページ管理の編集内容は本体 PageController がテーマ側 (app/template/<theme>/ProductReview44/Resource/template/default/) へ保存する。 @ProductReview44/... はプラグイン本体を指すため編集が反映されない - 名前空間なしのパスは twig.yaml の paths でテーマ → app/Plugin の順に解決され、 テーマ側のコピーが無い環境でもプラグイン本体へフォールバックする - dtb_page への登録と ProductReviewEvent の snippet 指定も同形式で揃う Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Symfony のフォームはバリデーションより前にデータマッピングを行う。 空白のみの入力は TrimListener で空文字になり、FormType の既定 empty_data が 非 compound で null を返すため、setter に null が渡って TypeError になる - プロパティ側の型 (?string / ?int) と揃える - 空白のみを保存して入力エラーになることを確認する回帰テストを追加する - 削除確認は assertNull で null であることを直接検証する - @phpstan-ignore-next-line を削除する。phpstan level 6 で抑止対象の指摘は出ない Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- 戻り値を使わない getStatusCode() の呼び出しをアサーションに置き換える Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- EccubeExtension::configurePlugins() はビルド時の DB 接続に失敗すると 全プラグインを無効とみなして正常終了するため、ルートが欠けていても cache:warmup は成功する - 欠けたまま phpunit を実行すると RouteNotFound が多発して原因が分かりにくいため、 warmup 直後に確定させる Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
再確認しました — LGTM
blocker 3 件を含む 9 件すべてにご対応いただき、ありがとうございます。blocker / major は実機で再現手順を再実行して解消を確認しました。
検証環境: EC-CUBE 4.4.0-dev (
89dec55c) / PHP 8.5.4 / Symfony 7.4.13 / Doctrine ORM 3.6.2 / DBAL 4.4.3 / SQLite /APP_ENV=prod/ symfony server (HTTPS)
実機検証
[blocker] setSQLLogger() / [blocker] 出力開始後のセッション参照
/admin/product_review/ を開いてから CSV をダウンロード:
HTTP/2 200
content-disposition: attachment; filename=product_review_20260806173622.csv
content-type: application/octet-stream
商品名,公開・非公開,投稿日,投稿者名,投稿者URL,性別,おすすめレベル,タイトル,コメント
チェリーアイスサンド,公開,"2026-07-28 18:42:51",レビュー太郎,,男性,4,検証タイトル44,...
HTML の混入なし・Shift_JIS で正常に出力されました。検索条件の復元を setCallback() の外へ移す形にご対応いただいたとおりです。
コールバック内でセッションを参照する状態に戻しても再現しなかった、という点について
こちらの環境（symfony server + php-fpm 8.5.4）では再現していました。ご報告の php-cgi 構成との差だと思われます。いずれにせよ、出力開始前に解決する現在の実装は SAPI に依存せず安全です。
[blocker] setter を非 nullable に狭めた件
管理画面のレビュー編集 /admin/product_review/2/edit で title / comment / reviewer_name に半角スペースのみを入力して保存 → 500 にならず「入力されていません。」のバリデーションエラーが表示されました。回帰テスト testReviewEditWithBlankOnlyRequiredValue の追加もありがとうございます。
[major] テンプレート参照の名前空間化
app/template/default/ProductReview44/Resource/template/default/index.twig に目印を入れて /product_review/2/review を取得 → 目印が出力に現れました（修正前は 0 件）。dtb_page.file_name（ProductReview44/Resource/template/default/index 等 4 件）と参照形式が一致しています。管理画面側を @ProductReview44/... のまま残した使い分けも適切です。
そのほかの指摘
| 指摘 | 対応 |
|---|---|
[minor] get(): ?ProductReviewConfig の null 対応 |
index() / download() とも ?-> に統一・log_error を残す形 |
| [minor] CI の再実行 | 現行 4.4 で 8 ジョブ green |
| [nit] 戻り値を使わない行 | assertTrue(...->isSuccessful()) に置換 |
[nit] assertNotInstanceOf |
assertNull に変更 |
[nit] @phpstan-ignore-next-line |
削除 |
CI に追加された bin/console debug:router | grep -q product_review_index は、EccubeExtension::configurePlugins() が DB 接続失敗時に「全プラグイン無効」で正常終了してしまう問題を CI で検知できる良いガードだと思います。
approve します。
|
@nanasess |
3 participants
概要(Overview・Refs Issue)
EC-CUBE 4.4 で動作するように改修しました。
Symfony 7 対応や PHP 8.2 以降の対応を含みます。
方針(Policy)
実装に関する補足(Appendix)
コード変更
Doctrine ORM 3 / Symfony 7 での動作に必要な修正
Configuration::setSQLLogger()の呼び出しを削除StreamedResponse::setCallback()の外へ移動session_start()が失敗し、出力済みの CSV に例外画面の HTML が混入しますempty_dataを経て null で渡るため、非 nullable にすると NotBlank の表示に到達せず TypeError になりますdtb_pageの登録とページ管理の保存先がapp/template/default/ProductReview44/...（テーマ側）のためです@ProductReview44/...はプラグイン本体に解決されるため、ページ管理での編集が反映されなくなりますProductReviewConfigRepository::get()が null を返す場合の扱いを呼び出し側で統一CI/CD ワークフロー
テスト（Test)
testReviewEditWithBlankOnlyRequiredValue）残タスク（Tasks）