Skip to content

Conversation

rdeutz
Copy link
Contributor

@rdeutz rdeutz commented Sep 11, 2025

Follow up for #45515 to fix know issues and redo of #45955

Summary of Changes

Cleanup and fix know issues

Note to version star fix: This is related to the calculation of a sha1 hash. While creating the history entry a hash will be calculated and saved in the history table. When someone saves again then the hash is calculated based on the current data. If in the history table a saved hash is the same and the version note is also the same (with the curretly calculated) then the a new history entry will NOT be created. The 2nd use of the hash is for the list of versions (click on "Versions" Button in edit view). The matching version will be maked with a star to indicate the currently used version.
I myself got confused because I saved and thought the hash must be the same when saving two times in a row. But this is wrong thinking.

Testing Instructions

This is now ready to test for the main functionality. Play around with versions for articles. You should be able to set different values for custom fields, tags, categories. Going back and force for versions should you always bring back to the values that where valid for the selected version.

Update1: Testing of banner, categories, contact, newsfeed is also possible, it works at least for me. Tags can also be tested, these are using the old system.

Update2: Looked into the preview and I think it is good enough. We are showing structured data, but we don't know what fields neither how the value is sturctured.

Update3: Fixed the compare view. It is pretty much the same as for the preview, we don't know what data we are showing so how it is yet is all what can be done. Obviously there is room for some data description concept but this would be a lot of work and we would have to do it for all data. For the usecase we have here I think it is fine.

Update4: Documentation will be written when it is merged, it will be only for developers how to use it.

Actual result BEFORE applying this Pull Request

Only core fields of an article are in versioning

Expected result AFTER applying this Pull Request

All fields are versioned.

Link to documentations

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 rdeutz mentioned this pull request Sep 11, 2025
9 tasks
@rdeutz rdeutz marked this pull request as draft September 11, 2025 12:11
@ChristineWk
Copy link

With the PR, the star is visible. Does it look OK?


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

@ChristineWk
Copy link

version

@brianteeman
Copy link
Contributor

image
  1. the id of the article has not changed - looks like you are comparing more than should be compared
  2. as above it should be just comparing the value between old and new
  3. should be showing the tag name and not the tag id. Also the old version had one tag and the second version had an extra tag. Doesnt look correct in this table.
  4. Related to above. dont know why it says the old value was undefined. It didnt exist so should be empty
  5. Should be language string not the component name

@brianteeman
Copy link
Contributor

brianteeman commented Sep 12, 2025

image

Dont know what "Saved on" is meant to mean here.

Found the string. There is supposed to be something after saved on

image

@brianteeman
Copy link
Contributor

Video of more tests. Shows more examples of the issues reported above

chrome_U42HfAVZxc.mp4

@rdeutz
Copy link
Contributor Author

rdeutz commented Sep 12, 2025

#46073 (comment)
1 & 2 should be fixed
3 & 4as I said we don't know what we are showing. It can be a tag or it can be something different just having the same name. Guessing and then making DB queries to get some data that might be the right one will not end well.
5 Same here, we don't know. I can use Text::_() for it, not sure if it makes so much sense.

#46073 (comment)
I deleted too much lines, its fixed

@brianteeman
Copy link
Contributor

tags

see video.

  1. It should not say undefined when there are no tags
  2. Just having the tag id isnt very useful
chrome_EXBk6cY2hV.mp4

@rdeutz
Copy link
Contributor Author

rdeutz commented Sep 12, 2025

I can remove the undefined that's not a problem. Question of taste.

Tags is a problem that I don't see how we solve that properly. I could try to find names for the id, but we are in a situation where the meaning/existens of a tag can change.

@brianteeman
Copy link
Contributor

I could try to find names for the id, but we are in a situation where the meaning/existens of a tag can change.

How is that different to anything else eg category where the ID can stay the same and the Text/Name can change. We always show the Text/Name at the time of the comparison not at the time it was created - dont we?

@rdeutz
Copy link
Contributor Author

rdeutz commented Sep 12, 2025

How is that different to anything else eg category where the ID can stay the same and the Text/Name can change. We always show the Text/Name at the time of the comparison not at the time it was created - dont we?

We do, but does it makes sense when we compare something that was created two years ago with category/tag id=2 what was at the time "blue underware" and now it is "yellow cap"?

@brianteeman
Copy link
Contributor

thats a valid comment and I would agree with you if that is what we did everywhere. But we dont. For categories and other things we show the current text and not the id number. I am just looking for consistency

@rdeutz
Copy link
Contributor Author

rdeutz commented Sep 12, 2025

@brianteeman I am still more on the site "not worth" but I added the ability to lookup a field when it is an array like tags. To make it work it needs a change in the #__content_types table.

{"sourceColumn":"tags","targetTable":"#__tags","targetColumn":"id","displayColumn":"title"}

must be added to the content_history_options column.

For convenience here is the value for article:

{"formFile":"administrator\/components\/com_content\/forms\/article.xml", "hideFields":["asset_id","checked_out","checked_out_time","version"],"ignoreChanges":["modified_by", "modified", "checked_out", "checked_out_time", "version", "hits", "ordering"],"convertToInt":["publish_up", "publish_down", "featured", "ordering"],"displayLookup":[{"sourceColumn":"catid","targetTable":"#__categories","targetColumn":"id","displayColumn":"title"},{"sourceColumn":"created_by","targetTable":"#__users","targetColumn":"id","displayColumn":"name"},{"sourceColumn":"access","targetTable":"#__viewlevels","targetColumn":"id","displayColumn":"title"},{"sourceColumn":"modified_by","targetTable":"#__users","targetColumn":"id","displayColumn":"name"},{"sourceColumn":"tags","targetTable":"#__tags","targetColumn":"id","displayColumn":"title"} ]}

@exlemor
Copy link

exlemor commented Sep 12, 2025

I have tested this item ✅ successfully on 7c18733

I have tested this successfully. Thanks @rdeutz!

As discussed in the PR Testing Group while testing this PR, if you create a tag on the fly, there is no trace in the Preview under Versions until after the 2nd save.


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

@brianteeman
Copy link
Contributor

brianteeman commented Sep 12, 2025

When you add images then the information is duplicated

image

Video

chrome_eOppk7rdvW.mp4

@brianteeman
Copy link
Contributor

the image duplication is fixed now

@brianteeman
Copy link
Contributor

When I do a preview in Joomla 6 the intro and full text are at the very bottom
When I do a preview in Joomla 5 they are at the top

See video

chrome_THFbTdH8NT.mp4

@softforge softforge merged commit 9fa62a0 into joomla:6.0-dev Sep 13, 2025
30 checks passed
@softforge softforge added this to the Joomla! 6.0.0 milestone Sep 13, 2025
@softforge
Copy link
Contributor

Thank you @rdeutz for all the work you have done on this. There are still a few rough edges which the testers, especially @brianteeman, have highlighted and need to be addressed before it's bulletproof, but there is a lot that has improved, and it's so much closer to the finish line. Thank you to all the testers and also @bembelimen and especially @LadySolveig who have put a lot of time into helping solve issues and nudging it along.

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