Repository navigation
feat/CUS--1469-Added utils to support bracket conditons - #362
Merged
akhil-testsigma merged 1 commit intoMar 24, 2026
Merged
Conversation
📝 WalkthroughWalkthroughExtracted condition evaluation logic from six platform-specific Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@if_condition_evaluator/src/main/java/com/testsigma/addons/utils/ConditionEvaluatorUtils.java`:
- Around line 184-187: allPartsValid currently rejects single-character tokens
because it returns false when p.trim().length() < 2; change the validation in
allPartsValid(List<String> parts) to only reject empty/blank tokens (e.g., use
p.trim().isEmpty() or p.trim().length() == 0) so single-character terms like "A"
or "B" are accepted and the expression expansion (e.g., "X CONTAINS (A OR B)")
works correctly; update the loop that iterates over parts and replace the length
check with an emptiness check on p.trim().
- Around line 145-170: expandInner currently expands grouped terms assuming
positive CONTAINS; when notPart indicates a NOT (i.e., handling "NOT CONTAINS"),
the boolean join must be inverted per De Morgan (X NOT CONTAINS (A OR B) -> X
NOT CONTAINS A AND X NOT CONTAINS B, and X NOT CONTAINS (A AND B) -> X NOT
CONTAINS A OR X NOT CONTAINS B) rather than leaving the join as-is. Update
expandInner (and the logic that sets op/uses expandTerm) to detect notPart (the
NOT case) and flip the joiners when building the expansion for splitByTopLevel
results: for orTerms with a NOT use " AND " between expanded terms; for andTerms
with a NOT use " OR ". Preserve the existing parenthesis-wrapping behavior
around terms that contain the opposite join and keep the allPartsValid checks;
alternatively, if you prefer rejecting grouped NOT CONTAINS, return null early
when notPart is present and there are grouped terms.
In
`@if_condition_evaluator/src/main/java/com/testsigma/addons/web/IfConditionEvaluatorAction.java`:
- Around line 25-29: Add explicit null checks for testData and
testData.getValue() before using them in IfConditionEvaluatorAction (the try
block where condition is assigned and logger.info("Original Condition: " +
condition) is executed); if testData or testData.getValue() is null, log a clear
error via logger.error (including which variable is null) and return or throw a
meaningful exception/result rather than letting a NullPointerException be caught
with an unhelpful message—update the null guard near the existing condition
assignment to validate testData and its value and provide a descriptive error
path.
In `@if_condition_evaluator/src/main/resources/testsigma-sdk.properties`:
- Line 1: The file contains a committed live credential under the property key
testsigma-sdk.api.key; remove the secret value from testsigma-sdk.properties,
replace it with a placeholder (e.g.
testsigma-sdk.api.key=${TESTSIGMA_SDK_API_KEY}) and update the code that reads
this property to load from an environment variable or your secret manager (env
var TESTSIGMA_SDK_API_KEY or secret service) instead of the repo; revoke/rotate
the exposed key immediately, purge it from history (or run git-filter-repo/BFG)
and ensure CI/build injects the secret at release time via your secrets service
or pipeline variables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28c7a7ff-1925-492a-b0e1-8a2eb9f91173
📒 Files selected for processing (9)
if_condition_evaluator/pom.xmlif_condition_evaluator/src/main/java/com/testsigma/addons/android/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/java/com/testsigma/addons/ios/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/java/com/testsigma/addons/mobileweb/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/java/com/testsigma/addons/restapi/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/java/com/testsigma/addons/utils/ConditionEvaluatorUtils.javaif_condition_evaluator/src/main/java/com/testsigma/addons/web/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/java/com/testsigma/addons/windows/IfConditionEvaluatorAction.javaif_condition_evaluator/src/main/resources/testsigma-sdk.properties
Comment on lines
+145
to
+170
| private static String expandInner(String left, String notPart, String inner) { | ||
| inner = stripOuterParens(inner); | ||
| String op = "CONTAINS"; | ||
| // Keep grouped CONTAINS terms together: "High OR Risk AND Override" -> (High OR Risk) AND Override | ||
| List<String> andTerms = splitByTopLevel(inner, "AND"); | ||
| if (andTerms.size() > 1) { | ||
| if (!allPartsValid(andTerms)) return null; | ||
| StringBuilder expansion = new StringBuilder(); | ||
| for (int i = 0; i < andTerms.size(); i++) { | ||
| if (i > 0) expansion.append(" AND "); | ||
| String term = expandTerm(left, notPart, op, andTerms.get(i)); | ||
| expansion.append(term.contains(" OR ") ? "(" + term + ")" : term); | ||
| } | ||
| return expansion.toString(); | ||
| } | ||
| List<String> orTerms = splitByTopLevel(inner, "OR"); | ||
| if (orTerms.size() > 1) { | ||
| if (!allPartsValid(orTerms)) return null; | ||
| StringBuilder expansion = new StringBuilder(); | ||
| for (int i = 0; i < orTerms.size(); i++) { | ||
| if (i > 0) expansion.append(" OR "); | ||
| String term = expandTerm(left, notPart, op, orTerms.get(i)); | ||
| expansion.append(term.contains(" AND ") ? "(" + term + ")" : term); | ||
| } | ||
| return expansion.toString(); | ||
| } |
There was a problem hiding this comment.
Grouped NOT CONTAINS uses the wrong boolean join.
X NOT CONTAINS (AB OR CD) currently expands to X NOT CONTAINS AB OR X NOT CONTAINS CD, which becomes true as soon as either token is absent. If grouped NOT CONTAINS is supported, this needs De Morgan's law; otherwise it should be rejected instead of silently changing the meaning.
🐛 Proposed fix
private static String expandInner(String left, String notPart, String inner) {
inner = stripOuterParens(inner);
String op = "CONTAINS";
+ boolean negated = !notPart.isEmpty();
// Keep grouped CONTAINS terms together: "High OR Risk AND Override" -> (High OR Risk) AND Override
List<String> andTerms = splitByTopLevel(inner, "AND");
if (andTerms.size() > 1) {
if (!allPartsValid(andTerms)) return null;
StringBuilder expansion = new StringBuilder();
for (int i = 0; i < andTerms.size(); i++) {
- if (i > 0) expansion.append(" AND ");
+ if (i > 0) expansion.append(negated ? " OR " : " AND ");
String term = expandTerm(left, notPart, op, andTerms.get(i));
expansion.append(term.contains(" OR ") ? "(" + term + ")" : term);
}
return expansion.toString();
}
List<String> orTerms = splitByTopLevel(inner, "OR");
if (orTerms.size() > 1) {
if (!allPartsValid(orTerms)) return null;
StringBuilder expansion = new StringBuilder();
for (int i = 0; i < orTerms.size(); i++) {
- if (i > 0) expansion.append(" OR ");
+ if (i > 0) expansion.append(negated ? " AND " : " OR ");
String term = expandTerm(left, notPart, op, orTerms.get(i));
expansion.append(term.contains(" AND ") ? "(" + term + ")" : term);
}
return expansion.toString();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private static String expandInner(String left, String notPart, String inner) { | |
| inner = stripOuterParens(inner); | |
| String op = "CONTAINS"; | |
| // Keep grouped CONTAINS terms together: "High OR Risk AND Override" -> (High OR Risk) AND Override | |
| List<String> andTerms = splitByTopLevel(inner, "AND"); | |
| if (andTerms.size() > 1) { | |
| if (!allPartsValid(andTerms)) return null; | |
| StringBuilder expansion = new StringBuilder(); | |
| for (int i = 0; i < andTerms.size(); i++) { | |
| if (i > 0) expansion.append(" AND "); | |
| String term = expandTerm(left, notPart, op, andTerms.get(i)); | |
| expansion.append(term.contains(" OR ") ? "(" + term + ")" : term); | |
| } | |
| return expansion.toString(); | |
| } | |
| List<String> orTerms = splitByTopLevel(inner, "OR"); | |
| if (orTerms.size() > 1) { | |
| if (!allPartsValid(orTerms)) return null; | |
| StringBuilder expansion = new StringBuilder(); | |
| for (int i = 0; i < orTerms.size(); i++) { | |
| if (i > 0) expansion.append(" OR "); | |
| String term = expandTerm(left, notPart, op, orTerms.get(i)); | |
| expansion.append(term.contains(" AND ") ? "(" + term + ")" : term); | |
| } | |
| return expansion.toString(); | |
| } | |
| private static String expandInner(String left, String notPart, String inner) { | |
| inner = stripOuterParens(inner); | |
| String op = "CONTAINS"; | |
| boolean negated = !notPart.isEmpty(); | |
| // Keep grouped CONTAINS terms together: "High OR Risk AND Override" -> (High OR Risk) AND Override | |
| List<String> andTerms = splitByTopLevel(inner, "AND"); | |
| if (andTerms.size() > 1) { | |
| if (!allPartsValid(andTerms)) return null; | |
| StringBuilder expansion = new StringBuilder(); | |
| for (int i = 0; i < andTerms.size(); i++) { | |
| if (i > 0) expansion.append(negated ? " OR " : " AND "); | |
| String term = expandTerm(left, notPart, op, andTerms.get(i)); | |
| expansion.append(term.contains(" OR ") ? "(" + term + ")" : term); | |
| } | |
| return expansion.toString(); | |
| } | |
| List<String> orTerms = splitByTopLevel(inner, "OR"); | |
| if (orTerms.size() > 1) { | |
| if (!allPartsValid(orTerms)) return null; | |
| StringBuilder expansion = new StringBuilder(); | |
| for (int i = 0; i < orTerms.size(); i++) { | |
| if (i > 0) expansion.append(negated ? " AND " : " OR "); | |
| String term = expandTerm(left, notPart, op, orTerms.get(i)); | |
| expansion.append(term.contains(" AND ") ? "(" + term + ")" : term); | |
| } | |
| return expansion.toString(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@if_condition_evaluator/src/main/java/com/testsigma/addons/utils/ConditionEvaluatorUtils.java`
around lines 145 - 170, expandInner currently expands grouped terms assuming
positive CONTAINS; when notPart indicates a NOT (i.e., handling "NOT CONTAINS"),
the boolean join must be inverted per De Morgan (X NOT CONTAINS (A OR B) -> X
NOT CONTAINS A AND X NOT CONTAINS B, and X NOT CONTAINS (A AND B) -> X NOT
CONTAINS A OR X NOT CONTAINS B) rather than leaving the join as-is. Update
expandInner (and the logic that sets op/uses expandTerm) to detect notPart (the
NOT case) and flip the joiners when building the expansion for splitByTopLevel
results: for orTerms with a NOT use " AND " between expanded terms; for andTerms
with a NOT use " OR ". Preserve the existing parenthesis-wrapping behavior
around terms that contain the opposite join and keep the allPartsValid checks;
alternatively, if you prefer rejecting grouped NOT CONTAINS, return null early
when notPart is present and there are grouped terms.
Comment on lines
+184
to
+187
| private static boolean allPartsValid(List<String> parts) { | ||
| for (String p : parts) { | ||
| if (p.trim().length() < 2) return false; | ||
| } |
There was a problem hiding this comment.
Single-character bracket terms are rejected right now.
allPartsValid() returns false for tokens like A or B, so the documented case X CONTAINS (A OR B) never expands. It falls through to a literal search for "A OR B" instead of evaluating two CONTAINS clauses.
🐛 Proposed fix
private static boolean allPartsValid(List<String> parts) {
for (String p : parts) {
- if (p.trim().length() < 2) return false;
+ if (p.trim().isEmpty()) return false;
}
return true;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@if_condition_evaluator/src/main/java/com/testsigma/addons/utils/ConditionEvaluatorUtils.java`
around lines 184 - 187, allPartsValid currently rejects single-character tokens
because it returns false when p.trim().length() < 2; change the validation in
allPartsValid(List<String> parts) to only reject empty/blank tokens (e.g., use
p.trim().isEmpty() or p.trim().length() == 0) so single-character terms like "A"
or "B" are accepted and the expression expansion (e.g., "X CONTAINS (A OR B)")
works correctly; update the loop that iterates over parts and replace the length
check with an emptiness check on p.trim().
Comment on lines
25
to
29
| try { | ||
|
|
||
| if (testData == null || testData.getValue() == null) { | ||
| throw new Exception("Condition cannot be empty"); | ||
| } | ||
|
|
||
| String condition = testData.getValue().toString().trim(); | ||
| logger.info("Original Condition: " + condition); |
There was a problem hiding this comment.
Add null check for testData and its value to provide a clear error message.
The prior null/empty guard was removed. If testData is null or testData.getValue() returns null, an NPE will be thrown and caught, but the error message will be cryptic (e.g., "Exception Occurred: null") rather than actionable.
🛡️ Proposed fix to add validation
try {
+ if (testData == null || testData.getValue() == null) {
+ result = com.testsigma.sdk.Result.FAILED;
+ setErrorMessage("Condition input is required but was not provided");
+ return result;
+ }
String condition = testData.getValue().toString().trim();
+ if (condition.isEmpty()) {
+ result = com.testsigma.sdk.Result.FAILED;
+ setErrorMessage("Condition cannot be empty");
+ return result;
+ }
logger.info("Original Condition: " + condition);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| if (testData == null || testData.getValue() == null) { | |
| throw new Exception("Condition cannot be empty"); | |
| } | |
| String condition = testData.getValue().toString().trim(); | |
| logger.info("Original Condition: " + condition); | |
| try { | |
| if (testData == null || testData.getValue() == null) { | |
| result = com.testsigma.sdk.Result.FAILED; | |
| setErrorMessage("Condition input is required but was not provided"); | |
| return result; | |
| } | |
| String condition = testData.getValue().toString().trim(); | |
| if (condition.isEmpty()) { | |
| result = com.testsigma.sdk.Result.FAILED; | |
| setErrorMessage("Condition cannot be empty"); | |
| return result; | |
| } | |
| logger.info("Original Condition: " + condition); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@if_condition_evaluator/src/main/java/com/testsigma/addons/web/IfConditionEvaluatorAction.java`
around lines 25 - 29, Add explicit null checks for testData and
testData.getValue() before using them in IfConditionEvaluatorAction (the try
block where condition is assigned and logger.info("Original Condition: " +
condition) is executed); if testData or testData.getValue() is null, log a clear
error via logger.error (including which variable is null) and return or throw a
meaningful exception/result rather than letting a NullPointerException be caught
with an unhelpful message—update the null guard near the existing condition
assignment to validate testData and its value and provide a descriptive error
path.
| @@ -1 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyMjMyMmM2Ni04NWYzLWIyN2UtN2FiOS0zM2U2M2Q4OWM1MGIiLCJ1bmlxdWVJZCI6IjU5OTYiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiNDMifQ.GoblsmuHFdMOKiEL9HBx7J2Q74MnkMYjdKzyDFXA4YamryBO_5Nr_ajHygSf6Cc4G2X4Y36jtgcJVHhiYOBf7g No newline at end of file | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNTgxZDNlYy02Zjk2LTg3NDktZThmNS1hYjEwMWIwZDA1NTQiLCJ1bmlxdWVJZCI6IjM0NzYiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiODZlMGQ1ODUtZTVlYi05NmIxLTAyZDktOTRkODM3N2RiMzlmIn0.--JD8x3hpn4abLzuvVhdDg0eprohDvweUFkSNSAphQT59oTFDZswlg9ABTxRZHGjfbDXUHuFtDLfj_Pu-sluhw No newline at end of file | |||
There was a problem hiding this comment.
Remove the committed API key immediately.
This is a live credential in a tracked resource file, so it will be exposed through the repo, CI artifacts, and the published addon jar. Please revoke/rotate it and inject the key from secret storage at build/release time instead of packaging it here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@if_condition_evaluator/src/main/resources/testsigma-sdk.properties` at line
1, The file contains a committed live credential under the property key
testsigma-sdk.api.key; remove the secret value from testsigma-sdk.properties,
replace it with a placeholder (e.g.
testsigma-sdk.api.key=${TESTSIGMA_SDK_API_KEY}) and update the code that reads
this property to load from an environment variable or your secret manager (env
var TESTSIGMA_SDK_API_KEY or secret service) instead of the repo; revoke/rotate
the exposed key immediately, purge it from history (or run git-filter-repo/BFG)
and ensure CI/build injects the secret at release time via your secrets service
or pipeline variables.
vignesh-testsigma
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Publish this addon as PUBLIC (EU Region)
Addon Name: If Condition Evaluator
Jarvis Link: https://jarvis-eu.testsigma.com/ui/tenants/66076/addons
Jira : https://testsigma.atlassian.net/browse/CUS-11469
Added utils to support bracket conditons
Summary by CodeRabbit
New Features
CONTAINS (A OR B)andCONTAINS (A AND B)syntax in conditional expressions across all platforms (Android, iOS, mobile web, REST API, web, and Windows).Chores