feat(EditorForm): support null model #6914
Merged
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
Reviewer's GuideThis PR enhances EditorForm to gracefully handle a null Model by wrapping its rendering logic (fields, groups, and buttons) in a null check, and adjusts the unit test to expect an empty form container rather than an exception. Class diagram for updated EditorForm null model handlingclassDiagram
class EditorForm {
+object? Model
+IEnumerable<GroupItem> GroupItems
+IEnumerable<GroupItem> UnsetGroupItems
+Func<object?, RenderFragment>? FieldItems
+RenderFragment? Buttons
+bool ShowUnsetGroupItemsOnTop
+IDictionary<string, object>? AdditionalAttributes
+string ClassString
+_editorItems
+RenderGroupItems(GroupItem)
+RenderUnsetGroupItems
}
EditorForm : BootstrapComponentBase
EditorForm --> "0..1" Model
EditorForm --> "*" GroupItems
EditorForm --> "*" UnsetGroupItems
EditorForm --> "0..1" FieldItems
EditorForm --> "0..1" Buttons
EditorForm --> "0..1" AdditionalAttributes
EditorForm --> "1" ClassString
EditorForm --> "1" _editorItems
EditorForm --> "1" RenderGroupItems
EditorForm --> "1" RenderUnsetGroupItems
Flow diagram for EditorForm rendering with null model supportflowchart TD
A["Start rendering EditorForm"] --> B{"Is Model null?"}
B -- Yes --> C["Render empty form container"]
B -- No --> D["Render FieldItems with Model"]
D --> E["Render form body"]
E --> F{"ShowUnsetGroupItemsOnTop?"}
F -- Yes --> G["If UnsetGroupItems.Any() then RenderUnsetGroupItems"]
G --> H["For each GroupItem, RenderGroupItems"]
F -- No --> I["For each GroupItem, RenderGroupItems"]
I --> J["If UnsetGroupItems.Any() then RenderUnsetGroupItems"]
E --> K{"Buttons not null?"}
K -- Yes --> L["Render Buttons in footer"]
K -- No --> M["Do not render footer"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds null-checking support for the Model property in the EditorForm component to prevent potential null reference exceptions. The change wraps the entire component content in a conditional block that only renders when Model is not null.
Key Changes
- Added null check wrapper around EditorForm component content to handle null Model scenarios
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6914 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 739 739
Lines 31773 31776 +3
Branches 4467 4468 +1
=========================================
+ Hits 31773 31776 +3
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:
|
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The test named Model_Error should be renamed to reflect that a null model now renders an empty form instead of throwing.
- Consider extracting the repeating RenderGroupItems/RenderUnsetGroupItems logic into a helper to flatten the nested branches and reduce duplication.
- Clarify whether the editor footer buttons should still render when Model is null, since they’re currently skipped inside the null check.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test named Model_Error should be renamed to reflect that a null model now renders an empty form instead of throwing.
- Consider extracting the repeating RenderGroupItems/RenderUnsetGroupItems logic into a helper to flatten the nested branches and reduce duplication.
- Clarify whether the editor footer buttons should still render when Model is null, since they’re currently skipped inside the null check.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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 #6913
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Make EditorForm support null Model by conditionally rendering only when Model is non-null and update associated tests accordingly.
Bug Fixes:
Tests: