dnnsoftware / Dnn.Platform

DNN (formerly DotNetNuke) is the leading open source web content management platform (CMS) in the Microsoft ecosystem.
https://dnncommunity.org/
MIT License
1.02k stars 745 forks source link

MVC Modules Do Not Support Areas #2166

Open SkyeHoefling opened 6 years ago

SkyeHoefling commented 6 years ago

Description

The current MVC Module Platform does not support asp.net MVC Areas which is a pretty big feature in the asp.net MVC space. Adding support for MVC Areas will allow developers to better organize their code and have better control over routing.

What is an Area?

An Area is a pre-defined route that groups a section of an MVC application. Suppose you are working on a DNN Module and there are 2 main pieces to it:

Instead of putting all of the controllers in the same folder you could group it into 2 Areas that map to the sections listed above.

In a native asp.net MVC project your routes may look like this:

Area Rroute
Admin https://localhost/admin/foo
Public https://localhost/public/foo
No Area https://localhost/foo

DNN MVC Routing Background

DNN currently uses key value pair matching in the url route to define the Controller and the Action in the MVC pattern. The default route is sometimes hidden but is clearly defined in the manifest file. Given the following controller/action:

Key Value
Controller Home
Action Index

You will get one of the 2 following routes:

Type Route
No Module Id https://localhost/some-dnn-page/
Module Id https://localhost/some-dnn-page/moduleId/1234/controller/Home/action/Index/

As you can see in the routes the controller and action are clearly stated.

The Problem

Since the DNN MVC Module Pattern relies on the Key Value Pair mapping in the routes for controllers and actions we should be able to extend this to Area's where applicable. Using our example from the beginning of this issue suppose with the following Areas:

Each area may have a controller and action of:

Key Value
Controller Home
Action Index

Currently this pattern would cause issues in the current DNN MVC implementation because it would not know how to route the same Controller/Action into the different Areas. Essentially we would have the following route:

That would mean 2 different controllers with 2 different sets of logic.

The Usage (Adding Areas to DNN MVC)

If we add a 3rd Key Value Pair that defines Areas this could solve the problem and extend the pattern to support more organized MVC code-bases. Suppose we have the following Key Value Pairs as part of the route:

With this new addition into the pattern we could easily have the module pattern check for an area and get the controller from that area. If the route does not provide an area it would just use the default controller folder. This would support the following routes with the exact same controller name & action:

Type Route
Admin Area https://localhost/some-dnn-page/moduleId/1234/area/Admin/controller/Home/action/Index
Public Area https://localhost/some-dnn-page/moduleId/1234/area/Public/controller/Home/action/Index
No Area https://localhost/some-dnn--page/moduleId/1234/controller/Home/action/Index

In this case here, we have 3 separate controllers/actions with the exact same same but different logic since they map to different controllers/methods/views. But since the route is handling the area correctly the module can easily define them separate and access them using the area.

Considerations

This is a pretty big change to implement because it effects the entire DNN MVC module pattern. We really should consider some other parts to the module pattern before going ahead with the change.

Manifest File

Currently the manifest file for DNN MVC works off of convention.

Given the following parameters

Name Value
Namespace Some.Module
Controller Home
Action Index

our manifest file route may look like this:

If we add a Public Area to the route it should look like this:

If there is no area defined it will just work like it currently does.

Url Routing and Redirect Routing

A powerful feature of MVC is the ability to route and redirect from the controller. Without this feature you kind of lose the purpose of MVC.

In ASP.NET MVC you may write the following code:

return RedirectToAction("action", "controller", new { Area = "area" });

in DNN MVC we handle the complex routing redirects with the following syntax

return Redirect(Url.Action("action", "controller"));

We just need to update this logic to match what asp.net MVC does and allow the developer to supply an area which will then properly generate the route:

New Syntax:

return Redirect(Url.Action("action", "controller", new { Area = "area" }));

Discussion

I don't believe this to be a completed spec on this space and would love to discuss thoughts on this topic before going ahead with any implementation.

valadas commented 5 years ago

@ahoefling I assigned the 10.0.0 milestone, I am just trying to close issues that are not actionable and assign each actionable one to a possible milestone, no engagement there :) This one looked to me like a possibility for 10 but that can be changed as per future discussions, I do not use the MVC pattern enough myself to have an opinion ;)

SkyeHoefling commented 5 years ago

Depending on the implementation this may be a breaking change if it forces all routes to now have an Area path in it, which would effectively change all routes in every MVC module on every DNN website that uses the implementation of this. I would encourage us to prevent this type of breaking change and only use the Area path if the module is using Area's.

@valadas could you mark this as help-wanted/up for grabs as I will not have capacity to work on this before v10.

If anyone is interested in trying to solve this, I can walk them through the code and show them what I was thinking of for an implementation, I just don't have the capacity to take this one all the way.

stale[bot] commented 4 years ago

This issue has been automatically marked as 'stale' because no activity has been detected in the last 90 days. The intent is to bring awareness to the issue and to alert to the fact that the issue will be closed if no further activity is detected within the next 14 days. If you are unable to make a pull request for this issue, then please consider partnering with another developer in the DNN community that is willing and able to assist. Thank you for your continued contributions.

SkyeHoefling commented 4 years ago

This is still a valid issue and something good for the platform to implement