[Feat] Project modules management (Added a smart table to display project modules with dynamic data loading.) #8550
Conversation
WalkthroughThe pull request introduces modifications across several components related to project module management in an Angular application. Key changes include updating button statuses for project creation, enhancing error handling with optional chaining, and introducing a new component for displaying project modules in a smart table format. Additionally, new HTML templates and SCSS styles are provided, along with the integration of a dialog for creating project modules. These changes collectively improve the user interface and functionality of the project management features. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
samuelmbabhazi
changed the title
samuelmbabhazi
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (18)
packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html (2)
10-17: Enhance table accessibility and stylingTwo improvements are suggested:
- Move the inline style to the component's SCSS file
- Add accessibility attributes for better screen reader support
Apply these changes:
<angular2-smart-table [settings]="settingsSmartTable" (userRowSelect)="selectItem($event)" [source]="smartTableSource" - style="cursor: pointer" #variantTable + role="grid" + aria-label="Project Modules" > </angular2-smart-table>Add to your SCSS file:
:host ::ng-deep { angular2-smart-table { cursor: pointer; } }
21-46: Improve action buttons consistency and UXConsider the following improvements:
- Add tooltip to edit button for consistency
- Use different button statuses for better visual distinction
- Consider extracting buttons into a reusable component
Apply these changes:
<button (click)="onEditProjectModuleDialog()" nbButton - status="basic" + status="primary" class="action primary" size="small" [disabled]="disableButton" + [nbTooltip]="'BUTTONS.EDIT' | translate" > <nb-icon icon="edit-outline"></nb-icon> {{ 'BUTTONS.EDIT' | translate }} </button> <button (click)="delete()" nbButton - status="basic" + status="danger" class="action" [disabled]="disableButton" size="small" [nbTooltip]="'BUTTONS.DELETE' | translate" > - <nb-icon status="danger" icon="trash-2-outline"> </nb-icon> + <nb-icon icon="trash-2-outline"> </nb-icon> </button>apps/gauzy/src/app/pages/projects/components/project-list/list.component.html (1)
Line range hint
24-31: Consider enhancing table accessibilityWhile the smart table implementation is functional, consider adding ARIA labels and role attributes to improve accessibility for screen readers.
<div class="table-scroll-container"> <angular2-smart-table style="cursor: pointer" (userRowSelect)="selectProject($event)" [settings]="settingsSmartTable" [source]="smartTableSource" + role="grid" + aria-label="{{ 'ORGANIZATIONS_PAGE.PROJECTS' | translate }}" ></angular2-smart-table> </div>packages/ui-core/shared/src/lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts (2)
131-137: Standardize null-checking pattern for consistencyThe null-checking pattern is inconsistent between form patching and selection setting. Consider using the same pattern throughout for better maintainability.
- members: (module.members || [])?.map((m) => m.id), + members: module.members?.map((m) => m.id) ?? [], - teams: (module.teams || [])?.map((t) => t.id), + teams: module.teams?.map((t) => t.id) ?? [], - this.selectedMembers = module.members?.map((m) => m.id); + this.selectedMembers = module.members?.map((m) => m.id) ?? []; - this.selectedTeams = module.teams?.map((t) => t.id); + this.selectedTeams = module.teams?.map((t) => t.id) ?? [];
Line range hint
1-338: Consider breaking down the component for better maintainabilityThe component has grown to handle multiple responsibilities including form management, data loading, and API interactions. Consider:
- Extracting form management into a separate service
- Creating a dedicated data loading service
- Breaking down the template into smaller, reusable components
This would improve:
- Testability
- Code reusability
- Maintenance
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.html (1)
Line range hint
492-511: Consider splitting project and module management into separate componentsThe current implementation mixes project and module management within the same component, which increases complexity and couples different concerns. Consider:
- Creating a separate route/component for module management
- Moving module-related functionality out of the project mutation component
- Using a dedicated service for module operations
This would improve:
- Component maintainability
- Code organization
- Feature isolation
- Reusability
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts (1)
Line range hint
1-733: Consider breaking down this large component for better maintainability.The component is handling multiple responsibilities including:
- Complex form management
- API interactions
- Dialog management
- File upload
- GitHub integration
Consider splitting this into smaller, more focused components and moving form logic to a dedicated service. This would:
- Improve maintainability
- Make the code easier to test
- Follow the Single Responsibility Principle
packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.ts (11)
82-85: Add error logging in catch blockIn the
loadModules()method, the error is not being logged in thecatchblock. Logging the error can aid in debugging and provide more insight into failures.Apply this diff to log the error:
- } catch { + } catch (error) { this.toastrService.danger('TOASTR.MESSAGE.SOMETHING_BAD_HAPPENED'); + console.error('Error loading modules:', error); } finally {
222-223: Add error logging in catch blockSimilarly, in the
updateModule()method, consider logging the error in thecatchblock to assist with debugging.Apply this diff:
- } catch { + } catch (error) { this.toastrService.danger('TOASTR.MESSAGE.SOMETHING_BAD_HAPPENED'); + console.error('Error updating module:', error); }
184-185: Add error logging in catch blockIn the
delete()method, logging the error in thecatchblock will help diagnose issues during deletion.Apply this diff:
- } catch { + } catch (error) { this.toastrService.danger('TOASTR.MESSAGE.SOMETHING_BAD_HAPPENED'); + console.error('Error deleting module:', error); }
218-221: Optimize module update without reloading all modulesAfter updating a module in
updateModule(), the entire module list is reloaded usingloadModules(). Consider updating the module directly in the table to improve performance.Modify the method to update the module in the table:
await firstValueFrom(this.organizationProjectModuleService.update(id, changes)); this.toastrService.success('TOASTR.MESSAGE.MODULE_UPDATED'); - await this.loadModules(); + const updatedModule = { ...this.selectedItem, ...changes }; + this.updateModuleInTable(updatedModule as IOrganizationProjectModule);
96-100: Ensure consistent localization for column titlesThe 'name' and 'isFavorite' column titles are inconsistently localized. All column titles should use
this.getTranslation()for consistency.Apply this diff:
name: { - title: this.getTranslation('ORGANIZATIONS_PAGE.NAME'), + title: this.getTranslation('PROJECT_MODULE.NAME'), type: 'string' }, isFavorite: { - title: 'isFavorite', + title: this.getTranslation('PROJECT_MODULE.IS_FAVORITE'),
216-224: Specify return type for asynchronous methodsFor better code clarity, consider specifying the return type for the
updateModule()method.Apply this diff:
- private async updateModule(id: ID, changes: Partial<IOrganizationProjectModule>) { + private async updateModule(id: ID, changes: Partial<IOrganizationProjectModule>): Promise<void> {
68-86: Handle potential undefineditemsin responseIn
loadModules(), ensure thatitemsis safely handled to prevent potential runtime errors.Apply this diff:
const { items = [] } = await firstValueFrom( this.organizationProjectModuleService.get<IOrganizationProjectModule>({ projectId: this.projectId }) ); - this.modules = items || []; + this.modules = items; this.smartTableSource.load(this.modules);
244-245: Refresh Smart Table after translation changesAfter reloading the Smart Table settings on language change, the table might not automatically refresh to display updated translations.
Consider refreshing the table source to apply the new settings:
this.loadSmartTable(); + this.smartTableSource.refresh();
173-186: Avoid reloading all modules after deletionIn the
delete()method, reloading all modules after deletion might be unnecessary. Consider removing the deleted module from the local array to optimize performance.Modify the
delete()method:await firstValueFrom(this.organizationProjectModuleService.delete(this.selectedItem.id)); this.toastrService.success('TOASTR.MESSAGE.MODULE_DELETED'); - await this.loadModules(); + this.modules = this.modules.filter(module => module.id !== this.selectedItem.id); + this.smartTableSource.load(this.modules);
15-17: Consistent initialization of class propertiesEnsure that all class properties are initialized in a consistent manner.
disableButtonis assigned astrue, whereasloadingis assigned astrueearlier.For clarity, group property initializations together.
loading = true; disableButton = true; + + modules: IOrganizationProjectModule[] = []; + selectedItem: IOrganizationProjectModule; + settingsSmartTable: object; + smartTableSource = new LocalDataSource();Also applies to: 49-50
176-177: Avoid early returns without bracesIn the
delete()method, using an early return without braces can reduce code readability.Apply this diff to add braces:
if (!result) return; + { // rest of the code + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
apps/gauzy/src/app/pages/projects/components/project-list/list.component.html(1 hunks)packages/ui-core/shared/src/lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts(3 hunks)packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html(1 hunks)packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.scss(1 hunks)packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.ts(1 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.html(2 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts(4 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.module.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.scss
🔇 Additional comments (7)
packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html (2)
3-7: Verify disable button state implementation
The button's disable state is properly bound, but let's verify its implementation in the component.
✅ Verification successful
Disable button state implementation is properly handled
The button's disable state is correctly implemented in the ProjectModuleTableComponent. The search results show that:
- The
disableButtonproperty is consistently updated based on selection state - The implementation follows the same pattern used across the application
- The disable state is toggled when a row is selected/deselected (
this.disableButton = !isSelected)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for disableButton property and its usage
ast-grep --pattern 'export class ProjectModuleTableComponent {
$$$
disableButton: boolean
$$$
}'
# Check for any state updates to disableButton
rg "this\.disableButton" -A 3
Length of output: 25447
1-46: Verify component integration and event handling
Let's verify the component's integration with parent components and proper event handling.
✅ Verification successful
Let me verify the component's integration and event handling based on the search results.
Component integration and event handling are properly implemented
The search results confirm that:
-
The component is properly integrated:
- It's declared in
ProjectMutationModuleand used inproject-mutation.component.html - Parent component correctly passes the required
projectIdinput
- It's declared in
-
Event handlers are properly implemented:
selectItemis used withuserRowSelectevent from smart tableonEditProjectModuleDialogis bound to edit button clickdeleteis bound to delete button clickdisableButtonproperty is properly bound to control button states
The component follows standard Angular patterns for event handling and parent-child communication, with no integration issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check component declaration and module imports
rg "ProjectModuleTableComponent" -A 5
# Verify parent component integration
ast-grep --pattern '<ngx-project-module-table
$$$
>'
# Check for event handlers implementation
rg "selectItem|onEditProjectModuleDialog|delete" -A 3
Length of output: 112972
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.module.ts (3)
8-8: LGTM! Import statements are well-organized.
The new imports follow Angular's style guide and maintain proper module organization.
Also applies to: 27-28
31-31: Consider exporting ProjectModuleTableComponent.
The component is declared but not exported. If this component needs to be used by other modules that import ProjectMutationModule, it should be added to the exports array.
Let's check if this component is used outside this module:
45-45: Verify SmartDataViewLayoutModule dependencies.
The addition of SmartDataViewLayoutModule might introduce new dependencies. Please ensure that all required dependencies for the smart table functionality are properly imported.
Let's check the module's dependencies:
Also applies to: 56-57
✅ Verification successful
SmartDataViewLayoutModule dependencies are properly configured
The module has all its required dependencies properly imported:
- Core UI dependencies from @nebular/theme (NbIconModule, NbSelectModule, NbToggleModule)
- Translation support via TranslateModule
- Smart table functionality via Angular2SmartTableModule
- Permission handling via NgxPermissionsModule
- Additional required modules: SharedModule, GauzyButtonActionModule, NoDataMessageModule
The module is widely used across the application and its dependencies are correctly configured in the project-mutation.module.ts file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check SmartDataViewLayoutModule's dependencies
ast-grep --pattern 'export class SmartDataViewLayoutModule {
$$$
}'
# Check if there are any required providers
rg -A 5 "providers.*SmartDataView"
Length of output: 108
Script:
#!/bin/bash
# Let's try to find the SmartDataViewLayoutModule file first
fd "smart.*layout.*module" --type f
# Then search for any imports in the project-mutation module
rg -A 2 "SmartDataViewLayoutModule" "packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.module.ts"
# Search for any smart data view layout related components and their dependencies
fd "smart.*layout" --type f
Length of output: 472
Script:
#!/bin/bash
# Let's examine the SmartDataViewLayoutModule content
cat "packages/ui-core/shared/src/lib/smart-data-layout/smart-data-view-layout.module.ts"
# Also check for any components or services it might be using
rg -l "SmartDataViewLayout" --type ts
Length of output: 5055
apps/gauzy/src/app/pages/projects/components/project-list/list.component.html (1)
92-92: LGTM! Improved UI consistency
The button status change from 'primary' to 'success' aligns with the application's design patterns, making it consistent with other creation/addition actions in the interface.
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts (1)
3-3: LGTM! Import statements and constructor injection are properly implemented.
The changes follow Angular's best practices for importing and injecting dependencies.
Also applies to: 49-50, 187-188
...-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html
Outdated
Show resolved
Hide resolved
.../lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts
Show resolved
Hide resolved
.../lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts
Outdated
Show resolved
Hide resolved
| <nb-tab [tabTitle]="'TASKS_PAGE.MODULE' | translate"> | ||
| <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table> | ||
| </nb-tab> |
There was a problem hiding this comment.
Add null check and consider conditional visibility for the Module tab
The project.id is passed directly to the child component without a null check, which could cause runtime errors if the project object is undefined (e.g., during project creation).
Consider applying this change:
- <nb-tab [tabTitle]="'TASKS_PAGE.MODULE' | translate">
- <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table>
- </nb-tab>
+ <nb-tab *ngIf="project?.id" [tabTitle]="'TASKS_PAGE.MODULE' | translate">
+ <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table>
+ </nb-tab>📝 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.
| <nb-tab [tabTitle]="'TASKS_PAGE.MODULE' | translate"> | |
| <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table> | |
| </nb-tab> | |
| <nb-tab *ngIf="project?.id" [tabTitle]="'TASKS_PAGE.MODULE' | translate"> | |
| <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table> | |
| </nb-tab> |
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.html
Outdated
Show resolved
Hide resolved
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts
Outdated
Show resolved
Hide resolved
...ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.ts
Show resolved
Hide resolved
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.html (2)
492-494: Consider using safe navigation operator for projectId bindingWhile the tab's visibility is properly guarded with
*ngIf, theprojectIdbinding could benefit from the safe navigation operator to prevent potential runtime errors.- <ngx-project-module-table [projectId]="project.id"></ngx-project-module-table> + <ngx-project-module-table [projectId]="project?.id"></ngx-project-module-table>
509-511: LGTM! Consider grouping with "Manage Sprints" buttonThe button implementation looks good:
- Proper conditional rendering
- Not tied to form validation
- Consistent styling with other action buttons
However, consider moving it next to the "Manage Sprints" button since both are project management actions that require an existing project.
- <button *ngIf="project?.id" class="mr-3" nbButton status="success" (click)="createProjectModuleDialog()"> - {{ 'BUTTONS.ADD_MODULE' | translate }} - </button> <ng-template [ngIf]="project"> + <button + *ngIf="project?.id" + nbButton + class="mr-3" + status="success" + (click)="createProjectModuleDialog()" + > + {{ 'BUTTONS.ADD_MODULE' | translate }} + </button> <button *ngIf="form.get('taskListType').value === TaskListTypeEnum.SPRINT" nbButton class="float-right" status="success" (click)="openTasksSettings()" > {{ 'BUTTONS.MANAGE_SPRINTS' | translate }} </button> </ng-template>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/ui-core/shared/src/lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts(3 hunks)packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html(1 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.html(2 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts(4 hunks)packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.module.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/ui-core/shared/src/lib/project/project-module/add-project-module-dialog/add-project-module-dialog.component.ts
- packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.html
- packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.component.ts
🔇 Additional comments (4)
packages/ui-core/shared/src/lib/project/project-mutation/project-mutation.module.ts (4)
8-8: LGTM! Nebular UI imports are properly organized.
The addition of NbIconModule and NbSpinnerModule is well-placed within the existing Nebular imports block.
Also applies to: 12-12
44-44: LGTM! Module imports are properly organized.
The new imports are well-organized:
- UI modules (
NbSpinnerModule,NbIconModule) are grouped with other Nebular imports - Feature modules (
RepositorySelectorModule,SmartDataViewLayoutModule) are placed at the end
Also applies to: 47-47, 58-59
28-29: Verify the component's location in the project structure.
The relative import path suggests the component is part of the project module feature. Let's verify its location and any potential circular dependencies.
✅ Verification successful
Component location verified and no circular dependencies detected
The component exists at the expected location packages/ui-core/shared/src/lib/project/project-module/module-table/project-module-table.component.ts, and the relative import path is correct. No circular dependencies were found as the component is only imported by the mutation module.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the component's location and potential circular dependencies
# Check if the component exists at the specified location
fd -t f "project-module-table.component.ts"
# Check for potential circular dependencies
rg -l "import.*ProjectMutationModule" | xargs rg "import.*ProjectModuleTableComponent"
Length of output: 427
32-33: Verify the intended usage scope of ProjectModuleTableComponent.
The component is declared but not exported, suggesting it's meant for internal use within this module. This aligns with the PR objective of displaying project modules within the project mutation component.
✅ Verification successful
ProjectModuleTableComponent's usage scope is correctly configured
The component is properly scoped as an internal component of the ProjectMutationModule:
- It's only imported and declared in project-mutation.module.ts
- Used exclusively in project-mutation.component's template
- Not exported, which aligns with its intended internal usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the component's usage is limited to this module
# Check where the component is being used
rg -l "ProjectModuleTableComponent" | grep -v "module-table"
Length of output: 142
Script:
#!/bin/bash
# Let's verify if ProjectModuleTableComponent is only used within project-mutation module files
rg "ProjectModuleTableComponent" -A 3 -B 3
Length of output: 2508
Script:
#!/bin/bash
# Let's check if ProjectModuleTableComponent is used in project-mutation.component's template
rg -t html "project-module-table|ProjectModuleTableComponent" packages/ui-core/shared/src/lib/project/project-mutation/
Length of output: 294
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Documentation