bug(LookupFilter): use first item as default value when value is null #6338
+126
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideThis PR implements default selection of the first lookup item when the filter value is null by converting the parameter setup to asynchronous in LookupFilter and assigning the first returned item, exposes this behavior via a new IsUseActiveWhenValueIsNull parameter in Select, extends unit tests to validate these scenarios with mock services, and refactors the FilterBase property placement. Sequence diagram for default value selection in LookupFiltersequenceDiagram
participant LookupFilter
participant TableColumnFilter
participant ILookupService
participant Items
LookupFilter->>TableColumnFilter: Access LookupService
alt _value is null or empty
LookupFilter->>ILookupService: GetItemsAsync(key, data)
ILookupService-->>LookupFilter: Return items
LookupFilter->>Items: Get first item
LookupFilter->>LookupFilter: Set _value to first item's Value
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- The new LookupService_Empty test doesn’t include any assertions—add checks to verify that no default value is set when the service returns an empty list.
- You introduced the IsUseActiveWhenValueIsNull parameter on Select but never use it in LookupFilter—consider gating the default‐value logic behind that flag.
- Overriding OnParametersSetAsync and awaiting item loading could trigger extra renders—review whether this default‐value logic belongs in OnInitializedAsync or should only run once to avoid performance issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new LookupService_Empty test doesn’t include any assertions—add checks to verify that no default value is set when the service returns an empty list.
- You introduced the IsUseActiveWhenValueIsNull parameter on Select but never use it in LookupFilter—consider gating the default‐value logic behind that flag.
- Overriding OnParametersSetAsync and awaiting item loading could trigger extra renders—review whether this default‐value logic belongs in OnInitializedAsync or should only run once to avoid performance issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6338 +/- ##
===========================================
- Coverage 100.00% 99.99% -0.01%
===========================================
Files 717 717
Lines 31561 31573 +12
Branches 4457 4461 +4
===========================================
+ Hits 31561 31572 +11
- Partials 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #6337
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Fix LookupFilter to default to the first lookup item when value is null, convert parameter initialization to async, add unit tests for lookup service scenarios, and update related docs and refactor FilterBase.
Bug Fixes:
Enhancements:
Documentation:
Tests: