AMM-118: Add time-based account lockout with auto-unlock #122
Conversation
📝 WalkthroughWalkthroughAdds admin UI controls to check and unlock user accounts, implements component methods to call new service endpoints, adds corresponding service methods and environment URLs, and surfaces fallback login error alerts. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant UI as EmployeeMasterComponent (UI)
participant Svc as EmployeeMasterNewServices
participant API as User Lock API
Admin->>UI: Click "Check lock status" (userID)
UI->>Svc: getUserLockStatus(userID)
Svc->>API: POST /user/getUserLockStatus { userId }
API-->>Svc: 200 { data: "...", statusCode: 200 }
Svc-->>UI: return response
UI-->>Admin: show lock-status alert
sequenceDiagram
actor Admin
participant UI as EmployeeMasterComponent (UI)
participant Svc as EmployeeMasterNewServices
participant API as User Lock API
Admin->>UI: Click "Unlock" (confirm)
UI->>Svc: unlockUserAccount(userID)
Svc->>API: POST /user/unlockUserAccount { userId }
API-->>Svc: 200 { statusCode: 200, message }
Svc-->>UI: return response
UI-->>Admin: show success alert, refresh list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.ts:
- Around line 1579-1619: The checkUserLockStatus method currently calls
JSON.parse(response.data) without guarding against malformed JSON; wrap the
parse in a try-catch around JSON.parse(response.data) (inside
checkUserLockStatus) to catch SyntaxError and handle it by logging the error and
showing a user-friendly dialogService.alert (e.g., 'Invalid lock status data
received' or fallback to the error path), and ensure subsequent logic only runs
when parsing succeeds (return early from the subscription error branch on parse
failure).
🧹 Nitpick comments (2)
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.html (2)
85-94: Inline styles for spacing could be consolidated.The
margin-right: 5px;styling is repeated across multiple buttons. While inline styles work, consider extracting these to a CSS class for better maintainability.♻️ Optional refactor to use CSS class
In your component CSS file, add:
.action-button-spacing { margin-right: 5px; }Then apply to buttons:
- style="margin-right: 5px;" + class="action-button-spacing"
95-104: Consider button color semantics for the Unlock action.The Unlock button uses
color="warn"(typically red), which might suggest a destructive or dangerous action. Since unlocking is a positive administrative action to restore access, consider usingcolor="accent"(green) to align with the Activate button's semantics.♻️ Suggested color adjustment
<button *ngIf="element.deleted === true" mat-raised-button - color="warn" + color="accent" matTooltip="Unlock account locked due to failed login attempts" (click)="unlockUserAccount(element.userID, element.userName)" style="margin-right: 5px;" > Unlock </button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.htmlsrc/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.tssrc/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.tssrc/app/user-login/login/login.component.tssrc/environments/environment.local.tssrc/environments/environment.prod.tssrc/environments/environment.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-25T11:01:28.621Z
Learnt from: devikasuresh20
Repo: PSMRI/ADMIN-UI PR: 50
File: src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts:1981-1981
Timestamp: 2024-11-25T11:01:28.621Z
Learning: In the `filterComponentList` method of `WorkLocationMappingComponent` in `src/app/app-provider-admin/provider-admin/activities/work-location-mapping/work-location-mapping.component.ts`, retain the manual filtering implementation as it provides better control over the filtering logic.
Applied to files:
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.htmlsrc/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.ts
🧬 Code graph analysis (1)
src/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.ts (3)
src/environments/environment.local.ts (1)
environment(40-513)src/environments/environment.prod.ts (1)
environment(40-514)src/environments/environment.test.ts (1)
environment(36-506)
🔇 Additional comments (9)
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.html (2)
105-114: LGTM - Consistent spacing added to Deactivate button.The margin-right styling maintains visual consistency with other action buttons.
115-121: LGTM - Check Lock Status button is well implemented.The button provides admins with proactive visibility into user lock status. The tooltip clearly describes the action, and the info icon is semantically appropriate.
src/environments/environment.test.ts (1)
225-229: LGTM - New API endpoints are well organized.The new User Account Lock Management endpoints are properly defined with clear comments and follow the existing URL pattern conventions. The placement near other user-related endpoints is logical.
src/environments/environment.prod.ts (1)
229-233: LGTM - Production environment endpoints are consistent.The endpoint definitions match the test environment configuration, ensuring consistency across environments.
src/app/user-login/login/login.component.ts (2)
179-218: Excellent improvement - unhandled errors now properly surface to users.The addition of the else branch ensures that error messages not matching specific patterns are displayed to users instead of being silently ignored. This improves the user experience and aids in debugging.
239-290: LGTM - Consistent error handling pattern for regular user authentication.The fallback error handler mirrors the Super Admin flow, providing consistent error messaging across both authentication paths.
src/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.ts (1)
192-202: LGTM - New service methods follow existing patterns.The
getUserLockStatus()andunlockUserAccount()methods are well implemented and consistent with the existing service architecture. They correctly use environment-based URLs and return Observables for reactive handling.♻️ Optional ES6 shorthand for cleaner syntax
The object literal can use ES6 property shorthand since the key and value names match:
getUserLockStatus(userId: number): Observable<any> { - return this.http.post(environment.getUserLockStatusUrl, { - userId: userId, - }); + return this.http.post(environment.getUserLockStatusUrl, { userId }); } unlockUserAccount(userId: number): Observable<any> { - return this.http.post(environment.unlockUserAccountUrl, { - userId: userId, - }); + return this.http.post(environment.unlockUserAccountUrl, { userId }); }src/environments/environment.local.ts (1)
230-234: LGTM!The new URL constants are properly structured, follow existing naming conventions, and are appropriately grouped with clear comments. The placement adjacent to related user account URLs is logical.
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.ts (1)
1538-1577: LGTM!The unlock implementation follows established patterns in this component (similar to
activateDeactivate). The confirmation dialog provides clear context, response handling is appropriate, and side effects (refresh list, reset search) align with user expectations.
...rovider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.ts
Show resolved
Hide resolved
varundeepsaini
force-pushed
the
AMM-118-time-based-account-lockout
branch
from
f86014d to
885dcbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.html`:
- Around line 95-104: The Unlock button is hidden by the wrong condition
(*ngIf="element.deleted === true"), preventing admins from unlocking active but
locked users; update the template where the button with
(click)="unlockUserAccount(element.userID, element.userName)" is declared so the
button is shown for all users (remove or replace the *ngIf) — or, if you prefer
stricter control, change the condition to a lock-specific check (e.g.,
element.locked === true) instead of element.deleted.
🧹 Nitpick comments (1)
src/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.ts (1)
191-202: LGTM with optional style refinement.The new service methods follow the existing patterns in this file. Consider using ES6 shorthand property syntax for cleaner code.
✨ Optional: Use ES6 shorthand property syntax
getUserLockStatus(userId: number): Observable<any> { return this.http.post(environment.getUserLockStatusUrl, { - userId: userId, + userId, }); } unlockUserAccount(userId: number): Observable<any> { return this.http.post(environment.unlockUserAccountUrl, { - userId: userId, + userId, }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.htmlsrc/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.tssrc/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.tssrc/app/user-login/login/login.component.tssrc/environments/environment.local.tssrc/environments/environment.prod.tssrc/environments/environment.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/environments/environment.local.ts
- src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.ts
- src/app/user-login/login/login.component.ts
- src/environments/environment.prod.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/app/app-provider-admin/provider-admin/activities/services/employee-master-new-services.service.ts (1)
src/environments/environment.test.ts (1)
environment(36-506)
🔇 Additional comments (2)
src/environments/environment.test.ts (1)
225-229: LGTM!The new API endpoint URLs for user lock management are properly defined using the
commonBaseURLtemplate literal, following the existing naming conventions in the file. The comment accurately identifies these as Common-API endpoints.src/app/app-provider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.html (1)
115-121: LGTM!The "Check lock status" button is correctly shown for all users without any conditional, allowing admins to check the lock status of any account regardless of its activation state.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...vider-admin/provider-admin/activities/employee-master-new/employee-master-new.component.html
Show resolved
Hide resolved
Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com>
varundeepsaini
force-pushed
the
AMM-118-time-based-account-lockout
branch
from
885dcbc to
0172709
Compare
|
1 participant
Part of PSMRI/AMRIT#118
📋 Description
Adds admin UI functionality to view and manage user account lock status. Admins can now check if an account is locked due to failed login attempts and manually unlock it before the 24-hour auto-unlock period.
✅ Type of Change
ℹ️ Additional Information
Changes:
checkUserLockStatus()method to display lock detailsunlockUserAccount()method for manual unlockTesting:
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.