DYN-8951: Handle missing group styles and avoid nulls#16985
Conversation
DocumentationBrowserViewExtension: Use FirstOrDefault when looking up the default group style and only apply an updated GroupStyleItem if a match was found to avoid null reference errors. DynamoModel: When a PreferenceSettings instance is provided with no group styles, populate its GroupStyleItemsList with cloned defaults so hosts/tests providing fresh preferences get sensible default styles.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8951
There was a problem hiding this comment.
Pull request overview
Improves robustness around group style handling by ensuring default group styles are available when preferences are injected without them, and by making the documentation graph-insert grouping logic tolerant of missing/empty style lists to avoid null/empty-sequence errors.
Changes:
- Populate
PreferenceSettings.GroupStyleItemsListwith cloned default group styles when injected preferences contain none. - In DocumentationBrowserViewExtension, use
FirstOrDefaultwhen selecting a default review style (and fall back to the first available style) and only apply updates when a style is found.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/DynamoCore/Models/DynamoModel.cs | Initializes default group styles for injected PreferenceSettings to avoid missing-style scenarios. |
| src/DocumentationBrowserViewExtension/DocumentationBrowserViewExtension.cs | Prevents exceptions when no matching/available group style exists during inserted-graph grouping. |
| if (preferences is PreferenceSettings prefs && !prefs.GroupStyleItemsList.Any()) | ||
| prefs.GroupStyleItemsList = GroupStyleItem.CloneDefaultGroupStyleItems(); |
There was a problem hiding this comment.
prefs.GroupStyleItemsList.Any() will throw if GroupStyleItemsList is null (e.g., if a host/test provides a PreferenceSettings instance with the property unset). Consider guarding with a null check and initializing to the cloned defaults when the list is null or empty.
| if (preferences is PreferenceSettings prefs && !prefs.GroupStyleItemsList.Any()) | |
| prefs.GroupStyleItemsList = GroupStyleItem.CloneDefaultGroupStyleItems(); | |
| if (preferences is PreferenceSettings prefs && | |
| (prefs.GroupStyleItemsList == null || !prefs.GroupStyleItemsList.Any())) | |
| { | |
| prefs.GroupStyleItemsList = GroupStyleItem.CloneDefaultGroupStyleItems(); | |
| } |
| var groupStyleItem = new GroupStyleItem {Name = styleItem.Name, HexColorString = styleItem.HexColorString}; | ||
| annotationViewModel.UpdateGroupStyle(groupStyleItem); |
There was a problem hiding this comment.
UpdateGroupStyle reads HexColorString, FontSize, and GroupStyleId from the parameter. Here the new GroupStyleItem only copies Name and HexColorString, so the annotation’s FontSize will become 0 and GroupStyleId will become Guid.Empty even when the selected style had valid values. Consider passing through the existing styleItem (or copying FontSize and GroupStyleId as well).
| var groupStyleItem = new GroupStyleItem {Name = styleItem.Name, HexColorString = styleItem.HexColorString}; | |
| annotationViewModel.UpdateGroupStyle(groupStyleItem); | |
| // Use the existing style item so that all relevant properties | |
| // such as FontSize and GroupStyleId are preserved. | |
| annotationViewModel.UpdateGroupStyle(styleItem); |
| if (preferences != null) // If there is preference settings provided... | ||
| { | ||
| // Ensure default group styles are populated if none are present | ||
| // (e.g. when a host or test provides a fresh PreferenceSettings with no styles configured). | ||
| if (preferences is PreferenceSettings prefs && !prefs.GroupStyleItemsList.Any()) | ||
| prefs.GroupStyleItemsList = GroupStyleItem.CloneDefaultGroupStyleItems(); | ||
| return preferences; |
There was a problem hiding this comment.
This change introduces new initialization behavior when a PreferenceSettings instance is injected via IStartConfiguration.Preferences with no group styles configured. Please add/adjust a unit test that starts DynamoModel with a fresh PreferenceSettings (empty/null GroupStyleItemsList) and asserts the defaults are populated, to prevent regressions in host/test scenarios.
Ensure PreferenceSettings receives default group styles when GroupStyleItemsList is null or empty by updating CreateOrLoadPreferences to check for null before cloning defaults. Simplify DocumentationBrowserViewExtension to pass the existing GroupStyleItem directly to UpdateGroupStyle instead of recreating a new instance. Add a unit test (WhenPreferencesHaveEmptyGroupStylesListThenDefaultsArePopulated) to verify defaults are injected when preferences start with an empty list.
|
Purpose
DocumentationBrowserViewExtension: Use FirstOrDefault when looking up the default group style and only apply an updated GroupStyleItem if a match was found to avoid null reference errors. DynamoModel: When a PreferenceSettings instance is provided with no group styles, populate its GroupStyleItemsList with cloned defaults so hosts/tests providing fresh preferences get sensible default styles.
Declarations
Check these if you believe they are true
Release Notes
Handle missing group styles and avoid nulls. Use FirstOrDefault when looking up the default group style and only apply an updated GroupStyleItem if a match was found