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 add_import_operator( $operators ) to inject the '~' => 'is_in' mapping and bumps the version 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 public function add_import_operator( $operators ) to add '~' => 'is_in'; updates version header 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

    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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • veryspry

Pre-merge checks and finishing touches

❌ 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 concisely and accurately summarizes the primary change: adding GPCP CSV import support for the is_in operator and identifies the modified file. Including the filename is slightly noisy but still relevant for quick identification, and the phrasing is specific enough for a reviewer scanning history to understand the main change.
Description Check ✅ Passed The PR description follows the repository template by including a Context section with the ticket link and a Summary that explains the change and links to the complementary GPCP PR, so the intent and dependency are clear to reviewers. The description is sufficiently complete for review, though adding brief QA/testing steps or a short note about backward compatibility would be a helpful non-critical enhancement.
✨ 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

📜 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 5d86f1c and 50d5fcf.

📒 Files selected for processing (1)
  • gravity-forms/gw-conditional-logic-operator-is-in.php (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • gravity-forms/gw-conditional-logic-operator-is-in.php

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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.

❤️ Share

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

Copy link

github-actions bot commented Sep 10, 2025

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

Generated by 🚫 dangerJS against 50d5fcf

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.

Copy link
Contributor

@saifsultanc saifsultanc left a comment

Choose a reason for hiding this comment

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

LGTM!

@SebastianWiz SebastianWiz merged commit 5850964 into master Sep 15, 2025
4 checks passed
@SebastianWiz SebastianWiz deleted the seb/add/88571-gpcp-is-in-integration branch September 15, 2025 18:59
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.

2 participants