Fix the "Account Number" column to display either the account number or the IBAN #59724
Conversation
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
Testing GuidelinesHi @woocommerce/moltres, 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:
|
…mmerce, woocommerce/client/admin
📝 WalkthroughWalkthroughA changelog entry was added for a WooCommerce plugin fix that updates the display logic for bank account details in the BACS payment method. The user interface now shows "Account Details" and displays either the account number or, if absent, the IBAN for each account. No public APIs were changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AdminUI
participant BankAccountsList
User->>AdminUI: View BACS payment method settings
AdminUI->>BankAccountsList: Request bank account details
BankAccountsList-->>AdminUI: Provide account number or IBAN for each account
AdminUI-->>User: Display "Account Details" column with account number or IBAN
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Size Change: +1.33 kB (+0.02%) Total Size: 5.86 MB
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/woocommerce/client/admin/client/settings-payments/components/bank-accounts-list/bank-accounts-list.tsx (1)
167-169: Minor: favour nullish-coalescing for clearer intentUsing
||may fall back to the IBAN whenaccount_numberis0, which is falsy but still meaningful. Switching to??keeps the fallback strictly fornull/undefined.-{ account.account_number || account.iban } +{ account.account_number ?? account.iban }plugins/woocommerce/client/admin/client/settings-payments/offline/settings-payments-bacs.tsx (1)
250-253: Grammar & consistency: replace “Accounts details”“Accounts details” sounds awkward; singular “Account details” (or “Bank account details”) is more natural and matches other WooCommerce copy.
-title={ __( 'Accounts details', 'woocommerce' ) } -description={ __( 'Configure your bank accounts details.', 'woocommerce' ) } +title={ __( 'Account details', 'woocommerce' ) } +description={ __( 'Configure your bank account details.', 'woocommerce' ) }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/woocommerce/changelog/59724-fix-show-iban-details(1 hunks)plugins/woocommerce/client/admin/client/settings-payments/components/bank-accounts-list/bank-accounts-list.tsx(2 hunks)plugins/woocommerce/client/admin/client/settings-payments/offline/settings-payments-bacs.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{php,js,ts,jsx,tsx}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-06-30T09:26:55.361Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.815Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Learnt from: prettyboymp
PR: woocommerce/woocommerce#59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:60-83
Timestamp: 2025-06-26T12:45:40.709Z
Learning: WooCommerce uses WordPress versioning conventions where minor versions in X.Y.Z format are constrained to 0-9 (Y cannot exceed 9). This means version increment logic should reset minor to 0 and increment major when minor reaches 9, rather than allowing two-digit minor versions like 9.10 or 9.11.
Learnt from: ralucaStan
PR: woocommerce/woocommerce#58782
File: plugins/woocommerce/client/blocks/assets/js/base/utils/render-frontend.tsx:0-0
Timestamp: 2025-06-16T16:12:12.148Z
Learning: For WooCommerce checkout blocks, lazy loading was removed in favor of direct imports to prevent sequential "popping" effects during component loading. This approach prioritizes user experience over code splitting, with minimal bundle size impact and improved performance (1.7s to 1.1s speed score improvement). The checkout flow benefits from having all components load together rather than incrementally.
Learnt from: opr
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-20T17:38:16.565Z
Learning: WooCommerce legacy JavaScript files in plugins/woocommerce/client/legacy/js/ must use older JavaScript syntax and cannot use modern features like optional chaining (?.) due to browser compatibility requirements. Explicit null checking with && operators should be used instead.
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/src/Internal/Admin/Settings/Payments.php:431-439
Timestamp: 2025-06-17T14:19:30.933Z
Learning: In plugins/woocommerce/src/Internal/Admin/Settings/Payments.php, the process_payment_provider_states() method intentionally filters out payment providers that don't have a _suggestion_id. This is by design to only track state changes for gateways from partner extensions, not core WooCommerce gateways or other installed gateways.
Learnt from: Aljullu
PR: woocommerce/woocommerce#59370
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-gallery/style.scss:206-233
Timestamp: 2025-07-03T10:08:59.936Z
Learning: WooCommerce uses Autoprefixer in their PostCSS configuration (plugins/woocommerce/client/blocks/postcss.config.js) with WordPress' standard browserslist configuration (@wordpress/browserslist-config) to automatically add vendor prefixes like -webkit-mask-image based on supported browsers, eliminating the need for manual prefix additions.
Learnt from: Aljullu
PR: woocommerce/woocommerce#59370
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-gallery/style.scss:206-233
Timestamp: 2025-07-03T10:08:59.936Z
Learning: WooCommerce uses Autoprefixer in their PostCSS configuration (plugins/woocommerce/client/blocks/postcss.config.js) with WordPress' standard browserslist configuration (@wordpress/browserslist-config) to automatically add vendor prefixes like -webkit-mask-image based on supported browsers, eliminating the need for manual prefix additions.
plugins/woocommerce/client/admin/client/settings-payments/components/bank-accounts-list/bank-accounts-list.tsx (1)
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx:43-50
Timestamp: 2025-06-18T07:56:06.961Z
Learning: In plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx, the user vladolaru prefers to keep the current setUpPlugin function signature with optional positional context parameter rather than refactoring to an options object or making context required.
plugins/woocommerce/changelog/59724-fix-show-iban-details (2)
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.815Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.
plugins/woocommerce/client/admin/client/settings-payments/offline/settings-payments-bacs.tsx (2)
Learnt from: vladolaru
PR: woocommerce/woocommerce#58784
File: plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx:43-50
Timestamp: 2025-06-18T07:56:06.961Z
Learning: In plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other-payment-gateways.tsx, the user vladolaru prefers to keep the current setUpPlugin function signature with optional positional context parameter rather than refactoring to an options object or making context required.
Learnt from: vladolaru
PR: woocommerce/woocommerce#59160
File: plugins/woocommerce/client/admin/client/settings-payments/onboarding/providers/woopayments/steps/payment-methods-selection/index.tsx:0-0
Timestamp: 2025-06-26T14:56:54.917Z
Learning: In WooCommerce payments onboarding components (like plugins/woocommerce/client/admin/client/settings-payments/onboarding/providers/woopayments/steps/payment-methods-selection/index.tsx), when updating local React state based on API calls, the local state should only be updated after the API call succeeds to prevent inconsistent state if the save operation fails. The pattern is to move setPaymentMethodsState calls inside the .then() callback of the API promise.
🔇 Additional comments (2)
plugins/woocommerce/client/admin/client/settings-payments/components/bank-accounts-list/bank-accounts-list.tsx (1)
145-149: Use translatable strings for the new column header
'Account Details'is hard-coded, while the rest of WooCommerce UI relies on__()for i18n. Drop-in translators will miss this string.-<div>Account Details</div> +<div>{ __( 'Account details', 'woocommerce' ) }</div>plugins/woocommerce/changelog/59724-fix-show-iban-details (1)
1-4: Changelog entry looks goodClear, concise, and placed in the correct directory.
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
This reverts commit 6d19173.
…e/woocommerce into fix/show-iban-details
This reverts commit 790af63.
|
I think it's due to the code rabbit suggestion. I reverted it |
mordeth
approved these changes
Jul 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes proposed in this Pull Request:
This pull request enhances the BACS (Direct Bank Transfer) settings page by ensuring that an account identifier is always visible in the list of bank accounts. Previously, if a bank account had an IBAN but no account number, the "Account Details" column would appear empty.
This change modifies the
BankAccountsListcomponent to display the IBAN as a fallback if theaccount_numberis not present. This improves the user experience by providing a consistent and informative view of all configured bank accounts.This PR also includes minor copy changes for better clarity.
Closes WOOPLUG-5006
Closes #59635
How to test the changes in this Pull Request:
Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fix the "Account Number" column in BACS to display either the account number or the IBAN
Changelog Entry Comment
Comment