gpeb-multi-field-sorting.php: Added snippet for multi field sorting on Entry Blocks.
#1107
Conversation
… on Entry Blocks.
|
Caution Review failedThe pull request is closed. """ WalkthroughA new class Changes
Sequence Diagram(s)sequenceDiagram
participant GravityForms
participant GPEB
participant MultiFieldSorter
GravityForms->>GPEB: Fetch entries for form 933
GPEB->>MultiFieldSorter: Apply gpeb_queryer_entries filter
MultiFieldSorter->>MultiFieldSorter: Sort entries by primary field (1.6), then secondary field (1.3)
MultiFieldSorter-->>GPEB: Return sorted entries
GPEB-->>GravityForms: Provide sorted entries
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (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
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: 2
🧹 Nitpick comments (4)
gp-entry-blocks/gpeb-multi-field-sorting.php (4)
6-6: Documentation comment doesn't match code functionality.The comment "Use confirmation message from Form Settings when editing an entry in GPEB" doesn't reflect what this code actually does (multi-field sorting). This might cause confusion for future developers.
-/** - * Gravity Perks // Entry Blocks // Multi Field Sorting - * https://gravitywiz.com/documentation/gravity-forms-entry-blocks/ - * - * Use confirmation message from Form Settings when editing an entry in GPEB. - * - * Instruction Video: https://www.loom.com/share/b9867d2735d44519bf563961e9b30bd2 - */ +/** + * Gravity Perks // Entry Blocks // Multi Field Sorting + * https://gravitywiz.com/documentation/gravity-forms-entry-blocks/ + * + * Adds multi-field sorting capability to Entry Blocks, allowing entries to be sorted + * by a primary field first and then by a secondary field when primary values are equal. + * + * Instruction Video: https://www.loom.com/share/b9867d2735d44519bf563961e9b30bd2 + */
13-15: Consider making the form ID configurable.The form ID is hardcoded to 933, which makes this snippet non-reusable for other forms without code modification. Consider making this configurable through a constant or filter.
- // Update the form ID to match your form. - if ( $gf_queryer->form_id != 933 ) { + // Define form ID at the top of the file or make it configurable + $target_form_id = apply_filters( 'gpeb_multi_sort_form_id', 933 ); + + // Only apply custom sorting to the target form + if ( $gf_queryer->form_id != $target_form_id ) { return $entries; }
18-20: Make sorting configuration more flexible.The sorting field IDs and direction are hardcoded, limiting flexibility. Consider making them configurable via filters for easier customization.
- // Update the field IDs to match your form. - $primary_sorting_field_id = '1.6'; - $secondary_sorting_field_id = '1.3'; - $sorting_direction = 'ASC'; // 'ASC' or 'DESC' + // Get sorting configuration with defaults that can be filtered + $sort_config = apply_filters( 'gpeb_multi_sort_config', array( + 'primary_field' => '1.6', + 'secondary_field' => '1.3', + 'direction' => 'ASC', // 'ASC' or 'DESC' + ), $gf_queryer->form_id ); + + $primary_sorting_field_id = $sort_config['primary_field']; + $secondary_sorting_field_id = $sort_config['secondary_field']; + $sorting_direction = $sort_config['direction'];
1-42: Add error handling and performance considerations.The code lacks error handling for edge cases and could benefit from performance improvements for larger datasets.
Consider adding:
- Error logging for invalid field IDs or sorting directions
- Limit on the number of entries to sort (for performance)
- Basic caching mechanism for repeated queries with the same parameters
Example implementation for error logging:
// Check if fields exist in the form before sorting $form = GFAPI::get_form($gf_queryer->form_id); $primary_field_exists = false; $secondary_field_exists = false; foreach ($form['fields'] as $field) { if ($field->id == $primary_sorting_field_id) { $primary_field_exists = true; } if ($field->id == $secondary_sorting_field_id) { $secondary_field_exists = true; } } if (!$primary_field_exists || !$secondary_field_exists) { // Log error using Gravity Forms logging GFCommon::log_error('GPEB Multi-field Sorting: One or more sorting fields do not exist in form ' . $gf_queryer->form_id); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-entry-blocks/gpeb-multi-field-sorting.php(1 hunks)
🔇 Additional comments (1)
gp-entry-blocks/gpeb-multi-field-sorting.php (1)
37-38: Validate sorting direction parameter.The code doesn't validate if $sorting_direction has a valid value. This could lead to unexpected behavior if an invalid value is provided.
- return ( $sorting_direction === 'DESC' ) ? -$cmp : $cmp; + // Validate sorting direction and apply appropriate comparison + $is_descending = strtoupper($sorting_direction) === 'DESC'; + return $is_descending ? -$cmp : $cmp;
| $cmp = strcasecmp( $a_primary, $b_primary ); | ||
| if ( $cmp === 0 ) { | ||
| $a_secondary = isset( $a[ $secondary_sorting_field_id ] ) ? $a[ $secondary_sorting_field_id ] : ''; | ||
| $b_secondary = isset( $b[ $secondary_sorting_field_id ] ) ? $b[ $secondary_sorting_field_id ] : ''; | ||
| $cmp = strcasecmp( $a_secondary, $b_secondary ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve comparison logic for different data types.
The current implementation uses strcasecmp which is only appropriate for strings. Add support for numeric and other data type comparisons.
- $cmp = strcasecmp( $a_primary, $b_primary );
- if ( $cmp === 0 ) {
- $a_secondary = isset( $a[ $secondary_sorting_field_id ] ) ? $a[ $secondary_sorting_field_id ] : '';
- $b_secondary = isset( $b[ $secondary_sorting_field_id ] ) ? $b[ $secondary_sorting_field_id ] : '';
- $cmp = strcasecmp( $a_secondary, $b_secondary );
- }
+ // Compare primary values based on data type
+ if ($is_numeric) {
+ $a_primary = (float) $a_primary;
+ $b_primary = (float) $b_primary;
+ $cmp = $a_primary <=> $b_primary; // Spaceship operator for numeric comparison
+ } else {
+ $cmp = strcasecmp( $a_primary, $b_primary );
+ }
+
+ // If primary values are equal, compare secondary values
+ if ( $cmp === 0 ) {
+ $a_secondary = isset( $a[ $secondary_sorting_field_id ] ) ? $a[ $secondary_sorting_field_id ] : '';
+ $b_secondary = isset( $b[ $secondary_sorting_field_id ] ) ? $b[ $secondary_sorting_field_id ] : '';
+
+ // Determine if secondary values are numeric
+ $is_secondary_numeric = is_numeric($a_secondary) && is_numeric($b_secondary);
+
+ if ($is_secondary_numeric) {
+ $a_secondary = (float) $a_secondary;
+ $b_secondary = (float) $b_secondary;
+ $cmp = $a_secondary <=> $b_secondary;
+ } else {
+ $cmp = strcasecmp( $a_secondary, $b_secondary );
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In gp-entry-blocks/gpeb-multi-field-sorting.php around lines 30 to 35, the
comparison uses strcasecmp which only works correctly for strings. Modify the
comparison logic to first check the data type of the values being compared; if
both are numeric, use numeric comparison, otherwise fall back to string
comparison with strcasecmp. Apply this improved logic for both primary and
secondary sorting fields to handle different data types correctly.
| $a_primary = isset( $a[ $primary_sorting_field_id ] ) ? $a[ $primary_sorting_field_id ] : ''; | ||
| $b_primary = isset( $b[ $primary_sorting_field_id ] ) ? $b[ $primary_sorting_field_id ] : ''; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add data type handling for field values.
The current implementation assumes string values for sorting. Consider adding support for different data types (numbers, dates) for more accurate sorting.
- $a_primary = isset( $a[ $primary_sorting_field_id ] ) ? $a[ $primary_sorting_field_id ] : '';
- $b_primary = isset( $b[ $primary_sorting_field_id ] ) ? $b[ $primary_sorting_field_id ] : '';
+ // Get field values with empty fallback
+ $a_primary = isset( $a[ $primary_sorting_field_id ] ) ? $a[ $primary_sorting_field_id ] : '';
+ $b_primary = isset( $b[ $primary_sorting_field_id ] ) ? $b[ $primary_sorting_field_id ] : '';
+
+ // Determine if values are numeric for appropriate comparison
+ $is_numeric = is_numeric($a_primary) && is_numeric($b_primary);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In gp-entry-blocks/gpeb-multi-field-sorting.php around lines 27 to 28, the code
currently assumes the sorting field values are strings, which can cause
incorrect sorting for numbers or dates. Update the code to detect the data type
of the field values and convert or cast them appropriately before comparison,
handling numbers as numeric types and dates as timestamps to ensure accurate
sorting.
There was a problem hiding this comment.
I dig this suggestion, but probably out of scope given that this is just a snippet
There was a problem hiding this comment.
@veryspry Understood! You're right that adding data type handling would be overkill for a simple snippet. The current string-based comparison will work fine for the basic use case. Let me know if you need anything else!
🐰✨ nods understandingly
| add_filter( 'gpeb_queryer_entries', function( $entries, $gf_queryer ) { | ||
|
|
||
| // Update the form ID to match your form. | ||
| if ( $gf_queryer->form_id != 933 ) { |
There was a problem hiding this comment.
Can you store the form id in a variable? (I think that's how we usually do it)
| $a_primary = isset( $a[ $primary_sorting_field_id ] ) ? $a[ $primary_sorting_field_id ] : ''; | ||
| $b_primary = isset( $b[ $primary_sorting_field_id ] ) ? $b[ $primary_sorting_field_id ] : ''; |
There was a problem hiding this comment.
I dig this suggestion, but probably out of scope given that this is just a snippet
| * | ||
| * Instruction Video: https://www.loom.com/share/b9867d2735d44519bf563961e9b30bd2 | ||
| */ | ||
| add_filter( 'gpeb_queryer_entries', function( $entries, $gf_queryer ) { |
There was a problem hiding this comment.
What do you think about wrapping all this in a function that takes an array arg so?
array(
'form_id' => $form_id,
'primary_sorting_field_id' => $primary_sorting_field_id,
'secondary_sorting_field_id' => $secondary_sorting_field_id,
'sorting_direction' => $sorting_direction, //defaults to ASC
);
… on Entry Blocks.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2945420521/83832
Summary
Add support for multi-level sorting in Entry Blocks, allowing users to define a primary and secondary sort field.
Demo:
https://www.loom.com/share/b9867d2735d44519bf563961e9b30bd2