Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR adds HTML support and phone-to-email conversion for mass mail: DB columns and UI fields for HTML and phone format, email sender logic updated to choose HTML per-entity and to convert phone contacts, validation for phone-to-email format, and subject made optional. Changes
Sequence DiagramsequenceDiagram
participant User as Web User
participant Bean as MassMailBean
participant Service as MassMailServiceImpl
participant DB as Database
participant Sender as MassMailEmailSender
participant Formatter as MessageFormat
User->>Bean: create/edit mass mail (html, phoneToEmailFormat, recipients)
Bean->>Service: createMassMail(inVO)
Service->>Formatter: MessageFormat.format(phoneToEmailFormat, "")
alt Format invalid
Formatter-->>Service: throws Exception
Service-->>Bean: throw MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT
else Format valid
Service->>DB: persist MassMail (html, phoneToEmailFormat, subject nullable)
DB-->>Service: persisted
Service-->>Bean: return MassMailOutVO
end
Bean->>Sender: send(massMail)
Sender->>Sender: isHtml(massMail) -> massMail.isHtml()
alt phoneToEmailFormat empty
Sender->>Sender: add only email contacts
else phoneToEmailFormat provided
Sender->>Formatter: MessageFormat.format(phoneToEmailFormat, phone)
Formatter-->>Sender: formattedEmail
Sender->>Sender: add formattedEmail for phone contacts
end
Sender-->>Bean: email sent/queued
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.41.1)core/src/main/java/org/phoenixctms/ctsms/util/ServiceUtil.javaThanks 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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/main/java/org/phoenixctms/ctsms/email/NotificationEmailSender.java`:
- Around line 53-57: The NotificationEmailSender currently allows empty subjects
because the else block that threw
L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT) was
commented out; restore subject validation in NotificationEmailSender by
re-enabling the check around CommonUtil.isEmptyString(subject) so that if
subject is empty the code throws the existing service exception (use
L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT))
instead of silently proceeding to mimeMessageHelper.setSubject, or alternatively
add a boolean flag (e.g., allowEmptySubject) to the NotificationEmailSender API
and use it to conditionally bypass the exception while preserving the default
behavior of throwing the SERVICE exception when not allowed.
In
`@core/src/main/java/org/phoenixctms/ctsms/service/massmail/MassMailServiceImpl.java`:
- Around line 228-234: The current validation only checks that
massMailIn.getPhoneToEmailFormat() is a valid MessageFormat pattern;
additionally require that the pattern contains a {0} placeholder so downstream
calls MessageFormat.format(..., phoneValue) are valid. After the existing
try/catch that calls MessageFormat.format(massMailIn.getPhoneToEmailFormat(),
""), add a check like if (!massMailIn.getPhoneToEmailFormat().contains("{0}"))
throw
L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT,
massMailIn.getPhoneToEmailFormat(), "missing {0} placeholder"); this keeps the
same exception code and message parameters (use the format string and a short
message) and ensures MassMailEmailSender.format calls will have the required
placeholder.
In `@core/src/main/java/org/phoenixctms/ctsms/util/ServiceUtil.java`:
- Around line 3379-3438: The method in ServiceUtil that formats the subject
currently returns null when format == null, which overwrites the existing safe
fallback in createMassMailTemplateModel
(MassMailMessageTemplateParameters.SUBJECT); change the method so that when
format is null it returns an empty string ("") instead of null, leaving the rest
of the formatting and exception handling (MessageFormat.format and the catch
that throws L10nUtil.initServiceException with
ServiceExceptionCodes.MASS_MAIL_INVALID_SUBJECT_FORMAT) untouched so
createMassMailTemplateModel retains its "" default.
In `@mda/src/main/uml/andromda-process.profile.uml`:
- Line 205: Multiple uml:Extension elements have their memberEnd entries in the
wrong order; locate each <packagedElement xmi:type="uml:Extension" ...> (e.g.
xmi:id="_11_5_e240333_1154527555609_685498_77" and the other Extension elements
flagged in the review) and ensure the memberEnd attribute lists the metaclass
end (the non-owned/non-navigable end) first and the ExtensionEnd (the
owned/navigable end whose id also appears in navigableOwnedEnd) second; for each
Extension, if the two ids in memberEnd are reversed, swap their order so the id
that matches navigableOwnedEnd is the second entry, making the first entry the
metaclass end.
In `@web/src/main/resources/org/phoenixctms/ctsms/web/massmail/labels.properties`:
- Around line 130-133: Fix the typo in the properties value for
mass_mail_phone_to_email_format_tooltip: replace "adresses" with "addresses" so
the tooltip reads "...to email addresses (eg.
+431231234567@email2sms.linkmobility.eu)." Update the value associated with the
key mass_mail_phone_to_email_format_tooltip accordingly.
In `@web/src/main/webapp/META-INF/includes/massmail/massMailMain.xhtml`:
- Around line 768-775: The preview currently forces escapeHtml="false" for the
multiLineOutputText include which causes plain-text previews to be rendered as
HTML; update the escapeHtml param on the ui:include (multiLineOutputText.xhtml)
to be data-driven: set the escapeHtml param to evaluate to true when not in HTML
mode and false when in HTML mode (use massMailBean.in.html to drive the
expression) so plain-text previews are escaped while HTML previews are not.
In `@web/src/main/webapp/META-INF/includes/shared/multiLineOutputText.xhtml`:
- Line 20: The f:param uses "#{multiLineText.length()}" which will NPE if
multiLineText is null when showLength is true; update the EL to guard against
null (for example use a conditional/ternary like check returning 0 or empty
string when multiLineText is null) so that the expression always evaluates
safely, and ensure the change targets the f:param referencing multiLineText and
the surrounding logic that depends on showLength.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b668013-4d89-4ea0-a82d-70ea0a658401
📒 Files selected for processing (37)
common/src/main/java/org/phoenixctms/ctsms/util/ServiceExceptionCodes.javacore/db/schema-create.sqlcore/db/schema-set-version.sqlcore/db/schema-up-master.sqlcore/src/main/java/org/phoenixctms/ctsms/email/EmailSender.javacore/src/main/java/org/phoenixctms/ctsms/email/MassMailEmailSender.javacore/src/main/java/org/phoenixctms/ctsms/email/NotificationEmailSender.javacore/src/main/java/org/phoenixctms/ctsms/service/massmail/MassMailServiceImpl.javacore/src/main/java/org/phoenixctms/ctsms/util/ServiceUtil.javacore/src/main/resources/ctsms-serviceexceptionmessages.propertiescore/src/main/resources/ctsms-serviceexceptionmessages_de.propertiesmda/src/main/uml/UML_Standard_Profile.MagicDraw_Profile.DSL_Customization.profile.umlmda/src/main/uml/UML_Standard_Profile.MagicDraw_Profile.Traceability_customization.profile.umlmda/src/main/uml/UML_Standard_Profile.MagicDraw_Profile.profile.umlmda/src/main/uml/UML_Standard_Profile.StandardProfileL2.profile.umlmda/src/main/uml/UML_Standard_Profile.StandardProfileL3.profile.umlmda/src/main/uml/UML_Standard_Profile.Validation_Profile.profile.umlmda/src/main/uml/andromda-common.andromda-persistence.profile.umlmda/src/main/uml/andromda-common.andromda-service.profile.umlmda/src/main/uml/andromda-common.profile.umlmda/src/main/uml/andromda-presentation.profile.umlmda/src/main/uml/andromda-process.profile.umlmda/src/main/uml/andromda-webservice.profile.umlmda/src/main/uml/andromda-xml.profile.umlmda/src/main/uml/ctsms.umlmda/src/main/uml/ctsms.xmlweb/src/main/java/org/phoenixctms/ctsms/web/model/massmail/MassMailBean.javaweb/src/main/java/org/phoenixctms/ctsms/web/util/DefaultSettings.javaweb/src/main/java/org/phoenixctms/ctsms/web/util/SettingCodes.javaweb/src/main/resources/org/phoenixctms/ctsms/web/labels.propertiesweb/src/main/resources/org/phoenixctms/ctsms/web/labels_de.propertiesweb/src/main/resources/org/phoenixctms/ctsms/web/massmail/labels.propertiesweb/src/main/resources/org/phoenixctms/ctsms/web/massmail/labels_de.propertiesweb/src/main/resources/org/phoenixctms/ctsms/web/settings.propertiesweb/src/main/webapp/META-INF/includes/massmail/massMailMain.xhtmlweb/src/main/webapp/META-INF/includes/shared/multiLineOutputText.xhtmlweb/src/main/webapp/resources/css/default.css
| if (!CommonUtil.isEmptyString(subject)) { | ||
| mimeMessageHelper.setSubject(subject); | ||
| } else { | ||
| throw L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT); | ||
| // } else { | ||
| // throw L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT); | ||
| } |
There was a problem hiding this comment.
Consider the implications of allowing empty subjects for notifications.
The empty subject validation is now commented out, meaning notification emails can be sent without a subject line. While this aligns with the PR's goal to make subjects optional for mass mails (SMS via email2sms may not need subjects), this change also affects NotificationEmailSender.
Emails without subjects may:
- Be flagged as spam by email providers
- Appear incomplete to recipients
Is this intentional for notifications as well, or should notification emails retain subject validation?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/main/java/org/phoenixctms/ctsms/email/NotificationEmailSender.java`
around lines 53 - 57, The NotificationEmailSender currently allows empty
subjects because the else block that threw
L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT) was
commented out; restore subject validation in NotificationEmailSender by
re-enabling the check around CommonUtil.isEmptyString(subject) so that if
subject is empty the code throws the existing service exception (use
L10nUtil.initServiceException(ServiceExceptionCodes.EMAIL_EMPTY_SUBJECT))
instead of silently proceeding to mimeMessageHelper.setSubject, or alternatively
add a boolean flag (e.g., allowEmptySubject) to the NotificationEmailSender API
and use it to conditionally bypass the exception while preserving the default
behavior of throwing the SERVICE exception when not allowed.
| if (!CommonUtil.isEmptyString(massMailIn.getPhoneToEmailFormat())) { | ||
| try { | ||
| MessageFormat.format(massMailIn.getPhoneToEmailFormat(), ""); | ||
| } catch (Exception e) { | ||
| throw L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT, massMailIn.getPhoneToEmailFormat(), e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sender_file="$(fd 'MassMailEmailSender.java' | head -n1)"
svc_file="$(fd 'MassMailServiceImpl.java' | head -n1)"
echo "Sender file: ${sender_file}"
rg -n -C3 'getPhoneToEmailFormat|MessageFormat\.format|phone' "${sender_file}"
echo
echo "Service validation callsite: ${svc_file}"
rg -n -C3 'phoneToEmailFormat|MessageFormat\.format' "${svc_file}"Repository: phoenixctms/ctsms
Length of output: 5234
Require a {0} placeholder in phoneToEmailFormat, not just valid MessageFormat syntax.
Lines 228–234 currently validate parseability only. Downstream code at MassMailEmailSender.java lines 464, 486, and 508 all call MessageFormat.format(massMail.getPhoneToEmailFormat(), <phoneValue>) with a phone number argument. A pattern without a {0} placeholder (e.g., "prefix-" or "no-placeholder") would pass validation but fail or produce unexpected behavior at runtime.
🔧 Proposed fix
if (!CommonUtil.isEmptyString(massMailIn.getPhoneToEmailFormat())) {
try {
- MessageFormat.format(massMailIn.getPhoneToEmailFormat(), "");
+ MessageFormat messageFormat = new MessageFormat(massMailIn.getPhoneToEmailFormat());
+ if (messageFormat.getFormatsByArgumentIndex().length == 0) {
+ throw new IllegalArgumentException("missing {0} placeholder");
+ }
+ MessageFormat.format(massMailIn.getPhoneToEmailFormat(), "");
} catch (Exception e) {
throw L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT, massMailIn.getPhoneToEmailFormat(), 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.
| if (!CommonUtil.isEmptyString(massMailIn.getPhoneToEmailFormat())) { | |
| try { | |
| MessageFormat.format(massMailIn.getPhoneToEmailFormat(), ""); | |
| } catch (Exception e) { | |
| throw L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT, massMailIn.getPhoneToEmailFormat(), e.getMessage()); | |
| } | |
| } | |
| if (!CommonUtil.isEmptyString(massMailIn.getPhoneToEmailFormat())) { | |
| try { | |
| MessageFormat messageFormat = new MessageFormat(massMailIn.getPhoneToEmailFormat()); | |
| if (messageFormat.getFormatsByArgumentIndex().length == 0) { | |
| throw new IllegalArgumentException("missing {0} placeholder"); | |
| } | |
| MessageFormat.format(massMailIn.getPhoneToEmailFormat(), ""); | |
| } catch (Exception e) { | |
| throw L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT, massMailIn.getPhoneToEmailFormat(), e.getMessage()); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/src/main/java/org/phoenixctms/ctsms/service/massmail/MassMailServiceImpl.java`
around lines 228 - 234, The current validation only checks that
massMailIn.getPhoneToEmailFormat() is a valid MessageFormat pattern;
additionally require that the pattern contains a {0} placeholder so downstream
calls MessageFormat.format(..., phoneValue) are valid. After the existing
try/catch that calls MessageFormat.format(massMailIn.getPhoneToEmailFormat(),
""), add a check like if (!massMailIn.getPhoneToEmailFormat().contains("{0}"))
throw
L10nUtil.initServiceException(ServiceExceptionCodes.MASS_MAIL_INVALID_PHONE_TO_EMAIL_FORMAT,
massMailIn.getPhoneToEmailFormat(), "missing {0} placeholder"); this keeps the
same exception code and message parameters (use the format string and a short
message) and ensures MassMailEmailSender.format calls will have the required
placeholder.
| </ownedEnd> | ||
| </packagedElement> | ||
| <packagedElement xmi:type="uml:Extension" xmi:id="_11_5_e240333_1154527555609_685498_77" name="" memberEnd="_11_5_e240333_1154527555609_612842_79 _11_5_e240333_1154527555609_639719_78" navigableOwnedEnd="_11_5_e240333_1154527555609_612842_79"> | ||
| <packagedElement xmi:type="uml:Extension" xmi:id="_11_5_e240333_1154527555609_685498_77" name="" memberEnd="_11_5_e240333_1154527555609_639719_78 _11_5_e240333_1154527555609_612842_79" navigableOwnedEnd="_11_5_e240333_1154527555609_612842_79"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine the specific lines mentioned
if [ -f "mda/src/main/uml/andromda-process.profile.uml" ]; then
echo "=== File exists, examining specific lines ==="
sed -n '200,275p' mda/src/main/uml/andromda-process.profile.uml | cat -n
else
echo "File not found at expected location"
# Try to find it
fd -t f "andromda-process.profile.uml" 2>/dev/null || echo "Could not find file"
fiRepository: phoenixctms/ctsms
Length of output: 10385
🏁 Script executed:
# Check git history to see what changes were made to this file
if git rev-parse --git-dir > /dev/null 2>&1; then
echo "=== Git diff for the UML file (last change) ==="
git diff HEAD~1 HEAD -- mda/src/main/uml/andromda-process.profile.uml 2>/dev/null | head -200
fiRepository: phoenixctms/ctsms
Length of output: 108
🏁 Script executed:
# Get more context around the memberEnd lines to understand the pattern
echo "=== Examining memberEnd occurrences in the file ==="
rg "memberEnd" mda/src/main/uml/andromda-process.profile.uml -n -B 1 -A 1 2>/dev/null | head -100Repository: phoenixctms/ctsms
Length of output: 6839
🏁 Script executed:
# Check git log for this file to see recent changes
git log --oneline -20 -- mda/src/main/uml/andromda-process.profile.uml 2>/dev/null || echo "No git history available"Repository: phoenixctms/ctsms
Length of output: 125
🏁 Script executed:
# Try to see if there are any uncommitted changes
git status -- mda/src/main/uml/andromda-process.profile.uml 2>/dev/null || echo "Git not available or file not tracked"Repository: phoenixctms/ctsms
Length of output: 126
🌐 Web query:
UML Extension memberEnd ordering specification semantics importance
💡 Result:
In UML, an Extension is a specialized Association between a Stereotype and a Metaclass in a Profile, used to extend the properties of the metaclass with those defined in the stereotype. The Extension has exactly two memberEnds: one ordinary Property end owned by the metaclass (non-ownedEnd, typed by the metaclass Class), and one ExtensionEnd owned by the Extension (ownedEnd, typed by the stereotype). This binary nature is enforced by the constraint memberEnd->size = 2. The memberEnd is an ordered association (Property[2..*] subsets Namespace::member), meaning the order of the two ends matters semantically. The first memberEnd (index 0) is conventionally the metaclass end (accessed via metaclassEnd query: memberEnd->reject(e | e.oclIsTypeOf(ExtensionEnd))->first), used to derive Extension::metaclass. The second is the ExtensionEnd (accessed via stereotypeEnd). This ordering specification is important because: - It enables unambiguous identification of ends without relying solely on ownership or type checks, simplifying queries and tool implementations (e.g., validateIsBinary, validateNonOwnedEnd). - It supports derivation of key attributes like Extension::metaclass and Extension::isRequired (derived from ownedEnd multiplicity). - UML tools and XMI serialization depend on this order for correct model interchange and validation. - Open issues like UMLR-78 ("consistent ordering of Association::memberEnd and ownedEnd") highlight its practical significance for metamodel consistency. Sources confirm Extensions are strictly binary Associations with ordered ends distinguishing metaclass from stereotype ends (UML 2.5 Superstructure ~p.672-676, Eclipse UML2 Javadoc). Disordered ends could lead to ambiguity in profile application and stereotype attachment semantics.
Citations:
- 1: https://stackoverflow.com/questions/2891377/when-to-use-memberend-and-when-navigableownedend-in-an-uml-class-diagram
- 2: https://www.omg.org/spec/UML/2.3/Superstructure/PDF
- 3: http://download.eclipse.org/modeling/mdt/uml2/javadoc/2.2.1/org/eclipse/uml2/uml/Extension.html
- 4: https://www.omg.org/spec/UML/2.5
- 5: https://issues.omg.org/issues/spec/UML/2.5?view=ALL
- 6: https://www.uml-diagrams.org/association.html
- 7: https://docs.nomagic.com/download/attachments/136711173/UML%20specification.pdf?version=1&modificationDate=1695226406576&api=v2
- 8: https://uml-diagrams.org/profile-extension.html
🌐 Web query:
AndroMDA UML profile memberEnd attribute order tooling requirements
💡 Result:
No specific documentation found on "memberEnd attribute order" requirements in AndroMDA UML profiles. AndroMDA primarily supports UML 1.4 and UML 2.x models exported as XMI from compatible tools like MagicDraw or Poseidon. Profiles (e.g., andromda-profile-common, persistence) define stereotypes and tagged values applied to UML metaclasses, where memberEnd refers to association ends in UML associations. Tooling must export XMI preserving UML model structure, including association order if semantically significant (e.g., for ordered associations). Official docs recommend MagicDraw for best UML feature support, implying reliable serialization of model elements like memberEnds. No explicit requirements for attribute order found in profiles or changes; project inactive since ~2014 with last release 3.5-SNAPSHOT.
Citations:
- 1: https://andromda.sourceforge.net/profiles.html
- 2: https://andromda.sourceforge.net/
- 3: https://andromda.sourceforge.net/starting.html
- 4: https://andromda.sourceforge.net/andromda-profiles/andromda-profiles-uml14/andromda-profile-persistence.html
- 5: https://andromda.sourceforge.net/andromda-cartridges/andromda-hibernate-cartridge/profile.html
- 6: https://myjeeva.com/uml-modeling-andromda.html
- 7: https://andromda.sourceforge.net/andromda-profiles/andromda-profiles-uml2/andromda-profile.html
- 8: https://andromda.sourceforge.net/changes-report.html
- 9: https://andromda.sourceforge.net/faq.html
- 10: https://andromda.sourceforge.net/andromda-metafacades/andromda-uml-metafacades/andromda-metafacades-uml/apidocs/org/andromda/metafacades/uml/ClassifierFacade.html
- 11: https://github.com/ojmakhura/andromda
Fix memberEnd ordering to comply with UML Extension semantics
According to the UML Specification (2.3/2.5 Superstructure), Extension memberEnd is an ordered association where the order is semantically significant: the first memberEnd must be the metaclass end (non-ownedEnd), and the second must be the ExtensionEnd (ownedEnd). This ordering is required for:
- Correct derivation of Extension::metaclass and related attributes
- Proper XMI serialization and model interchange
- Unambiguous tool interpretation and code generation
The file currently shows inconsistent memberEnd ordering across Extension elements:
- Lines 205, 217, 253 have correct ordering (metaclass end first)
- Lines 211, 223, 247, 265 and others have incorrect ordering (ExtensionEnd first)
Standardize all Extension elements to place the non-navigable metaclass end first and the navigableOwnedEnd second in the memberEnd attribute to ensure consistent, spec-compliant semantics across the profile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mda/src/main/uml/andromda-process.profile.uml` at line 205, Multiple
uml:Extension elements have their memberEnd entries in the wrong order; locate
each <packagedElement xmi:type="uml:Extension" ...> (e.g.
xmi:id="_11_5_e240333_1154527555609_685498_77" and the other Extension elements
flagged in the review) and ensure the memberEnd attribute lists the metaclass
end (the non-owned/non-navigable end) first and the ExtensionEnd (the
owned/navigable end whose id also appears in navigableOwnedEnd) second; for each
Extension, if the two ids in memberEnd are reversed, swap their order so the id
that matches navigableOwnedEnd is the second entry, making the first entry the
metaclass end.
web/src/main/resources/org/phoenixctms/ctsms/web/massmail/labels.properties
Show resolved
Hide resolved
| <ui:include src="../shared/multiLineOutputText.xhtml"> | ||
| <ui:param name="multiLineText" | ||
| value="#{massMailBean.previewText}" /> | ||
| <ui:param name="multiLineTextClass" value="ctsms-monospaced" /> | ||
| <ui:param name="rendered" value="#{not massMailBean.in.html}" /> | ||
| <ui:param name="showLength" value="true" /> | ||
| <ui:param name="escapeHtml" value="false" /> | ||
| </ui:include> |
There was a problem hiding this comment.
Verify escapeHtml="false" for plain text preview mode.
In plain text mode, escapeHtml is set to false (line 774). This means any HTML-like characters in the plain text template will be interpreted as HTML rather than displayed literally. For SMS messages, if a user enters text like <test> in plain text mode, it would be rendered as HTML rather than shown as literal text.
Consider whether this is intentional. If plain text should display content literally, escapeHtml should be true for the non-HTML mode preview.
Suggested fix
<ui:include src="../shared/multiLineOutputText.xhtml">
<ui:param name="multiLineText"
value="#{massMailBean.previewText}" />
<ui:param name="multiLineTextClass" value="ctsms-monospaced" />
<ui:param name="rendered" value="#{not massMailBean.in.html}" />
<ui:param name="showLength" value="true" />
- <ui:param name="escapeHtml" value="false" />
+ <ui:param name="escapeHtml" value="true" />
</ui:include>📝 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.
| <ui:include src="../shared/multiLineOutputText.xhtml"> | |
| <ui:param name="multiLineText" | |
| value="#{massMailBean.previewText}" /> | |
| <ui:param name="multiLineTextClass" value="ctsms-monospaced" /> | |
| <ui:param name="rendered" value="#{not massMailBean.in.html}" /> | |
| <ui:param name="showLength" value="true" /> | |
| <ui:param name="escapeHtml" value="false" /> | |
| </ui:include> | |
| <ui:include src="../shared/multiLineOutputText.xhtml"> | |
| <ui:param name="multiLineText" | |
| value="#{massMailBean.previewText}" /> | |
| <ui:param name="multiLineTextClass" value="ctsms-monospaced" /> | |
| <ui:param name="rendered" value="#{not massMailBean.in.html}" /> | |
| <ui:param name="showLength" value="true" /> | |
| <ui:param name="escapeHtml" value="true" /> | |
| </ui:include> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/main/webapp/META-INF/includes/massmail/massMailMain.xhtml` around
lines 768 - 775, The preview currently forces escapeHtml="false" for the
multiLineOutputText include which causes plain-text previews to be rendered as
HTML; update the escapeHtml param on the ui:include (multiLineOutputText.xhtml)
to be data-driven: set the escapeHtml param to evaluate to true when not in HTML
mode and false when in HTML mode (use massMailBean.in.html to drive the
expression) so plain-text previews are escaped while HTML previews are not.
web/src/main/webapp/META-INF/includes/shared/multiLineOutputText.xhtml
Outdated
Show resolved
Hide resolved
|
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores