Dnn.platform: TabInfo HasChildren falsely reports True with former child page(s) in the Recycle Bin

[jeremy-farrance]

Description of bug

When using the property TabInfo .HasChildren from code, if the page has a page or pages in the recycle bin that were previously children of that page, the HasChildren property will still be true.

Steps to reproduce

List the steps to reproduce the behavior

1. Go to Content / Pages
2. Add a page or two to an existing page
3. Delete the new pages so they are in the recycle bin
4. Add a quick RazorHost module to the page, run this script:

<p>Dnn.Tab.HasChildren = @Dnn.Tab.HasChildren</p>

Current result

HasChildren = True

Expected result

HasChildren = False

DNN Version
[x] 9.3.2

Affected browser

All

[sleupold]

we may need to be very careful where this is used. if e.g. a check for children is done after moving it within the navigation tree or whether a duplicate name check might need to be performed when creating a sub page, "true" would be appropriate, while when building up a menu, this should be false.

[jeremy-farrance]

we may need to be very careful where this is used...

My case (a DDRMenu using cshtml and custom sidemenu using RazorHost), both are in navigation and used to decide dropdown and expando menu stuff.

[sleupold]

@Accuraty I understand your use case, but we need to investigate, where this property is used in platform and how it is interpreted.

[mitchelsellers]

Sadly, I believe the current behavior is in fact correct. There are children tabs under the parent, it just so happens that the tab is deleted, however, it is a soft delete.

There are other situations in menu templates for example where the "HasChildren" property isn't exactly proper either. For example, if all children have been hidden from the menu HasChildren would also be true, however, it would not have any items to include in a menu, etc.

Looking at your desired behavior, there is a real thought of a "HasVisibleChildren" property that would be helpful for menu construction.

[sleupold]

I agree with @mitchelsellers, the best option would be a "hasActiveChildren", which checks isdeleted, display dates and workflow states as well.

[valadas]

Yep, that makes sense to me.

[jeremy-farrance]

It sounds like HasChildren can survive as is and what we need is 1 or more overrides that let the caller specifically request how the result should be determined. Maybe just pass in a few Booleans to trigger the desired logic? In summary, we should be able to get a result that matches what the currently logged in user should see (respect permissions, IsDeleted, display dates, etc.), What other options might be needed for Admin, UI, etc? An option to trigger 'ignore-the-cache' might be important? Is there any chance this has already been tackled somewher? Maybe the DDRMenu code's nodes deliver a different logic for the results of HasChildren?

[valadas]

We need to also do performance testing on this, I remember there was some performance issues, not sure on the details but would be good to check we don't make it worse or that we actually improve it.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jeremy-farrance]

Stale? This is still a real issue and needs to be addressed by use case.

[valadas]

@Accuraty Stalebot is an utility to help move issues along, so if an issue has no activity for a long time, it warns subscribers about it and if no further comment is posted, it closes it after some more time. It keeps the ball rolling, any comment removes it's stale status like it did here.

[valadas]

Thinking about this, #2925 added an overload to include deleted children or not, I think that solves this issue right?

[jeremy-farrance]

Thinking about this, #2925 added an overload to include deleted children or not, I think that solves this issue right?

Yes, I do believe that will cover my needs - that prompted the initial issue. However, in reading above, I think some additional interesting point (edge-cases) were brought up?

[valadas]

So the question is, does anybody intend to submit a PR with more or is the solution in #2925 enough and we close this issue?

[valadas]

I am going to assume #2925 is a good enough solution, if not, feel free to comment