feat: 受注管理用メモ機能を追加 (#6821)#6840
Open
ttokoro20240902 wants to merge 6 commits into
Open
Conversation
商品に管理者専用の「受注管理用メモ」を登録できるようにし、注文確定時に その商品のメモを受注明細(OrderItem)へスナップショットとしてコピーする。 - Entity: Product / OrderItem に order_memo (TEXT, nullable) を追加 - Form: ProductType にフリーエリア直後で order_memo を追加(プレーンテキスト・ eccube_lltext_len 上限・purify なし) - Service: OrderMemoPreprocessor を新規追加し、購入フロー(flow_type: shopping)で 商品明細にのみメモをコピー(確定後は再コピーせずスナップショット保持) - Template: 商品登録画面にメモカード、受注編集画面に明細アイコン＋モーダル (表示専用・メモがある商品明細のみ)を追加 - CSV: 商品CSV・配送/出荷CSVに order_memo 列を追加(dtb_csv 初期データ＋ 既存環境向け INSERT マイグレーション)、商品CSVインポートで取り込み - i18n: ラベル/ツールチップ/CSV列名の翻訳キーを ja/en に追加 - Test: OrderMemoPreprocessor のユニットテスト、ProductController の保存/ バリデーションテスト、admin-product の E2E を追加 フロント画面にはメモを一切表示しない。受注編集のメモはモーダルで Twig 自動 エスケープにより安全に表示する。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
カバレッジの薄かった以下を自動テストで担保する。 - OrderMemoFlowTest(新規): purchaseflow.yaml の配線を検証。確定フロー (shopping)に OrderMemoPreprocessor が登録され、受注フロー(order)には 登録されていないこと(確定後の再コピーを防ぎスナップショットを保持)を PurchaseFlow::dump() で確認する。振る舞い自体は OrderMemoPreprocessorTest が担保するため、ここでは flow_type の絞り込みのみを対象とする。 - CsvImportControllerTest: 商品CSVインポートで「受注管理用メモ」列が Product.order_memo に取り込まれることを検証するテストを追加。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
受注編集画面で、メモを持つ商品明細にアイコンが表示され、クリックで モーダルにメモ全文が表示されること、送料・手数料・値引き等の明細には アイコンが出ないことを検証する E2E を admin-order.spec.ts に追加。 - setup-fixtures.php: 受注明細に order_memo を持つ確認用受注(注文者: 受注メモ確認用)を生成。注文確定時のコピー結果を再現する。 - admin-order.spec.ts: 一覧検索→受注編集へ遷移し、メモリンクが1件のみ (商品明細のみ)であること、モーダルの全文表示と閉じる動作を検証。 Flaky 対策として、アイコンではなくモーダルトリガ(a[data-bs-target])を 対象にし、クリック前にツールチップのオーバーレイを除去、閉じる判定は .modal.show の件数0を自動リトライで待つ。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough商品エンティティ（ Changes受注管理用メモ機能
Sequence Diagram(s)sequenceDiagram
participant Admin as 管理者ブラウザ
participant ProductController as ProductController
participant Product as Product Entity
participant ShoppingFlow as PurchaseFlow (shopping)
participant OrderMemoPreprocessor as OrderMemoPreprocessor
participant OrderItem as OrderItem Entity
Admin->>ProductController: 商品編集フォーム送信 (order_memo含む)
ProductController->>Product: setOrderMemo(text)
ProductController-->>Admin: リダイレクト
Note over Admin,OrderItem: 顧客が購入確定時
Admin->>ShoppingFlow: process(Order, context)
ShoppingFlow->>OrderMemoPreprocessor: process(Order, context)
loop 商品明細のみ
OrderMemoPreprocessor->>Product: getOrderMemo()
Product-->>OrderMemoPreprocessor: memoText
OrderMemoPreprocessor->>OrderItem: setOrderMemo(memoText)
end
ShoppingFlow-->>Admin: 購入確定完了
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/DoctrineMigrations/Version20260617000000.php`:
- Around line 34-39: The sort_no value obtained from the SELECT MAX(sort_no) + 1
query can be NULL when the dtb_csv table is empty, and directly embedding this
NULL into the SQL string will break the INSERT statement syntax. Modify the
fetchOne query to wrap the MAX expression with COALESCE to provide a default
value (such as 1), then use parameter binding by replacing the $sortNo string
interpolation in the addSql call with a placeholder (?) and pass the fetched
sort_no value as a parameter in the second argument array to the addSql method
call.
In `@src/Eccube/Controller/Admin/Product/CsvImportController.php`:
- Around line 299-305: The order_memo field in the CSV import block (in the
setOrderMemo call within CsvImportController) is being saved without character
length validation, which creates a mismatch with the form-based validation using
the eccube_lltext_len constraint. After applying StringUtil::trimAll() to the
order_memo value, add validation to enforce the same character limit that is
applied during form-based imports. Check the length of the trimmed value against
the defined maximum length constraint and either truncate or skip the value if
it exceeds the limit to ensure consistent validation behavior between CSV and
form imports.
In `@tests/Eccube/Tests/Service/PurchaseFlow/OrderMemoFlowTest.php`:
- Around line 40-41: The container service retrieval calls in lines 40-41 using
get('eccube.purchase.flow.shopping') and get('eccube.purchase.flow.order') are
flagged by Rector as requiring modernization. Update both the shoppingFlow and
orderFlow property assignments to use the container service retrieval syntax
that Rector expects, following modern Symfony/EC-CUBE patterns for accessing
typed container services instead of string-based get() calls.
In
`@tests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.php`:
- Around line 71-73: The entityManager::find() method on line 71 can return
null, but the returned value is passed directly to setOrderItemType() on line 73
without null checking, which creates a type safety issue. Add a null check after
the find() call for the $DeliveryFeeType variable to ensure it is not null
before calling setOrderItemType() on the $FeeItem object. If the DeliveryFeeType
cannot be found, decide on an appropriate action such as throwing an exception
with a descriptive error message or handling the missing type gracefully to
prevent the type error that Rector is detecting.
In `@tests/Eccube/Tests/Web/Admin/Product/CsvImportControllerTest.php`:
- Around line 909-910: The assertion on line 909 uses assertNotNull to verify
that $Product exists, but according to the Rector CI rule, it should be changed
to assertInstanceOf which provides stronger type checking. Replace the
assertNotNull($Product) call with assertInstanceOf(Product::class, $Product) to
ensure the variable is not only non-null but also an instance of the Product
class.
In `@tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php`:
- Around line 1288-1290: The test is asserting on a stale entity by directly
checking the `$Product` object that was obtained before the request was made.
After the HTTP request completes and the response is verified as a redirect in
line 1288, you need to ensure you are validating against the current persisted
state in the database. Either re-fetch the `$Product` entity from the database
using the repository, or call the refresh method on the entity to reload it from
the database before asserting on `getOrderMemo()` in line 1289 to ensure you are
validating the actual saved data rather than the pre-request object state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6271765e-5644-4add-8884-579ddd68bcf4
⛔ Files ignored due to path filters (2)
src/Eccube/Resource/doctrine/import_csv/en/dtb_csv.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/dtb_csv.csvis excluded by!**/*.csv
📒 Files selected for processing (18)
app/DoctrineMigrations/Version20260617000000.phpapp/config/eccube/packages/purchaseflow.yamle2e/setup-fixtures.phpe2e/tests/admin-order.spec.tse2e/tests/admin-product.spec.tssrc/Eccube/Controller/Admin/Product/CsvImportController.phpsrc/Eccube/Entity/OrderItem.phpsrc/Eccube/Entity/Product.phpsrc/Eccube/Form/Type/Admin/ProductType.phpsrc/Eccube/Resource/locale/messages.en.yamlsrc/Eccube/Resource/locale/messages.ja.yamlsrc/Eccube/Resource/template/admin/Order/edit.twigsrc/Eccube/Resource/template/admin/Product/product.twigsrc/Eccube/Service/PurchaseFlow/Processor/OrderMemoPreprocessor.phptests/Eccube/Tests/Service/PurchaseFlow/OrderMemoFlowTest.phptests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.phptests/Eccube/Tests/Web/Admin/Product/CsvImportControllerTest.phptests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php
- OrderMemoPreprocessorTest: nullable な find() 結果に assertInstanceOf を追加 - CsvImportControllerTest: assertNotNull を assertInstanceOf(Product::class) へ - rector.php: OrderMemoFlowTest は shopping/order の購入フローを名前付きサービスで 区別するため ContainerGetNameToTypeInTestsRector を当該ファイルのみ skip Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
商品の受注管理用メモを、フロント確定だけでなく管理画面の受注作成・編集時にも 商品明細(OrderItem)へ反映する。OrderMemoPreprocessor を shopping/order 両フローに 登録し、明細メモに商品メモの文言が含まれていなければ追記する(同一文言は重複させず、 既存メモは保持)。あわせて dtb_csv マイグレーションを既存パターン(const NAME と hasTable ガード)に合わせる。 - OrderMemoPreprocessor: 上書きから冪等な追記へ変更 - purchaseflow.yaml: order フローにも登録 - Version20260617000000: const NAME / hasTable ガードを追加 - テスト: order フロー登録の検証へ変更、追記の冪等性テストを追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Version20260617000000: dtb_csv が空の場合に MAX(sort_no)+1 が NULL となり INSERT が壊れるのを防ぐため COALESCE で既定値を確保し、sort_no をパラメータ バインディングに変更 - ProductControllerTest::testEditWithOrderMemo: POST 後に entityManager->refresh で DB の永続化状態を再読込してからアサートするよう変更 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.php (1)
66-79: ⚡ Quick win部分一致の誤判定ケースもテストに含めてください。
現状の冪等テストだと、既存メモが商品メモを部分文字列として含むケース（
str_contains起因）を検出できません。ここを1ケース追加すると、重複判定の退行を防げます。差分例（境界ケース追加）
+ public function testSubstringMemoIsStillAppendedAsDifferentMemo(): void + { + $this->Product->setOrderMemo('商品メモ'); + + foreach ($this->Order->getProductOrderItems() as $OrderItem) { + $OrderItem->setOrderMemo('既存メモ: 商品メモ詳細'); + } + + $this->processor->process($this->Order, new PurchaseContext()); + + foreach ($this->Order->getProductOrderItems() as $OrderItem) { + $this->assertSame("既存メモ: 商品メモ詳細\n商品メモ", $OrderItem->getOrderMemo()); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.php` around lines 66 - 79, The testSameMemoIsNotAppendedTwice method only tests the case where the existing memo and product memo are identical. Add an additional test case to verify the boundary condition where an existing order item memo contains the product memo as a substring but they are not identical (for example, existing memo is "割れ物注意" while product memo is "梱包時は割れ物注意"). This will catch potential regressions if the duplicate detection logic incorrectly uses substring matching (str_contains) instead of exact matching, ensuring the processor still appends the memo when the strings are different even if one contains the other.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@tests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.php`:
- Around line 66-79: The testSameMemoIsNotAppendedTwice method only tests the
case where the existing memo and product memo are identical. Add an additional
test case to verify the boundary condition where an existing order item memo
contains the product memo as a substring but they are not identical (for
example, existing memo is "割れ物注意" while product memo is "梱包時は割れ物注意"). This will
catch potential regressions if the duplicate detection logic incorrectly uses
substring matching (str_contains) instead of exact matching, ensuring the
processor still appends the memo when the strings are different even if one
contains the other.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a02485e8-c86d-43b4-ab63-16a628a6863c
📒 Files selected for processing (6)
app/DoctrineMigrations/Version20260617000000.phpapp/config/eccube/packages/purchaseflow.yamlsrc/Eccube/Service/PurchaseFlow/Processor/OrderMemoPreprocessor.phptests/Eccube/Tests/Service/PurchaseFlow/OrderMemoFlowTest.phptests/Eccube/Tests/Service/PurchaseFlow/Processor/OrderMemoPreprocessorTest.phptests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Eccube/Service/PurchaseFlow/Processor/OrderMemoPreprocessor.php
- app/DoctrineMigrations/Version20260617000000.php
- app/config/eccube/packages/purchaseflow.yaml
- tests/Eccube/Tests/Web/Admin/Product/ProductControllerTest.php
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6840 +/- ##
==========================================
+ Coverage 74.88% 74.91% +0.02%
==========================================
Files 463 464 +1
Lines 24029 24064 +35
==========================================
+ Hits 17994 18027 +33
- Misses 6035 6037 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
商品に管理者専用の「受注管理用メモ」を登録できるようにし、受注の作成・編集時にその商品のメモを受注明細（
OrderItem）へ冪等に追記する機能を追加します（同一文言は重複させず、既存メモは保持）。フロント画面にはメモを一切表示しません。Issue: #6821
変更内容
Product/OrderItemにorder_memo（TEXT, nullable）を追加ProductTypeにフリーエリア直後でorder_memoを追加（プレーンテキスト・eccube_lltext_len上限・purify なし）OrderMemoPreprocessorを新規追加し、購入フロー（shopping）と受注フロー（order）の両方で、商品明細のメモに商品メモの文言が無ければ追記（冪等・既存メモ保持）order_memo列を追加（dtb_csv初期データ＋既存環境向け INSERT マイグレーション）、商品CSVインポートで取り込み設計上のポイント
OrderMemoPreprocessorをshopping・order両フローに登録し、明細メモに商品メモの文言が含まれていなければ追記する（同一文言は重複させず、既存メモは消さない）。受注の作成時だけでなく編集時にも反映され、商品マスタのメモを後から変更した場合は旧文言を残したまま新文言が追記される。スクリーンショット
1. 商品登録／編集画面：受注管理用メモカード
2. 受注編集画面：商品明細のメモアイコン
3. 受注編集画面：メモモーダル（全文表示）
テスト
OrderMemoPreprocessorTest: コピー・冪等追記挙動のユニットテスト（同一文言は追記しない／別文言は改行追記し既存メモを保持／送料等の非商品明細は対象外）OrderMemoFlowTest:purchaseflow.yamlの配線検証（shopping・order 両方に登録）ProductControllerTest: 保存・バリデーションCsvImportControllerTest: 商品CSVインポートでの取り込みadmin-product.spec.ts/admin-order.spec.ts）: メモ登録、受注編集のアイコン／モーダル表示TODO（Draft）
🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
テスト