Skip to content

Conversation

SebastianWiz
Copy link
Contributor

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 the gpcp_supported_import_operators filter.

Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Extends 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 a method to inject the '~' => 'is_in' operator via the gpcp_supported_import_operators filter. Version updated from 1.1 to 1.2.

Changes

Cohort / File(s) Summary
GPCP import operator support
gravity-forms/gw-conditional-logic-operator-is-in.php
In init(), conditionally adds filter gpcp_supported_import_operators when GP_Conditional_Pricing exists; introduces add_import_operator( $operators ) to add '~' => 'is_in'; bumps version to 1.2.

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

    Admin->>GPCP: Start CSV import
    Note over CLO: On init(): if GP_Conditional_Pricing exists<br/>add_filter('gpcp_supported_import_operators', add_import_operator)
    GPCP->>WP: Request supported import operators
    WP->>CLO: Apply gpcp_supported_import_operators
    CLO-->>WP: Return operators + {'~': 'is_in'}
    WP-->>GPCP: Filtered operators
    GPCP->>GPCP: Parse CSV using '~' mapped to 'is_in'
    GPCP-->>Admin: Import proceeds with operator support
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • veryspry

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.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 title accurately summarizes the main change — adding GPCP CSV import support for the is_in operator — and directly maps to the changeset, so it is related and clear for reviewers; it is slightly verbose because it includes the filename and backticks but remains correct and focused.
Description Check ✅ Passed The PR description follows the repository template by providing a Context section with the Helpscout ticket link and a Summary that concisely explains the change and references the related GPCP PR, so the required template information is present and useful for reviewers.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch seb/add/88571-gpcp-is-in-integration

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.

Copy link

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

Generated by 🚫 dangerJS against 5d86f1c

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 (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 the class_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 like in 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6d89429 and 5d86f1c.

📒 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 the gpcp_supported_import_operators filter (per gwconditionalpricing PR #76). Helps support/clients avoid confusion when running older GPCP.

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.

1 participant