diff --git a/src/Eccube/Controller/Admin/Order/OrderController.php b/src/Eccube/Controller/Admin/Order/OrderController.php index a2060883b8..0440818bca 100644 --- a/src/Eccube/Controller/Admin/Order/OrderController.php +++ b/src/Eccube/Controller/Admin/Order/OrderController.php @@ -185,12 +185,7 @@ public function index(Request $request, ?int $page_no = null): array $qb = $this->orderRepository->getQueryBuilderBySearchDataForAdmin($searchData); - // null を配列オフセットに使うのは PHP 8.5 で非推奨。null は '' として扱われるため挙動は変わらない - $sortKey = $searchData['sortkey'] ?? ''; - $paginate_options = ['wrap-queries' => true]; - if (empty($this->orderRepository::COLUMNS[$sortKey]) || $sortKey == 'order_status') { - $paginate_options = []; - } + $paginate_options = $this->createPaginateOptions($this->extractSortKey($searchData)); $event = new EventArgs( [ @@ -299,8 +294,13 @@ protected function exportCsv(Request $request, int $csvTypeId, string $fileName) // タイムアウトを無効にする. set_time_limit(0); + // 一覧画面と同じ paginate オプションを使う. + // sortkey は HiddenType なので, セッションに入っている値をそのまま参照できる. + $sortKey = $this->extractSortKey($this->session->get('eccube.admin.order.search', [])); + $paginate_options = $this->createPaginateOptions($sortKey); + $response = new StreamedResponse(); - $response->setCallback(function () use ($request, $csvTypeId): void { + $response->setCallback(function () use ($request, $csvTypeId, $paginate_options): void { // CSV種別を元に初期化. $this->csvExportService->initCsvType($csvTypeId); @@ -353,7 +353,7 @@ protected function exportCsv(Request $request, int $csvTypeId, string $fileName) // 出力. $csvService->fputcsv($ExportCsvRow->getRow()); } - }); + }, $paginate_options); }); $response->headers->set('Content-Type', 'application/octet-stream'); @@ -362,6 +362,38 @@ protected function exportCsv(Request $request, int $csvTypeId, string $fileName) return $response; } + /** + * 検索条件からソートキーを取り出す. + * + * セッション由来の値も渡るため, 文字列以外は未指定として扱う. + * (null をそのまま配列オフセットに使うのは PHP 8.5 で非推奨) + */ + private function extractSortKey(mixed $searchData): string + { + $sortKey = is_array($searchData) ? $searchData['sortkey'] ?? null : null; + + return is_string($sortKey) ? $sortKey : ''; + } + + /** + * 受注一覧・受注CSV・配送CSVで共通の paginate オプションを組み立てる. + * + * 受注検索のクエリは Shipping を fetch join しているため (OrderItem は join のみ), to-many 側の列 + * (s.shipping_date, s.tracking_number, s.name01 等) でソートすると LimitSubqueryWalker が例外を投げる. + * wrap-queries を有効にするとサブクエリで包まれ, ソートを保ったまま解消できる. + * order_status は association (o.OrderStatus) をソート対象にするため, 従来どおり対象外とする. + * + * @return array + */ + private function createPaginateOptions(string $sortKey): array + { + if (empty($this->orderRepository::COLUMNS[$sortKey]) || $sortKey === 'order_status') { + return []; + } + + return ['wrap-queries' => true]; + } + /** * Update to order status */ diff --git a/src/Eccube/Controller/Admin/Product/ProductController.php b/src/Eccube/Controller/Admin/Product/ProductController.php index 2c9be180bc..6f3cc982ca 100644 --- a/src/Eccube/Controller/Admin/Product/ProductController.php +++ b/src/Eccube/Controller/Admin/Product/ProductController.php @@ -185,12 +185,7 @@ public function index(Request $request, $page_no = null): array $qb = $this->productRepository->getQueryBuilderBySearchDataForAdmin($searchData); - // null を配列オフセットに使うのは PHP 8.5 で非推奨。null は '' として扱われるため挙動は変わらない - $sortKey = $searchData['sortkey'] ?? ''; - $paginate_options = ['wrap-queries' => true]; - if (empty($this->productRepository::COLUMNS[$sortKey]) || $sortKey == 'code' || $sortKey == 'status') { - $paginate_options = []; - } + $paginate_options = $this->createPaginateOptions($this->extractSortKey($searchData)); $event = new EventArgs( [ @@ -935,8 +930,17 @@ public function export(Request $request): StreamedResponse // タイムアウトを無効にする. set_time_limit(0); + // 一覧画面と同じ paginate オプションを使う. + // sortkey は HiddenType なので, セッションに入っている値をそのまま参照できる. + $sortKey = $this->extractSortKey($this->session->get('eccube.admin.product.search', [])); + $paginate_options = $this->createPaginateOptions($sortKey); + + // ProductClass の列でソートしている場合は, その列を select 句に載せる必要がある. + $sortColumn = $this->productRepository::COLUMNS[$sortKey] ?? ''; + $hiddenSortColumn = str_starts_with($sortColumn, 'pc.') ? $sortColumn : null; + $response = new StreamedResponse(); - $response->setCallback(function () use ($request): void { + $response->setCallback(function () use ($request, $paginate_options, $hiddenSortColumn): void { // CSV種別を元に初期化. $this->csvExportService->initCsvType(CsvType::CSV_TYPE_PRODUCT); @@ -963,12 +967,24 @@ public function export(Request $request): StreamedResponse // http://uedatakeshi.blogspot.jp/2010/04/distinct-oeder-by-postgresmysql.html $qb->resetDQLPart('select'); + // stock_status は SearchProductType に無く, コアからは設定されない + // (管理画面の在庫切れ絞り込みは別キーの stock を使う). プラグイン等が + // セッションへ入れたときだけ通る経路なので, 従来の形を維持する. if ($isOutOfStock) { $qb->select('p, pc') ->distinct(); } else { $qb->select('p') ->distinct(); + + // ProductClass の列でソートしている場合は, その列を HIDDEN で select 句に載せる. + // DISTINCT と併用するため, ORDER BY の対象が select 句に無いと + // PostgreSQL が「ORDER BY expressions must appear in select list」で拒否する. + // pc を fetch join すると ProductClasses が pc.visible の条件で部分初期化され, + // 非表示の規格の行が出力から落ちてしまうため, HIDDEN で取得対象には含めない. + if ($hiddenSortColumn !== null) { + $qb->addSelect($hiddenSortColumn.' AS HIDDEN sort_key_value'); + } } // データ行の出力. $this->csvExportService->setExportQueryBuilder($qb); @@ -1012,7 +1028,7 @@ public function export(Request $request): StreamedResponse // 出力. $csvService->fputcsv($ExportCsvRow->getRow()); } - }); + }, $paginate_options); }); $now = new \DateTime(); @@ -1025,6 +1041,38 @@ public function export(Request $request): StreamedResponse return $response; } + /** + * 検索条件からソートキーを取り出す. + * + * セッション由来の値も渡るため, 文字列以外は未指定として扱う. + * (null をそのまま配列オフセットに使うのは PHP 8.5 で非推奨) + */ + private function extractSortKey(mixed $searchData): string + { + $sortKey = is_array($searchData) ? $searchData['sortkey'] ?? null : null; + + return is_string($sortKey) ? $sortKey : ''; + } + + /** + * 商品一覧・商品CSVで共通の paginate オプションを組み立てる. + * + * 商品検索のクエリは ProductClass を to-many で join しているため, ProductClass 側の列 + * (pc.code, pc.stock) でソートすると LimitSubqueryWalker が例外を投げる. + * wrap-queries を有効にするとサブクエリで包まれ, ソートを保ったまま解消できる. + * status は association (p.Status) をソート対象にするため, 従来どおり対象外とする. + * + * @return array + */ + private function createPaginateOptions(string $sortKey): array + { + if (empty($this->productRepository::COLUMNS[$sortKey]) || $sortKey === 'status') { + return []; + } + + return ['wrap-queries' => true]; + } + /** * ProductCategory作成 */ diff --git a/src/Eccube/Service/CsvExportService.php b/src/Eccube/Service/CsvExportService.php index 1a60019e15..0715a18995 100644 --- a/src/Eccube/Service/CsvExportService.php +++ b/src/Eccube/Service/CsvExportService.php @@ -170,8 +170,10 @@ public function exportHeader(): void /** * クエリビルダにもとづいてデータ行を出力する. * このメソッドを使う場合は, 事前にsetExportQueryBuilder($qb)で出力対象のクエリビルダをわたしておく必要がある. + * + * @param array $paginateOptions KnpPaginator に渡すオプション. 一覧画面と同じ値をわたす. */ - public function exportData(\Closure $closure): void + public function exportData(\Closure $closure, array $paginateOptions = []): void { if (is_null($this->qb) || is_null($this->entityManager)) { throw new \LogicException('query builder not set.'); @@ -183,7 +185,7 @@ public function exportData(\Closure $closure): void $page = 1; $limit = 100; - while ($results = $this->paginator->paginate($this->qb, $page, $limit)) { + while ($results = $this->paginator->paginate($this->qb, $page, $limit, $paginateOptions)) { /** @var AbstractPagination $results */ if (!$results->valid()) { break; diff --git a/tests/Eccube/Tests/Web/Admin/AbstractAdminWebTestCase.php b/tests/Eccube/Tests/Web/Admin/AbstractAdminWebTestCase.php index 93e0aae8ef..712cf23d35 100644 --- a/tests/Eccube/Tests/Web/Admin/AbstractAdminWebTestCase.php +++ b/tests/Eccube/Tests/Web/Admin/AbstractAdminWebTestCase.php @@ -15,6 +15,7 @@ namespace Eccube\Tests\Web\Admin; +use Eccube\Common\EccubeConfig; use Eccube\Tests\Web\AbstractWebTestCase; abstract class AbstractAdminWebTestCase extends AbstractWebTestCase @@ -41,4 +42,44 @@ public function logIn(mixed $user = null) return $user; } + + /** + * CSV のレコード数を返す（ヘッダ行を除く）. + */ + protected function countCsvRows(string $csv): int + { + return count($this->parseCsv($csv)) - 1; + } + + /** + * CSV をレコード単位にパースする（ヘッダ行を含む）. + * + * 項目の値に改行が含まれるため, 行数は改行では数えられない. + * 出力は eccube_csv_export_encoding のエンコーディングなので UTF-8 に戻してから読む + * (SJIS は 2 バイト目に 0x5C を含む文字があり, escape と誤認して行が結合される). + * escape は PHP 8.4 以降の既定値に合わせて '' を明示する + * (省略すると deprecation。'\\' はデータ中のバックスラッシュで行が結合される). + * + * @return array> + */ + protected function parseCsv(string $csv): array + { + $eccubeConfig = static::getContainer()->get(EccubeConfig::class); + $csv = (string) mb_convert_encoding($csv, 'UTF-8', $eccubeConfig->get('eccube_csv_export_encoding')); + + $fp = fopen('php://memory', 'r+'); + $this->assertNotFalse($fp); + fwrite($fp, $csv); + rewind($fp); + + $records = []; + while (($row = fgetcsv($fp, null, $eccubeConfig->get('eccube_csv_export_separator'), '"', '')) !== false) { + if ($row !== [null]) { + $records[] = $row; + } + } + fclose($fp); + + return $records; + } } diff --git a/tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php b/tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php index 58a7a5bdcb..439e215918 100644 --- a/tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php +++ b/tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php @@ -39,6 +39,11 @@ final class OrderControllerTest extends AbstractAdminWebTestCase { use MailerAssertionsTrait; + /** + * Shipping (to-many) の列をソート対象にするキー. + */ + private const TO_MANY_SORT_KEYS = ['shipping_status', 'tracking_number', 'delivery']; + protected ?OrderStatusRepository $orderStatusRepository = null; protected ?PaymentRepository $paymentRepository = null; @@ -286,6 +291,151 @@ public function testExportOrder() $this->assertMatchesRegularExpression('/user-[0-9]@example.com/', $content); } + /** + * ソートしてから受注CSVをダウンロードしてもエラーにならないことのテスト. + * + * 受注検索のクエリは Shipping を fetch join しているため, Shipping 側の列でソートすると + * LimitSubqueryWalker が例外を投げ, CSV が最後まで出力されなかった. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + #[DataProvider(methodName: 'dataSortKeyProvider')] + public function testExportOrderWithSortKey(string $sortKey): void + { + $this->client->request( + Request::METHOD_POST, + $this->generateUrl('admin_order'), + [ + 'admin_search_order' => [ + '_token' => 'dummy', + 'email' => 'user-', + 'sortkey' => $sortKey, + 'sorttype' => 'a', + ], + ] + ); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + $this->client->request( + Request::METHOD_GET, + $this->generateUrl('admin_order_export_order') + ); + + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + // ヘッダ行だけでなくデータ行が出力されていること. + $content = $this->client->getInternalResponse()->getContent(); + $this->assertMatchesRegularExpression('/user-[0-9]@example.com/', $content); + } + + /** + * 配送CSVも同じクエリビルダを使うため, 同様にソート後もエラーにならないこと. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + #[DataProvider(methodName: 'dataSortKeyProvider')] + public function testExportShippingWithSortKey(string $sortKey): void + { + $this->client->request( + Request::METHOD_POST, + $this->generateUrl('admin_order'), + [ + 'admin_search_order' => [ + '_token' => 'dummy', + 'email' => 'user-', + 'sortkey' => $sortKey, + 'sorttype' => 'a', + ], + ] + ); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + $this->client->request( + Request::METHOD_GET, + $this->generateUrl('admin_order_export_shipping') + ); + + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + $content = $this->client->getInternalResponse()->getContent(); + $this->assertMatchesRegularExpression('/user-[0-9]@example.com/', $content); + } + + /** + * ソートの有無で受注CSVの行数が変わらないことのテスト. + * + * to-many の関連を select 句に載せると, 関連側の絞り込み条件でコレクションが部分初期化され, + * ソートしたときだけ出力行数が減ることがある. 行数で担保して出力内容の変化を検知する. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + public function testExportOrderRowCountIsNotAffectedBySort(): void + { + $expected = $this->countExportedRows('admin_order_export_order', ''); + $this->assertGreaterThan(0, $expected); + + foreach (self::TO_MANY_SORT_KEYS as $sortKey) { + $this->assertSame($expected, $this->countExportedRows('admin_order_export_order', $sortKey), $sortKey); + } + } + + /** + * ソートの有無で配送CSVの行数が変わらないことのテスト. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + public function testExportShippingRowCountIsNotAffectedBySort(): void + { + $expected = $this->countExportedRows('admin_order_export_shipping', ''); + $this->assertGreaterThan(0, $expected); + + foreach (self::TO_MANY_SORT_KEYS as $sortKey) { + $this->assertSame($expected, $this->countExportedRows('admin_order_export_shipping', $sortKey), $sortKey); + } + } + + /** + * ソートしてから CSV を出力し, ヘッダ行を除いたレコード数を返す. + */ + private function countExportedRows(string $route, string $sortKey): int + { + $this->client->request( + Request::METHOD_POST, + $this->generateUrl('admin_order'), + [ + 'admin_search_order' => [ + '_token' => 'dummy', + 'email' => 'user-', + 'sortkey' => $sortKey, + 'sorttype' => 'a', + ], + ] + ); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + $this->client->request(Request::METHOD_GET, $this->generateUrl($route)); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + return $this->countCsvRows($this->client->getInternalResponse()->getContent()); + } + + /** + * @return \Iterator, array{string}> + */ + public static function dataSortKeyProvider(): \Iterator + { + // Shipping (to-many) の列。#6713 で報告されたエラーになる3キー。 + yield ['shipping_status']; + yield ['tracking_number']; + yield ['delivery']; + // association (o.OrderStatus) のため wrap-queries の対象外。従来どおり動くこと。 + yield ['order_status']; + // Order (to-one) の列。従来どおり動くこと。 + yield ['purchase_price']; + // ソート未指定。 + yield ['']; + } + /** * Test for issue 1995 * diff --git a/tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php b/tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php index 897885a853..cf98cefd11 100644 --- a/tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php +++ b/tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php @@ -1050,6 +1050,143 @@ public static function dataEditRoundingTypeProvider(): array ]; } + /** + * ソートしてから商品CSVをダウンロードしてもエラーにならないことのテスト. + * + * 商品検索のクエリは ProductClass を to-many で join しているため, ProductClass 側の列 + * (pc.code, pc.stock) でソートすると LimitSubqueryWalker が例外を投げ, + * CSV が最後まで出力されなかった. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + #[DataProvider(methodName: 'dataProductSortKeyProvider')] + public function testExportProductWithSortKey(string $sortKey): void + { + $searchForm = $this->createSearchForm(); + $searchForm['sortkey'] = $sortKey; + $searchForm['sorttype'] = 'a'; + $this->searchProduct($searchForm); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + // ヘッダ行だけでなくデータ行が出力されていること. + $this->assertGreaterThan(0, $this->countCsvRows($this->exportProductCsv())); + } + + /** + * ソートの有無で商品CSVの行数が変わらないことのテスト. + * + * ProductClass を select 句に載せると pc.visible の条件でコレクションが部分初期化され, + * ソートしたときだけ非表示の規格の行が落ちることがある. 行数で担保して検知する. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + public function testExportProductRowCountIsNotAffectedBySort(): void + { + // 規格を 2 件登録した商品. 規格を登録すると 規格なし既定の ProductClass は非表示になるが, + // ソートしない CSV には 3 行とも出力される. + $productName = 'Product for csv sort '.uniqid(); + $Product = $this->createProduct($productName, 2); + + $searchForm = $this->createSearchForm(); + $searchForm['id'] = $productName; + + $expected = $this->countExportedRows($searchForm, ''); + $this->assertSame(count($Product->getProductClasses()), $expected); + + foreach (['product_code', 'stock'] as $sortKey) { + $this->assertSame($expected, $this->countExportedRows($searchForm, $sortKey), $sortKey); + } + } + + /** + * ソートが商品CSVの並び順に反映されることのテスト. + * + * ソートを保ったまま LimitSubqueryWalker の例外を解消するのが目的なので, + * エラーにならないことだけでなく並び順そのものを担保する. + * + * @see https://github.com/EC-CUBE/ec-cube/issues/6713 + */ + public function testExportProductKeepsSortOrder(): void + { + // ソート未指定時の既定は p.update_date DESC, p.id DESC なので, 登録順を b, c, a にして + // 既定の並び (a, c, b) が昇順 (a, b, c) とも降順 (c, b, a) とも一致しないようにする. + // 一致していると, ソートが効かなくてもその向きのアサートが通ってしまう. + // 規格を登録すると 規格なし既定の ProductClass は非表示になるが, CSV には出力される. + $prefix = 'Product for sort order '.uniqid(); + foreach (['b', 'c', 'a'] as $code) { + $Product = $this->createProduct($prefix.' '.$code, 1); + $no = 1; + foreach ($Product->getProductClasses() as $ProductClass) { + $ProductClass->setCode($code.$no++); + } + } + $this->entityManager->flush(); + + $searchForm = $this->createSearchForm(); + $searchForm['id'] = $prefix; + $searchForm['sortkey'] = 'product_code'; + + $searchForm['sorttype'] = 'a'; + $this->searchProduct($searchForm); + $this->assertSame(['a', 'a', 'b', 'b', 'c', 'c'], $this->extractSortedProductGroups($this->exportProductCsv())); + + $searchForm['sorttype'] = 'd'; + $this->searchProduct($searchForm); + $this->assertSame(['c', 'c', 'b', 'b', 'a', 'a'], $this->extractSortedProductGroups($this->exportProductCsv())); + } + + /** + * CSV から商品コードの先頭 1 文字を出力順に取り出す. + * + * 商品コードは `<商品を表す 1 文字><規格ごとの連番>` で登録する. + * 商品内の行順は `Product::$ProductClasses` の取得順（`#[ORM\OrderBy]` が無く DB 依存） + * に左右されるため, 商品間の並びだけを見るよう連番を落として比較する. + * + * @return array + */ + private function extractSortedProductGroups(string $csv): array + { + $records = $this->parseCsv($csv); + $header = array_shift($records); + $this->assertIsArray($header); + $index = array_search('商品コード', $header, true); + $this->assertIsInt($index); + + return array_map(fn (array $row): string => substr((string) $row[$index], 0, 1), $records); + } + + /** + * ソートしてから商品CSVを出力し, ヘッダ行を除いたレコード数を返す. + * + * @param array $searchForm + */ + private function countExportedRows(array $searchForm, string $sortKey): int + { + $searchForm['sortkey'] = $sortKey; + $searchForm['sorttype'] = 'a'; + $this->searchProduct($searchForm); + $this->assertTrue($this->client->getResponse()->isSuccessful()); + + return $this->countCsvRows($this->exportProductCsv()); + } + + /** + * @return \Iterator, array{string}> + */ + public static function dataProductSortKeyProvider(): \Iterator + { + // ProductClass (to-many) の列。修正前はエラーになる2キー。 + yield ['product_code']; + yield ['stock']; + // association (p.Status) のため wrap-queries の対象外。従来どおり動くこと。 + yield ['status']; + // Product (to-one) の列。従来どおり動くこと。 + yield ['product_id']; + yield ['name']; + // ソート未指定。 + yield ['']; + } + /** * 商品検索を実行し, 検索条件をセッションに保持する. *