Display integration control plane group id only for jdbc userstore #230
Conversation
WalkthroughAdds backend and frontend support to detect whether a JDBC user store is configured, exposes a new backend endpoint, wires a new HTTP client call, updates group visibility logic in the UI, and includes minor defensive and whitespace-only edits elsewhere. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as GroupSelector (UI)
participant HTTP as HTTPClient
participant API as ConfigsApi (Backend)
participant Delegate as ConfigsDelegate
participant USM as Realm / UserStore
User->>UI: Open dashboard / GroupSelector mounts
UI->>HTTP: isJdbcUserStoreConfigured()
HTTP->>API: GET /configs/isJdbcUserStoreConfigured
API->>Delegate: isJdbcUserStoreConfigured()
Delegate->>USM: Load realm configuration / userStoreClass
alt realm indicates JDBCUserStoreManager
Delegate-->>API: true
else not JDBC (file/external)
Delegate-->>API: false
end
API-->>HTTP: { "isJdbcUserStore": boolean }
HTTP-->>UI: boolean
UI->>UI: set isJdbcUserStore state
UI->>UI: filter groups (hide or show ICP_NAME based on flag & route)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
| } | ||
|
|
||
| return groupList; |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
| } | |
| return groupList; | |
| } | |
| logger.info("Successfully fetched {} groups", groupList.size()); | |
| return groupList; |
| public Response getSuperAdmin() { | ||
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
| public Response getSuperAdmin() { | |
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); | |
| public Response getSuperAdmin() { | |
| log.info("Retrieving super admin configuration"); | |
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); |
| public Response isJdbcUserStoreConfigured() { | ||
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
| public Response isJdbcUserStoreConfigured() { | |
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); | |
| public Response isJdbcUserStoreConfigured() { | |
| log.info("Checking JDBC user store configuration"); | |
| ConfigsDelegate configsDelegate = new ConfigsDelegate(); |
| public boolean isJdbcUserStoreConfigured() { | ||
| log.debug("Checking if JDBC user store is configured."); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
| public boolean isJdbcUserStoreConfigured() { | |
| log.debug("Checking if JDBC user store is configured."); | |
| public boolean isJdbcUserStoreConfigured() { | |
| log.info("Checking if JDBC user store is configured"); | |
| log.debug("Checking if JDBC user store is configured."); |
| String userStoreClass = realmConfig.getUserStoreClass(); | ||
| boolean isJdbc = "org.wso2.dashboard.security.user.core.jdbc.JDBCUserStoreManager" | ||
| .equals(userStoreClass); | ||
| log.info("User store class: " + userStoreClass + ", is JDBC: " + isJdbc); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 6
| log.info("User store class: " + userStoreClass + ", is JDBC: " + isJdbc); | |
| log.info("User store class: " + userStoreClass + ", is JDBC: " + isJdbc); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Determined user store type - Class: " + userStoreClass + ", is JDBC: " + isJdbc); | |
| } |
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
![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
🧹 Nitpick comments (6)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/ApiSideDrawer.js (3)
77-77: Good defensive default for urlList.The empty array default prevents runtime errors when
urlListis missing, which aligns with addressing the blank page issue mentioned in Issue #227.Note: The truthiness check on line 92 (
urls && urls.map) is now redundant sinceurlsis guaranteed to be an array. You may optionally simplify it to justurls.map(url =>.
92-94: Consider adding key prop to mapped elements.The
.map()call is missing akeyprop on the renderedCopyToClipboardRowcomponents, which can trigger React warnings.Apply this diff to add the key prop:
- {urls && urls.map(url => - <CopyToClipboardRow text={url} /> + {urls.map((url, index) => + <CopyToClipboardRow key={index} text={url} /> )}
108-127: Consider adding key prop to mapped ExpansionPanel components.The
.map()call is missing akeyprop on the renderedExpansionPanelcomponents.Apply this diff to add the key prop:
- return (resources.map(resource => ( - <ExpansionPanel> + return (resources.map((resource, index) => ( + <ExpansionPanel key={resource.url || index}> <ExpansionPanelSummaryNote: Using
resource.urlas the key assumes URLs are unique; otherwise, fall back toindex.components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/api/ConfigsApi.java (1)
57-71: Prefer proper JSON serialization over manual string construction.Line 68 manually constructs the JSON response as a string. This approach is error-prone and bypasses type safety. Consider creating a dedicated response model class and using JAX-RS's automatic JSON serialization.
Create a response model:
public class JdbcUserStoreResponse { private boolean isJdbcUserStore; public JdbcUserStoreResponse(boolean isJdbcUserStore) { this.isJdbcUserStore = isJdbcUserStore; } public boolean isJdbcUserStore() { return isJdbcUserStore; } public void setJdbcUserStore(boolean jdbcUserStore) { isJdbcUserStore = jdbcUserStore; } }Then update the endpoint:
public Response isJdbcUserStoreConfigured() { ConfigsDelegate configsDelegate = new ConfigsDelegate(); boolean isJdbc = configsDelegate.isJdbcUserStoreConfigured(); - Response.ResponseBuilder responseBuilder = Response.ok().entity("{\"isJdbcUserStore\": " + isJdbc + "}"); + Response.ResponseBuilder responseBuilder = Response.ok().entity(new JdbcUserStoreResponse(isJdbc)); HttpUtils.setHeaders(responseBuilder); return responseBuilder.build(); }components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/delegates/configs/ConfigsDelegate.java (1)
45-74: Consider extracting the hardcoded JDBC class name and improving error handling.The implementation has two areas for improvement:
Hardcoded class name (lines 62-63): The JDBC user store class name is hardcoded. Extract it to a constant for maintainability.
Generic exception handling (line 70): Catching generic
Exceptioncan mask important errors. Consider catching specific exceptions or at least logging more details about unexpected errors.Apply this diff:
public class ConfigsDelegate { private static final Log log = LogFactory.getLog(ConfigsDelegate.class); + private static final String JDBC_USER_STORE_CLASS = "org.wso2.dashboard.security.user.core.jdbc.JDBCUserStoreManager"; public SuperAdminUser getSuperUser() { log.debug("Retrieving super user from system properties."); SuperAdminUser superAdminUser = new SuperAdminUser(); String superAdminUserName = (String) ConfigParser.getParsedConfigs().get("super_admin.username"); superAdminUser.setUsername(superAdminUserName); return superAdminUser; } public boolean isJdbcUserStoreConfigured() { log.debug("Checking if JDBC user store is configured."); try { if (UserStoreManagerUtils.isFileBasedUserStoreEnabled()) { log.debug("File-based user store is enabled. JDBC user store is not configured."); return false; } RealmConfiguration realmConfig = DataHolder.getInstance().getRealmConfig(); if (realmConfig == null) { // Try to initialize user store to get realm config UserStoreManagerUtils.getUserStoreManager(); realmConfig = DataHolder.getInstance().getRealmConfig(); } if (realmConfig != null) { String userStoreClass = realmConfig.getUserStoreClass(); - boolean isJdbc = "org.wso2.dashboard.security.user.core.jdbc.JDBCUserStoreManager" - .equals(userStoreClass); + boolean isJdbc = JDBC_USER_STORE_CLASS.equals(userStoreClass); log.info("User store class: " + userStoreClass + ", is JDBC: " + isJdbc); return isJdbc; } log.debug("Realm configuration is null. JDBC user store is not configured."); return false } catch (Exception e) { - log.error("Error while checking JDBC user store configuration", e); + log.error("Unexpected error while checking JDBC user store configuration. Defaulting to non-JDBC.", e); return false; } } }components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js (1)
67-72: Add error handling for JSON.parse.Line 69 parses
node.detailswithout error handling. If the details field contains invalid JSON, this will throw an exception and break the UI.Apply this diff:
function loadNodesForGroup(group, dispatch) { HTTPClient.getAllNodes(group).then(response => { - response.data = response.data.map(node => ({ ...node, details: JSON.parse(node.details) })); + response.data = response.data.map(node => { + try { + return { ...node, details: JSON.parse(node.details) }; + } catch (e) { + console.error(`Failed to parse node details for node ${node.nodeId}:`, e); + return { ...node, details: {} }; + } + }); dispatch(selectGroup(group, response.data)); }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/data/manager/InMemoryDataManager.java(11 hunks)components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/api/ConfigsApi.java(1 hunks)components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/delegates/configs/ConfigsDelegate.java(2 hunks)components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/ApiSideDrawer.js(2 hunks)components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js(1 hunks)components/org.wso2.micro.integrator.dashboard.web/web-app/src/utils/HTTPClient.js(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/ApiSideDrawer.js (2)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/DataServicesSideDrawer.js (2)
nodeData(69-69)resources(189-189)components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/ProxySideDrawer.js (2)
props(36-36)props(62-62)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js (2)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/utils/HTTPClient.js (1)
HTTPClient(25-237)components/org.wso2.micro.integrator.dashboard.web/web-app/src/redux/Actions.js (4)
changeGroup(19-24)changeGroup(19-24)selectGroup(62-67)selectGroup(62-67)
components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/delegates/configs/ConfigsDelegate.java (1)
components/org.wso2.dashboard.security.user.core/src/main/java/org/wso2/dashboard/security/user/core/UserStoreManagerUtils.java (1)
UserStoreManagerUtils(37-101)
🪛 Biome (2.1.2)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (7)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/commons/sideDrawers/ApiSideDrawer.js (1)
107-108: Good defensive default for resources.The empty array default and direct map iteration are clean and prevent runtime errors when
resourcesis missing.components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/data/manager/InMemoryDataManager.java (1)
73-73: LGTM! Formatting improvements.The whitespace and formatting changes improve code readability without altering behavior.
Also applies to: 90-90, 94-94, 103-103, 109-109, 120-120, 139-139, 158-158, 175-175, 182-182, 188-188, 195-195, 200-200, 226-226, 240-240, 254-254, 266-266
components/org.wso2.ei.dashboard.core/src/main/java/org/wso2/ei/dashboard/core/rest/api/ConfigsApi.java (1)
45-55: LGTM! Formatting improvements.The formatting adjustments improve readability without changing behavior.
components/org.wso2.micro.integrator.dashboard.web/web-app/src/utils/HTTPClient.js (4)
23-23: LGTM! Import formatting improved.The spacing adjustment improves consistency.
45-47: LGTM! Good defensive programming.Adding a default empty object for the
paramsparameter prevents potential issues when the method is called without the second argument.
74-76: LGTM! New method follows established patterns.The
isJdbcUserStoreConfigured()method correctly uses theget()method to call the backend endpoint and follows the same pattern as other methods in this class.
100-100: LGTM! Formatting improvements.The spacing and formatting adjustments improve code consistency and readability.
Also applies to: 136-136, 177-180, 182-185, 189-198, 215-218
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
Show resolved
Hide resolved
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js (1)
21-63: Nice fix for the async “who wins first” issue; consider stale-route + unmount safety.
The Promise.all sequencing is good (and addresses the earlier race), but this effect readslocation.pathnamewhile the requests are in-flight (Line 45-47). If the user navigates before the promises resolve, you can filter/select using a stale route and dispatch the wrong initial group. Also, there’s no unmount guard beforesetIsJdbcUserStore/setGroupList.React.useEffect(() => { - // Wait for both JDBC user store check and groups to load before selecting initial group + let cancelled = false; Promise.all([ HTTPClient.isJdbcUserStoreConfigured().catch(error => { console.error("Error checking external user store configuration:", error); return { data: { isJdbcUserStore: false } }; }), HTTPClient.getGroups() ]).then(([userStoreResponse, groupsResponse]) => { + if (cancelled) return; // Set JDBC user store flag first const jdbcUserStore = userStoreResponse.data.isJdbcUserStore; setIsJdbcUserStore(jdbcUserStore); + const pathname = window.location.pathname; // avoid stale route // Filter groups based on current location and JDBC user store status - const shouldShowICP = (location.pathname.startsWith("/users") || - location.pathname.startsWith("/roles") || - location.pathname.startsWith("/update-password")) && jdbcUserStore; + const shouldShowICP = (pathname.startsWith("/users") || + pathname.startsWith("/roles") || + pathname.startsWith("/update-password")) && jdbcUserStore; ... }).catch(error => { console.error("Error loading groups:", error); }); -}, []) + return () => { cancelled = true; }; +}, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js(1 hunks)
🔇 Additional comments (1)
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js (1)
146-151: Style change looks fine (minHeight/lineHeight pairing).
No concerns here.
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
Show resolved
Hide resolved
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
Show resolved
Hide resolved
components/org.wso2.micro.integrator.dashboard.web/web-app/src/home/GroupSelector.js
Show resolved
Hide resolved
2 participants
Purpose
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.