gpnf-sortable-entries.php: Fixed an issue where duplicated entries appeared in sortable child entries.
#1165
Conversation
…appeared in sortable child entries.
WalkthroughUpdates the sorting of Nested Form entries to use strict string ID comparison against cookie values and to deduplicate entries. Tracks already-added IDs, builds a new ordered list, and replaces the original entries array with the sorted, de-duplicated result. No public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant C as Cookie (entry IDs)
participant P as Server Plugin
participant S as SortEntries()
B->>P: Request entries (with cookie)
P->>C: Read ordered entry IDs
P->>S: Sort using cookie order
note right of S: For each cookie ID:<br/>- Convert to string<br/>- Find matching entry with strict ID match<br/>- Skip if already added (dedupe)<br/>- Append to sorted list
S-->>P: Sorted, de-duplicated entries
P-->>B: Response with sorted entries
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
![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
🧹 Nitpick comments (4)
gp-nested-forms/gpnf-sortable-entries.php (4)
117-128: Reduce sorting to O(n) with an index map (and use a set for membership).Avoid nested scans by pre‑indexing entries by ID and using an associative set for O(1) lookups.
- $sorted_entries = array(); - $valid_entry_ids = array(); - - foreach ( $cookie_entries as $entry_id ) { - $entry_id_str = (string) $entry_id; - - foreach ( $args['entries'] as $entry ) { - // Use strict comparison and prevent duplicates - if ( (string) $entry['id'] === $entry_id_str && ! in_array( $entry_id_str, $valid_entry_ids, true ) ) { - $sorted_entries[] = $entry; - $valid_entry_ids[] = $entry_id_str; - break; - } - } - } + $sorted_entries = array(); + $seen = array(); // set + $entries_by_id = array(); + foreach ( $args['entries'] as $e ) { + $entries_by_id[ (string) $e['id'] ] = $e; + } + foreach ( $cookie_entries as $entry_id ) { + $entry_id_str = (string) $entry_id; + if ( isset( $entries_by_id[ $entry_id_str ] ) && ! isset( $seen[ $entry_id_str ] ) ) { + $sorted_entries[] = $entries_by_id[ $entry_id_str ]; + $seen[ $entry_id_str ] = true; + } + }
130-130: Guard against stale cookies dropping entries.If the cookie misses IDs (new entries, cleared cache, etc.), unmatched entries are currently omitted. Append any not referenced by the cookie to preserve visibility.
- $args['entries'] = $sorted_entries; + // Append any entries not present in the cookie order. + foreach ( $args['entries'] as $entry ) { + $id = (string) $entry['id']; + if ( ! in_array( $id, $valid_entry_ids, true ) ) { + $sorted_entries[] = $entry; + $valid_entry_ids[] = $id; + } + } + $args['entries'] = $sorted_entries;If you adopt the O(n) refactor above, swap $valid_entry_ids for $seen lookups.
272-276: Use strict match client‑side to mirror PHP normalization.Normalize both sides to strings and use
===to avoid mixed‑type surprises.- return currentEntries.find(function(entry) { - return entry.id == entryId; - }); + return currentEntries.find(function(entry) { + return String(entry.id) === String(entryId); + });Optional: filter out unfound items to avoid
undefinedentries:var sortedEntries = sortedEntryIds.map(...).filter(Boolean);
292-292: Remove debug logging.
console.log(gpnf)is noisy in production.- console.log(gpnf)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gp-nested-forms/gpnf-sortable-entries.php(1 hunks)
🔇 Additional comments (1)
gp-nested-forms/gpnf-sortable-entries.php (1)
115-125: Good fix: strict ID comparison + dedupe prevents duplicate child entries.Casting to string and using strict checks with a seen set resolves the mixed‑type duplication issue.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3069089927/89053?viewId=3808239
Summary
When using GP Nested Forms with both sortable entries and duplication features enabled, extra child entries would appear after duplicating an entry, editing it, and navigating between form pages.
Quick loom showing issue: https://www.loom.com/share/246831998d5344c6bfdc937cca04b470
The issue was that entry IDs were stored in different formats (strings and numbers) but compared using loose matching (
==). This made the system think identical entries were different, causing duplicate entries to be added when users navigated between pages.Solution: Fixed the issue by making sure all entry IDs are compared consistently. We now convert all entry IDs to strings and use strict matching (
===) for comparison. We also track which entries have already been processed to make sure each entry is only added onceQuick after loom: https://www.loom.com/share/4f2af783bfc64c2782a8fd46b9ee1126