EPMRPP-104088 || Old recipient in Email notifications rules still dis… #4470
Conversation
…play as login name after upgrading to version 25.2
BlazarQSO
requested review from
AmsterGet,
maria-hambardzumian and
allaprischepa
as code owners
WalkthroughThe changes update localization strings in multiple languages to require email addresses instead of usernames or recipients for notification rules. The UI components for adding/editing notification recipients were refactored to only accept and validate email addresses, removing support for usernames and simplifying validation logic. Redux dependencies were removed from the recipients container component. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddEditNotificationModal
participant RecipientsContainer
participant Validator
User->>AddEditNotificationModal: Open notification modal
AddEditNotificationModal->>RecipientsContainer: Render recipient input
User->>RecipientsContainer: Enter email address
RecipientsContainer->>Validator: Validate email
Validator-->>RecipientsContainer: Return valid/invalid
RecipientsContainer-->>User: Show error if invalid, accept if valid
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/addEditNotificationModal.jsx (1)
93-96: Rename the label to indicate pluralityThe input accepts multiple addresses (
RecipientsContainerreturns an array), but the label has been changed to the singular “Email”. Consider switching to something clearer like “Emails” or “Email addresses” to avoid misleading users.- defaultMessage: 'Email', + defaultMessage: 'Emails',This is a copy-text update only; no functional impact.
app/localization/translated/zh.json (2)
76-76: Consider using the more common Chinese term “邮箱”.“电子邮件” is perfectly correct, but native UIs usually label an e-mail field as “邮箱” for brevity and consistency with other screens (e.g.,
CreateUserModal.email).
If you want full consistency across the app, you might want to switch to “邮箱”.-"AddEditNotificationCaseModal.recipientsLabel": "电子邮件", +"AddEditNotificationCaseModal.recipientsLabel": "邮箱",
80-82: Mixed-language error messages – unify for consistency.
AddEditNotificationCaseModal.recipientsHint(line 75) is already in Chinese, butAddEditNotificationModal.recipientsErroris left in English.
Stick to one language (English placeholders are OK per dev-practice, but then the hint above should also stay English). Decide and align both keys.-"AddEditNotificationModal.recipientsError": "Email is incorrect. Please enter correct email", +# Example if you prefer Chinese: +"AddEditNotificationModal.recipientsError": "邮箱输入错误，请输入正确的邮箱。",app/localization/translated/uk.json (1)
80-81: Duplication & maintenance overhead for email-validation textYou introduced a bespoke error/placeholder pair here, but almost identical strings already exist under
Common.validation.email(line 337). Re-using the common key would
- keep wording consistent across the UI,
- eliminate future translation drift, and
- cut translator workload.
- "AddEditNotificationModal.recipientsError": "Вказано невірну адресу електронної пошти. Будь ласка, введіть правильну адресу електронної пошти", - "AddEditNotificationModal.recipientsPlaceholder": "Електронна пошта", + "AddEditNotificationModal.recipientsError": "@:Common.validation.email", + "AddEditNotificationModal.recipientsPlaceholder": "Електронна пошта"(The exact syntax for key aliasing depends on your i18n tooling; adjust accordingly.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/localization/translated/be.json(1 hunks)app/localization/translated/es.json(1 hunks)app/localization/translated/ru.json(1 hunks)app/localization/translated/uk.json(1 hunks)app/localization/translated/zh.json(1 hunks)app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/addEditNotificationModal.jsx(1 hunks)app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/recipientsContainer/recipientsContainer.jsx(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/recipientsContainer/recipientsContainer.jsx (1)
Learnt from: Guria
PR: reportportal/service-ui#4391
File: app/src/pages/inside/common/emptyStatePage/emptyStatePage.jsx:111-111
Timestamp: 2025-06-09T21:33:49.377Z
Learning: PropTypes.elementType accepts strings, functions, and React special elements (like React.Fragment, Suspense, etc.) through React's isValidElementType function. It's more flexible than PropTypes.string and can be used when a prop might receive different types of valid React element types including strings.
app/localization/translated/es.json (1)
undefined
<retrieved_learning>
Learnt from: AmsterGet
PR: #4409
File: app/localization/translated/es.json:1648-1652
Timestamp: 2025-06-18T16:24:46.188Z
Learning: In the ReportPortal project, Spanish and Chinese localization entries are intentionally left in English during development. Native speakers handle these translations later in the process, so having English text in Spanish (es.json) and Chinese (zh.json) locale files is expected and acceptable.
</retrieved_learning>
app/localization/translated/zh.json (1)
Learnt from: AmsterGet
PR: reportportal/service-ui#4409
File: app/localization/translated/es.json:1648-1652
Timestamp: 2025-06-18T16:24:46.188Z
Learning: In the ReportPortal project, Spanish and Chinese localization entries are intentionally left in English during development. Native speakers handle these translations later in the process, so having English text in Spanish (es.json) and Chinese (zh.json) locale files is expected and acceptable.
🧬 Code Graph Analysis (1)
app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/recipientsContainer/recipientsContainer.jsx (1)
app/src/common/utils/validation/validate.js (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Call feature Docker build / Build and export to AWS ECR
- GitHub Check: build (20)
- GitHub Check: build (20)
🔇 Additional comments (11)
app/localization/translated/ru.json (1)
80-82: Error message out-of-sync with new validation rule
AddEditNotificationModal.recipientsErrorstill allows “имя пользователя … или … e-mail”, contradicting the updated placeholder. Align the error text:-"AddEditNotificationModal.recipientsError": "Пожалуйста, введите существующее имя пользователя в вашем проекте или действительный адрес электронной почты", +"AddEditNotificationModal.recipientsError": "Пожалуйста, введите действительный адрес электронной почты",Do the same for other affected keys (
NotificationCase.recipientsHint, etc.) in this file to avoid mixed guidance.⛔ Skipped due to learnings
Learnt from: allaprischepa PR: reportportal/service-ui#4418 File: app/localization/translated/zh.json:2125-2130 Timestamp: 2025-06-23T06:22:27.254Z Learning: For the ReportPortal project, translation work should be delegated to dedicated translators rather than being handled directly during code review. English keys can serve as placeholders until proper translations are provided by the translation team.Learnt from: allaprischepa PR: reportportal/service-ui#4418 File: app/localization/translated/es.json:2127-2131 Timestamp: 2025-06-23T06:21:47.861Z Learning: The user allaprischepa prefers to have professional translators handle localization work rather than implementing suggested translations in code reviews.app/localization/translated/uk.json (1)
75-77:recipientsHintnow contradicts the email-only UXLine 75 still tells the user to enter “an existing username or a valid email”, while the surrounding label (76) and the functional code have switched to email-only input. This mixed messaging will confuse users and is likely to be reported as a bug.
- "AddEditNotificationCaseModal.recipientsHint": "Будь ласка, введіть існуюче ім'я користувача у вашому проекті або дійсну адресу електронної пошти", + "AddEditNotificationCaseModal.recipientsHint": "Будь ласка, введіть дійсну адресу електронної пошти"Update the hint (and any other language variants) to stay consistent with the new validation logic.
⛔ Skipped due to learnings
Learnt from: allaprischepa PR: reportportal/service-ui#4418 File: app/localization/translated/es.json:2127-2131 Timestamp: 2025-06-23T06:21:47.861Z Learning: The user allaprischepa prefers to have professional translators handle localization work rather than implementing suggested translations in code reviews.Learnt from: allaprischepa PR: reportportal/service-ui#4418 File: app/localization/translated/zh.json:2125-2130 Timestamp: 2025-06-23T06:22:27.254Z Learning: For the ReportPortal project, translation work should be delegated to dedicated translators rather than being handled directly during code review. English keys can serve as placeholders until proper translations are provided by the translation team.app/localization/translated/be.json (1)
80-81: Align all “recipient” strings across dialogsThe placeholder & error text for
AddEditNotificationModalwere updated, but the parallel keys for the Case modal (AddEditNotificationCaseModal.recipientsPlaceholder,NotificationCase.recipientsHint,NotificationCase.recipientsLabel) still carry the old wording. Keep the localisation consistent to prevent mixed terminology in the UI.app/src/pages/inside/projectSettingsPageContainer/content/notifications/modals/addEditNotificationModal/recipientsContainer/recipientsContainer.jsx (8)
17-18: LGTM: Modern React patterns adopted.The refactor from class component with Redux connect to functional component with hooks is a positive modernization that improves code readability and maintainability.
29-29: LGTM: Clearer user guidance with email-specific placeholder.The placeholder change from "Recipients" to "Email" provides clearer guidance to users about the expected input format.
33-33: LGTM: Improved error message specificity.The error message now clearly indicates that email format is required, which aligns with the component's new email-only validation approach.
37-37: LGTM: Simplified component structure.The component name change and direct export are consistent with removing the Redux connect wrapper, resulting in cleaner code structure.
62-63: Error handling logic is sound but depends on validation function fix.The error message generation is appropriate, but it relies on the
recipientsWithErrorstate that's updated through the side effect ingetEmailValidationError. Once the validation function is fixed, this error handling will work correctly.
73-73: Prop usage is correct but depends on validation function fix.The
getItemValidationErrorTypeprop correctly uses the validation function, but the underlying validation function needs to be fixed to remove side effects as mentioned in the earlier comment.
82-88: LGTM: Simplified and appropriate prop definitions.The PropTypes and defaultProps are correctly simplified to match the refactored component structure, with only the necessary
errorprop defined.
69-69: Confirm endpoint returns email addresses for autocompleteThe recipients autocomplete is now email-only, but it still uses
getURI={URLS.projectUsernamesSearch(projectKey)}which (per app/src/common/urls.js) constructs
/project/${projectKey}/usernames?filter.cnt.users=${searchTerm}– an endpoint that likely returns usernames rather than email addresses.
Please verify that the API response includes user emails (not just usernames). If it doesn’t, switch to an email-search endpoint or update the autocomplete logic accordingly.
• app/src/pages/inside/projectSettingsPageContainer/.../recipientsContainer/recipientsContainer.jsx (line 69):
getURI={URLS.projectUsernamesSearch(projectKey)}
• app/src/common/urls.js:projectUsernamesSearchdefinition
...nt/notifications/modals/addEditNotificationModal/recipientsContainer/recipientsContainer.jsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4470 +/- ##
===========================================
- Coverage 67.20% 66.80% -0.41%
===========================================
Files 84 84
Lines 991 997 +6
Branches 140 140
===========================================
Hits 666 666
- Misses 294 300 +6
Partials 31 31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
…play as login name after upgrading to version 25.2
PR Checklist
developfor features/bugfixes, other if mentioned in the task)npm run lint) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build)?devnpm script)manage:translationsscript?Visuals
Summary by CodeRabbit
New Features
Refactor