-
Notifications
You must be signed in to change notification settings - Fork 16
[PRE-xyz] addon to filter the mailbox messages based on subject. #202
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
WalkthroughA new Maven module named Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GetMailBoxContentBasedOnFilter
participant Mailbox
participant RuntimeVariableStore
User->>GetMailBoxContentBasedOnFilter: execute()
GetMailBoxContentBasedOnFilter->>Mailbox: setEmailId(emailId)
GetMailBoxContentBasedOnFilter->>Mailbox: getAllMessages()
GetMailBoxContentBasedOnFilter->>GetMailBoxContentBasedOnFilter: filter messages by type/text
GetMailBoxContentBasedOnFilter->>GetMailBoxContentBasedOnFilter: extract content with regex
alt Content found
GetMailBoxContentBasedOnFilter->>RuntimeVariableStore: store extracted content
GetMailBoxContentBasedOnFilter-->>User: return SUCCESS
else No content or error
GetMailBoxContentBasedOnFilter-->>User: return FAILED
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
gmail_api_with_filters/pom.xml(1 hunks)gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java(1 hunks)
🔇 Additional comments (2)
gmail_api_with_filters/pom.xml (1)
75-105: LGTM - Build configuration is appropriate for addon packaging.The Maven Shade Plugin configuration correctly creates an uber-jar with all dependencies, and the Maven Source Plugin ensures source code is available. This setup is well-suited for TestSigma addon deployment.
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java (1)
17-39: LGTM - Well-structured class annotations and field declarations.The class is properly annotated with clear action description and appropriate data binding. The use of Lombok's @DaTa annotation reduces boilerplate code effectively.
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java
Outdated
Show resolved
Hide resolved
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java
Outdated
Show resolved
Hide resolved
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java
Show resolved
Hide resolved
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java
Show resolved
Hide resolved
gmail_api_with_filters/src/main/java/com/testsigma/addons/web/GetGmailContent.java
Show resolved
Hide resolved
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: 2
🧹 Nitpick comments (4)
mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java (4)
19-25: Fix typo in the action description.There's a grammatical error in line 23: "store the content based ont the filter" should be "store the content based on the filter".
- description = "Get email content from mailbox using emailID and" + - " store the content based ont the filter in a runtime variable.", + description = "Get email content from mailbox using emailID and" + + " store the content based on the filter in a runtime variable.",
97-100: Consider regex pattern flags and compilation safety.The Pattern.DOTALL flag makes
.match newlines, which might not always be desired. Also, consider handling potential PatternSyntaxException for invalid regex patterns.- Pattern pattern = Pattern.compile( - regexCondition.getValue().toString(), - Pattern.DOTALL - ); + try { + Pattern pattern = Pattern.compile( + regexCondition.getValue().toString() + ); + } catch (PatternSyntaxException e) { + logger.warn("Invalid regex pattern: " + e.getMessage()); + output = messageContent; // Fallback to full content + continue; // Skip regex processing + }
117-125: Simplify null checking logic.Since
outputis initialized as an empty string on line 90, the null check on line 117 is redundant.- if(output != null && !output.isEmpty()){ + if(!output.isEmpty()){ logger.info("setting runtime variable " + output); runTimeData.setValue(output); runTimeData.setKey(runtimeVariable.getValue().toString()); } else { logger.info("No content found in the email"); setErrorMessage("No content found in the email"); return Result.FAILED; }
103-108: Improve regex matching logic.Consider using capturing groups for more precise content extraction instead of returning the entire match.
if (matcher.find()) { - output = matcher.group(); + // Use group(1) if regex has capturing groups, otherwise use group() + output = matcher.groupCount() > 0 ? matcher.group(1) : matcher.group(); } else { // If no regex match found, return the full content output = messageContent; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mailbox_actions/pom.xml(1 hunks)mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java(1 hunks)mailbox_actions/src/main/resources/testsigma-sdk.properties(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- mailbox_actions/src/main/resources/testsigma-sdk.properties
- mailbox_actions/pom.xml
🔇 Additional comments (2)
mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java (2)
1-17: LGTM!Package declaration and imports are well-organized and all appear to be used in the implementation.
73-85: Fix case sensitivity in switch statement.The switch statement uses incorrect case for "sentto" and "receivedfrom" which doesn't match the allowedValues in the annotation.
switch (currentFilterType.toLowerCase()) { case "subject": return message.getSubject() != null && message.getSubject().contains(filterValue); - case "sentto": + case "sentto": return message.getSentTo() != null && message.getSentTo().contains(filterValue); - case "receivedfrom": + case "receivedfrom": return message.getReceivedFrom() != null && message.getReceivedFrom().contains(filterValue); default: return false; }Likely an incorrect or invalid review comment.
| @TestData(reference = "filter-type", allowedValues = {"subject","sentTo","receivedFrom"}) | ||
| private com.testsigma.sdk.TestData filterType; |
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.
Fix case sensitivity mismatch between allowedValues and switch statement.
The allowedValues use camelCase ("sentTo", "receivedFrom") but the switch statement in lines 77-82 checks for lowercase ("sentto", "receivedfrom"). This mismatch could cause the filter to always return false for these cases.
- @TestData(reference = "filter-type", allowedValues = {"subject","sentTo","receivedFrom"})
+ @TestData(reference = "filter-type", allowedValues = {"subject","sentto","receivedfrom"})Or alternatively, update the switch statement to match the camelCase values.
📝 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.
| @TestData(reference = "filter-type", allowedValues = {"subject","sentTo","receivedFrom"}) | |
| private com.testsigma.sdk.TestData filterType; | |
| @TestData(reference = "filter-type", allowedValues = {"subject","sentto","receivedfrom"}) | |
| private com.testsigma.sdk.TestData filterType; |
🤖 Prompt for AI Agents
In
mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java
around lines 36-37 and 77-82, the allowedValues for filterType use camelCase
("sentTo", "receivedFrom") but the switch statement checks for lowercase
("sentto", "receivedfrom"). To fix this, update the switch statement cases to
use the exact camelCase strings as in allowedValues to ensure correct matching.
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.
I have added toLowerCase so this should not cause any issue
switch (currentFilterType.toLowerCase()) {
case "subject":
return message.getSubject() != null &&
message.getSubject().contains(filterValue);
....
}
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.
@ManojTestsigma You're absolutely right! Adding toLowerCase() to the switch condition is an excellent solution that resolves the case sensitivity issue. This approach is actually better than my suggested fix because:
- It preserves the more readable camelCase format in the
allowedValuesannotation - It makes the code more robust by handling any case variations that might be passed at runtime
- It ensures consistent behavior regardless of how users input the filter type
Your implementation correctly converts the input to lowercase before comparison with the lowercase case labels, which eliminates the mismatch I flagged. Thank you for the clarification!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| try { | ||
| for( MailboxMessage messageIterator : messages ) { | ||
| logger.info("message : " + messageIterator.getSubject()); | ||
| logger.info("content : " + messageIterator.getTextMessage()); | ||
| logger.info("from : " + messageIterator.getReceivedFrom()); | ||
| } | ||
| } catch (Exception e) { | ||
| // | ||
| logger.info("ignoring messages"); | ||
| } |
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.
🛠️ Refactor suggestion
Improve error handling and consider information disclosure.
The empty catch block silently ignores exceptions, which could hide important errors. Additionally, logging email content and sender information could expose sensitive data.
- try {
- for( MailboxMessage messageIterator : messages ) {
- logger.info("message : " + messageIterator.getSubject());
- logger.info("content : " + messageIterator.getTextMessage());
- logger.info("from : " + messageIterator.getReceivedFrom());
- }
- } catch (Exception e) {
- //
- logger.info("ignoring messages");
- }
+ try {
+ for( MailboxMessage messageIterator : messages ) {
+ logger.debug("Processing message with subject: [REDACTED]");
+ // Avoid logging sensitive email content and addresses
+ }
+ } catch (Exception e) {
+ logger.warn("Failed to process message details: " + e.getMessage());
+ }📝 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.
| try { | |
| for( MailboxMessage messageIterator : messages ) { | |
| logger.info("message : " + messageIterator.getSubject()); | |
| logger.info("content : " + messageIterator.getTextMessage()); | |
| logger.info("from : " + messageIterator.getReceivedFrom()); | |
| } | |
| } catch (Exception e) { | |
| // | |
| logger.info("ignoring messages"); | |
| } | |
| try { | |
| for( MailboxMessage messageIterator : messages ) { | |
| logger.debug("Processing message with subject: [REDACTED]"); | |
| // Avoid logging sensitive email content and addresses | |
| } | |
| } catch (Exception e) { | |
| logger.warn("Failed to process message details: " + e.getMessage()); | |
| } |
🤖 Prompt for AI Agents
In
mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java
around lines 57 to 66, the catch block currently ignores exceptions silently,
which can hide errors, and the logs expose potentially sensitive email content
and sender information. Modify the catch block to log the exception details at
an appropriate level (e.g., warning or error) instead of ignoring it. Also,
remove or mask sensitive information such as email content and sender details
from the logs to prevent information disclosure.
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.
this is for debugging purpose, ignoring this in case of empty subject etc..
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.
@ManojTestsigma, I understand this is for debugging purposes to handle cases like empty subjects. Since you need the debugging information, here's a compromise that maintains your debugging capability while addressing potential security concerns:
try {
for( MailboxMessage messageIterator : messages ) {
logger.debug("message : " + (messageIterator.getSubject() != null ? "[SUBJECT_PRESENT]" : "[EMPTY_SUBJECT]"));
logger.debug("content length : " + (messageIterator.getTextMessage() != null ? messageIterator.getTextMessage().length() : 0));
logger.debug("from : " + (messageIterator.getReceivedFrom() != null ? "[SENDER_PRESENT]" : "[NO_SENDER]"));
}
} catch (Exception e) {
logger.debug("Exception while processing messages: " + e.getMessage());
}This way, you can still debug empty subjects and other issues without exposing the actual email content in logs. But if you prefer to keep the current implementation for your debugging workflow, that's your call!
(๑•́ ◡ •̀๑) ♪
✏️ Learnings added
Learnt from: ManojTestsigma
PR: testsigmahq/testsigma-addons#202
File: mailbox_actions/src/main/java/com/testsigma/addons/web/test/GetMailBoxContentBasedOnFilter.java:57-66
Timestamp: 2025-08-05T09:16:20.323Z
Learning: In the GetMailBoxContentBasedOnFilter addon, the logging of email subjects, content, and sender information is intentionally kept for debugging purposes, particularly to handle cases with empty subjects. The user ManojTestsigma prefers this approach for debugging the mailbox filtering functionality.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
jira
no jira created for this
Addon Name : Gmail Api With filters
Addon account: [email protected]
jarvis link : https://jarvis.testsigma.com/ui/tenants/2817/addons
fix
this addon can be used for mailbox alias function
to filter the emails based on subject, sender etc..
Summary by CodeRabbit