coderedcorp / coderedcms

Wagtail + CodeRed Extensions enabling rapid development of marketing-focused websites.
https://www.coderedcorp.com/cms
Other
668 stars 137 forks source link

Add ability to disable registration of CRX layout and analytics settings #623

Closed michaelsteigman closed 1 month ago

michaelsteigman commented 4 months ago

Allow developers who need to store additional site-specific settings and do not wish to have multiple "Settings" menus to disable the default models and create custom settings models.

vsalvino commented 4 months ago

Thanks for doing this work. I think this implementation may be a bit over-engineered though. I don't plan on supporting having users create drop-in replacement settings models, as that could also get quite messy.

My though would be a Django setting which simply does not register our settings models in the Wagtail Admin (UI hides them in the UI). However the models would still be intact as-is. As you've probably learned, too much stuff touches those models that the change of breakage becomes extremely high if someone customizes them.

The assumption would be if you are disabling them in the Wagtail Admin, you'll need to customize the templates in which those settings are used, otherwise you'll end up getting the default values.

michaelsteigman commented 4 months ago

Thanks for doing this work. I think this implementation may be a bit over-engineered though. I don't plan on supporting having users create drop-in replacement settings models, as that could also get quite messy.

My though would be a Django setting which simply does not register our settings models in the Wagtail Admin (UI hides them in the UI). However the models would still be intact as-is. As you've probably learned, too much stuff touches those models that the change of breakage becomes extremely high if someone customizes them.

I admit I was having fun and got carried away! It did exactly what I was hoping (and would not have affected existing installs) but I realize it was more than what you were expecting. A simple mechanism to disable the registration of the CRX models can also work for us. Just means a little more duplication on our side.

The assumption would be if you are disabling them in the Wagtail Admin, you'll need to customize the templates in which those settings are used, otherwise you'll end up getting the default values.

I started working on a template tag to replace the settings references in the templates but stopped short of going there!

I've backed out everything except the decorator that conditionally will register the layout and analytics models. Let me know if this will work for you.

michaelsteigman commented 3 months ago

@vsalvino - back to bump this. Is current PR acceptable or did you have another approach in mind?

vsalvino commented 1 month ago

Thanks, this looks great! Appreciate the tests as well! I am going to do a bit of modification and split this setting into a couple individual settings.