gpecf-deduct-deposit.php: Fixed an issue where disabled deposit field caused issues with calculations.
#1149
Conversation
…ld caused issues with calculations.
WalkthroughAdded an early-return guard in the gform_product_total filter: when the target form ID matches and the deposit input is disabled, the filter returns the original total without running deposit deduction. Existing deposit retrieval and subtraction logic runs only when the deposit field is enabled. Changes
Sequence Diagram(s)sequenceDiagram
participant GF as Gravity Forms (Total Calc)
participant Plugin as GW_Deduct_Deposit
GF->>Plugin: gform_product_total(formId, total)
alt formId matches target
Plugin->>Plugin: Check deposit input enabled?
alt Deposit input disabled
Plugin-->>GF: Return original total (no changes)
else Deposit input enabled
Plugin->>Plugin: Read deposit price & quantity
Plugin->>Plugin: Compute depositValue = price * qty
Plugin-->>GF: Return total - depositValue (existing logic)
end
else formId not matched
Plugin-->>GF: Return original total
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
gp-ecommerce-fields/gpecf-deduct-deposit.php (5)
73-91: Harden the guard and numeric handling; avoid NaN and missing-input cases.Edge cases to cover:
- If either the price or quantity input is missing (not rendered on the current page), jQuery returns an empty set;
.is(':disabled')on an empty set is false, and subsequent.val()calls returnundefined, leading toNaNtotals.- Quantity is not normalized to a number; rely on
gformToNumberfor both inputs.- Prefer strict equality for the
formIdcheck.Proposed refactor:
- self.init = function() { - gform.addFilter( 'gform_product_total', function( total, formId ) { - if ( formId == self.formId ) { - // If deposit is disabled or hidden, return total as it is. - if ( $( 'input[name="input_' + self.depositFieldId + '.2"]' ).is( ':disabled' ) ) { - return total; - } - - var depositPrice = $( 'input[name="input_' + self.depositFieldId + '.2"]' ).val(); - var depositQuantity = $( 'input[name="input_' + self.depositFieldId + '.3"]' ).val(); - - depositValue = gformToNumber(depositPrice) * depositQuantity; - - // since the depositValue (product field) would have been added to the total, - // it must first be removed to get base value and then discount applied (second substract). - total = total - depositValue - depositValue; - } - return total; - } ); - }; + self.init = function() { + gform.addFilter( 'gform_product_total', function( total, formId ) { + if ( formId === self.formId ) { + var $price = $( 'input[name="input_' + self.depositFieldId + '.2"]' ); + var $qty = $( 'input[name="input_' + self.depositFieldId + '.3"]' ); + + // If deposit inputs are missing or disabled, return total as-is. + if ( !$price.length || !$qty.length || $price.is(':disabled') || $qty.is(':disabled') ) { + return total; + } + + var depositPrice = gformToNumber( $price.val() ); + var depositQuantity = gformToNumber( $qty.val() || 0 ); + var depositValue = depositPrice * depositQuantity; + + if ( ! isFinite( depositValue ) ) { + return total; + } + + // Since the depositValue (product field) has already been added to the total, + // subtract it to get the base, then subtract again to apply the discount. + total -= ( depositValue * 2 ); + } + return total; + } ); + };
17-21: PHPCS precision-alignment error: remove aligned arrows in array.The pipeline is failing on WordPress.WhiteSpace.PrecisionAlignment. Replace column-aligned arrows with single spaces.
- $this->_args = wp_parse_args( $args, array( - 'form_id' => false, - 'deposit_field_id' => false, - ) ); + $this->_args = wp_parse_args( + $args, + array( + 'form_id' => false, + 'deposit_field_id' => false, + ) + );
110-117: PHPCS precision-alignment warning: standardize spacing in $args and $slug.Remove precision alignment to satisfy WPCS and unblock the pipeline.
- $args = array( - 'formId' => $this->_args['form_id'], - 'depositFieldId' => $this->_args['deposit_field_id'], - ); + $args = array( + 'formId' => $this->_args['form_id'], + 'depositFieldId' => $this->_args['deposit_field_id'], + ); $script = 'new ' . __CLASS__ . '( ' . json_encode( $args ) . ' );'; - $slug = implode( '_', array( strtolower( __CLASS__ ), $this->_args['form_id'] ) ); + $slug = implode( '_', array( strtolower( __CLASS__ ), $this->_args['form_id'] ) );
157-160: PHPCS precision-alignment warning: configuration array at bottom.Same rule applies here; avoid aligning arrows with spaces.
-new GW_Deduct_Deposit( array( - 'form_id' => 123, // Update "123" to the ID of your form. - 'deposit_field_id' => 4, // Update the "4" to your deposit field ID. -) ); +new GW_Deduct_Deposit( + array( + 'form_id' => 123, // Update "123" to the ID of your form. + 'deposit_field_id' => 4, // Update the "4" to your deposit field ID. + ) +);
132-141: Guard against missing deposit product and fix reference timing.If the deposit field is conditionally hidden, it may not be present in
$order['products'], and referencing it by key by reference will emit notices. Also, the reference is taken before$orderis reassigned, which is fragile.Refactor to process calculations first, then safely obtain the reference, and bail if the product is absent.
- $deposit =& $order['products'][ $this->_args['deposit_field_id'] ]; - - // Run this first so calculations are reprocessed before we convert deposit to a negative number. - $order = gp_ecommerce_fields()->add_ecommerce_fields_to_order( $order, $form, $entry ); + // Reprocess calculations before converting the deposit to a negative number. + $order = gp_ecommerce_fields()->add_ecommerce_fields_to_order( $order, $form, $entry ); + + // If the deposit product isn't present (e.g., field hidden/disabled), nothing to deduct. + if ( empty( $order['products'][ $this->_args['deposit_field_id'] ] ) ) { + return $order; + } + + $deposit = &$order['products'][ $this->_args['deposit_field_id'] ];
🧹 Nitpick comments (1)
gp-ecommerce-fields/gpecf-deduct-deposit.php (1)
85-87: Typo: “substract” → “subtract” in the comment.Keeps the inline docs clean and professional.
- // it must first be removed to get base value and then discount applied (second substract). + // it must first be removed to get the base value and then discount applied (second subtract).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
gp-ecommerce-fields/gpecf-deduct-deposit.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: PHPCS (Files Changed)
gp-ecommerce-fields/gpecf-deduct-deposit.php
[warning] 79-79:
Found precision alignment of 1 spaces.
🪛 GitHub Actions: PHP Lint (PR)
gp-ecommerce-fields/gpecf-deduct-deposit.php
[warning] 79-79: PHPCS warning: WordPress.WhiteSpace.PrecisionAlignment.Found - Found precision alignment of 1 spaces.
[error] 1-1: PHPCS error: Found precision alignment of 1 spaces.
🔇 Additional comments (1)
gp-ecommerce-fields/gpecf-deduct-deposit.php (1)
75-79: Good guard: prevents double-subtraction when deposit input is disabled.This early return addresses the reported frontend miscalculation when the deposit field is conditionally disabled/hidden. Nice, targeted fix.
|
Tested and confirmed by Samuel and customer |
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3042483189/88050
Summary
The snippet to use a product field to deduct deposit is causing GF to display incorrect calculations on the frontend when the Deposit field is conditionally hidden.
BEFORE (Credits Samuel):
https://www.loom.com/share/ad6470f9886c4a0983e253b23b796cd0
AFTER:
https://www.loom.com/share/4448c9551ec246c4b8756e3a64e839c0