joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[6.0] RFC extend versioning, save releated information in history table and restore historical data

Open rdeutz opened this issue 7 months ago • 2 comments

Our versioning works on databse table level, this leads to situations that we save an article with custom fields but the CF are not included in the history. This is unexpected from a user perspective. This PR starts to change this behaviour so that releated information are also saved.

Summary of Changes

I have added an interface VersionableModelInterface, if a model implements this interface then in AdminModel the save of the histroy data is done in the save method (calls saveHistory) of the Model. This is the first step and we have now the data with releated information in the history table.

If you want to restore a version then in loadHistory (VersionableModelTrait) set the data from the history table as session data and loadForm use this data when you edit an item.

It works for CF so far. At the moment not for Tags (tags is a pain, we might change more on tags for 6 so I haven't looked into the details why not).

This is a b/c break and I don't know how many are affected but I think that it can't be too much hassle and what we get when we finish this is worth the work for extensions developers.

  • [ ] Look into tags
  • [ ] Extend to other core items where we are using versions (Banner, Banner Client, newfeeds, Categories Tags).
  • [ ] Remove behaviour plugin
  • [ ] Fix tests

Testing Instructions

It is only done for Articles and Contacts!

  • Create some custom fields
  • Create an article and save some versions of it
  • Restore versions back and force

Actual result BEFORE applying this Pull Request

Custom fields have always the last saved value

Expected result AFTER applying this Pull Request

Custom fields have the value from the saved version

Link to documentations

Documentation will be provided at a later stage.

Please select:

  • [ ] Documentation link for docs.joomla.org:

  • [ ] No documentation changes for docs.joomla.org needed

  • [ ] Pull Request link for manual.joomla.org:

  • [ ] No documentation changes for manual.joomla.org needed

rdeutz avatar May 22 '25 08:05 rdeutz

I have tested this item :white_check_mark: successfully on eb8533b796f43a9a9fa097b85f2971f7fa8882e0

Successful tested with a text-, integer-, list- and checkboxfield(s).

B.t.w. Comparing versions is throwing an error: https://prnt.sc/bvWwN0TaKwSq with or without this patch


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

peterpeter avatar May 31 '25 20:05 peterpeter

I have tested this item :white_check_mark: successfully on eb8533b796f43a9a9fa097b85f2971f7fa8882e0

Successful tested with a text-, integer-, list- and checkboxfield(s).

B.t.w. Comparing versions is throwing an error: https://prnt.sc/bvWwN0TaKwSq with or without this patch


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

peterpeter avatar May 31 '25 20:05 peterpeter

I now know why the tests are failing, when creating a banner over the api only the table class is used and not the model. So the problem is "only" an API problem.

rdeutz avatar Aug 04 '25 18:08 rdeutz

I also got the An error has occurred. 0 htmlspecialchars(): Argument #1 ($string) must be of type string, array given error when comparing with or without, but the PR certainly restores the custom fields and tags restored, really nice feature

softforge avatar Aug 08 '25 15:08 softforge

@rdeutz - there are some issues with this PR ;(

Sometimes it restores with a blank article, sometimes not, (no error message), Tags not restored :/

(sorry)

exlemor avatar Aug 09 '25 03:08 exlemor

I now know why the tests are failing, when creating a banner over the api only the table class is used and not the model. So the problem is "only" an API problem.

@rdeutz Then the failing API test should be commented out with a to do comment so that system tests are passing. Merging this PR as it is would mean to break system tests in the 6.0-dev branch.

richard67 avatar Aug 11 '25 20:08 richard67

I now know why the tests are failing, when creating a banner over the api only the table class is used and not the model. So the problem is "only" an API problem.

Maybe @heelc29 or @alikon could help with the API so that the system test works again?

muhme avatar Aug 12 '25 05:08 muhme

Test and the compare view is fixed, so this here is ready to test again.

@exlemor could you give some more information what you did. As you can see it is working for other people, so it would be good to know more details.

rdeutz avatar Aug 14 '25 11:08 rdeutz

I have tested this item :red_circle: unsuccessfully on 541f78ecf38f196ec669cfbc90098d57e61c14d1


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

brianteeman avatar Aug 14 '25 11:08 brianteeman

I installed the blog sample data I edited the content of the typography article I entered some data in the author field of the typography article Then I clicked ion the versions button and selected the two versions and clicked on compare

An error has occurred. htmlspecialchars(): Argument #1 ($string) must be of type string, array given

Call Stack

Function Location
1 () JROOT\administrator\components\com_contenthistory\tmpl\compare\compare.php:105
2 htmlspecialchars() JROOT\administrator\components\com_contenthistory\tmpl\compare\compare.php:105
3 include() JROOT\libraries\src\MVC\View\HtmlView.php:416
4 Joomla\CMS\MVC\View\HtmlView->loadTemplate() JROOT\libraries\src\MVC\View\HtmlView.php:204
5 Joomla\CMS\MVC\View\HtmlView->display() JROOT\administrator\components\com_contenthistory\src\View\Compare\HtmlView.php:64
6 Joomla\Component\Contenthistory\Administrator\View\Compare\HtmlView->display() JROOT\libraries\src\MVC\Controller\BaseController.php:697
7 Joomla\CMS\MVC\Controller\BaseController->display() JROOT\libraries\src\MVC\Controller\BaseController.php:730
8 Joomla\CMS\MVC\Controller\BaseController->execute() JROOT\libraries\src\Dispatcher\ComponentDispatcher.php:143
9 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() JROOT\libraries\src\Component\ComponentHelper.php:361
10 Joomla\CMS\Component\ComponentHelper::renderComponent() JROOT\libraries\src\Application\AdministratorApplication.php:150
11 Joomla\CMS\Application\AdministratorApplication->dispatch() JROOT\libraries\src\Application\AdministratorApplication.php:205
12 Joomla\CMS\Application\AdministratorApplication->doExecute() JROOT\libraries\src\Application\CMSApplication.php:315
13 Joomla\CMS\Application\CMSApplication->execute() JROOT\administrator\includes\app.php:58
14 require_once() JROOT\administrator\index.php:32

brianteeman avatar Aug 14 '25 11:08 brianteeman

Test and the compare view is fixed, so this here is ready to test again.

@exlemor could you give some more information what you did. As you can see it is working for other people, so it would be good to know more details.

Hi Robert, I'll try to remember but to be honest, after the medication I have been on due to hospital visit on Monday, not entirely sure my memory will recollect 100%, but basically I tested your PR as you instructed and with your PR activated, I would try to restore an article and I would get a blank page (i.e. code view = blank), and at other times not but no error or console messages but now that a bunch of work has been done on it. I will retest it for you sometime today, I'm not 100% yet and with the heat wave not helping, but I'll do my best.

exlemor avatar Aug 14 '25 11:08 exlemor

Thanks for testing @brianteeman I think I have missed a case. I made a change and pushed it. I tried to check locally but -funny enough- the sampledata plugins are not working here. Please retest if I have made the correct change.

rdeutz avatar Aug 14 '25 14:08 rdeutz

I like this so much. I will test it later as soon as I find the time and would be very happy if we could get it in for the Release.

The only thing that strikes me right away is that we definitely need an addition to the documentation for developers. Previously, everything that was saved directly via $table->store for example was also versioned, but that is no longer the case and, in my experience, is sometimes used to bend data. We should at least issue a note about this.

LadySolveig avatar Aug 14 '25 16:08 LadySolveig

Applied the latest changes and did the exact same check

Now the page is full with this warning

Warning: Array to string conversion in D:\repos\j6\administrator\components\com_contenthistory\tmpl\compare\compare.php on line 102

brianteeman avatar Aug 14 '25 16:08 brianteeman

@brianteeman thanks I will try to get sampledata installed and try on my own.

@LadySolveig The only thing you need to do, if you have used versioning in your component, is in the model add "implements VersionableModelInterface". That's all. But I agree we need documentation and I will write it, when it works without errors.

rdeutz avatar Aug 14 '25 17:08 rdeutz

I have tested this item :red_circle: unsuccessfully on 5d68b8283cda45031f2db805d8d317b0f0812261

I have test this unsuccessfully. ;( Sorry @rdeutz

I downloaded the Update Package from the Download button built on Thu Aug 14 18:28:10 UTC 2025

When comparing 2 versions of the article, I get the same error Brian got:

An error has occurred. 0 htmlspecialchars(): Argument #1 ($string) must be of type string, array given

Going further, and this MAY be beyond this PR, I apologize as I rarely restore older versions of Articles,

  1. when I preview an article that has an image in it, the image preview is broken?

  2. when having added/removed tags from a version, I do see a newTags ["5","4"] and Tags ["5","4"] section in the popup preview window but those are numbers not the Tags themselves? (perhaps expected but not very user friendly)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

exlemor avatar Aug 15 '25 05:08 exlemor

At the moment it is more important to test if the functionallity works. So can you restore an older version of an article, banner etc.

The problem with the compare/preview view is that you have a structure to show that is not fixed, it is array of array ... of array and to have the diff between it you need to convert this into a flat table. I have made a fix to show the data without error but it is not pretty.

@exlemor do you know the line where the error was?

rdeutz avatar Aug 15 '25 07:08 rdeutz

I have tested this item :white_check_mark: successfully on 5d68b8283cda45031f2db805d8d317b0f0812261

Article, tags and custom fields tested with different versions and restore - works for me.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

LadySolveig avatar Aug 15 '25 13:08 LadySolveig

I have tested this item :white_check_mark: successfully on 5d68b8283cda45031f2db805d8d317b0f0812261

I was able to successfully. Thanks to @rdeutz, @LadySolveig, @softforge, @Bodge-IT !


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

exlemor avatar Aug 15 '25 15:08 exlemor

I have tested this item :red_circle: unsuccessfully on 5d68b8283cda45031f2db805d8d317b0f0812261


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

brianteeman avatar Aug 15 '25 15:08 brianteeman

When I compare two versions of an article that does not have any image the comparison shows

image

Note all the "undefined" references

I would expect it to be like this

image

The same is true for most other options

image

brianteeman avatar Aug 15 '25 15:08 brianteeman

When you have some tags it gets even weirder

There is this section right at the top

image

Then another reference to tags

image

And finally a third reference right at the bottom

image

brianteeman avatar Aug 15 '25 15:08 brianteeman

I have tested this item :red_circle: unsuccessfully on 5d68b8283cda45031f2db805d8d317b0f0812261

Version Note not working image


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

heelc29 avatar Aug 15 '25 15:08 heelc29

@exlemor @LadySolveig did you not get these errors

brianteeman avatar Aug 15 '25 15:08 brianteeman

This needs more work if the display of information is to be useful

Another example is when scheme is not defined

image

You only get that value of extendJed when you have not enabled any schema plugins

image

If it is to be displayed in the comparison then the string needs to be translated into human speak

brianteeman avatar Aug 15 '25 15:08 brianteeman

Restore of 1st version is not working correctly

  • buttons disappear (link new article) image
  • when I try to save again image

heelc29 avatar Aug 15 '25 15:08 heelc29

@LadySolveig thank you for your time explaining the need to push this through (because of its b/c breaking changes) and that the visual display will be fixed before final release or it will have to be reverted

brianteeman avatar Aug 15 '25 16:08 brianteeman

Confirmed the problem with the version note and fixed it.

Didn't get the button disappear problem reproduced, but will try later.

rdeutz avatar Aug 16 '25 09:08 rdeutz

I have tested this item :white_check_mark: successfully on c027b4dcb8e051797c4dd4f01aece5766b8bd91e

I have re-tested this successfully. Thanks @rdeutz! Keep up the great work!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45515.

exlemor avatar Aug 16 '25 13:08 exlemor

When you restore to the initial version the buttons disappear AND you cannot save the article

See video

https://github.com/user-attachments/assets/68c2ea91-4c85-4cad-ae6b-10a11fc8ca64

brianteeman avatar Aug 16 '25 13:08 brianteeman