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.03k stars 751 forks source link

Add Lists Management User Interfaces #3066

Open david-poindexter opened 6 years ago

david-poindexter commented 6 years ago

Description of problem

The ability to manage host/admin Lists in DNN was removed in DNN 9 and this is a vital feature that needs to be added in.

Description of solution

We need to be able to edit Lists (Countries, Currency, etc.) via the UI. Following the DNN 9 naming conventions, these could be implemented as Global Lists and Site Lists (similar to the way Assets are handled currently).

Description of alternatives considered

I see two approaches:

  1. We add in a new Persona Bar extension(s) to resurface this feature.
  2. We add back in the old WebForms based module(s) as a temporary "fix" and simply link to it/them from a new Persona Bar menu item(s) similar to the way Global Assets and Site Assets are handled now.

Screenshots

Host > Lists

screencapture_ 2018-09-20 11 18 am _1942

Site > Lists

screencapture_ 2018-09-20 11 18 am _1943

Additional context

n/a

Affected version

Affected browser

sleupold commented 6 years ago

IMO the main issue of Lists is the limited data structure it uses - it looks like you can do everything with it like simple text lists (like forbidden passwords), dictionaries (banned words with substitution), lists with value and text (without specification what they mean) and even hierarchical lists, where lists might be nested or lists may contain nested values or nested lists may contain nested values. There is no option for the framework or the database to validate items and there is a high risk of orphaned data from 3rd party modules, which may easily result in inefficient table and platform performance. From the database performance, the list should at least be recreated and migrated to fulfill 3NF. However, this does not solve the issue of inappropriate data structure for many of the contained items: Countries are needed for any site with public registration but e.g. ecommerce sites need additional information and the data structure fails (Country may require currency and address format etc.) There should be a major benefit in reviewing the core data stored in the lists to decide, whether they need to be moved to a dedicated table - including the option to reference it. Referencing country and region in current user profile are unveiling clearly the performance and integrity issues of the current solution

sleupold commented 6 years ago

PS_ it even looks like the table is overengineered - as some of the columns doesn't seem to be populated at all

david-poindexter commented 6 years ago

@sleupold I certainly agree some attention needs to be given to the architecture of this solution. Perhaps that would be best served under a separate issue? This issue is really about resurfacing a UI for what was there prior to the Persona Bar implementation.

My vote would be to implement this as a new Persona Bar Extension rather than bringing back the WebForms based solution. Thoughts from others?

sleupold commented 6 years ago

@nvisionative do you really want to re-implement all features? do we need hierarchical lists except for Countries and Regions? Shouldn't we have a global button "add List" and an AddItem command (right click or whatever) for a list to add an item instead of an "add item" button and a dropdown for the list in the add form? I volunteer for the proper SQL part (with backwards compatibility) and might sketch changes for the API, but don't have the time to test it to be ready to build the UI, at least not for 9.3.0.

david-poindexter commented 6 years ago

@sleupold again, this issue is about getting back what was - that's all. 😉 If that is not what people want, then it is going to be a much more complicated endeavor. I am personally fine either way, but it is easier to get back to a previous baseline than it is to rearchitect it all. My vote would be, as a first step, to get back to what it was without rearchitecting the solution.

sleupold commented 6 years ago

@nvisionative if you want to stick to this crap, go ahead. But it should be smashed into the bin before migrating to .Net Core. And we should not encourage 3rd party developers to use it.

david-poindexter commented 6 years ago

@sleupold Are you reading my comments? I am trying to be objective here. That's all. If this is not something the community wants or if the community wants something different, then, by all means, that's what we shall do. I am just documenting the issue for consideration. There is no need to get ugly here. ☚ī¸

sleupold commented 6 years ago

@nvisionative it is not about what the community wants or understands. The current data structure is not suiting the problem and does not prevent inconsistent data and doesn't perform appropriately. I am trying to improve the database regarding performance and stability, making it fit for the future - but this doesn't seem to be a common goal.

david-poindexter commented 6 years ago

@sleupold those goals are 100% great! They just shouldn't be injected into the context of an issue that has nothing to do with that. This issue, as stated before, is about bringing back a missing UI for what was previously there. That's all. A separate issue should be created to address your stated goals.

sleupold commented 6 years ago

@nvisionative if you implement a UI for the current structure, it will cause a rewrite, once the data layer is corrected. I myself volunteer to fix the data layer, but I will never touch the UI.

david-poindexter commented 6 years ago

Excellent - we'll cross that bridge if/when we get there.

sleupold commented 6 years ago

the problem are not the lists used by DNN Platform, the problem are 3rd party lists, which may use or even abuse it. If we bring back old UI, I fear, they will be encouraged to use it. We should at least comment that there are plans to improve it and apply additional constraints

WillStrohl commented 6 years ago

@sebastian It seems like you're super passionate about this feature. Why don't you create a product development plan for it?

In the meantime, only one thing can be done at a time. This won't be built all at once, and no matter how it's built, it won't serve every possible use case.

Also, please try to be less argumentative when engaging publicly with the community. Everyone sees and can read this. If the community is to thrive, then we need for all people to feel like their opinions matter and can be heard. We're all on the same team. Let's work with each other more. 😊

valadas commented 5 years ago

Since there was no move on revamping the lists feature, I am moving this from rfc to a new feature and assigning Dnn 10, if things change in that matter we can adjust.

Timo-Breumelhof commented 4 years ago

@david-poindexter Am I correct in assuming that the "old" list module should still work in DNN 9?

david-poindexter commented 4 years ago

@Timo-Breumelhof I'm not 100% sure, but it may (obviously as a non-PB module though).

WillStrohl commented 4 years ago

@Timo-Breumelhof I looked into this too. ;)

Unfortunately, the old list module is using telerik for the tree view, dropdown lists, and grid. :(

https://github.com/dnnsoftware/Dnn.Platform/blob/release%2F7.4.2/Website/DesktopModules/Admin/Lists/ListEntries.ascx https://github.com/dnnsoftware/Dnn.Platform/blob/release%2F7.4.2/Website/DesktopModules/Admin/Lists/ListEntries.ascx.cs

This needs to be rebuilt.

WillStrohl commented 4 years ago

I dug into this more, and there is a weird behavior that I can't think of a reason for. Why would the replacement value (Value) need to be unique? There's a unique constraint on that column. It makes sense for Text since you don't want to have multiple instances of the same text trying to get replaced.

This may be an artificial restraint based on the other use cases in the Lists feature. If that's the case, this logic should be moved to the BLL and not be in the DAL.

image

WillStrohl commented 4 years ago

For reference, this is the constraint: [IX_Lists_ListName_Value_Text_ParentID]

sleupold commented 4 years ago

@hismightiness EntryValue needs to be unique, as the display text is just a friendly name for it - in most lists. For Country and Region, EntryValue holds the unique ISO code, similar for the various type lists. And for these lookup lists, the display text should be unique as well (per list), otherwise you would get duplicate DDL items displayed. Of course, the requirements for ProfanityFilter or banned passwords are different - and therefore IMO they should be moved to dedicated tables with appropriate indexes.

However, for Profanity Filter items, text should contain the text to be search and value the text to replace it with. Index IX_Lists_ListName_Value_Text_ParentID just makes sure, there are no duplicate tuples with same Value AND Text within a list (and parent). You are encountering duplicate items in index IX_Lists_ParentID, which makes Value unique for a List and Parent Value (including Null).

WillStrohl commented 4 years ago

FYI - I'm sure "someone" will find an issue with this. :D
https://github.com/hismightiness/dnn-profanity

JoeAucoin commented 3 years ago

I'm too am still using the Lists module from DNN 9.0.1 release . . . \DNN_Platform_9.0.1.142_Upgrade\Install\Module\DNNCE_Lists_09.00.01_Install.zip Does anyone know if this module will be updated to remove the Telerik components? I use the module to support many custom modules I've built that leverage the "Lists" table. I use the module to support dropdown lists in many other custom modules. My goal is to update >50 DNN sites to 9.9 and remove all traces of Telerik.

david-poindexter commented 3 years ago

@JoeAucoin my assumption is it wouldn't be too crazy to pull out the old module source, refactor it to a stand-alone module, and then replace any Telerik controls.

rodrigoratan commented 3 years ago

my 2cents

I agree that some important data used by the platform, like profile data, can and should be improved to get their own tables, but the issue here is that currently the platform already uses the Lists table, so we should have some way to administrators to manage this, not only for Country and Region, but also for when you create a profile property of type "Lists", so somewhat this generic approach will be still need..

david-poindexter commented 2 years ago

A new DNN Community repo has been set up for @JoeAucoin to attempt the creation of a standalone version of the module with Telerik removed. See https://github.com/DNNCommunity/DNN.Lists.