Show correct heading for billing address when Local Pickup is selected#58220
Show correct heading for billing address when Local Pickup is selected#58220senadir merged 5 commits intowoocommerce:trunkfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdjusts checkout billing address block text when Local Pickup is selected and billing is forced: the UI shows "Billing address" instead of "Billing and shipping address" by treating Local Pickup as exempt from forced-billing wording. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Checkout Billing Component
participant Hook as useShippingData Hook
participant Utils as Title/Description Utils
participant UI as Rendered UI
Component->>Hook: request shipping data
Hook-->>Component: returns shipping rates (includes pickup selection)
Component->>Utils: get title/description (forcedBilling, hasSelectedLocalPickup)
Utils-->>Component: computed title/description
Component->>UI: render billing block with title/description
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Testing GuidelinesHi @woocommerce/rubik, Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
|
Hi, I'll be AFK until 20th February, and due to capacity constraints it's not likely this will be reviewed before then, so I appreciate your patience and I will review when I get back. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/utils.tsx (2)
24-40:⚠️ Potential issue | 🟠 MajorSame parameter issue applies here.
This function also requires the new
isLocalPickupparameter that isn't passed fromedit.tsx. Apply the same fix (make parameter optional with defaultfalseor updateedit.tsx).Make parameter optional
export const getBillingAddresssBlockDescription = ( description: string, forcedBillingAddress: boolean, - isLocalPickup: boolean + isLocalPickup: boolean = false ): string => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/utils.tsx` around lines 24 - 40, The getBillingAddresssBlockDescription function signature must accept an optional isLocalPickup so callers that haven't been updated (like edit.tsx) won't break; update the function declaration (getBillingAddresssBlockDescription) to make isLocalPickup default to false (so callers can omit it) and ensure the implementation uses that parameter as-is, or alternatively update edit.tsx to pass the new isLocalPickup boolean from component state/props whenever calling getBillingAddresssBlockDescription.
11-22:⚠️ Potential issue | 🟠 MajorMissing third parameter in
edit.tsxcall site.The function signature now requires 3 parameters (
title/description,forcedBillingAddress,isLocalPickup), butedit.tsx(lines 42–49) calls both functions with only 2 parameters. This breaks backwards compatibility and will cause TypeScript errors or undefined behavior at runtime.Since the editor context doesn't support local pickup selection, either:
- Make
isLocalPickupoptional with a default offalse, or- Update
edit.tsxto passfalseexplicitlyThe frontend implementation (lines 55–64) correctly passes the third parameter, showing the expected usage pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/utils.tsx` around lines 11 - 22, The call site in edit.tsx invokes getBillingAddresssBlockTitle with only two args causing errors because the function now requires isLocalPickup; make the third parameter optional with a default false (e.g., change the signature of getBillingAddresssBlockTitle to accept isLocalPickup?: boolean = false) so existing two-arg calls in edit.tsx continue to work, and ensure any other call sites that do know local-pickup pass the boolean explicitly.
🧹 Nitpick comments (1)
plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/frontend.tsx (1)
43-49: Use the existinghasSelectedLocalPickupfrom the hook instead of re-deriving it.The
useShippingDatahook already exportshasSelectedLocalPickup(returned at line 126 of use-shipping-data.ts). This manual computation duplicates existing logic and should be replaced with the hook's property.♻️ Simplify by using the hook's existing property
- const { shippingRates } = useShippingData(); - - const hasSelectedLocalPickup = shippingRates.some( ( pkg ) => - pkg.shipping_rates.some( - ( rate ) => rate.method_id === 'pickup_location' && rate.selected - ) - ); + const { hasSelectedLocalPickup } = useShippingData();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/frontend.tsx` around lines 43 - 49, The code is recomputing hasSelectedLocalPickup by iterating shippingRates; instead import and use the existing hasSelectedLocalPickup returned by useShippingData instead of re-deriving it—remove the manual computation that references shippingRates and replace uses of the local hasSelectedLocalPickup with the hook-provided hasSelectedLocalPickup from useShippingData to avoid duplicated logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/utils.tsx`:
- Around line 24-40: The getBillingAddresssBlockDescription function signature
must accept an optional isLocalPickup so callers that haven't been updated (like
edit.tsx) won't break; update the function declaration
(getBillingAddresssBlockDescription) to make isLocalPickup default to false (so
callers can omit it) and ensure the implementation uses that parameter as-is, or
alternatively update edit.tsx to pass the new isLocalPickup boolean from
component state/props whenever calling getBillingAddresssBlockDescription.
- Around line 11-22: The call site in edit.tsx invokes
getBillingAddresssBlockTitle with only two args causing errors because the
function now requires isLocalPickup; make the third parameter optional with a
default false (e.g., change the signature of getBillingAddresssBlockTitle to
accept isLocalPickup?: boolean = false) so existing two-arg calls in edit.tsx
continue to work, and ensure any other call sites that do know local-pickup pass
the boolean explicitly.
---
Nitpick comments:
In
`@plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/frontend.tsx`:
- Around line 43-49: The code is recomputing hasSelectedLocalPickup by iterating
shippingRates; instead import and use the existing hasSelectedLocalPickup
returned by useShippingData instead of re-deriving it—remove the manual
computation that references shippingRates and replace uses of the local
hasSelectedLocalPickup with the hook-provided hasSelectedLocalPickup from
useShippingData to avoid duplicated logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 96ff3f85-9338-4c67-9b5c-e37dba380b82
📒 Files selected for processing (3)
plugins/woocommerce/changelog/fix-58213plugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/frontend.tsxplugins/woocommerce/client/blocks/assets/js/blocks/checkout/inner-blocks/checkout-billing-address-block/utils.tsx
Instead of manually checking for 'pickup_location' method_id which doesn't cover all local pickup methods, use the existing hasSelectedLocalPickup property from useShippingData hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
#58220) * Show correct heading for billing address when Local Pickup is selected * Refactor: Use shippingRates to reliably detect Local Pickup selection * Add the changelog file * Use hasSelectedLocalPickup from useShippingData directly Instead of manually checking for 'pickup_location' method_id which doesn't cover all local pickup methods, use the existing hasSelectedLocalPickup property from useShippingData hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
#58220) * Show correct heading for billing address when Local Pickup is selected * Refactor: Use shippingRates to reliably detect Local Pickup selection * Add the changelog file * Use hasSelectedLocalPickup from useShippingData directly Instead of manually checking for 'pickup_location' method_id which doesn't cover all local pickup methods, use the existing hasSelectedLocalPickup property from useShippingData hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
#58220) * Show correct heading for billing address when Local Pickup is selected * Refactor: Use shippingRates to reliably detect Local Pickup selection * Add the changelog file * Use hasSelectedLocalPickup from useShippingData directly Instead of manually checking for 'pickup_location' method_id which doesn't cover all local pickup methods, use the existing hasSelectedLocalPickup property from useShippingData hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Seghir Nadir <nadir.seghir@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #58213 .
(For Bug Fixes) Bug introduced in PR # .
Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
How to test the changes in this Pull Request
Enable Local Pickup
WooCommerce > Settings > Shipping.Enable "Force shipping to the customer billing address"
WooCommerce > Settings > Shipping > Shipping destination.Navigate to Checkout
Verify heading behavior
This ensures the heading accurately reflects the actual checkout scenario.
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
Fix - Show 'Billing address' instead of 'Billing and shipping address' when Local Pickup is selected and shipping is forced to the billing address.