Make properties nullable in filter objects #76
Conversation
| return this.ApplyFilterTo(ordered); | ||
|
|
||
| return base.ApplyFilterTo(query).ToPaged(Page, PerPage); | ||
| return base.ApplyFilterTo(query).ToPaged(Page ?? 1, PerPage ?? 10); |
There was a problem hiding this comment.
When Page or PrePage is null, consider skipping pagination.
There was a problem hiding this comment.
This should not be modified, it is required to pass in paging parameters here. What's your opinion.
There was a problem hiding this comment.
We have ApplyFilterWithoutPagination() method to ignore pagination. It's not client-controlled since skipping pagination may cause big performance problems and a bottleneck in the main application. When required, calling query.ApplyFilterWithoutPagination(filter) method in the code is a good approach. Makes the developer know about no-filtering instead of being managed by the client. But it depends on case, but this one is more common
There was a problem hiding this comment.
Pull Request Overview
This PR makes the filter properties nullable to allow for better handling of missing filter criteria.
- Changed string filter properties to be nullable in StringFilter.
- Modified pagination properties to be nullable in PaginationFilterBase and IPaginationFilter with proper defaulting.
- Updated ordering properties to be nullable in OrderableBase and IOrderable.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/AutoFilterer/Types/StringFilter.cs | Made string properties nullable |
| src/AutoFilterer/Types/PaginationFilterBase.cs | Updated pagination properties to be nullable and added null coalescing for default values |
| src/AutoFilterer/Types/OrderableBase.cs | Changed Sort property to be nullable |
| src/AutoFilterer/Abstractions/IPaginationFilter.cs | Updated pagination interface with nullable ints |
| src/AutoFilterer/Abstractions/IOrderable.cs | Updated ordering interface with nullable string |
Comments suppressed due to low confidence (1)
src/AutoFilterer/Types/StringFilter.cs:28
- Using 'Equals' as a property name can cause confusion with the Object.Equals method. Consider renaming it to a more descriptive name (e.g., 'EqualTo').
public virtual new string? Equals { get; set; }
Resolves #75
All tests are passed but I'm not sure if it's a breaking-change or not. I'll investigate the progress with preview release