fix(admin): description missing in client view details and alignment issues in fields #2475
Conversation
…issues in fields (#2471)
faisalsiddique4400
requested review from
duttarnab and
syntrydy
as code owners
📝 WalkthroughWalkthroughRefactors Gluu detail components to add typed public props, configurable sizes and label styles, memoization, and a computed row class; standardizes Clients detail layout with module-level style objects, badge-list helper, consistent FormGroup-based rows, and a toggleable secret display when appropriate. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App / Store
participant ClientUI as ClientDetailPage
participant Row as GluuFormDetailRow
participant Secret as GluuSecretDetail
Note over ClientUI,Secret: ClientDetailPage renders detail rows using shared styles and sizing props
App->>ClientUI: supply client row data
ClientUI->>Row: render label + value (appliedLabelStyle, lsize/rsize)
ClientUI->>Secret: render secret row (label,value,lsize,rsize,labelStyle)
Secret->>Secret: determine hasToggle = (value !== '-')
alt secret present
User->>Secret: click Toggle
Secret->>Secret: toggle masked/unmasked (useCallback)
Secret-->>ClientUI: displayed secret updated in DOM
else secret absent
Secret-->>ClientUI: render dash, no Toggle
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (2)
20-30: Normalize description value to a single string
extractDescriptioncurrently returnsresult[0].values, which is likely an array. That meansdescriptioncan be either a string (row.description) or an array (from customAttributes), and an empty array will still be truthy so the final|| dashfallback will not kick in. This can lead to odd rendering (concatenated values or blank output).Consider normalizing to a single trimmed string:
- function extractDescription(customAttributes) { - const result = customAttributes.filter((item) => item.name === 'description') - if (result && result.length >= 1) { - return result[0].values - } - return '' - } + function extractDescription(customAttributes = []) { + const attr = customAttributes.find((item) => item.name === 'description') + const values = attr?.values + const firstValue = Array.isArray(values) ? values[0] : values + return typeof firstValue === 'string' ? firstValue.trim() : '' + } const dash = '-' - const description = row.description || extractDescription(row.customAttributes || []) || dash + const description = + (typeof row.description === 'string' && row.description.trim()) || + extractDescription(row.customAttributes || []) || + dashThis guarantees
descriptionis always a string (or-), which should make rendering and formatting more predictable.
98-115: Consider adding a dash fallback forapplicationType
subjectTypefalls back todash, butapplicationTypeis passed directly. Ifrow.applicationTypeis ever missing/empty, this row will render a blank value instead of the placeholder you use elsewhere.You can make it consistent with a small tweak:
- <GluuFormDetailRow - label="fields.application_type" - value={row.applicationType} + <GluuFormDetailRow + label="fields.application_type" + value={row.applicationType || dash}This keeps the visual language uniform across all scalar fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx(2 hunks)admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (8)
admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx (1)
7-7: Secret detail props and responsive sizing look goodExtending the props with
lsize/rsizeand providing defaults keeps existing usage working while allowing better alignment control in detail views. No issues here.admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (7)
31-45: Detail layout style objects look consistent with the rest of the viewThe
detailContainerStyle,detailRowStyle,detailLabelStyle, anddetailValueStyledefinitions give you a clear, centralized way to standardize spacing and typography across all rows in this panel. This directly addresses the alignment/spacing issues described in the PR.
59-79: Top row (client_id + client_secret) uses the new standardized layout correctlyThe first row now uses
detailRowStyleplusGluuFormDetailRowforclient_idand the updatedGluuSecretDetailfor the secret, which should align both columns cleanly and match the rest of the detail panel styling.
80-97: Description is now surfaced with sensible fallbacksUsing
displayName = row.clientName || row.displayName || dashanddescription = row.description || extractDescription(...) || dashin combination withGluuFormDetailRowensures both fields are always present in View Details, fulfilling the issue’s core requirement about the missing description. The label–value layout here is consistent with other rows.
116-149: Trusted-client and status rows now match the shared detail-row patternThe new
FormGroup+Label+Colstructure withdetailLabelStyleanddetailValueStyle, plus theme-aware badges for trusted client and status, brings these booleans in line with the rest of the detail view. Alignment and spacing should be much more consistent than before.
151-168: Scopes and grant types correctly leveragerenderBadgeListUsing
FormGroup rowwith labelsm={4}and valuesm={8}, together withrenderBadgeList(clientScopes)andrenderBadgeList(row.grantTypes), gives a clean, multi-value display that aligns with other badge-based rows. This should fix the earlier misalignment for these collections.
169-186: Response types and login URIs rows are visually consistent with scopes/grant types
response_typesandlogin_urisnow follow the same pattern (labelsm={4}, valuesm={8},renderBadgeListfor arrays), so these sections will respect the shared spacing and alignment rules and avoid the previous layout drift.
187-212: Logout redirect URIs and authentication method rows align with the new detail layoutThe
logout_redirect_urisrow uses the same badge helper and grid as the other URI lists, andauthentication_methodnow renders a themed badge when present or a dash otherwise, insidedetailValueStyle. This keeps the bottom rows visually consistent with the rest of the detail panel while covering the “no value” case explicitly.
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js
Outdated
Show resolved
Hide resolved
…issues in fields (#2471)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
20-26: Handle array return value from extractDescription.The
valuesproperty (plural naming suggests an array) is returned directly. Ifresult[0].valuesis an array like['description text']or[], passing it toGluuFormDetailRowmay cause rendering issues since the component likely expects a string. An empty array[]is truthy, so the dash fallback on line 29 won't trigger even when there's no description.Apply this diff to properly handle array values:
function extractDescription(customAttributes) { const result = customAttributes.filter((item) => item.name === 'description') if (result && result.length >= 1) { - return result[0].values + const values = result[0].values + return Array.isArray(values) ? (values.length > 0 ? values.join(', ') : '') : values } return '' }
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
46-56: LGTM! Past review feedback addressed.The
renderBadgeListhelper now consistently wraps both the badge list and the dash placeholder in the same<div style={detailValueStyle}>container, ensuring uniform label–value alignment across all rows regardless of whether items are present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx(2 hunks)admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (4)
admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx (3)
7-7: LGTM! Flexible sizing parameters added.The addition of optional
lsizeandrsizeparameters with sensible defaults (6/6) allows callers to customize column widths while maintaining backward compatibility. This supports the PR objective of standardizing layout across different views.
16-19: LGTM! Alignment and styling improvements applied.The changes correctly address the alignment issues mentioned in the PR objectives:
align-items-centerensures proper vertical alignment between label and valuemb-2provides consistent spacing between rows- Bold labels create visual hierarchy
- Dynamic
lsizeenables flexible layoutThese changes standardize the detail row layout as intended.
20-31: LGTM! Refactoring successfully implements the previous review feedback.The value column restructuring looks excellent:
- Flex layout with proper alignment and spacing addresses the formatting issues
wordBreak: 'break-all'handles long secret values gracefully- Line 26 correctly implements the fix from the previous review comment—the condition
(value === '-' || up)ensures the dash placeholder renders even whenupisfalse, preventing blank rows for clients without secrets- Toggle appropriately hidden for dash placeholders
- Dynamic
rsizeenables flexible responsive layoutThe refactoring successfully standardizes the detail row layout as intended by the PR objectives.
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
58-212: Well-structured refactor that addresses the PR objectives.The layout refactor successfully:
- Restores the missing description field with proper fallback logic
- Standardizes alignment using consistent style objects (
detailRowStyle,detailLabelStyle,detailValueStyle)- Consolidates badge rendering into the reusable
renderBadgeListhelper- Applies uniform spacing and structure across all detail rows
The changes align well with the stated goal of fixing alignment issues and displaying the description field in the Clients View Details panel.
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js
Outdated
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
41-51:extractDescriptionreturns an array instead of a string, causing incorrect UI rendering.The
customAttributesstructure storesvaluesas an array (confirmed by codebase patterns atadmin-ui/plugins/user-management/helper/validations.ts:104-105). Whenresult[0].valuesis an array like['My Description'], it gets passed toGluuFormDetailRowwhich expects a string, resulting invalue?.toString()rendering as a comma-joined string instead of a single value.function extractDescription(customAttributes) { const result = customAttributes.filter((item) => item.name === 'description') if (result && result.length >= 1) { - return result[0].values + const values = result[0].values + return Array.isArray(values) ? values[0] : values } return '' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
admin-ui/app/routes/Apps/Gluu/GluuFormDetailRow.tsx(3 hunks)admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx(1 hunks)admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx (3)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
useTranslation(33-33)admin-ui/plugins/services/Components/Configuration/PersistenceDetail.js (1)
labelStyle(37-39)admin-ui/plugins/admin/components/Webhook/ShortcodePopover.js (1)
Label(91-118)
🔇 Additional comments (11)
admin-ui/app/routes/Apps/Gluu/GluuFormDetailRow.tsx (3)
1-19: LGTM! Well-structured typed interface with appropriate prop definitions.The new
GluuFormDetailRowPropsinterface provides good type safety. The optional props with sensible defaults and the inclusion ofCSSPropertiesforlabelStylealign well with the PR's goal of standardizing alignment.
37-40: Good defensive programming with optional chaining on theme context.The
theme?.state?.themesafely handles cases where the theme context might not be fully initialized, preventing potential runtime errors.
59-59: LGTM! Memoization is appropriate for this presentational component.Since the component's output depends solely on its props and theme context,
memohelps avoid unnecessary re-renders when parent components update.admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (4)
53-63: LGTM! Badge list helper correctly handles empty state with consistent alignment.The
renderBadgeListfunction now wraps both populated and empty states in the samedetailValueStylecontainer, ensuring consistent label-value alignment as suggested in previous reviews.
67-118: LGTM! Description field is now properly displayed with consistent styling.The
descriptionfield is correctly mapped at line 109-117 with appropriate fallback logic, directly addressing the PR objective. The consistent use oflsize={4},rsize={8},detailLabelStyle, androwClassName="align-items-start"ensures uniform alignment across all fields.
146-178: LGTM! Boolean fields rendered consistently with badge styling.The
is_trusted_clientandstatusfields use the same layout pattern with proper badge colors anddetailValueStylewrapper for alignment consistency.
181-241: LGTM! Badge list fields follow consistent layout pattern.All multi-value fields (scopes, grant_types, response_types, login_uris, logout_redirect_uris, authentication_method) use the
renderBadgeListhelper with consistentsm={4}/sm={8}column sizing and uniform styling.admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx (4)
7-16: LGTM! Well-defined props interface with appropriate optional fields.The
GluuSecretDetailPropsinterface provides good type safety and consistency withGluuFormDetailRowProps, supporting the unified styling approach across detail components.
33-35: LGTM! Proper use ofuseCallbackfor stable handler reference.Using
useCallbackwith an empty dependency array ensures thehandleSecretfunction maintains referential stability, which works well with thememowrapper to prevent unnecessary re-renders.
51-56: LGTM! Dash placeholder rendering issue has been correctly addressed.The condition
(value === '-' || up)ensures the dash placeholder is displayed when no secret exists, while maintaining the reveal toggle behavior for actual secrets. This addresses the concern from previous reviews.
63-63: LGTM! Memoization is appropriate for this component.The component is a good candidate for
memoas it's primarily presentational with stable props.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/routes/Apps/Gluu/GluuFormDetailRow.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/app/routes/Apps/Gluu/GluuFormDetailRow.tsx (3)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (3)
useTranslation(33-33)theme(34-34)selectedTheme(35-35)admin-ui/plugins/services/Components/Configuration/PersistenceDetail.js (1)
labelStyle(37-39)admin-ui/plugins/admin/components/Webhook/ShortcodePopover.js (1)
Label(91-118)
🔇 Additional comments (1)
admin-ui/app/routes/Apps/Gluu/GluuFormDetailRow.tsx (1)
7-19: Typed props, layout knobs, and memoization look goodThe props interface, default values (including
lsize/rsize),rowClassNamepass‑through, andmemoexport make this detail row more predictable and reusable without introducing behavioral regressions.Also applies to: 23-35, 59-59
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
51-57: Excellent! I found critical evidence. In ClientListPage.js line 121, there's a similar pattern:org = results[0].values[0]This confirms that
valuesis indeed an array, and the correct pattern is to accessvalues[0]to get the actual value. TheextractDescriptionfunction in ClientDetailPage.js returnsresult[0].values(the entire array) instead ofresult[0].values[0](the first element), which would render as[object Array]in the UI.The original review comment is verified as correct.
Fix required: extractDescription returns an array instead of a string value.
The function returns
result[0].values, which is an array. Based on the pattern in ClientListPage.js (line 121:results[0].values[0]), the correct approach is to return the first element of the values array.Apply this fix:
function extractDescription(customAttributes) { const result = customAttributes.filter((item) => item.name === 'description') if (result && result.length >= 1) { - return result[0].values + return result[0].values[0] || '' } return '' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js (3)
11-40: LGTM: Static styles properly moved to module scope.The style objects are now correctly defined at module scope, addressing the previous review comment. This prevents unnecessary object recreation on each render.
63-73: LGTM: Badge list helper with consistent container wrapping.The
renderBadgeListhelper correctly addresses the previous review comment by always wrapping the output (badges or dash) in the same styled container, ensuring consistent alignment across all rows.
80-143: LGTM: Column 1 fields with consistent layout and styling.All fields in the first column follow a consistent pattern with proper label sizing (lsize=4/rsize=8 or sm=4/sm=8), uniform label styling, and appropriate use of the
renderBadgeListhelper for array fields.
4 participants
fix(admin): description missing in client view details and alignment issues in fields (#2471)
Summary
The View Details mode in the Auth Server → Clients page does not display the
descriptionfield. Several field values are also misaligned, resulting in inconsistent formatting throughout the details panel.Issues Fixed
Steps to Reproduce
Expected Behavior
Fix Summary
descriptionattribute.Closes
Closes: #2471
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.