feat(checkout-form): warn when required field is missing for products#1146
feat(checkout-form): warn when required field is missing for products#1146superdav42 merged 1 commit intomainfrom
Conversation
Add admin notices in the checkout form editor when products configured in pricing_table or products fields require a companion field that the form does not provide: - Period Selection notice: products with price variations need a period_selection field for customers to switch between billing cycles. - Template Selection notice: products that don't force a specific site template (site_templates limitation mode != MODE_ASSIGN_TEMPLATE) need a template_selection field for customers to pick a template. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds conditional admin notices to the checkout form editor. Backend helper methods analyze products to identify which are missing required selection fields (template or period), compute product name lists, pass them to the editor view, and the view renders warnings with actionable instructions when products need selection capability. ChangesCheckout Form Editor Missing Selection Field Notices
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
views/base/checkout-forms/steps.php (1)
12-12: 💤 Low valueDefensive: guard against the variables not being defined.
$price_variation_notice_productsand$template_selection_notice_productsare always passed by the current caller inrender_steps(), but this view is plainwu_get_template-extracted state, so a missed callsite would surface as anundefined variablewarning at the top of the editor. A cheapisset()(or?? []) keeps the view robust if it's ever rendered from another path.🛡️ Suggested guard
- <?php if ( ! empty($price_variation_notice_products)) : ?> + <?php if ( ! empty($price_variation_notice_products ?? [])) : ?> ... - <?php if ( ! empty($template_selection_notice_products)) : ?> + <?php if ( ! empty($template_selection_notice_products ?? [])) : ?>Also applies to: 32-32
🤖 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 `@views/base/checkout-forms/steps.php` at line 12, The view uses $price_variation_notice_products and $template_selection_notice_products directly which can trigger "undefined variable" warnings if this template is rendered outside render_steps(); before any conditional checks (e.g., the if(...) that references these variables) ensure each variable is defined/defaulted (use isset() or the null coalescing operator to set them to an empty array) so the existing conditionals work safely even when the caller forgets to pass them.inc/admin-pages/class-checkout-form-edit-admin-page.php (1)
1116-1265: ⚡ Quick winExtract shared product-ID collection to remove duplication.
get_template_selection_notice_products()andget_price_variation_notice_products()share ~30 lines of identical logic for parsingpricing_table/productsfield IDs and turning them into a deduped product-ID set. Only the per-product predicate differs. Extracting a shared helper keeps the two notice methods focused on their predicates and avoids future drift between them (e.g., if a new product-ID-bearing field type is added, only one place needs updating).♻️ Suggested refactor
+ /** + * Collects product IDs referenced by `pricing_table` / `products` fields on the form. + * + * `@since` 2.4.13 + * + * `@param` \WP_Ultimo\Models\Checkout_Form $form The checkout form. + * `@return` int[] Deduped, positive product IDs. + */ + protected function collect_product_ids_from_form($form): array { + + $product_id_fields = $form->get_all_fields_by_type(['pricing_table', 'products']); + + if (empty($product_id_fields)) { + return []; + } + + $product_ids = []; + + foreach ($product_id_fields as $field) { + $type = $field['type'] ?? ''; + + if ('pricing_table' === $type) { + $ids_string = (string) ($field['pricing_table_products'] ?? ''); + } elseif ('products' === $type) { + $ids_string = (string) ($field['products'] ?? ''); + } else { + continue; + } + + if ('' === $ids_string) { + continue; + } + + foreach (explode(',', $ids_string) as $id) { + $id = absint(trim($id)); + + if ($id) { + $product_ids[ $id ] = $id; + } + } + } + + return $product_ids; + }Then both methods reduce to:
- $product_id_fields = $form->get_all_fields_by_type(['pricing_table', 'products']); - - if (empty($product_id_fields)) { - return []; - } - - $product_ids = []; - - foreach ($product_id_fields as $field) { - ... - } - - if (empty($product_ids)) { - return []; - } + $product_ids = $this->collect_product_ids_from_form($form); + + if (empty($product_ids)) { + return []; + }🤖 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 `@inc/admin-pages/class-checkout-form-edit-admin-page.php` around lines 1116 - 1265, Both notice methods duplicate the logic that parses 'pricing_table'/'products' fields into a deduplicated list of product IDs; extract that logic into a single helper (e.g., protected function collect_product_ids_from_form($form): array) and call it from get_template_selection_notice_products() and get_price_variation_notice_products(). The helper should accept the Checkout_Form instance (or the form object returned by $this->get_object()), iterate the fields returned by get_all_fields_by_type(['pricing_table','products']), build the same deduped array of absint(trim($id)) product IDs, skip empty strings, and return [] when none found so existing per-product predicates (template/limitations or price variations) remain unchanged. Update both notice methods to remove the duplicated loops and use the new helper while preserving all original behavior and return values.
🤖 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 `@inc/admin-pages/class-checkout-form-edit-admin-page.php`:
- Around line 1116-1265: Both notice methods duplicate the logic that parses
'pricing_table'/'products' fields into a deduplicated list of product IDs;
extract that logic into a single helper (e.g., protected function
collect_product_ids_from_form($form): array) and call it from
get_template_selection_notice_products() and
get_price_variation_notice_products(). The helper should accept the
Checkout_Form instance (or the form object returned by $this->get_object()),
iterate the fields returned by
get_all_fields_by_type(['pricing_table','products']), build the same deduped
array of absint(trim($id)) product IDs, skip empty strings, and return [] when
none found so existing per-product predicates (template/limitations or price
variations) remain unchanged. Update both notice methods to remove the
duplicated loops and use the new helper while preserving all original behavior
and return values.
In `@views/base/checkout-forms/steps.php`:
- Line 12: The view uses $price_variation_notice_products and
$template_selection_notice_products directly which can trigger "undefined
variable" warnings if this template is rendered outside render_steps(); before
any conditional checks (e.g., the if(...) that references these variables)
ensure each variable is defined/defaulted (use isset() or the null coalescing
operator to set them to an empty array) so the existing conditionals work safely
even when the caller forgets to pass them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d28a2e53-2f4b-4ce6-9ff2-446feb107c7d
📒 Files selected for processing (2)
inc/admin-pages/class-checkout-form-edit-admin-page.phpviews/base/checkout-forms/steps.php
Summary
Test plan
Merged via PR #1146 to main. aidevops.sh v3.14.83 spent 32s on this as a headless bash routine. |
|
Performance Test Results Performance test results for 7722bef are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
1 participant
Summary
period_selectionfield is on the form.site_templateslimitation mode !=MODE_ASSIGN_TEMPLATE) are configured but notemplate_selectionfield is on the form.Test plan
pricing_tablefield referencing a product that has price variations, and noperiod_selectionfield → notice appears.period_selectionfield → notice disappears.productsfield referencing a product whose site template limitation mode is notassign_template, and notemplate_selectionfield → notice appears.template_selectionfield, OR set the product's site template mode toassign_template→ notice disappears.🤖 Generated with Claude Code
Summary by CodeRabbit