gpi-waiting-list.php: Added waiting list message on order summary page.
#1117
Conversation
WalkthroughA new filter hook is introduced to modify the order summary markup in the waiting list functionality. The code now appends a waiting list message to product names in the order summary when their available stock is zero or less. This is achieved by parsing and updating the HTML markup before it is displayed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OrderSummary
participant GPI_Waiting_List
User->>OrderSummary: View order summary
OrderSummary->>GPI_Waiting_List: Apply gform_order_summary filter (markup, form, entry)
GPI_Waiting_List->>GPI_Waiting_List: add_waiting_list_to_order_summary()
GPI_Waiting_List->>OrderSummary: Return modified markup with waiting list messages
OrderSummary->>User: Display updated order summary
✨ 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
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: 3
🧹 Nitpick comments (2)
gp-inventory/gpi-waiting-list.php (2)
291-303: Optimize nested loops and improve string comparison reliability.The nested loops could be inefficient with many products. Also, direct string comparison may fail due to whitespace or HTML entity differences.
-foreach ( $productNodes as $node ) { - foreach ( $products['products'] as $product_id => $product ) { - if ( $product['name'] == $node->nodeValue ) { +// Create a map for efficient lookup +$product_map = array(); +foreach ( $products['products'] as $product_id => $product ) { + $product_map[ trim( $product['name'] ) ] = array( 'id' => $product_id, 'product' => $product ); +} + +foreach ( $productNodes as $node ) { + $node_text = trim( $node->nodeValue ); + if ( isset( $product_map[ $node_text ] ) ) { + $product_id = $product_map[ $node_text ]['id']; + $product = $product_map[ $node_text ]['product'];
294-296: Extract inventory instance determination to reduce code duplication.This logic is duplicated from the
add_waiting_list_to_single_productmethod. Consider extracting it to a private helper method.+private function get_inventory_instance( $field ) { + return gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) ? gp_inventory_type_simple() : gp_inventory_type_advanced(); +} + // Then in both methods: -$gpi_instance = gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) ? gp_inventory_type_simple() : gp_inventory_type_advanced(); +$gpi_instance = $this->get_inventory_instance( $field );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-inventory/gpi-waiting-list.php(2 hunks)
🔇 Additional comments (1)
gp-inventory/gpi-waiting-list.php (1)
70-70: LGTM: Filter hook registration is correct.The hook registration follows WordPress best practices with appropriate priority and argument count.
| $markup = $dom->saveHTML(); | ||
| return $markup; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve original markup structure when returning modified HTML.
The saveHTML() method may add DOCTYPE and wrapper elements that weren't in the original markup, potentially breaking the layout.
-$markup = $dom->saveHTML();
-return $markup;
+// Get only the body content to preserve original structure
+$body = $dom->getElementsByTagName( 'body' )->item( 0 );
+if ( $body ) {
+ $markup = '';
+ foreach ( $body->childNodes as $child ) {
+ $markup .= $dom->saveHTML( $child );
+ }
+} else {
+ $markup = $dom->saveHTML();
+}
+return $markup;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $markup = $dom->saveHTML(); | |
| return $markup; | |
| // Get only the body content to preserve original structure | |
| $body = $dom->getElementsByTagName( 'body' )->item( 0 ); | |
| if ( $body ) { | |
| $markup = ''; | |
| foreach ( $body->childNodes as $child ) { | |
| $markup .= $dom->saveHTML( $child ); | |
| } | |
| } else { | |
| $markup = $dom->saveHTML(); | |
| } | |
| return $markup; |
🤖 Prompt for AI Agents
In gp-inventory/gpi-waiting-list.php around lines 305 to 306, the use of
$dom->saveHTML() returns the entire document including added DOCTYPE and wrapper
elements, which can alter the original markup structure and break layout. To fix
this, extract and return only the inner HTML of the specific element you
modified instead of the whole document. This preserves the original markup
structure without extra wrappers.
| libxml_use_internal_errors( true ); | ||
| $dom = new DOMDocument(); | ||
| $dom->loadHTML( mb_convert_encoding( $markup, 'HTML-ENTITIES', 'UTF-8' ) ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle DOM parsing errors and consider markup preservation.
The DOMDocument parsing could fail or alter the original markup structure. Consider adding error handling and preserving the original markup format.
libxml_use_internal_errors( true );
$dom = new DOMDocument();
-$dom->loadHTML( mb_convert_encoding( $markup, 'HTML-ENTITIES', 'UTF-8' ) );
+$success = $dom->loadHTML( mb_convert_encoding( $markup, 'HTML-ENTITIES', 'UTF-8' ), LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD );
+if ( ! $success ) {
+ return $markup; // Return original markup if parsing fails
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| libxml_use_internal_errors( true ); | |
| $dom = new DOMDocument(); | |
| $dom->loadHTML( mb_convert_encoding( $markup, 'HTML-ENTITIES', 'UTF-8' ) ); | |
| libxml_use_internal_errors( true ); | |
| $dom = new DOMDocument(); | |
| $success = $dom->loadHTML( mb_convert_encoding( $markup, 'HTML-ENTITIES', 'UTF-8' ), LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD ); | |
| if ( ! $success ) { | |
| return $markup; // Return original markup if parsing fails | |
| } |
🤖 Prompt for AI Agents
In gp-inventory/gpi-waiting-list.php around lines 285 to 287, the DOMDocument
loadHTML call lacks error handling and may alter the original markup. Add checks
to capture and handle any parsing errors from libxml after calling loadHTML, and
consider preserving the original markup by storing it before parsing or using
alternative methods that maintain the markup structure.
| $field = GFAPI::get_field( $form, $product_id ); | ||
| $gpi_instance = gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) ? gp_inventory_type_simple() : gp_inventory_type_advanced(); | ||
| $available = (int) $gpi_instance->get_available_stock( $field ); | ||
| if ( $available <= 0 ) { | ||
| $node->nodeValue .= $this->waitlist_message; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for inventory field compatibility.
The code assumes the field is an inventory field but doesn't validate this before calling inventory methods, which could cause errors.
$field = GFAPI::get_field( $form, $product_id );
+
+// Validate that this is an inventory field
+if ( ! gp_inventory_type_simple()->is_applicable_field( $field ) &&
+ ! gp_inventory_type_advanced()->is_applicable_field( $field ) ) {
+ continue;
+}
+
$gpi_instance = gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) ? gp_inventory_type_simple() : gp_inventory_type_advanced();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $field = GFAPI::get_field( $form, $product_id ); | |
| $gpi_instance = gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) ? gp_inventory_type_simple() : gp_inventory_type_advanced(); | |
| $available = (int) $gpi_instance->get_available_stock( $field ); | |
| if ( $available <= 0 ) { | |
| $node->nodeValue .= $this->waitlist_message; | |
| } | |
| $field = GFAPI::get_field( $form, $product_id ); | |
| // Validate that this is an inventory field | |
| if ( ! gp_inventory_type_simple()->is_applicable_field( $field ) && | |
| ! gp_inventory_type_advanced()->is_applicable_field( $field ) ) { | |
| continue; | |
| } | |
| $gpi_instance = gp_inventory_type_simple()::$type === rgar( $field, 'gpiInventory' ) | |
| ? gp_inventory_type_simple() | |
| : gp_inventory_type_advanced(); | |
| $available = (int) $gpi_instance->get_available_stock( $field ); | |
| if ( $available <= 0 ) { | |
| $node->nodeValue .= $this->waitlist_message; | |
| } |
🤖 Prompt for AI Agents
In gp-inventory/gpi-waiting-list.php around lines 294 to 299, the code assumes
the field is an inventory field without validating it, which may cause errors
when calling inventory methods. Add a check to confirm the field is indeed an
inventory field before proceeding with inventory-related operations. If the
field is not compatible, skip the inventory logic to prevent potential errors.
There was a problem hiding this comment.
@saifsultanc I do have an alternate approach suggestion / idea.
https://www.loom.com/share/b0750f623ff84ed5af1590d6c09241fc
Let me know what you think!
Edit: Something I didn't say in the video is that creating a "custom" view template would also make this a little more resilient to GF core template changes.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2950621761/84154
Summary
When using the waiting list snippet feature on a Product choice field, it doesn't show any sign or indication in the order summary on the entry page, that an exhausted selected choice is on the waiting list.
BEFORE:
AFTER:
