[LOCKLITE-84] Be able to edit members about a vault#139
Conversation
This reverts commit 997b612.
vbetsch
added
enhancement
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements functionality to allow editing of vault members, adding comprehensive UI components and improving test coverage. The changes introduce member management capabilities through a new sharing modal interface.
Key changes:
- Added vault member editing functionality with a ShareVaultModal component
- Implemented extensive test coverage for Logger and API modules with updated error handling patterns
- Reorganized file structure by migrating DTOs and types to module-specific locations
Reviewed Changes
Copilot reviewed 97 out of 126 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/projects/ui/modules/vaults/components/modals/ShareVaultModal.tsx | New modal for editing vault member permissions |
| src/projects/ui/modules/vaults/components/core/atoms/AvatarMultiSelect.tsx | Multi-select component for choosing vault members |
| tests/units/projects/shared/logs/logger.test.ts | Comprehensive test coverage for Logger class with new error handling |
| tests/units/projects/api/modules/users/infra/users.repository.test.ts | Complete test suite for UsersRepository |
| src/projects/shared/logs/logger.ts | Updated error handling to use structured error objects |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ); | ||
|
|
||
| const onChange = (members: VaultMemberModelDto[]): void => { | ||
| setSelectedMembers([...members]); |
There was a problem hiding this comment.
The selectedMembers state is initialized but never used in the component logic. This appears to be dead code that should be removed to improve maintainability.
| setSelectedMembers([...members]); | |
| const onChange = (members: VaultMemberModelDto[]): void => { |
| Logger.critical({ message: 'test', error: mockError }); | ||
|
|
||
| const timesThree: number = 3; | ||
| expect(consoleLogSpy).toHaveBeenCalledTimes(timesThree); |
There was a problem hiding this comment.
[nitpick] Using magic numbers with descriptive variable names like timesThree and timesTwo makes the code less readable. Consider using the literal numbers directly or using more descriptive names that explain the business logic behind these counts.
| expect(consoleLogSpy).toHaveBeenCalledTimes(timesThree); | |
| expect(consoleLogSpy).toHaveBeenCalledTimes(3); |
| ); | ||
|
|
||
| const onChange = (members: VaultMemberModelDto[]): void => { | ||
| setSelectedMembers([...members]); |
There was a problem hiding this comment.
The internal onChange function updates unused state selectedMembers and then calls props.onChange. The state update serves no purpose and should be removed.
| setSelectedMembers([...members]); | |
| const onChange = (members: VaultMemberModelDto[]): void => { |
# Conflicts: # package.json # src/projects/api/app/adapters/vault.adapter.ts # src/projects/api/modules/auth/domain/usecases/register.usecase.ts # src/projects/api/modules/auth/domain/usecases/signin.usecase.ts # src/projects/api/modules/seed/domain/upsert-user-with-vaults.usecase.ts # src/projects/api/modules/users/app/user.adapter.ts # src/projects/api/modules/users/domain/current-user.service.ts # src/projects/api/modules/vaults/domain/usecases/create-vault.usecase.ts # src/projects/api/modules/vaults/domain/usecases/get-my-vaults.usecase.ts # tests/units/projects/api/modules/auth/domain/usecases/signin.usecase.test.ts
No description provided.