Conversation
* fix: UI Issue in Option Translations Modal * feedback integration
…lates) (#8880) * fix(accounting-template): resolve duplicate templates and incorrect language filtering * Remove console log * feedback integration * Remove duplicated comment
* fix:Inventory Filters * fix: remove filter by name * feedback Integration
…o Assign Managers/Members in Project Creation (#8836) * feat: allows employees to select others * chore: add translations for SELECT_EMPLOYEE permission * Fix: retrieved employees * Integration of feedback * Fix: remove CHANGE_SELECTED_EMPLOYEE permission * Fix: Ensure both CHANGE_SELECTED_EMPLOYEE and SELECT_EMPLOYEE permissions are applied --------- Co-authored-by: mbabhaziSamuel <mbabhazisamuel@gmail.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces significant changes to employee permissions and product filtering functionality across the Ever Gauzy platform, with extensive internationalization updates.
Key changes:
- Added new
SELECT_EMPLOYEEpermission alongsideCHANGE_SELECTED_EMPLOYEEin core services and controllers - Implemented product category and type filtering components with conditional label rendering
- Added translations for
SELECT_EMPLOYEEacross 15+ language files - Enhanced
AccountingTemplateServicewith newfindOneByIdStringmethod and language filtering - Modified employee filtering logic in
EmployeeServiceto support both employee selection permissions
The changes appear well-structured but there are some potential issues:
- Empty
ngOnChangeshooks in new filter components need implementation or removal - TableFiltersModule has empty exports array, preventing component access
- Some permission checks could be more robust for null/undefined cases
💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
33 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings | Greptile
| async findOneByIdString(id: string): Promise<AccountingTemplate> { | ||
| const tenantId = RequestContext.currentTenantId(); | ||
|
|
||
| const query = this.typeOrmRepository.createQueryBuilder('template'); | ||
|
|
||
| query.where((qb: SelectQueryBuilder<AccountingTemplate>) => { | ||
| qb.andWhere( | ||
| new Brackets((bck: WhereExpressionBuilder) => { | ||
| bck.andWhere(p(`"${qb.alias}"."id" = :id`), { id }); | ||
| bck.andWhere(p(`"${qb.alias}"."tenantId" = :tenantId`), { tenantId }); | ||
| }) | ||
| ); | ||
|
|
||
| qb.orWhere( | ||
| new Brackets((bck: WhereExpressionBuilder) => { | ||
| bck.andWhere(p(`"${qb.alias}"."id" = :id`), { id }); | ||
| bck.andWhere(p(`"${qb.alias}"."tenantId" IS NULL`)); | ||
| }) | ||
| ); | ||
| }); | ||
|
|
||
| return await query.getOne(); |
There was a problem hiding this comment.
logic: Missing organization scope check in findOneByIdString - should match getAccountTemplate's fallback pattern
| <label class="label"> | ||
| {{ 'INVENTORY_PAGE.PRODUCT_CATEGORY' | translate }} | ||
| <label class="label" *ngIf="label !== ''"> | ||
| {{ label || 'INVENTORY_PAGE.PRODUCT_CATEGORY' | translate }} |
There was a problem hiding this comment.
logic: Using || operator here could cause issues if label is false or 0, should use nullish coalescing (??) instead
| <span class="language-label">{{ ln.value }}</span> | ||
| <input | ||
| fullWidth | ||
| id="groupNameInput_{{ln.value}}" |
There was a problem hiding this comment.
logic: ID 'groupNameInput_{{ln.value}}' is duplicated from line 27 - should be unique for options, e.g. 'optionNameInput_{{ln.value}}'
| id="groupNameInput_{{ln.value}}" | |
| id="optionNameInput_{{ln.value}}" |
| padding-left: 3.5rem; | ||
|
|
||
|
|
||
| &[nbInput] { | ||
| padding-left: 0.5rem; | ||
| } |
There was a problem hiding this comment.
style: Inconsistent padding-left between default input (3.5rem) and nbInput (0.5rem) could cause alignment issues. Consider standardizing the padding values.
| border: none !important; | ||
| border-radius: 0 !important; | ||
| box-shadow: none !important; | ||
| } |
There was a problem hiding this comment.
style: Multiple !important declarations should be avoided. Consider restructuring the CSS specificity instead of forcing overrides.
| <label class="label" *ngIf="label !== ''"> | ||
| {{ label || 'INVENTORY_PAGE.PRODUCT_TYPE' | translate }} |
There was a problem hiding this comment.
logic: Empty string check will not catch undefined/null values for label property. Consider using a more robust check like label?.trim() to handle all falsy cases.
| export * from './product-category-filter.component'; | ||
| export * from './product-type-filter.component'; |
There was a problem hiding this comment.
style: Consider maintaining alphabetical order by moving these exports between lines 5-6 to group with other product-related filters
| * | ||
| * @param changes | ||
| */ | ||
| ngOnChanges(changes: SimpleChanges) {} |
There was a problem hiding this comment.
logic: Empty lifecycle hook implementation. Either implement the hook or remove it if not needed.
| ngOnChanges(changes: SimpleChanges) {} |
| * | ||
| * @param changes | ||
| */ | ||
| ngOnChanges(changes: SimpleChanges) {} |
There was a problem hiding this comment.
style: Empty ngOnChanges method implementation - should be removed if not being used
| ngOnChanges(changes: SimpleChanges) {} |
| /** | ||
| * | ||
| * @param changes | ||
| */ |
There was a problem hiding this comment.
style: Empty JSDoc comments should either be removed or filled with meaningful documentation
| /** | |
| * | |
| * @param changes | |
| */ |
|
View your CI Pipeline Execution ↗ for commit 9b8a443.
☁️ Nx Cloud last updated this comment at |
3 participants
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.