octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.03k stars 2.21k forks source link

cms.databaseTemplates option should only look at meta fields #4575

Closed CptMeatball closed 5 years ago

CptMeatball commented 5 years ago

Description:

In PR #3908 the feature was implemented that the entire CMS section can be set to use the database if CMS items are edited. Only thing is that it focuses on the entire file, and not on the specific items like the title, meta title or other non-code related fields.

This issue might "work as intended", but it does not make deployment easier. When syncing the theme files, it would sync the entire database to a file or the file to the database, but this does not take into account that the client can edit a title, and the developer can edit the html template. When syncing, either up or down, on of those changes will go lost.

Am I using this feature incorrectly, or is this outside the scope of this feature?

Steps To Reproduce:

  1. Set cms.databaseTemplates to true
  2. Edit page in the CMS itself
  3. Update the file HTML without the CMS
  4. See that the code hasn't change.
LukeTowers commented 5 years ago

@CptMeatball This is intentional. When database content for a given file is found it is treated as preferred over the actual file contents to protect against client changes being overwritten by newer developer updates. There are features built in (Commit & Reset, and the theme:sync command) to tell the system which changes to prefer, but it sounds like you're looking for a more complicated merge conflict resolution system which would merge the changes from both sources at the same time.

I'm not really sure what such an interface would look like let alone how it would function. What are your thoughts on that?

CptMeatball commented 5 years ago

I'm not looking for a merge solution, I'm looking for a way to decouple meta information from the actual code. We've ran into this problem on multiple occasions that the client or a marketing agency have made changes the meta information of a page, which got overwritten by a deployment. It creates a scenario in which you overwrite meta information, without the possibility to do a rollback, as the meta information isn't part of the version control.

The code/markup is so tightly coupled to the meta information, that you can not safely deploy updates, without first merging meta changes that are on the actual live website.

Preferably the theme:sync command should have an option to just sync the "important stuff".

LukeTowers commented 5 years ago

@CptMeatball I'm not sure how we'd handle that, since the "meta" information is directly a part of the overall Halcyon object. I'm not even sure that's a use case that we should be encouraging TBH since if a client has access to edit the meta information of a CMS template then they generally have access to edit the main meat of the template, so I don't want to encourage developers to choose which datasource to prefer for which sections since they could easily be shooting themselves in the foot if the client makes changes to the "meat" of the template.

What are your thoughts on that?

LukeTowers commented 5 years ago

Also I wouldn't mind having a "Merge" option in the backend (along with the commit & reset) to popup a git diff like window for choosing which sources to prefer, perhaps one in the command too. It's just that it would be a complex feature to implement.

CptMeatball commented 5 years ago

Maybe I'm thinking sideways, but the fact that the client has full access to the CMS template to just edit some titles and/or description feels backwards. Personally, I haven't ran into any situation in which the clients wants to edit the actual markup of a cms page. I have however, ran into lots of situations in which the client has a marketing agency or content writer that wants to edit the title and the description of the page.

The problem we're (I'm?) facing is that the title and description is currently either:

In both cases you need to manually track any changes and merge them into your deployment branch.

I just found this plugin (https://github.com/devemio/oc-page-settings-plugin), that does exactly what I'm currently looking for, so it might be not that big of a deal, but I still think storing meta in the cms files is causing problems that aren't necessary.

Anyway, if I'm the only odd duck in the pond, I'll close my case, but I thought it's worth a discussion :) .

CptMeatball commented 5 years ago

Having a merge option feels like a complex feature indeed! And I think it would also introduce new problems regarding the use of automated deploy tools like DeployBot for existing 🤔

So if we want to tackle this problem, having a merge system wouldn't be the way to go I think!

LukeTowers commented 5 years ago

Hmm. I'm not really sure what the best solution is. Perhaps @bennothommo can add his thoughts. I know for my clients personally none of them have access to the CMS menu at all, they only have access to the Pages (RainLab.Pages) menu. I pretty much only use CMS pages for highly dynamic pages that wouldn't really ever need their title / description changed.

bennothommo commented 5 years ago

@CptMeatball I'm in the same boat as @LukeTowers - my company generally gives clients access to the RainLab Pages plugin, and locks them out of the CMS section. That way, there's a clear separation between what we control as the developer (the templating, assets and CSS) and they control as the client (content, configuration).

The cms.databaseTemplates option allows us to make adjustments to the content on our copies without it being pushed to the live website and overwriting the content added by the client - that's where I've found the most use for the feature.

That being said, I understand where you're coming from. A theoretical feature change to the database templates could be made to allow the developer to select whether they want just the content of the CMS object stored in DB, or the meta, or both (by default). It would mean both a DB read and a file read to merge the two pieces together if they're coming from different sources, but given that it's all cached in the end, it shouldn't be too much of a performance hit.

LukeTowers commented 5 years ago

For now @CptMeatball it sounds like the plugin you linked will solve your problems so unless you have anything else you'd like to add or anyone else chimes in I'll close the issue for now.