First code submission for MVC Pipeline#6749
First code submission for MVC Pipeline#6749donker wants to merge 252 commits intofeature/mvc-pipelinefrom
Conversation
…tomizations for the MVC pipeline.
Fix layout issue
client dependecy for mvc (because it not exist in nuget)
Html module
…auwaen/Dnn.Platform into feature/mvc-pipeline-old
# Conflicts: # DNN Platform/DotNetNuke.Web.Mvc/Framework/Controllers/DnnController.cs # DNN Platform/DotNetNuke.Web.Mvc/Framework/Modules/ResultCapturingActionInvoker.cs # DNN Platform/DotNetNuke.Web.Mvc/Routing/MvcRoutingManager.cs # DNN Platform/Library/Entities/Portals/PortalSettings.cs # DNN Platform/Library/Entities/Portals/PortalSettingsController.cs # DNN Platform/Library/Entities/Urls/AdvancedUrlRewriter.cs # DNN Platform/Library/UI/Modules/Html5/Html5HostControl.cs # DNN Platform/Library/UI/Modules/Html5/Html5ModuleControlFactory.cs # DNN Platform/Modules/DDRMenu/Common/DNNContext.cs # DNN Platform/Modules/DDRMenu/Localisation/Localiser.cs # DNN Platform/Modules/DDRMenu/MenuBase.cs # DNN Platform/Modules/DDRMenu/TemplateEngine/TemplateDefinition.cs # DNN Platform/Modules/HTML/Components/HtmlTextController.cs # DNN Platform/Providers/HtmlEditorProviders/DNNConnect.CKE/Web/EditorControl.cs # Dnn.AdminExperience/Dnn.PersonaBar.Extensions/admin/personaBar/Dnn.SiteSettings/App_LocalResources/SiteSettings.resx
|
I’d like to check the status of this PR. There hasn’t been any review activity for several months—are there any issues or blockers? How can I help move things forward? It might also be worth bringing this up during a TAG meeting ? I completely understand that reviewing this PR is a significant effort. At the same time, on the otherhand inoticing quite a few major changes being made elsewhere, which also require substantial merge work on my side. I believe it’s generally more efficient to process PRs in order (except for bug fixes, of course). Otherwise, each new PR tends to create additional merge overhead for those still pending. |
I wish this PR was a custom package that could be installed and uninstalled like a plugin. |
DNN Platform/DotNetNuke.Web.Mvc/StandardTabAndModuleInfoProvider.cs
Outdated
Show resolved
Hide resolved
DNN Platform/DotNetNuke.Web.MvcPipeline/Commons/PropertyHelper.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Base controller for DNN MVC page controllers, exposing common services and portal context. | ||
| /// </summary> | ||
| public abstract class DnnPageController : Controller, IMvcController |
There was a problem hiding this comment.
Similar question here for the fate of the existing IDNNController and related items, as I can see a lot of confusion without some level of guidance being provided at a programming perspective with different behaviors and purposes. For example this ONLY provides PortalSettings the legacy one provides much more rich context (User, etc)
There was a problem hiding this comment.
This one is indended to work in mvc pipeline, real mvc. IDNNController is for webforms , have a reference to a Page. Dous not use standard mvc. Use hacks in mvc to write the output to a string and generate module actions in a way that is not compatible with real mvc.
There was a problem hiding this comment.
@donker @valadas @bdukes @david-poindexter given the above comment from @sachatrauwaen do we need to think about the addition of an Obsolete or other attribute on the others? I'm worried about multiple items that do similar things and those not being aware of what exactly this is, or this important difference
There was a problem hiding this comment.
Are you suggesting that we mark the current MVC pattern as obsolete?
There was a problem hiding this comment.
Maybe? I'm worried that with multiple ways of doing something, and/or without doing so, it is going to create even more confusion? Maybe it's too early for that?
There was a problem hiding this comment.
Yeah, I think it's too early for that. We're just going to have to live with two MVC patterns side-by-side for a while.
There was a problem hiding this comment.
In the mvc pipeline there is a way to run actual mvc modules as child controllers. But with some limitations. And when looking at the way actuel mvc modules are implemented is i think the only way that was possible in mvc 3. It's really bad hacking witch bring a lot of restructions to the real MS MVC. So opinion, is that depreciating the actual mvc modules is not a bad thing. Also because this patern can not work in dotnet core (because in use child controllers witch is not available in dotnet core).
Depreciating the actual mvc modules, can also make this PR smalller.
| /// Builds an edit URL for the current module instance. | ||
| /// </summary> | ||
| /// <returns>The edit URL.</returns> | ||
| public string EditUrl() |
There was a problem hiding this comment.
I echo @valadas comments on this duplication of exploding items that are already there, the long-term maintenance of this, as we have seen with other changes, is not exactly "fun". I would prefer exposing the ModuleContext and leaving it at that personally
There was a problem hiding this comment.
This a base controller is specifically for module controls implemented as a child controller. But is in reality not realy the recommended way because removed by MS in dotnet core. Maybe better to remove it ?
There was a problem hiding this comment.
I think in many ways, yes, it would be a "bigger lift" for somone converting an existing module, but at the same time, very easy document using ModuleContext
| .Register(); | ||
|
|
||
| /* | ||
| if (Host.CdnEnabled && !string.IsNullOrEmpty(jsl.ObjectName)) |
There was a problem hiding this comment.
This commented out code should be removed
There was a problem hiding this comment.
this commended code, use DnnJsIncludeFallback with is actually not implemented in the mvc pipeline. Do you think this have some sense this days ? Is it needed to be implement in the mvc pipeline ?
There was a problem hiding this comment.
This may have already been lost with @donker's CDF revamp.
We look at it, but not possible do the the use of lot of code that is not exposed for external project and need some UI changes in the core |
| /// <summary> | ||
| /// Gets the current portal settings. | ||
| /// </summary> | ||
| public PortalSettings PortalSettings |
There was a problem hiding this comment.
I would prefer that we only expose IPortalSettings (i.e. via the interface)
There was a problem hiding this comment.
I have tryed, but endup in a nogo, because ActiveTab was not exposed thue IPortalSettings and found no way to get ActiveTab an oter way. Or do you know a way to get ActiveTab without PortalSettings ?
There was a problem hiding this comment.
TabController.CurrentPage is what I've been using. We want to have a better way to get that context, but it hasn't been fully designed yet.
6 participants
Core Purpose
Introduces a new ASP.NET MVC-based rendering pipeline as an alternative to the traditional WebForms pipeline, allowing DNN to modernize its architecture while maintaining backward compatibility.
Key Features
Dual Pipeline Architecture:
Video showcase the general usage
Module Support:
More info about MVC Module Control Implementation
Razor+ Module Development Guide
How to compile Samples
Configuration:
More info about MVC pipeline (routing / settings)
New MVC Pipeline Projects
DotNetNuke.Web.MvcPipeline.csproj - New core MVC pipeline library
Razor Module Sample
NewDDRMenu
Core Platform Projects
DotNetNuke.Library
DotNetNuke.Abstractions
DotNetNuke.Web.Mvc
HTML
Dnn.PersonaBar.UI
UI for pipeline settings in Site and Page Settings
DNNConnect.CKE
Skins/Aperture
Skin conversion guide
Website
AdminExperiance
This PR is a first submission of the work of the MVC Pipeline team to bring the main project up to date with what has been developed in a forked repo.
Documentation
MVC Module Control Implementation
MVC pipeline (routing / settings)
Razor Module Development Guide
How to compile Samples
DNN MVC Pipeline - Developer User Guide