[Fix] Fix and Improve Total Value Filter on Accounting Screen #8759
Conversation
WalkthroughThis PR replaces a single value input filter with a range filter for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RFilter as RangeFilterComponent
participant InvComp as InvoicesReceivedComponent
participant InvService as InvoiceService
participant DB as Database
User->>RFilter: Enter min/max values
RFilter->>InvComp: Emit updated range filter event
InvComp->>InvService: Call pagination with range filter {min, max}
InvService->>DB: Query invoices with range conditions
DB-->>InvService: Return filtered invoices
InvService-->>InvComp: Send filtered results
InvComp-->>User: Render updated invoice list
Possibly related issues
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ 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 (
|
|
View your CI Pipeline Execution ↗ for commit 9529748.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/ui-core/shared/src/lib/table-filters/range-filter.component.ts (3)
48-48: Remove empty lifecycle hook.The
ngOnChangesmethod is empty and unused. Remove it to improve code maintainability.- ngOnChanges(changes: SimpleChanges) {}
29-31: Remove unnecessary constructor.The constructor only calls super() and doesn't perform any other initialization.
- constructor() { - super(); - }🧰 Tools
🪛 Biome (1.9.4)
[error] 29-31: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
10-19: Enhance template accessibility.Add ARIA labels and role attributes to improve accessibility.
<div class="d-flex"> <input [formControl]="rangeControl.controls.min" class="form-control me-2" placeholder="Min" type="number" + aria-label="Minimum value" /> - <span>-</span> + <span aria-hidden="true">-</span> <input [formControl]="rangeControl.controls.max" class="form-control" placeholder="Max" type="number" + aria-label="Maximum value" /> </div>packages/core/src/lib/invoice/invoice.service.ts (1)
304-314: Add type safety and validation for range values.While the implementation is correct, consider these improvements:
- Add type safety for the filter parameters
- Validate that min is not greater than max when both are provided
- if ('totalValue' in where) { - const { min, max } = where.totalValue; + if ('totalValue' in where && where.totalValue) { + const { min, max } = where.totalValue as { min?: number; max?: number }; + + if (min !== undefined && max !== undefined && min > max) { + throw new BadRequestException('Minimum value cannot be greater than maximum value'); + } if (min !== undefined && max !== undefined) { filter.where.totalValue = Between(min, max); } else if (min !== undefined) { filter.where.totalValue = MoreThanOrEqual(min); } else if (max !== undefined) { filter.where.totalValue = LessThanOrEqual(max); } }apps/gauzy/src/app/pages/invoices/invoices-received/invoices-received.component.ts (1)
389-396: Add type safety and validation to the range filter.While the range filter implementation is good, it could benefit from additional type safety and validation.
Consider applying these improvements:
-filterFunction: (range) => { - const { min, max } = range; - this.setFilter({ - field: 'totalValue', - search: { min, max } - }); +filterFunction: (range: { min?: number; max?: number }) => { + const { min, max } = range; + // Validate range values + if ((min !== undefined && min < 0) || (max !== undefined && max < 0)) { + console.warn('Invalid range values: Values cannot be negative'); + return; + } + if (min !== undefined && max !== undefined && min > max) { + console.warn('Invalid range values: Minimum value cannot be greater than maximum value'); + return; + } + this.setFilter({ + field: 'totalValue', + search: { min, max } + }); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/gauzy/src/app/pages/invoices/invoices-received/invoices-received.component.ts(3 hunks)packages/core/src/lib/invoice/invoice.controller.ts(0 hunks)packages/core/src/lib/invoice/invoice.service.ts(2 hunks)packages/ui-core/shared/src/lib/table-filters/index.ts(1 hunks)packages/ui-core/shared/src/lib/table-filters/range-filter.component.ts(1 hunks)packages/ui-core/shared/src/lib/table-filters/table-filters.module.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/lib/invoice/invoice.controller.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/ui-core/shared/src/lib/table-filters/range-filter.component.ts
[error] 29-31: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (4)
packages/ui-core/shared/src/lib/table-filters/index.ts (1)
6-7: LGTM!The new export follows the existing pattern and maintains consistency with other exports.
packages/ui-core/shared/src/lib/table-filters/range-filter.component.ts (1)
33-46: LGTM on subscription handling!The subscription cleanup in ngOnDestroy is properly implemented, preventing memory leaks. The use of RxJS operators (debounceTime, distinctUntilChanged) is also a good practice for optimizing filter updates.
packages/ui-core/shared/src/lib/table-filters/table-filters.module.ts (1)
22-22: LGTM!The RangeFilterComponent is properly imported and declared in the module.
Also applies to: 46-46
apps/gauzy/src/app/pages/invoices/invoices-received/invoices-received.component.ts (1)
36-36: LGTM!Clean import of the
RangeFilterComponentwithin the existing imports block.
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
New Features
Refactor