Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the UI login/feature-discovery flow so that a user without permission to call listLdapConfigurations is not blocked from logging in, while still keeping the UI’s LDAP-enabled flag updated when LDAP config changes.
Changes:
- Stop rejecting the
GetInfoflow whenlistLdapConfigurationsfails (e.g., due to missing API permission). - Rename the Vuex action used for refreshing the LDAP-enabled flag from
UpdateConfigurationtoUpdateLdapConfigurationFlag. - Update
AutogenViewto dispatch the renamed action after LDAP configuration add/delete actions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ui/src/views/AutogenView.vue | Dispatches the renamed Vuex action after LDAP config mutations. |
| ui/src/store/modules/user.js | Makes LDAP config listing non-fatal during GetInfo; renames the LDAP refresh action. |
Comments suppressed due to low confidence (1)
ui/src/store/modules/user.js:561
UpdateLdapConfigurationFlagwraps the API call in anew Promise(...)but never callsresolve(...)on success, so any caller awaitingdispatch('UpdateLdapConfigurationFlag')will hang indefinitely. Either callresolve(ldapEnable)inside thethen(...)(and possiblyresolve(false)on handled errors), or simplify by returning theapi('listLdapConfigurations')promise chain directly.
UpdateLdapConfigurationFlag ({ commit }) {
return new Promise((resolve, reject) => {
api('listLdapConfigurations').then(response => {
const ldapEnable = (response.ldapconfigurationresponse.count > 0)
commit('SET_LDAP', ldapEnable)
}).catch(error => {
reject(error)
})
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13152 +/- ##
=============================================
- Coverage 16.39% 4.16% -12.23%
=============================================
Files 5669 405 -5264
Lines 501541 33079 -468462
Branches 60925 5903 -55022
=============================================
- Hits 82228 1378 -80850
+ Misses 410104 31525 -378579
+ Partials 9209 176 -9033
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I opened a stacked follow-up PR against this PR's source branch with the review hardening from the reproduced 4.22 issue: shapeblue#136 It keeps |
|
Update: I pushed the follow-up hardening directly into this PR branch now that I confirmed write access to The PR now includes commit
The temporary stacked PR is now merged/superseded: shapeblue#136 |
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17859 |
|
@wido we need some help ;) |
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@sudo87 can you solve the conflict? |
61d5a48 to
7d925b1
Compare
|
@sudo87 , did you address the comments #13152 (comment) and #13152 (comment) ? |
|
@DaanHoogland yes conflicts are resolved in this pr. Is there any other comment to address? |
|
@sudo87 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
Description
This PR fixes the login flow for users who do not have permission to access the listLdapConfigurations API. Currently, login is blocked for these users regardless of whether LDAP is configured.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?