[4.4] マイページ注文履歴詳細にお問い合わせ番号を表示 (#6818)#6826
Open
ttokoro20240902 wants to merge 4 commits into
Open
Conversation
フロント会員がマイページの注文履歴詳細画面で配送のお問い合わせ番号 (追跡番号) を確認できるようにする。 - history.twig: 配送情報ループ内、お届け時間の直後に表示ブロックを追加。 値が未入力 (null/空文字列) の場合は項目自体を非表示。複数配送先は 配送先ごとに評価。出力は自動エスケープ (XSS 対策)。 - messages.ja/en.yaml: front.mypage.tracking_number を追加 (お問い合わせ番号 / Tracking No.、admin.order.tracking_number と統一)。 - MypageControllerTest: 表示/null非表示/空文字非表示/複数配送/一部のみ入力/ XSSエスケープ/翻訳定義 の主要ケースを追加。非表示判定はセレクタ件数で検証し、 受注生成・配送追加・履歴取得をヘルパーへ集約。 - front-mypage.spec.ts: 管理画面でお問い合わせ番号を設定しマイページで 表示を確認する E2E を追加。受注番号をキーに検索・直接遷移することで 一覧の並び順に依存しない (フレーキー回避)。 データソースは既存の dtb_shipping.tracking_number。DB・エンティティ・ コントローラ・ルートの変更は不要 (表示のみ)。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughマイページ注文履歴詳細に、Shipping.tracking_number が存在する場合のみ「お問い合わせ番号」を表示するテンプレート追加、日英メッセージ追加、関連のユニットテストとE2Eテストを実装しました。 Changesマイページ注文履歴詳細への追跡番号表示機能
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
e2e/tests/front-mypage.spec.ts (1)
316-318: ⚡ Quick win非表示検証はテキスト全体ではなくラベル要素不在で判定した方が安定します。
div.ec-orderDelivery全体へのnot.toContainTextは将来の文言変更に弱いので、dtラベルの不在で直接判定する方が堅牢です。修正例
- const delivery = page.locator('div.ec-orderDelivery'); - await expect(delivery).not.toContainText('お問い合わせ番号'); + await expect( + page.locator('div.ec-orderDelivery dt', { hasText: 'お問い合わせ番号' }) + ).toHaveCount(0);🤖 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 `@e2e/tests/front-mypage.spec.ts` around lines 316 - 318, Replace the brittle full-text assertion on the delivery block with a structural check for the absent label: locate the existing delivery locator (delivery = page.locator('div.ec-orderDelivery')) and then assert that the child dt element with the label text 'お問い合わせ番号' does not exist (e.g., get delivery.locator for the dt with that text and expect its count to be zero). This makes the check robust to unrelated wording changes while still verifying the label is not present.
🤖 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 `@e2e/tests/front-mypage.spec.ts`:
- Around line 185-189: 現在のコード uses adminPage.locator(...).first() to pick the
first order link which can target the wrong row; instead locate the table row
whose order number cell matches the expected orderNo and then select the anchor
within that row, e.g. find the tbody tr that contains orderNo (use the existing
orderNo variable) and call locator on that row to get the 'a[href*="/order/"]'
link, then read its href into orderEditHref and navigate; replace usages of
orderLink = adminPage.locator(...).first() and the blind navigation with this
targeted row-based lookup so only the matching order is followed.
In `@tests/Eccube/Tests/Web/Mypage/MypageControllerTest.php`:
- Around line 218-327: The listed test methods (testHistoryWithTrackingNumber,
testHistoryWithoutTrackingNumber, testHistoryWithEmptyTrackingNumber,
testHistoryWithMultipleShippings,
testHistoryWithMultipleShippingsPartiallyFilled, testHistoryTrackingNumberXss,
testTrackingNumberLabelTranslations) lack a return type; update each public
function declaration to include the void return type (e.g., change "public
function testHistoryWithTrackingNumber()" to "public function
testHistoryWithTrackingNumber(): void") so all tests in MypageControllerTest use
consistent ": void" signatures.
---
Nitpick comments:
In `@e2e/tests/front-mypage.spec.ts`:
- Around line 316-318: Replace the brittle full-text assertion on the delivery
block with a structural check for the absent label: locate the existing delivery
locator (delivery = page.locator('div.ec-orderDelivery')) and then assert that
the child dt element with the label text 'お問い合わせ番号' does not exist (e.g., get
delivery.locator for the dt with that text and expect its count to be zero).
This makes the check robust to unrelated wording changes while still verifying
the label is not present.
🪄 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: 4af7d162-7e20-4548-b7ed-8d92beb9c1de
📒 Files selected for processing (5)
e2e/tests/front-mypage.spec.tssrc/Eccube/Resource/locale/messages.en.yamlsrc/Eccube/Resource/locale/messages.ja.yamlsrc/Eccube/Resource/template/default/Mypage/history.twigtests/Eccube/Tests/Web/Mypage/MypageControllerTest.php
- MypageControllerTest: getContent() の string|false に (string) キャストを付与し Crawler を import 経由に変更 (rector StringCastAssertStringContainsStringRector) - 追加したテストメソッドに : void 戻り値型を付与 - E2E: 受注一覧から .first() ではなく受注番号で行を特定してリンクを辿る - E2E: お問い合わせ番号未表示の検証をラベル dt のカウント 0 へ構造化 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e2e-test.yml の matrix は許可リストであり, 列挙されていない spec は CI で実行されない。 front-order(16件) / front-mypage(10件+skip 1) / front-invoice(2件) は テストカバレッジ向上で追加された際から matrix 未登録のままで, 一度も CI で実行されていなかった。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6826 +/- ##
=======================================
Coverage 74.88% 74.88%
=======================================
Files 463 463
Lines 24029 24029
=======================================
+ Hits 17994 17995 +1
+ Misses 6035 6034 -1
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:
|
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.
概要
フロント会員がマイページの注文履歴詳細画面（
/mypage/history/{order_no}）で、配送のお問い合わせ番号（追跡番号）を確認できるようにします。Issue #6818 の実装です。データソースは既存の
dtb_shipping.tracking_number。DB・エンティティ・コントローラ・ルートの変更は不要で、表示のみを追加します（Order.Shippings経由で取得済みのため追加クエリ・N+1 なし）。closes #6818
変更内容
history.twig: 配送情報ループ内、「お届け時間」の直後に表示ブロックを追加。値が未入力（null/空文字列）の場合は項目自体を非表示。複数配送先は配送先ごとに評価。{{ Shipping.tracking_number }}（|raw不使用）で自動エスケープ。messages.ja/en.yaml:front.mypage.tracking_numberを追加（お問い合わせ番号/Tracking No.、既存admin.order.tracking_numberと統一）。MypageControllerTest: 表示／null非表示／空文字非表示／複数配送／一部のみ入力／XSSエスケープ／翻訳定義 を追加。front-mypage.spec.ts（E2E）: 管理画面でお問い合わせ番号を設定→マイページで表示を確認。受注番号をキーに検索・直接遷移し一覧の並び順に依存しない。スコープ外
動作確認
MypageControllerTest全件パス（既存含む 17 件）<script>でエスケープ・未実行）エビデンス画像
管理画面の受注編集でお問い合わせ番号を入力
マイページ注文履歴詳細（配送情報セクション拡大）
マイページ注文履歴詳細（ページ全体）
<script>alert(1)</script>を設定 → エスケープ表示で実行されないtracking_number = NULL→ 項目自体が非表示tracking_number = ''（空文字列）→ 項目自体が非表示受入基準（Issue §9）
null／空文字列 → 項目非表示🤖 Generated with Claude Code
Summary by CodeRabbit
新機能
ドキュメント（ローカライズ）
テスト
CI