Skip to content

[4.0] Refactoring Content Versioning#29217

Merged
wilsonge merged 8 commits intojoomla:4.0-devfrom
Hackwar:j4history2
May 29, 2020
Merged

[4.0] Refactoring Content Versioning#29217
wilsonge merged 8 commits intojoomla:4.0-devfrom
Hackwar:j4history2

Conversation

@Hackwar
Copy link
Copy Markdown
Member

@Hackwar Hackwar commented May 25, 2020

This is a redo of #23432.

The content versioning in Joomla is a rather obscure feature. Parts of it run/are available for every extension that uses the base classes, others you have to specifically add to your code. There isn't really a way to not use that system. It is also tied into the UCM system and is unnecessarily complex. This PR tries to improve on that in a few ways. This does not claim to be the perfect solution (there is a lot of stuff to do in the future), but it is hopefully a step around the backwards compatibility issues that we would have if we want to work on this in 4.1 or later.

What does this PR do?

  • Decoupling of the versioning system from the UCM.
  • Simplifying code
  • Moving all versioning code to interfaces and traits
  • Instead of using magic fields/attributes, the support of versioning is clearly signaled by interfaces
  • Moved all the necessary code to a proper subfolder...

What did I do?

I changed the name of the history table from #__ucm_history to #__history, dropped the ucm_type_id column and changed the ucm_item_id column to a varchar and changed how it is used. That column now contains keys of the type <component>.<type>.<id>, which is also what is used everywhere to identify the content item to edit. By this, you don't need to read the content types table first to get the right item type ID. But more importantly for me, you can now look into the table and read what type of data a row is. I consider that a better solution for debugging and better than a composite key out of 2 columns.

Status

This PR is ready for testing. However, while migrating this code to the current 4.0-dev, several bugs in the 4.0-dev codebase have surfaced, which need to be fixed beforehand. I will report back here regarding those bugs.

How to test

  1. Apply the PR
  2. Apply the database changes of this PR
  3. Go to a core component that supports versioning, com_content for example, and create a content item. Save the item and edit it again, changing something. Saving again should create a new version. Editing again and saving without any changes shouldn't create a new version.
  4. In the edit view, click on the versions toolbar button and see that 2 versions have been created (Notice that the edit without a change does not show up here)
  5. Compare 2 versions and restore an older one. All of this should work.

Comment on lines +87 to +90
$jform = $input->get('jform', array(), 'array');
$versionNote = '';

if ($aliasParts[0] && ComponentHelper::getParams($aliasParts[0])->get('save_history', 0))
if (isset($jform['version_note']))
Copy link
Copy Markdown
Contributor

@SharkyKZ SharkyKZ May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way not to hardcode control group and fields? This looks like the opposite of:

Instead of using magic fields/attributes, the support of versioning is clearly signaled by interfaces

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've taken a lot of existing code and didn't rewrite everything. If you have a better solution, I'm all ears.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at the moment. But would be great to stop relying on input. This breaks when editing items programmatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh this is a good point. By now having this in a plugin is it going to cause issues with the API?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the time pressure, can we fix this in another PR and merge this one for the time being?

Co-authored-by: SharkyKZ <sharkykz@gmail.com>
@Hackwar
Copy link
Copy Markdown
Member Author

Hackwar commented May 29, 2020

All fixed. Can we merge this now? Given the current time pressure. ;-)

Co-authored-by: Harald Leithner <leithner@itronic.at>
Co-authored-by: Harald Leithner <leithner@itronic.at>
@wilsonge wilsonge merged commit 269e1e4 into joomla:4.0-dev May 29, 2020
@wilsonge
Copy link
Copy Markdown
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants