-
Notifications
You must be signed in to change notification settings - Fork 122
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
Store notification content as binary to support unicode #286
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #286 +/- ##
============================================
- Coverage 33.43% 33.29% -0.15%
Complexity 507 507
============================================
Files 72 72
Lines 4621 4641 +20
Branches 559 560 +1
============================================
Hits 1545 1545
- Misses 2922 2942 +20
Partials 154 154
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
...gt/org.wso2.carbon.email.mgt/src/main/java/org/wso2/carbon/email/mgt/util/I18nEmailUtil.java
Outdated
Show resolved
Hide resolved
...gt/org.wso2.carbon.email.mgt/src/main/java/org/wso2/carbon/email/mgt/util/I18nEmailUtil.java
Outdated
Show resolved
Hide resolved
notificationTemplateResult.setBody(templateContent[1]); | ||
notificationTemplateResult.setFooter(templateContent[2]); | ||
} else { | ||
throw new SQLException("Invalid content data."); |
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.
Is it ok to throw a SQL Exception here?
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.
Most simple answer is "Yes".. 😃
Let me know if you have any specific concerns..
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.
To add more context, this util is used in the DAO impls.. Specifically this is used as a instruction of the RowMapper.mapRow()
functional param of the jdbc templates. The RowMapper.mapRow()
only allowed to throw exceptions in the type of SQLException.
Proposed changes in this pull request
$subject. Related to,
When should this PR be merged
After merging,
Follow up actions
When updating product-is versions, both the framework change and this should be update together.
PS:
Done with: