-
Notifications
You must be signed in to change notification settings - Fork 92
gw-conditional-logic-operator-is-in.php
: Added GPCP CSV Import support for is_in
operator.
#1162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ort for `is_in` operator.
WalkthroughExtends the "Is In" operator to support GP Conditional Pricing CSV imports by conditionally registering an import-operator mapping when GP_Conditional_Pricing is available. Adds Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin (CSV Import)
participant GPCP as GP Conditional Pricing
participant WP as WP Filters
participant CLO as GF_CLO_Is_In
Note over CLO: Plugin init() checks for GP_Conditional_Pricing<br/>and registers filter gpcp_supported_import_operators
Admin->>GPCP: Start CSV import
GPCP->>WP: Apply 'gpcp_supported_import_operators' filter
WP->>CLO: Call add_import_operator(operators)
CLO-->>WP: Return operators + {'~': 'is_in'}
WP-->>GPCP: Filtered operator map
GPCP->>GPCP: Parse CSV using '~' mapped to 'is_in'
GPCP-->>Admin: Import completes with operator recognized
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this 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 (2)
gravity-forms/gw-conditional-logic-operator-is-in.php (2)
34-37
: Consider registering the filter unconditionally
The guard is safe, but unnecessary.add_filter()
is a no-op if the tag is never applied, and removing theclass_exists()
check avoids any load-order edge cases or future class name changes.- // Add support for Conditional Pricing import operators - if ( class_exists( 'GP_Conditional_Pricing' ) ) { - add_filter( 'gpcp_supported_import_operators', array( $this, 'add_import_operator' ) ); - } + // Add support for Conditional Pricing import operators (safe even if the filter is never applied). + add_filter( 'gpcp_supported_import_operators', array( $this, 'add_import_operator' ) );
208-212
: Confirm token choice (‘~’) and add phpdoc for clarity
- Verify that
~
isn’t already used by GPCP’s CSV operators and is documented for customers. If there’s any chance of collisions, consider adding an alias likein
as well.- Minor: add
@since
and param/return docs.- // Register 'is_in' operator for GP Conditional Pricing CSV imports. - public function add_import_operator( $operators ) { + /** + * Register CSV import operator(s) for GP Conditional Pricing. + * + * Maps the "~" token to the internal 'is_in' operator. + * + * @since 1.2 + * @param array $operators Operator map of CSV token => internal operator. + * @return array + */ + public function add_import_operator( $operators ) { $operators['~'] = 'is_in'; return $operators; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-conditional-logic-operator-is-in.php
(3 hunks)
🔇 Additional comments (1)
gravity-forms/gw-conditional-logic-operator-is-in.php (1)
13-13
: Version bump to 1.2 — note dependency and docs sync
Please add a short changelog/readme note that CSV import support for "is_in" requires the GPCP version that introduces thegpcp_supported_import_operators
filter (per gwconditionalpricing PR #76). Helps support/clients avoid confusion when running older GPCP.
…ort for `is_in` operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/3056241699/88571?viewId=8172236
Summary
This PR adds
is_in
as a custom import operator to GP Conditional Pricing. This goes hand-in-hand with another new PR for GPCP, which adds thegpcp_supported_import_operators
filter.