Shutov/ #2582 [tech Admin/ Users] Added functionality to proccesses deleting of child`s data #2587
Conversation
…n' should contain the information about selection specified on the workshop page (#2584) * Created 'getWorkshopCompetitiveSelectionDescriptionById' method in UserWorkshopService Changed info-status template to show competitive selection description if user has applicationStatus AcceptedForSelection * fix tests * lint fix * lint fix * replaced 'toString' method with enum in 'if' statement * fixed formatting of inside *ngIf
…ng fix (#2594) * First version * Dvorak / 2549 Burger menu, name of the menu tab and indicator positioning fix * Minor update * Changed camelCase to kebab-case
…parent already has 20 children (#2585) * Disabled 'Add another child' button when the parent already has 20 children. Added information button with pop-up message when the parent already has 20 children. * error fix * error fix * Changed checking of children array length in template to a variable * renamed 'has20Children' variable to 'isChildLimitReached'. Replaced comparing to 20 with CHILDREN_AMOUNT_MAX constant
* added method of unreadMessages in chat * Removed question mark in translation * Add applications count example endpoint, state, badges for cabinet * Add provider.service tests * Update chat.state.ts * Update uk.json * Changed endpoint, refactored application service docs, moved method to application service, add tests * Made pendingApplications as search response, add role check * Add null-safety for template * Add test * Add support icon & fixed notification badge style * Add unreadNotificationsCount support * Removed notifications-list amount test assertions since it's managed by notifications.component * Add accepted for selection notification declination * Fixed fetching of pending application after application update and badge empty bug * Fixed changing notifications amount by tab navigation * Test enhancement * Resolved code review comments * Removed unused imports in personal-cabinet.component --------- Co-authored-by: doliinyk <[email protected]> Co-authored-by: Denys Oliinyk <[email protected]>
…orkshop's administrator (#2597)
* Fixed lint warnings * Fixed Sonar issues * Added lint-staged and pre-commit hook by husky * Lint & Prettier fixes * lintstagedrc config change * Lint/prettier fix
|
|
WalkthroughThis pull request introduces functionality to delete child users with role-based restrictions. The changes span multiple files, adding a new delete option for child users in the admin tools section. The implementation includes conditional rendering of the delete button, a new modal confirmation type, and methods to handle child user deletion with appropriate dialog and action dispatching. Changes
Sequence DiagramsequenceDiagram
participant User
participant UsersListComponent
participant UsersComponent
participant DialogService
participant Store
User->>UsersListComponent: Click Delete Button
UsersListComponent->>UsersComponent: Emit delete event
UsersComponent->>DialogService: Open Confirmation Dialog
DialogService-->>UsersComponent: Dialog Result
alt Confirmed
UsersComponent->>Store: Dispatch DeleteChildById Action
else Cancelled
UsersComponent->>UsersComponent: No Action
end
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/app/shared/models/child.model.ts (1)
19-19: Consider using a more descriptive property name.The property name
pibis not self-descriptive. Consider renaming it to something more meaningful likedisplayNameorfullNameto better indicate its purpose in the UI.- pib?: string; + displayName?: string;src/app/shell/admin-tools/data/users/users.component.ts (1)
127-129: Improve method documentation.The current documentation is too basic. Add more details about the parameters and the confirmation flow.
/** - * This method delete child By Id + * Deletes a child after showing a confirmation dialog. + * @param child - The child entity to be deleted + * @throws {Error} When deletion fails */src/app/shell/admin-tools/data/users/users.component.spec.ts (1)
224-234: Enhance mock data quality in tests.The mock child object contains several empty strings and null values. Consider using meaningful test data to make the tests more realistic and maintainable.
mockChild = { id: 'childId', firstName: 'John', lastName: 'Doe', - parentId: '', - dateOfBirth: '', + parentId: 'parent123', + dateOfBirth: '2010-01-01', gender: 0, - isParent: null, - placeOfStudy: '', + isParent: false, + placeOfStudy: 'School #1', pib: 'qwerty' };src/app/shared/components/users-list/users-list.component.html (1)
136-136: Consider extracting the role check to a method.The condition
user.role === UserTabsTitles.childcould be moved to a method in the component class for better reusability and testability.
- Add a method to the component class:
isChildUser(user: User): boolean { return user.role === UserTabsTitles.child; }
- Update the template:
-<button *ngIf="user.role === UserTabsTitles.child" mat-menu-item (click)="delete.emit(user)"> +<button *ngIf="isChildUser(user)" mat-menu-item (click)="delete.emit(user)">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/i18n/en.jsonis excluded by!src/assets/**and included bysrc/**src/assets/i18n/uk.jsonis excluded by!src/assets/**and included bysrc/**
📒 Files selected for processing (7)
src/app/shared/components/users-list/users-list.component.html(1 hunks)src/app/shared/components/users-list/users-list.component.ts(2 hunks)src/app/shared/enum/modal-confirmation.ts(3 hunks)src/app/shared/models/child.model.ts(1 hunks)src/app/shell/admin-tools/data/users/users.component.html(1 hunks)src/app/shell/admin-tools/data/users/users.component.spec.ts(2 hunks)src/app/shell/admin-tools/data/users/users.component.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: SonarCloud
🔇 Additional comments (4)
src/app/shared/components/users-list/users-list.component.ts (1)
9-9: LGTM!The addition of
UserTabsTitlesfollows Angular best practices for component properties and supports role-based conditional rendering.Also applies to: 38-38
src/app/shell/admin-tools/data/users/users.component.spec.ts (2)
254-264: LGTM! Comprehensive test coverage.The test properly verifies that the
deleteChildmethod is called with correct parameters upon confirmation.
242-246: Verify the modal width constant.Add a test to verify that the
Constants.MODAL_SMALLvalue matches the expected width for consistency across the application.src/app/shell/admin-tools/data/users/users.component.html (1)
35-39: LGTM! Clean event binding implementation.The delete event binding follows the established pattern and maintains consistency with other event bindings.
| private deleteChild(id: string, params: ChildrenParameters): void { | ||
| this.store.dispatch(new DeleteChildById(id, params)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add return type for better error handling.
The deleteChild method should return the Observable from the store dispatch for proper error handling in the calling method.
-private deleteChild(id: string, params: ChildrenParameters): void {
- this.store.dispatch(new DeleteChildById(id, params));
+private deleteChild(id: string, params: ChildrenParameters): Observable<any> {
+ return this.store.dispatch(new DeleteChildById(id, params));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private deleteChild(id: string, params: ChildrenParameters): void { | |
| this.store.dispatch(new DeleteChildById(id, params)); | |
| } | |
| private deleteChild(id: string, params: ChildrenParameters): Observable<any> { | |
| return this.store.dispatch(new DeleteChildById(id, params)); | |
| } |
| /** | ||
| * This method delete child By Id | ||
| */ | ||
| public onDeleteChild(child: Child): void { | ||
| this.matDialog | ||
| .open(ConfirmationModalWindowComponent, { | ||
| width: Constants.MODAL_SMALL, | ||
| data: { | ||
| type: ModalConfirmationType.deleteChild, | ||
| property: child.pib | ||
| } | ||
| }) | ||
| .afterClosed() | ||
| .pipe(filter(Boolean)) | ||
| .subscribe(() => { | ||
| this.deleteChild(child.id, this.childrenParams); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for the delete operation.
The delete operation should handle potential errors to provide feedback to the user when deletion fails.
public onDeleteChild(child: Child): void {
this.matDialog
.open(ConfirmationModalWindowComponent, {
width: Constants.MODAL_SMALL,
data: {
type: ModalConfirmationType.deleteChild,
property: child.pib
}
})
.afterClosed()
.pipe(filter(Boolean))
.subscribe(() => {
- this.deleteChild(child.id, this.childrenParams);
+ this.deleteChild(child.id, this.childrenParams).pipe(
+ catchError(error => {
+ // Handle error (e.g., show error message)
+ console.error('Failed to delete child:', error);
+ return EMPTY;
+ })
+ ).subscribe();
});
}Committable suggestion skipped: line range outside the PR's diff.
| @@ -33,6 +33,7 @@ export enum ModalConfirmationType { | |||
| updateEmployeeDeputy = 'updateEmployeeDeputy', | |||
| unBlockParent = 'unBlockParent', | |||
| blockParent = 'blockParent', | |||
| deleteParent = 'deleteParent', | |||
There was a problem hiding this comment.
💡 Codebase verification
Remove unused deleteParent enum values
The deleteParent enum values are not used anywhere in the codebase. The PR implements child deletion functionality and consistently uses ModalConfirmationType.deleteChild. Remove these unused enum values to maintain code cleanliness:
deleteParentfromModalConfirmationTypedeleteParentfromModalConfirmationTitledeleteParentfromModalConfirmationText
🔗 Analysis chain
Verify if deleteParent enum is needed.
The PR objectives mention functionality for deleting child's data, but these changes add enum values for deleting parent. The code uses ModalConfirmationType.deleteChild. Please verify if the deleteParent enum values are necessary for this PR.
Also applies to: 64-64, 112-112
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of deleteParent in the codebase
rg "deleteParent" --type ts
Length of output: 335
Script:
#!/bin/bash
# Search for ModalConfirmationType and the translation keys
rg "ModalConfirmationType\.deleteParent" --type ts
rg "SERVICE_MESSAGES\.MODAL_CONFIRMATIONS\.TITLES\.DELETE_PARENT" --type ts
rg "SERVICE_MESSAGES\.MODAL_CONFIRMATIONS\.DELETE_PARENT" --type ts
# Search for any file that might be importing and using these enums
rg -l "modal-confirmation" --type ts
Length of output: 3557
Script:
#!/bin/bash
# Check parent-related components for delete functionality
echo "=== parent-applications.component.ts ==="
rg "delete" -A 5 -B 5 src/app/shell/personal-cabinet/parent/parent-applications/parent-applications.component.ts
echo -e "\n=== create-parent.component.ts ==="
rg "delete" -A 5 -B 5 src/app/shell/personal-cabinet/parent/create-parent/create-parent.component.ts
echo -e "\n=== children.component.ts ==="
rg "delete" -A 5 -B 5 src/app/shell/personal-cabinet/parent/children/children.component.ts
Length of output: 861
also added modal window, translate, and enums for "deleting Parent`s data method" for further implementation
Summary by CodeRabbit
Release Notes
New Features
Enhancements
pibproperty to child modelBug Fixes