Add AdminNavigationProvider to cleanup AdminMenu

[MikeAlhayek]

Instead of the NavigationHelper.IsAdminMenu helper that was introduced in this version, AdminNavigationProvider allows us to cleanup our code and build the navigation with less and cleaner code.

Since we already introduced a breaking change in 2.0.0 for INavigationProvider, I think this PR should be part of 2.0.0 this way people can modify their Admin Menu once and address all the improvements at once.

[hishamco]

Can we hold this after 2.0.0 we don't want to introduce any bugs accidentally, every PR in this stage is critical

[MikeAlhayek]

I am hoping for 2.0.0 because we did lots of enhancements to the AdminMenu including introducing the NavigationHelper.IsAdminMenu Helper which this PR removes. Plus since people have to already touch every AdminMenu due to the interface braking change, it's a good time for people to make this change once and for all.

[hishamco]

I understand but I have many things to do before 2.0.0, but last week you are telling that we needed to publish the release on TUE or THUR for that I deferred everything after even the PO Extractor Tool update

I suggest organizing the release period every time giving some time for test and integrating the newly added functionality and rejecting any modification in the preview unless it's critical. FYI I already saw such thing in many OSS including ASP.NET Core itself

[MikeAlhayek]

I deferred everything after even the PO Extractor Tool update

Nothing related to translation was changed here. So it should not impact anything you have done.

But I understand your concern. My concern is to reduce the amount of work needed that is going to be needed during upgrade. In this case, I think this PR is fair and will make things easy. Maybe instead of debating, checkout the PR and test it out?

[hishamco]

Maybe instead of debating, checkout the PR and test it out?

To be honest I have many things in PO Extractor Tool & Orchard Core Contrib. I told you PRs in the release time need time for testing & integration

[sebastienros]

OK with me on these conditions, and it's fine if you can find someone else to OK it without my conditions, you pick which approval you need ;)

• Rename NavigationProviderBase to NamedNavigationProvider such that it only adds the verification of the name, and there is a single async method to implement protected virtual ValueTask BuildNavigationAsync(NavigationBuilder builder). This class also has a single constructor with a string name such that concrete implementations have to call into base(name). The Name property is protected { get; } only.
• Rename AdminNavigationProvider to AdminMenuNavigationProvider, inheriting from NamedNavigationProvider.

[hishamco]

I already something quite similar to Seb suggestion in Orchard Core Contrib long time ago, I will check the PR very soon, but we need to fix the build issue first

[hishamco]

Why the functional tests are skipped?

[MikeAlhayek]

@hishamco they are triggered after approval.

[sebastienros]

they are triggered after approval.

I like this 💯