feat(admin-ui): revamp SMTP module as per Figma#2696
Merged
Conversation
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR comprehensively revamps the SMTP management module with new validation, theming, and UI components while performing major cleanup of deprecated UI components (FloatGrid, IconWithBadge, NavSearch, etc.). It also removes the SMTP Redux slice in favor of a different state management approach and updates the type system throughout the admin-ui. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/app/redux/sagas/AuthSaga.ts (1)
111-115: 🛠️ Refactor suggestion | 🟠 MajorRemove the stale
cedarlingLogTypeChangedcontract.Lines 111–115 now branch entirely on
toastMessage, butcedarlingLogTypeChangedremains inPutConfigMeta(authSlice.ts:12), the saga's _meta type annotation (line 102), and multiple test cases (lines 41, 65, 105, 126–127, 169–170). The test at line 99 ("should dispatch generic success toast when cedarlingLogTypeChanged is false") explicitly tests behavior driven by this flag, which the saga no longer consults. RemovecedarlingLogTypeChangedfrom the interface and update the tests to focus ontoastMessageonly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/redux/sagas/AuthSaga.ts` around lines 111 - 115, Remove the stale cedarlingLogTypeChanged field from the PutConfigMeta interface in authSlice.ts and update the AuthSaga _meta type annotation (the PutConfigMeta usage in the saga) so the saga only branches on toastMessage; then update any unit tests that reference cedarlingLogTypeChanged (remove that flag from test fixtures and assertions) so they assert behavior solely based on toastMessage (e.g., tests that previously checked "generic success toast when cedarlingLogTypeChanged is false" should instead verify presence/absence/content of toastMessage).admin-ui/app/locales/es/translation.json (1)
8-8:⚠️ Potential issue | 🟡 MinorConcatenation error in translation string.
The value
"AceptarAgregar mapeo de atributos"appears to be two strings incorrectly merged. It should likely be"Agregar mapeo de atributos"(without the leading "Aceptar").🐛 Proposed fix
- "add_attribute_mapping": "AceptarAgregar mapeo de atributos", + "add_attribute_mapping": "Agregar mapeo de atributos",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/es/translation.json` at line 8, The translation value for the key "add_attribute_mapping" contains a concatenation bug ("AceptarAgregar mapeo de atributos"); update the value to the correct Spanish phrase "Agregar mapeo de atributos" by replacing the current string assigned to the "add_attribute_mapping" key in the locales JSON so it no longer includes the erroneous "Aceptar" prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/locales/en/translation.json`:
- Around line 871-874: The two translation keys keystore_edit_enabled and
keystore_edit_disabled currently have identical text; update the value for
keystore_edit_disabled (or both keys) so they show distinct messages for the
enabled vs disabled states—e.g., keep keystore_edit_enabled as "Keystore
configuration updated successfully." and change keystore_edit_disabled to
something like "Keystore configuration saved but editing is disabled." to
clearly reflect the disabled state; modify the translation.json entries for
keystore_edit_enabled and keystore_edit_disabled accordingly.
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 834-835: The two translation keys keystore_edit_enabled and
keystore_edit_disabled currently have identical Portuguese text; update the
values so they clearly differ (e.g., one indicating "keystore editing enabled"
and the other "keystore editing disabled"). Edit the entries for
"keystore_edit_enabled" and "keystore_edit_disabled" in the pt translation file
to use distinct, user-facing messages that reflect their respective states
(successful enabling versus successful disabling).
- Around line 832-833: The keystore tooltip strings ("keystore_fields_disabled"
and "keystore_field_disabled_named") reference the non-existent label 'Editar
Keystore'; update those translation values to use the actual Portuguese toggle
label "Editar armazenamento de chaves" (and keep the {{field}} placeholder for
the named key) so the text matches the UI; verify SmtpManagement/SmtpForm.tsx
(SmtpForm component) continues to consume these keys and will surface the
corrected copy to users.
- Around line 185-186: Rename the misspelled localization key
server_success_stmp to server_success_smtp across all locale JSON files (e.g.,
en, fr, es, pt) so it matches server_fails_smtp; update each locale entry where
server_success_stmp appears and keep the translated string unchanged, then
ensure the references in code (GluuInfo.tsx usage that combines
server_success_smtp and server_fails_smtp) point to the corrected key name.
In `@admin-ui/app/routes/Apps/Gluu/GluuTooltip.tsx`:
- Line 62: The spread currently uses a truthiness check that drops offset when
it's 0; in GluuTooltip (the JSX that spreads props using {...(offset && { offset
})}) change the condition to explicitly test for undefined (e.g., use offset !==
undefined or offset != null) so offset={0} is passed through; update the spread
expression to {...(offset !== undefined && { offset })} (or similar) to preserve
zero as a valid value.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.ts`:
- Around line 32-36: The disabled style block in GluuInputRow.style.ts uses
'&:disabled' and sets pointerEvents: 'none', which prevents hover and hides the
cursor change; remove the pointerEvents: 'none' entry from the '&:disabled' rule
and keep opacity: 0.5 and cursor: 'not-allowed' so the native disabled attribute
still blocks interaction but the not-allowed cursor is visible on hover.
In
`@admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx`:
- Around line 323-337: The test currently clicks Apply on SmtpForm but doesn't
assert the commit dialog or final submit; update the test that renders SmtpForm
(renderAndWait(<SmtpForm ... />)) to wait for and assert the commit dialog
appears (e.g., query/get by the dialog title/text or role) after clicking the
Apply button, then simulate confirming the commit (click the dialog confirm
button) and assert that the submit handler (the mocked
handleSubmit/defaultProps.handleSubmit) is called; ensure you use await/waitFor
around dialog visibility and the mock assertion so the full commit flow is
verified.
In `@admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx`:
- Around line 121-138: The forEach callback in submitForm currently uses an
implicit-return arrow (Object.keys(errors).forEach((k) => (touched[k] = true)))
which returns a value unnecessarily; change the callback to a void-returning
body so it doesn't return (e.g., Object.keys(errors).forEach((k) => { touched[k]
= true })) or replace with a for...of loop to populate the touched map before
calling formik.setTouched; update code in the submitForm function around the
touched population logic.
- Around line 462-486: The toggle currently does an optimistic update of
optimisticKeystoreEdit before calling putConfigWorker, but on API failure the
Redux allowSmtpKeystoreEdit never changes so the useEffect won't revert the
optimistic state; fix by capturing the previous value (const prev =
optimisticKeystoreEdit) before calling setOptimisticKeystoreEdit, then dispatch
putConfigWorker as now, and ensure the saga/error path triggers a revert: either
have the saga dispatch a specific failure action (e.g. PUT_CONFIG_FAILURE)
containing the previous value so the component can dispatch
setOptimisticKeystoreEdit(prev) or handle that failure action directly inside
the component (via useEffect watching the failure flag from the store) to call
setOptimisticKeystoreEdit(prev), or alternatively dispatch a dedicated revert
action from the saga to reset allowSmtpKeystoreEdit in Redux—use the identifiers
Toggle, optimisticKeystoreEdit, allowSmtpKeystoreEdit,
setOptimisticKeystoreEdit, and putConfigWorker to locate and implement the
change.
In
`@admin-ui/plugins/smtp-management/components/SmtpManagement/styles/SmtpFormPage.style.ts`:
- Around line 187-193: The focus rules for the selector '& input:focus, &
input:active, & select:focus, & select:active' remove the browser's native focus
indicator without providing an accessible replacement; restore a visible
keyboard focus state by replacing outline/boxShadow:none with an explicit,
high-contrast focus style (for example a visible outline or ring using
themeColors.fontColor or a dedicated focus color and a subtle boxShadow) while
keeping formInputBg and inputBorderColor; update the rule in
SmtpFormPage.style.ts so focused inputs/selects have a clear, persistent visual
indicator (outline or boxShadow) that meets contrast and visibility
requirements.
In `@admin-ui/plugins/smtp-management/helper/validations.ts`:
- Around line 20-29: The current object-form Yup.when usage for
smtp_authentication_account_username and smtp_authentication_account_password
(depending on requires_authentication) triggers the noThenProperty lint rule and
fails to allow null credentials; replace the object-form when(...) with the
callback overload (use the (values, schema) => ... form) and in the branch where
requires_authentication is false return schema.nullable() (instead of
schema.notRequired()), while keeping schema.required(...) when true; update both
fields (smtp_authentication_account_username,
smtp_authentication_account_password) accordingly so validation accepts null
when authentication is disabled.
---
Outside diff comments:
In `@admin-ui/app/locales/es/translation.json`:
- Line 8: The translation value for the key "add_attribute_mapping" contains a
concatenation bug ("AceptarAgregar mapeo de atributos"); update the value to the
correct Spanish phrase "Agregar mapeo de atributos" by replacing the current
string assigned to the "add_attribute_mapping" key in the locales JSON so it no
longer includes the erroneous "Aceptar" prefix.
In `@admin-ui/app/redux/sagas/AuthSaga.ts`:
- Around line 111-115: Remove the stale cedarlingLogTypeChanged field from the
PutConfigMeta interface in authSlice.ts and update the AuthSaga _meta type
annotation (the PutConfigMeta usage in the saga) so the saga only branches on
toastMessage; then update any unit tests that reference cedarlingLogTypeChanged
(remove that flag from test fixtures and assertions) so they assert behavior
solely based on toastMessage (e.g., tests that previously checked "generic
success toast when cedarlingLogTypeChanged is false" should instead verify
presence/absence/content of toastMessage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad51d2a5-3710-40ef-8096-194fe2f30129
📒 Files selected for processing (27)
admin-ui/.gitignoreadmin-ui/app/components/App/AuthenticatedRouteSelector.tsxadmin-ui/app/constants/ui.tsadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/app/redux/sagas/AuthSaga.tsadmin-ui/app/redux/types/index.tsadmin-ui/app/routes/Apps/Gluu/GluuInfo.tsxadmin-ui/app/routes/Apps/Gluu/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuTooltip.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuInfo.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/types/css.d.tsadmin-ui/plugins/scripts/components/CustomScripts/styles/CustomScriptFormPage.style.tsadmin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpEditPage.test.tsxadmin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsxadmin-ui/plugins/smtp-management/components/SmtpManagement/SmtpEditPage.tsxadmin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsxadmin-ui/plugins/smtp-management/components/SmtpManagement/styles/SmtpFormPage.style.tsadmin-ui/plugins/smtp-management/helper/index.tsadmin-ui/plugins/smtp-management/helper/utils.tsadmin-ui/plugins/smtp-management/helper/validations.tsadmin-ui/plugins/smtp-management/redux/features/smtpSlice.tsadmin-ui/plugins/smtp-management/redux/types/SmtpApi.type.tsadmin-ui/plugins/smtp-management/types/smtp-types.ts
💤 Files with no reviewable changes (3)
- admin-ui/plugins/smtp-management/redux/features/smtpSlice.ts
- admin-ui/app/components/App/AuthenticatedRouteSelector.tsx
- admin-ui/plugins/smtp-management/redux/types/SmtpApi.type.ts
admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx
Show resolved
Hide resolved
admin-ui/plugins/smtp-management/components/SmtpManagement/styles/SmtpFormPage.style.ts
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (2)
admin-ui/app/locales/pt/translation.json (1)
832-835:⚠️ Potential issue | 🟡 MinorUse the translated PT term consistently in the keystore copy.
Line 627 and Line 628 already use
armazenamento de chaves, but these messages still fall back tokeystore. That leaves the same setting with two names in one flow.Suggested copy
- "keystore_fields_disabled": "Ative 'Editar armazenamento de chaves' para modificar as configurações do keystore.", + "keystore_fields_disabled": "Ative 'Editar armazenamento de chaves' para modificar as configurações do armazenamento de chaves.", - "keystore_edit_enabled": "A edição do keystore foi ativada.", - "keystore_edit_disabled": "A edição do keystore foi desativada.", + "keystore_edit_enabled": "A edição do armazenamento de chaves foi ativada.", + "keystore_edit_disabled": "A edição do armazenamento de chaves foi desativada.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/pt/translation.json` around lines 832 - 835, Replace inconsistent use of "keystore" with the Portuguese term "armazenamento de chaves" across the translation entries keystore_fields_disabled, keystore_field_disabled_named, keystore_edit_enabled, and keystore_edit_disabled so the UI uses the same term as lines 627–628 (e.g., change "keystore" → "armazenamento de chaves" in each value and in any quoted phrases like "Editar armazenamento de chaves" to maintain consistent wording).admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx (1)
323-339:⚠️ Potential issue | 🟡 MinorFinish the commit confirmation path in this test.
This still only proves that the dialog opens. It never confirms the dialog or asserts that
handleSubmitran, so the actual Apply flow is still uncovered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx` around lines 323 - 339, The test currently only verifies the commit dialog opens but never completes the apply flow; update the test to simulate confirming the dialog and assert the submit handler was invoked: after waiting for the dialog (screen.getByRole('dialog')), find and click the dialog's confirm button (e.g., use screen.getByRole('button', { name: /Confirm|Apply|Yes/i }) or query within the dialog element) inside an act/fireEvent click, then waitFor and expect the mocked handleSubmit (from defaultProps or the prop passed into SmtpForm) toHaveBeenCalledTimes(1) (or toHaveBeenCalledWith expected args); ensure you reference SmtpForm and the mocked handleSubmit/defaultProps in your assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/EmptyLayout/EmptyLayout.tsx`:
- Line 49: The div in EmptyLayout component uses a misspelled className
"fullscrenn__section__child" which prevents the correct CSS from applying;
update the className on the wrapper div in EmptyLayout.tsx to
"fullscreen__section__child" (replace the double "n" typo) so the centered child
wrapper receives the intended styles, and run a quick dev build to verify
styling renders correctly.
- Around line 21-35: The useEffect in EmptyLayout currently depends on the
entire pageConfig object (and its bound functions from Layout), causing an
infinite re-trigger loop; change the effect to run only once by capturing
pageConfig.setElementsVisibility into a ref (e.g., const setVisibilityRef =
useRef(pageConfig?.setElementsVisibility) or assign inside a small synchronous
block) and call setVisibilityRef.current({...}) inside the effect, then use an
empty dependency array [] so setElementsVisibility is not re-created every
render; also correct the CSS class typo fullscrenn__section__child to
fullscreen__section__child.
In `@admin-ui/app/locales/es/translation.json`:
- Line 310: The translation for the key "connect_protection" uses sentence case
while other keys like "host_name" and "data_type" use title case, causing
inconsistency; pick a consistent casing convention (either sentence case or
title case) for all field label keys and update the JSON entries
accordingly—e.g., change "connect_protection" to match the chosen convention or
update "host_name" and "data_type" to sentence case—ensuring all related keys in
the file follow the same pattern.
- Around line 337-338: The translations for "from_name" and "from_email_address"
use lowercase "remitente" and should be made consistent with the other sender
translations that use capitalized "Remitente"; update the values for the keys
"from_name" and "from_email_address" to use "Remitente" (e.g., "Nombre del
Remitente", "Correo del Remitente") so casing matches the existing sender
translations.
- Around line 368-373: The translation uses inconsistent terminology: replace
the English "Keystore" in the value for "keystore_configuration" with the
Spanish equivalent used elsewhere ("almacén de claves") so all keys are
consistent (e.g., update the value for the key keystore_configuration to match
key_store, key_store_password, key_store_alias). Also scan related keys like
allow_keystore_edit to ensure the same Spanish phrase is used uniformly.
- Around line 870-873: Replace inconsistent use of "keystore" with the Spanish
term "almacén de claves" in the locale keys keystore_fields_disabled,
keystore_field_disabled_named, keystore_edit_enabled, and keystore_edit_disabled
so all four messages consistently use "almacén de claves" (e.g., change "La
edición del keystore ha sido habilitada." to "La edición del almacén de claves
ha sido habilitada."). Ensure the placeholder {{field}} remains unchanged in
keystore_field_disabled_named and keep punctuation/accents consistent with
existing translations.
In `@admin-ui/app/locales/fr/translation.json`:
- Around line 831-834: Update the French translations to use the existing French
term "magasin de clés" consistently: replace occurrences of "keystore" in the
values for the keys keystore_fields_disabled, keystore_field_disabled_named,
keystore_edit_enabled, and keystore_edit_disabled with "magasin de clés" so the
messages match the wording used elsewhere (e.g., lines 615–616).
In `@admin-ui/app/routes/Apps/Gluu/GluuInfo.tsx`:
- Around line 55-65: The new translation key "actions.ok" is used in the
GluuInfo component inside the GluuButton (on the JSX that renders
{t('actions.ok')}) but wasn't added to the FR/PT locale bundles; add
"actions.ok" with appropriate localized strings to each locale file touched by
this PR (e.g., French and Portuguese translation JSON/modules) so the CTA
resolves properly; ensure the key name matches exactly "actions.ok" and include
the same English fallback value in the default locale.
- Around line 49-51: The UI currently renders a hardcoded "false" in GluuText
which is misleading; update the GluuInfo component usage so it either receives
and displays the real backend failure detail (prop name e.g. smtpError,
serverResponse, or similar) and render that value inside GluuText, or replace
the line with a translated generic failure message (use
t('actions.server_response_unavailable') or similar) so it doesn't show a
literal boolean; adjust SmtpEditPage to pass the actual error into GluuInfo if
choosing the detailed route, or change the GluuInfo rendering to use the
translation key if you cannot pass the backend message.
In `@admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx`:
- Around line 93-107: The form's onSubmit (configured in useFormik) only toggles
the modal and doesn't set commitOperations, causing Enter-key submits to miss
building changes; unify submission by routing formik's onSubmit to the same
logic as handleApply: call
setCommitOperations(buildSmtpChangedFieldOperations(initialValues,
formik.values, t)) and then toggle() from the form submit path (or refactor
handleApply into a reusable function and invoke it from both the form's onSubmit
and the existing handleApply) so both keyboard (formik.handleSubmit) and mouse
Apply use the exact same flow.
- Around line 136-139: The code currently runs trimObjectStrings on
formik.values (cast to SmtpFormValues) which will strip whitespace from secret
fields like smtp_authentication_account_password and key_store_password and can
corrupt credentials; instead, build a normalized payload that only trims
non-secret fields (e.g., create a shallow copy of formik.values, run
trimObjectStrings only on whitelisted fields or on a new object of allowed keys)
and keep smtp_authentication_account_password and key_store_password untouched,
then pass that normalized object into toSmtpConfiguration and handleSubmit;
update references around trimObjectStrings, formik.values, SmtpFormValues,
toSmtpConfiguration, and handleSubmit accordingly.
In `@admin-ui/plugins/smtp-management/helper/validations.ts`:
- Around line 6-29: Update the Yup schema to normalize non-secret string inputs
and the port before validation: for host, from_name, from_email_address,
connect_protection and smtp_authentication_account_username add .trim() (or a
.transform((v)=>typeof v==='string'?v.trim():v)) before .required() so
whitespace-only values are treated as empty and trigger your translated required
messages; for port add a .transform((value, original) => (original === '' ||
original == null ? undefined : Number(original))) so empty strings don't produce
Yup's generic type error and your .required(t('messages.smtp_port_required'))
and .min/.max/.integer checks run as intended; do not trim or transform
smtp_authentication_account_password (leave password fields unchanged).
---
Duplicate comments:
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 832-835: Replace inconsistent use of "keystore" with the
Portuguese term "armazenamento de chaves" across the translation entries
keystore_fields_disabled, keystore_field_disabled_named, keystore_edit_enabled,
and keystore_edit_disabled so the UI uses the same term as lines 627–628 (e.g.,
change "keystore" → "armazenamento de chaves" in each value and in any quoted
phrases like "Editar armazenamento de chaves" to maintain consistent wording).
In
`@admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx`:
- Around line 323-339: The test currently only verifies the commit dialog opens
but never completes the apply flow; update the test to simulate confirming the
dialog and assert the submit handler was invoked: after waiting for the dialog
(screen.getByRole('dialog')), find and click the dialog's confirm button (e.g.,
use screen.getByRole('button', { name: /Confirm|Apply|Yes/i }) or query within
the dialog element) inside an act/fireEvent click, then waitFor and expect the
mocked handleSubmit (from defaultProps or the prop passed into SmtpForm)
toHaveBeenCalledTimes(1) (or toHaveBeenCalledWith expected args); ensure you
reference SmtpForm and the mocked handleSubmit/defaultProps in your assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2ee300f-7286-4bd0-a564-e8872fc06423
📒 Files selected for processing (30)
admin-ui/app/components/EmptyLayout/EmptyLayout.tsxadmin-ui/app/components/EmptyLayout/EmptyLayoutSection.tsxadmin-ui/app/components/EmptyLayout/index.tsadmin-ui/app/components/FloatGrid/Col.tsxadmin-ui/app/components/FloatGrid/Grid.tsxadmin-ui/app/components/FloatGrid/Ready.tsxadmin-ui/app/components/FloatGrid/Row.tsxadmin-ui/app/components/FloatGrid/floatGridContext.tsxadmin-ui/app/components/FloatGrid/index.tsadmin-ui/app/components/IconWithBadge/IconWithBadge.tsxadmin-ui/app/components/IconWithBadge/index.tsadmin-ui/app/components/InputGroupAddon/InputGroupAddon.tsxadmin-ui/app/components/InputGroupAddon/index.tsadmin-ui/app/components/NavSearch/index.tsadmin-ui/app/components/NavSearch/search.tsxadmin-ui/app/components/NavSearch/styles.tsadmin-ui/app/components/NavbarThemeProvider/NavbarThemeProvider.tsxadmin-ui/app/components/NavbarThemeProvider/index.tsadmin-ui/app/components/index.tsxadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/app/routes/Apps/Gluu/GluuInfo.tsxadmin-ui/app/routes/Apps/Gluu/GluuTooltip.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/routes/Pages/ByeBye.tsxadmin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsxadmin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsxadmin-ui/plugins/smtp-management/helper/validations.ts
💤 Files with no reviewable changes (16)
- admin-ui/app/components/EmptyLayout/EmptyLayoutSection.tsx
- admin-ui/app/components/NavbarThemeProvider/NavbarThemeProvider.tsx
- admin-ui/app/components/IconWithBadge/IconWithBadge.tsx
- admin-ui/app/components/FloatGrid/Col.tsx
- admin-ui/app/components/NavbarThemeProvider/index.ts
- admin-ui/app/components/FloatGrid/Ready.tsx
- admin-ui/app/components/NavSearch/search.tsx
- admin-ui/app/components/NavSearch/styles.ts
- admin-ui/app/components/FloatGrid/Row.tsx
- admin-ui/app/components/IconWithBadge/index.ts
- admin-ui/app/components/InputGroupAddon/InputGroupAddon.tsx
- admin-ui/app/components/FloatGrid/Grid.tsx
- admin-ui/app/components/InputGroupAddon/index.ts
- admin-ui/app/components/FloatGrid/floatGridContext.tsx
- admin-ui/app/components/FloatGrid/index.ts
- admin-ui/app/components/NavSearch/index.ts
admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
admin-ui/app/routes/Apps/Gluu/GluuInfo.tsx (1)
48-52:⚠️ Potential issue | 🟡 MinorRedundant detail text repeats the status message.
The detail section displays
{t('actions.server_response')}: {t('actions.server_fails_smtp')}, butt('actions.server_fails_smtp')is already shown on line 45 as the status message. This adds no diagnostic value.To provide meaningful detail, either:
- Extend
GluuInfoItemto accept an optionalerrorMessage?: stringfrom the backend and display it here, or- Remove this redundant detail section entirely.
,
Option A: Remove redundant detail section
</div> - {!item.testStatus && ( - <GluuText variant="p" className={classes.detailText}> - {t('actions.server_response')}: {t('actions.server_fails_smtp')} - </GluuText> - )} </ModalBody>Option B: Accept and display actual error message
Update the interface:
interface GluuInfoItem { openModal: boolean testStatus: boolean errorMessage?: string // Add optional error detail }Then update the detail section:
{!item.testStatus && ( <GluuText variant="p" className={classes.detailText}> - {t('actions.server_response')}: {t('actions.server_fails_smtp')} + {t('actions.server_response')}: {item.errorMessage || t('actions.server_fails_smtp')} </GluuText> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/routes/Apps/Gluu/GluuInfo.tsx` around lines 48 - 52, The current detail text in GluuInfo.tsx redundantly repeats the status message; either remove the conditional GluuText block that renders "{t('actions.server_response')}: {t('actions.server_fails_smtp')}" when !item.testStatus, or extend the GluuInfoItem type to include an optional errorMessage?: string and change the detail rendering to show that backend error (e.g., render "{t('actions.server_response')}: {item.errorMessage}" only when item.errorMessage exists), updating the component props and any callers that construct GluuInfoItem accordingly so GluuText only displays meaningful diagnostic text.admin-ui/app/locales/pt/translation.json (1)
532-535:⚠️ Potential issue | 🟡 MinorUse one Portuguese term for keystore across the SMTP screen.
Line 628, Line 629, and Lines 833-836 already use
armazenamento de chaves, but these field labels switch torepositório de chaves. Mixing both terms on the same form makes the field names and helper text look unrelated.💬 Suggested copy
- "key_store": "Repositório de chaves", - "key_store_password": "Senha do repositório de chaves", - "key_store_alias": "Alias do repositório de chaves", - "signing_algorithm": "Algoritmo de assinatura do repositório de chaves", + "key_store": "Armazenamento de chaves", + "key_store_password": "Senha do armazenamento de chaves", + "key_store_alias": "Alias do armazenamento de chaves", + "signing_algorithm": "Algoritmo de assinatura do armazenamento de chaves",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/pt/translation.json` around lines 532 - 535, The Portuguese translations for the SMTP form are inconsistent: the keys "key_store", "key_store_password", "key_store_alias", and "signing_algorithm" use "repositório de chaves" while other labels use "armazenamento de chaves"; update the values for these keys to use the same term "armazenamento de chaves" (and equivalent phrasing for password/alias/algorithm) so the SMTP screen uses one consistent Portuguese term across all related field labels and helper text.admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx (1)
95-98:⚠️ Potential issue | 🟠 MajorBuild the commit diff from the same normalized values you submit.
submitForm()trims non-secret fields beforetoSmtpConfiguration(...), butcommitOperationsis computed here from rawformik.values. A trailing-space edit can therefore appear in the commit dialog/audit list even though the submitted SMTP config is unchanged.🔧 Proposed fix
+ const normalizeSmtpFormValues = useCallback((values: SmtpFormValues): SmtpFormValues => { + const { smtp_authentication_account_password, key_store_password, ...rest } = values + const trimmedRest = trimObjectStrings( + rest as unknown as Record<string, unknown>, + ) as unknown as Omit< + SmtpFormValues, + 'smtp_authentication_account_password' | 'key_store_password' + > + return { + ...trimmedRest, + smtp_authentication_account_password, + key_store_password, + } + }, []) + const formik = useFormik<SmtpFormValues>({ initialValues, - onSubmit: () => { + onSubmit: (values) => { if (readOnly) return - setCommitOperations(buildSmtpChangedFieldOperations(initialValues, formik.values, t)) + const normalizedValues = normalizeSmtpFormValues(values) + setCommitOperations(buildSmtpChangedFieldOperations(initialValues, normalizedValues, t)) toggle() },- const { smtp_authentication_account_password, key_store_password, ...rest } = formik.values - const trimmedRest = trimObjectStrings( - rest as unknown as Record<string, unknown>, - ) as unknown as Omit< - SmtpFormValues, - 'smtp_authentication_account_password' | 'key_store_password' - > - const trimmedValues: SmtpFormValues = { - ...trimmedRest, - smtp_authentication_account_password, - key_store_password, - } - handleSubmit(toSmtpConfiguration(trimmedValues), userMessage) + const normalizedValues = normalizeSmtpFormValues(formik.values) + handleSubmit(toSmtpConfiguration(normalizedValues), userMessage)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx` around lines 95 - 98, The commit operations are built from raw formik.values while submitForm() first normalizes/trims values (via the same logic used by toSmtpConfiguration), causing spurious diffs; change the onSubmit handler to compute the normalized/trimmed values exactly as submitForm() uses (e.g., call the same normalization function or toSmtpConfiguration(...) with the trimmed payload) and then call setCommitOperations(buildSmtpChangedFieldOperations(initialValues, normalizedValues, t)) before toggle(), ensuring the commit diff matches the actual submitted SMTP configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/components/EmptyLayout/EmptyLayout.tsx`:
- Around line 62-65: The current cast loses the concrete props of
EmptyLayoutBase; instead remove the broad ReturnType cast and keep the actual
typed result of withPageConfig(EmptyLayoutBase), then augment that concrete
component type with the Section static by using a narrowed intersection cast so
the original props are preserved. Concretely: assign the result of
withPageConfig(EmptyLayoutBase) to EmptyLayout without using ReturnType, then
set EmptyLayout = EmptyLayout as typeof EmptyLayout & { Section: typeof Section
} (or equivalent) and then assign EmptyLayout.Section = Section so the exported
EmptyLayout keeps the correct children/className props and also exposes Section.
In `@admin-ui/plugins/smtp-management/helper/validations.ts`:
- Around line 7-14: The port schema using Yup.number() with a transform
currently lacks a translated typeError for non-numeric inputs, so NaN cases
trigger Yup's default untranslated message; update the port validator (the port
field using Yup.number(), transform(...), required(...), min(...), max(...),
integer(...)) to chain .typeError(t('messages.smtp_port_numeric')) (or similar
translated key) before .required() so non-numeric values produce the localized
error message.
---
Duplicate comments:
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 532-535: The Portuguese translations for the SMTP form are
inconsistent: the keys "key_store", "key_store_password", "key_store_alias", and
"signing_algorithm" use "repositório de chaves" while other labels use
"armazenamento de chaves"; update the values for these keys to use the same term
"armazenamento de chaves" (and equivalent phrasing for password/alias/algorithm)
so the SMTP screen uses one consistent Portuguese term across all related field
labels and helper text.
In `@admin-ui/app/routes/Apps/Gluu/GluuInfo.tsx`:
- Around line 48-52: The current detail text in GluuInfo.tsx redundantly repeats
the status message; either remove the conditional GluuText block that renders
"{t('actions.server_response')}: {t('actions.server_fails_smtp')}" when
!item.testStatus, or extend the GluuInfoItem type to include an optional
errorMessage?: string and change the detail rendering to show that backend error
(e.g., render "{t('actions.server_response')}: {item.errorMessage}" only when
item.errorMessage exists), updating the component props and any callers that
construct GluuInfoItem accordingly so GluuText only displays meaningful
diagnostic text.
In `@admin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsx`:
- Around line 95-98: The commit operations are built from raw formik.values
while submitForm() first normalizes/trims values (via the same logic used by
toSmtpConfiguration), causing spurious diffs; change the onSubmit handler to
compute the normalized/trimmed values exactly as submitForm() uses (e.g., call
the same normalization function or toSmtpConfiguration(...) with the trimmed
payload) and then call
setCommitOperations(buildSmtpChangedFieldOperations(initialValues,
normalizedValues, t)) before toggle(), ensuring the commit diff matches the
actual submitted SMTP configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c42e349-fecc-4fd2-9b1d-897e1058af19
📒 Files selected for processing (13)
admin-ui/app/components/EmptyLayout/EmptyLayout.tsxadmin-ui/app/components/StatusBadge/index.tsadmin-ui/app/components/StatusBadge/types.tsadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/app/routes/Apps/Gluu/GluuInfo.tsxadmin-ui/app/utils/types.tsadmin-ui/plugins/admin/components/MAU/ActiveUsersGraph.tsxadmin-ui/plugins/admin/components/Reports/ReportPage.tsxadmin-ui/plugins/jans-lock/types/jans-lock-types.tsadmin-ui/plugins/smtp-management/components/SmtpManagement/SmtpForm.tsxadmin-ui/plugins/smtp-management/helper/validations.ts
💤 Files with no reviewable changes (6)
- admin-ui/plugins/jans-lock/types/jans-lock-types.ts
- admin-ui/app/components/StatusBadge/index.ts
- admin-ui/app/components/StatusBadge/types.ts
- admin-ui/plugins/admin/components/Reports/ReportPage.tsx
- admin-ui/plugins/admin/components/MAU/ActiveUsersGraph.tsx
- admin-ui/app/utils/types.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 532-535: The translations for the keystore fields ("key_store",
"key_store_password", "key_store_alias", "signing_algorithm") use "repositório
de chaves", which conflicts with other keys that use "armazenamento de chaves";
update the values for these four keys to use the same Portuguese term
"armazenamento de chaves" (and corresponding phrasing for password, alias and
signing algorithm) so the SMTP form uses a single consistent term for
keystore-related labels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d2f3ed1-6d52-4c6d-a728-c1905b72529e
📒 Files selected for processing (6)
admin-ui/app/components/EmptyLayout/EmptyLayout.tsxadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/plugins/smtp-management/helper/validations.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: Faisal Siddique <71010439+faisalsiddique4400@users.noreply.github.com>
|
duttarnab
approved these changes
Mar 11, 2026
moabu
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(admin-ui): revamp SMTP module as per Figma (#2633)
Summary
This PR revamps the SMTP module in the Admin UI to align with the latest Figma designs as part of the ongoing Admin UI redesign initiative.
Changes Included
Result
Parent Initiative
This work is part of the following parent feature:
feat(admin-ui): Revamp the existing design of the admin-ui according to the new design🔗 Ticket
Closes: #2633
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removed Components