Skip to content

Commit 69dff49

Browse files
committed
Merge branch 'bugfix/HierarchicalTopicMappingService-model-validation' into develop
The `HierarchicalTopicMappingService`'s `GetRootViewModelAsync()` and `GetViewModelAsync()` methods accept a `validationDelegate' for determining if each individual topic should be added to the mapped hierarchy. Except that's not how it was actually working. Due to a bug in where the `validationDelegate` was called, it was effectively validating the parent topic—but only after that parent topic had already been mapped. This still allowed it to bypass mapping all children, but prevented it from being used to prune individual children based on their individual attributes, which is the far more intuitive and expected behavior—and also the behavior indicated by the documentation. This fixes that bug, and also updates the logic of the one implementation which specifies a `validationDelegate` to ensure it's compatible with the new placement. (Fortunately, we have visibility into all other implementations, and `validationDelegate` isn't otherwise called; as such, we can be confident this won't break other implementations that worked around this bug by accommodating its incorrect behavior.) While I was at it, I also updated `MenuViewComponentBase<T>` to filter out `List` content types—something traditionally done in the view—as there's no expected scenario where we'd want nested topic lists to show up in the main navigation, since a) they're not routable by `TopicController`, and b) intended to represent internal page content, not part of the site hierarchy.
2 parents d63f2b5 + f7fc6ff commit 69dff49

2 files changed

Lines changed: 3 additions & 3 deletions

File tree

OnTopic.AspNetCore.Mvc/Components/MenuViewComponentBase{T}.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ IHierarchicalTopicMappingService<T> hierarchicalTopicMappingService
102102
await HierarchicalTopicMappingService.GetRootViewModelAsync(
103103
navigationRootTopic!,
104104
3,
105-
t => t.ContentType is not "PageGroup"
105+
t => t is not { ContentType: "List" } and not { Parent: { ContentType: "PageGroup" } }
106106
).ConfigureAwait(false);
107107

108108
/*==========================================================================================================================

OnTopic/Mapping/Hierarchical/HierarchicalTopicMappingService{T}.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ private static int DistanceFromRoot(Topic? sourceTopic) {
189189
/*------------------------------------------------------------------------------------------------------------------------
190190
| Request mapping of children
191191
\-----------------------------------------------------------------------------------------------------------------------*/
192-
if (tiers >= 0 && validationDelegate(sourceTopic) && viewModel.Children.Count == 0) {
193-
foreach (var topic in sourceTopic.Children.Where(t => t.IsVisible())) {
192+
if (tiers >= 0 && viewModel.Children.Count == 0) {
193+
foreach (var topic in sourceTopic.Children.Where(t => t.IsVisible() && validationDelegate(t))) {
194194
taskQueue.Add(GetViewModelAsync(topic, tiers, validationDelegate));
195195
}
196196
}

0 commit comments

Comments
 (0)