Skip to content

Conversation

@SebastianWiz
Copy link
Contributor

Context

⛑️ Ticket(s): https://secure.helpscout.net/link/to/conversation

Summary

Issue: When using the taxonomy depth filter snippet with Multiple Choice fields (configured with anything other than "Select One" input type), selected taxonomy terms were not being saved. This affected any Multiple Choice field using checkbox-style inputs with dynamically populated taxonomy choices.

The issue occurred because GF introduced persistent choices for Multiple Choice fields, where each choice requires a unique key property for proper tracking and saving.

Fix: Updated the gppa_input_choices filter to keep existing choice data instead of replacing it. The fix maintains the unique keys that GPPA already generated for each choice and only creates new MD5 keys when needed. Uses has_persistent_choices() to identify fields that need this approach while staying compatible with older field types.

@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

The change modifies choice processing in the filter-terms-by-depth script, implementing a two-stage approach with a filtered_choices array and per-item choice objects. It introduces conditional merging logic for multi-select fields with persistent choices, matching terms by ID and preserving properties, while generating md5-based keys for new choices.

Changes

Cohort / File(s) Summary
Multi-select choice filtering and persistence logic
gp-populate-anything/gppa-filter-terms-by-depth.php
Replaces direct choice accumulation with two-stage processing using filtered_choices array. Converts each term to a $choice object with conditional merging logic: for persistent choice fields, matches by term_id and merges properties to preserve existing keys; if no match, assigns a key generated via md5(term_id). Returns $filtered_choices instead of $choices.

Sequence Diagram(s)

sequenceDiagram
    participant Func as gppa_filter_terms_by_depth()
    participant Terms as terms[]
    participant Filtered as filtered_choices[]
    participant Orig as original_choices[]
    participant Logic as Merge Logic

    Func->>Terms: iterate terms
    rect rgb(230, 240, 255)
        Note over Func: create per-term $choice object
        Func->>Filtered: append tentative $choice
    end

    alt field supports persistent choices
        rect rgb(220, 245, 220)
            Note over Logic: lookup by term_id in original_choices
            Orig->>Logic: provide matching choice?
            Logic-->>Filtered: merge properties into existing key if match
        end
    else no match / non-persistent field
        rect rgb(255, 235, 235)
            Note over Logic: ensure key exists
            Logic->>Logic: generate key = md5(term_id) if needed
            Logic-->>Filtered: add new choice with generated key
        end
    end

    Func->>Func: return filtered_choices
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Key areas requiring attention:
    • Matching and merging logic for original choices by term_id
    • Consistency and collisions risk of md5(term_id) key generation
    • Correct return and downstream expectations for filtered_choices vs original $choices

Suggested reviewers

  • veryspry
  • saifsultanc

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title clearly identifies the file being modified and accurately describes the primary issue being fixed: taxonomy selections not saving in Multiple Choice fields with checkbox input types. The title is specific and directly relates to the core problem addressed by the changeset, which involves updating the gppa_input_choices filter to preserve choice data with persistent keys for Gravity Forms fields. The title is concise and provides enough context for someone scanning the commit history to understand the main change without ambiguity.
Description Check ✅ Passed The pull request description follows the required template structure with both Context and Summary sections properly populated. The Context section includes a ticket reference, which provides traceability. The Summary section comprehensively explains the issue (taxonomy selections not saving), its root cause (Gravity Forms persistent choices requiring unique keys), and the implemented fix (preserving existing choice data while maintaining unique keys). The description is complete and provides sufficient detail for code reviewers to understand the problem domain and the solution approach.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch seb/fix/91863-multi-choice-taxonomy-selection-saving

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 308ecd3 and 60fde0e.

📒 Files selected for processing (1)
  • gp-populate-anything/gppa-filter-terms-by-depth.php (1 hunks)
🔇 Additional comments (3)
gp-populate-anything/gppa-filter-terms-by-depth.php (3)

46-53: LGTM! Clean two-stage processing approach.

The introduction of $filtered_choices and explicit choice object creation improves code clarity and sets up the structure needed for the persistent choice preservation logic.


73-73: Correct return value for the refactored logic.

Returning $filtered_choices properly completes the two-stage processing approach and ensures all the persistent choice preservation logic is applied.


55-68: The loose comparison at line 59 is inconsistent with the codebase's strict comparison pattern, but not definitively incorrect.

Verification findings:

  1. Line 59 loose comparison confirmed: The == comparison exists and is inconsistent with line 33's pattern (intval( $term->parent ) === intval( $parent_id )), which uses strict comparison with explicit type casting.

  2. isset() check already present: Line 59 includes isset( $original_choice['object'] ), so the property access is protected. WP_Term objects always provide term_id as an integer.

  3. Type handling pattern: The codebase uses intval() and === for strict comparisons where type matters (line 33), but uses loose == at line 59. Consider aligning this for consistency—either use $original_choice['object']->term_id === $object->term_id or cast both with intval().

  4. Nested loop complexity: The O(n×m) concern is noted but unlikely problematic given typical form field choice sizes.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 28, 2025

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against 60fde0e

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
gp-populate-anything/gppa-filter-terms-by-depth.php (1)

55-68: Core fix looks solid; consider minor defensive improvements.

The persistent choice preservation logic correctly addresses the bug by:

  • Using method_exists() for backward compatibility
  • Merging original choices to preserve existing keys
  • Generating stable MD5 keys for new choices

Consider these improvements:

  1. Use strict comparison for term_id matching (Line 59):
-if ( isset( $original_choice['object'] ) && $original_choice['object']->term_id == $object->term_id ) {
+if ( isset( $original_choice['object'], $original_choice['object']->term_id ) && $original_choice['object']->term_id === $object->term_id ) {

This adds a property existence check and uses strict comparison to avoid type coercion edge cases.

  1. Optional: Optimize lookup for large datasets by indexing original choices first:
// Before the foreach loop at line 48
$original_choices_by_term_id = array();
foreach ( $choices as $original_choice ) {
    if ( isset( $original_choice['object']->term_id ) ) {
        $original_choices_by_term_id[ $original_choice['object']->term_id ] = $original_choice;
    }
}

Then replace lines 58-63 with a direct lookup. This reduces complexity from O(n×m) to O(n+m).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8ed9f2 and 308ecd3.

📒 Files selected for processing (1)
  • gp-populate-anything/gppa-filter-terms-by-depth.php (1 hunks)
🔇 Additional comments (3)
gp-populate-anything/gppa-filter-terms-by-depth.php (3)

46-47: LGTM: Clean separation of input and output.

The two-stage approach with $filtered_choices improves clarity and correctly implements the choice preservation logic.


49-53: LGTM: Correct choice structure.

The choice array structure properly maps term data to GPPA's expected format.


73-73: LGTM: Correct return value.

Returning $filtered_choices properly completes the two-stage filtering and choice preservation logic.

…tions were not saving in Multiple Choice fields when using checkbox input types.
@SebastianWiz SebastianWiz force-pushed the seb/fix/91863-multi-choice-taxonomy-selection-saving branch from 308ecd3 to 60fde0e Compare October 28, 2025 02:29
@SebastianWiz SebastianWiz changed the title Fixed an issue where taxonomy selections were not saving in Multiple Choice fields when using checkbox input types. gppa-filter-terms-by-depth.php: Fixed an issue where taxonomy selections were not saving in Multiple Choice fields when using checkbox input types. Oct 28, 2025
@saifsultanc saifsultanc merged commit 668f586 into master Nov 3, 2025
5 checks passed
@saifsultanc saifsultanc deleted the seb/fix/91863-multi-choice-taxonomy-selection-saving branch November 3, 2025 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants