Fix behavioural change introduced in v1 API with input validation in Email senders #1048

AfraHussaindeen · 2026-01-03T09:44:45Z

Purpose

$Subject

To be merged after

Fix behavioural change introduced in v1 API with input validation in Email senders wso2-extensions/identity-event-handler-notification#358

Public Issue

Unable to set or modify an sub-org's email "From" address product-is#25959

Summary by CodeRabbit

Internal Improvements
- Enhanced email notification sender configuration handling for improved operational control.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…Email senders

coderabbitai · 2026-01-03T09:44:59Z

Walkthrough

Two service classes in the notification sender API (v1 and v2) have their internal method calls updated to pass an additional boolean parameter—true in v1, false in v2—to addEmailSender and updateEmailSender methods. The public API surface remains unchanged.

Changes

Cohort / File(s)	Summary
Notification Sender v1 `components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v1/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java`	Updated internal service calls: `addEmailSender(dto, true)` and `updateEmailSender(dto, true)`. Method signatures changed to accept boolean parameter.
Notification Sender v2 `components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java`	Updated internal service calls: `addEmailSender(dto, false)` and `updateEmailSender(dto, false)`. Method signatures changed to accept boolean parameter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A flag and a call, two versions align,
One sends with true, one sets to fine,
Parameters pass through the digital line,
Notifications dispatch—everything fine! 🐇📨✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete and missing most required template sections.	Complete the PR description with Goals, Approach, User Stories, Developer Checklist, Release Note, Documentation, Training, Certification, Marketing, Automation Tests, Security Checks, Samples, Related PRs, Migrations, Test Environment, and Learning sections.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main behavioral change being fixed in the v1 API related to email sender input validation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

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.}

...bon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java

wso2-engineering · 2026-01-03T09:45:06Z

...bon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java


        EmailSenderDTO dto = buildEmailSenderDTO(senderName, emailSenderUpdateRequest);
        try {
-            EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto);
+            EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto, true);


Log Improvement Suggestion No: 2

Suggested change

EmailSenderDTO dto = buildEmailSenderDTO(senderName, emailSenderUpdateRequest);

try {

EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto);

EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto, true);

public EmailSender updateEmailSender(String senderName, EmailSenderUpdateRequest emailSenderUpdateRequest) {

EmailSenderDTO dto = buildEmailSenderDTO(senderName, emailSenderUpdateRequest);

log.info("Updating email sender: " + senderName);

try {

EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto, true);

...bon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java

wso2-engineering

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.

✅ Before merging this pull request:

Review all AI-generated comments for accuracy and relevance.
Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.

Comment	Accepted (Y/N)	Reason
#### Log Improvement Suggestion No: 1
#### Log Improvement Suggestion No: 2
#### Log Improvement Suggestion No: 3
#### Log Improvement Suggestion No: 4

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java (1)
247-256: LGTM - Consistent with addEmailSender changes.

The update method correctly mirrors the pattern established in addEmailSender by passing false to maintain v2 backward compatibility.
💡 Optional: Add inline documentation for the boolean parameter

Consider adding a brief comment explaining the boolean parameter's purpose for future maintainability:
 public EmailSender updateEmailSender(String senderName, EmailSenderUpdateRequest emailSenderUpdateRequest) {

     EmailSenderDTO dto = buildEmailSenderDTO(senderName, emailSenderUpdateRequest);
     try {
+        // Pass false to disable strict validation for v2 API backward compatibility
         EmailSenderDTO emailSenderDTO = notificationSenderManagementService.updateEmailSender(dto, false);
         return buildEmailSenderFromDTO(emailSenderDTO);
     } catch (NotificationSenderManagementException e) {
Similarly for addEmailSender at line 89.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f12eae5 and 7c99975.

📒 Files selected for processing (2)

components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v1/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java
components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: Thisara-Welmilla
Repo: wso2/identity-api-server PR: 1032
File: components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java:384-387
Timestamp: 2025-11-06T05:02:47.521Z
Learning: In components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java, SMS sender property validation is handled by the service layer builder class (SMSSenderDTO.Builder), not at the API layer, because the validation logic is complex and depends on other attributes of the object.

📚 Learning: 2025-11-06T05:02:47.521Z

Learnt from: Thisara-Welmilla
Repo: wso2/identity-api-server PR: 1032
File: components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java:384-387
Timestamp: 2025-11-06T05:02:47.521Z
Learning: In components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java, SMS sender property validation is handled by the service layer builder class (SMSSenderDTO.Builder), not at the API layer, because the validation logic is complex and depends on other attributes of the object.

Applied to files:

components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java
components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v1/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java

🔇 Additional comments (3)

components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v2/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java (1)

85-94: Confirmed: The boolean parameter correctly controls validation behavior.

The change from true (v1) to false (v2) for the addEmailSender and updateEmailSender method calls is correct. The boolean parameter is part of the external NotificationSenderManagementService (from org.wso2.carbon.identity.event.handler.notification framework), where:

true enables input validation (v1 behavior)

false disables validation to preserve backward compatibility (v2 behavior)

This aligns with the PR objective to fix the behavioral difference between v1 and v2 APIs, with v2 maintaining its original less-strict validation while v1 adopts stricter validation.

components/org.wso2.carbon.identity.api.server.notification.sender/org.wso2.carbon.identity.api.server.notification.sender.v1/src/main/java/org/wso2/carbon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java (2)

235-244: LGTM! Consistent API versioning strategy across v1 and v2.

The updateEmailSender method correctly passes true to enable input validation, matching the addEmailSender behavior. This v1 approach is intentionally differentiated from v2, which disables validation by passing false to both operations—a deliberate versioning strategy.

73-82: LGTM! The service layer method signature supports this boolean parameter.

The addition of the true parameter to enable input validation in the v1 API aligns with the PR objective to fix the behavioral change. The v1 implementation consistently passes true for both add and update operations, while v2 passes false, confirming intentional API versioning. The public method signatures remain unchanged, maintaining backward compatibility.

Fix behavioural change introduced in v1 API with input validation in …

7c99975

…Email senders

wso2-engineering bot reviewed Jan 3, 2026

View reviewed changes

...bon/identity/api/server/notification/sender/v1/core/NotificationSenderManagementService.java Show resolved Hide resolved

wso2-engineering bot reviewed Jan 3, 2026

View reviewed changes

...bon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java Show resolved Hide resolved

wso2-engineering bot reviewed Jan 3, 2026

View reviewed changes

...bon/identity/api/server/notification/sender/v2/core/NotificationSenderManagementService.java Show resolved Hide resolved

wso2-engineering bot suggested changes Jan 3, 2026

View reviewed changes

coderabbitai bot reviewed Jan 3, 2026

View reviewed changes

This was referenced Jan 3, 2026

Fix behavioural change introduced in v1 API with input validation in Email senders wso2-extensions/identity-event-handler-notification#358

Open

Unable to set or modify an sub-org's email "From" address wso2/product-is#25959

Open

SujanSanjula96 approved these changes Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix behavioural change introduced in v1 API with input validation in Email senders #1048

Fix behavioural change introduced in v1 API with input validation in Email senders #1048

AfraHussaindeen commented Jan 3, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

wso2-engineering bot Jan 3, 2026

Uh oh!

Uh oh!

Uh oh!

wso2-engineering bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix behavioural change introduced in v1 API with input validation in Email senders #1048

Are you sure you want to change the base?

Fix behavioural change introduced in v1 API with input validation in Email senders #1048

Conversation

AfraHussaindeen commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

To be merged after

Public Issue

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

wso2-engineering bot Jan 3, 2026

Choose a reason for hiding this comment

Log Improvement Suggestion No: 2

Uh oh!

Uh oh!

Uh oh!

wso2-engineering bot left a comment

Choose a reason for hiding this comment

AI Agent Log Improvement Checklist

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AfraHussaindeen commented Jan 3, 2026 •

edited

Loading

coderabbitai bot commented Jan 3, 2026 •

edited

Loading