? Pending

User tests: Successful: Unsuccessful:

avatar alikon
alikon
19 Sep 2022

Pull Request for Issue #8244 .

Summary of Changes

update modified field when exists and when publish/unpublish from the list

Testing Instructions

see #8244 .

Actual result BEFORE applying this Pull Request

the Modified Date is not changed

Expected result AFTER applying this Pull Request

the Modified Date is changed

avatar alikon alikon - open - 19 Sep 2022
avatar alikon alikon - change - 19 Sep 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Sep 2022
Category Libraries
avatar coolcat-creations
coolcat-creations - comment - 19 Sep 2022

So you only would update the modified state if it exists in the table? I think that does not make sense because modified is modified, no matter if the field is visible or not. Thanks!

avatar laoneo
laoneo - comment - 19 Sep 2022

Honestly I'm a bit hesitant to modify this field in the base table class where almost every extension inherits from. As this can have some unknown backward compatibility implications, I would rather do this in a model. If really needed in the base class then not before 5.0.

avatar laoneo
laoneo - comment - 19 Sep 2022

@coolcat-creations the check is done if the table has a field modified at all, not if it is visible in the form.

avatar coolcat-creations
coolcat-creations - comment - 19 Sep 2022

Ok - thanks for clarifying- is the PR ready to test or still to discuss?

avatar alikon
alikon - comment - 19 Sep 2022

@laoneo i've based the change on this #8244 (comment)

avatar HLeithner
HLeithner - comment - 19 Sep 2022

I have 2 questions here.

  1. in the documentation the field is called modified_time.
  2. we never change the modified column in JTable, not for reordering or moving, so should this field be updated on such "trivial" change and should the user doing this change not also be updated.

George mentioned that we do the update in his comment #8244 (comment) but I can't find it.

So if we update modified_time on publish we should be consistent and update it everywhere and also include the "who" did it. Beside that we don't trigger versioning on publish/unpublish (or did this changed already?)

avatar chmst
chmst - comment - 19 Sep 2022

For articles and categories: The modified data (both user and datetime) are changed if the content is edited (means when the save button is used).
Changing the state in the list view makes no change in modified data. The change is made in function store() in respective Tables.
This PR would be a huge b/c break.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38783.
avatar richard67
richard67 - comment - 20 Sep 2022

I agree with Harald: When the „modified time“ is updated and the table has also a „modified by“ column, then that column should be updated, too.

avatar alikon
alikon - comment - 20 Sep 2022

make sense i'll do it when i'll found some free time

avatar chmst
chmst - comment - 21 Sep 2022

This PR causes a massive b/c break.

avatar coolcat-creations
coolcat-creations - comment - 21 Sep 2022

@chmst why so? It fixes a bug that the time is not updated or not?

avatar chmst
chmst - comment - 21 Sep 2022

Module Articles latest allows ordering "recently modified first".

In my application I have articles with information.
Sometimes we unpubish an article because the information seems to be wrong, but publish again when it has been checked by the responsile.
The information then is the same as years ago.
It is wrong to show this information as "recently changed".

avatar coolcat-creations
coolcat-creations - comment - 21 Sep 2022

@chmst But if you unpublish something inside the article the modified date is also modified. Do you really take care where you unpublish it exactly?

avatar richard67
richard67 - comment - 21 Sep 2022

The PR changes the meaning of the modified columns. Right now, they show when the item was edited last time and by whom. With the PR it will show when the item was edited or it's status was modified by whom. When you now show the time of last modification in frontend, it might have a completely different and possibly undesired meaning. One may want to see this the other that on his site.

A clean way out would be to have another 2 columns.

With a good naming we would have an "edited" time and user which does what the modified time and user do now, and the "modified" time and user would work like it is done with this PR.

But this would be a b/c break, so we might have to find a different naming so the "modified" time and user continue to work like before.

avatar chmst
chmst - comment - 21 Sep 2022

Yes as I wrote - if I open the article for editing, the modified date is changed with function store().
I don't want to decide what is correct and will find a workaround for my stuff.
But all the features which use ordering with modified date could change their behaviour.

avatar brianteeman
brianteeman - comment - 21 Sep 2022

@chmst is correct. This will have significant impact on users websites.

avatar coolcat-creations
coolcat-creations - comment - 21 Sep 2022

Then how to fix the bug without a b/c break?

avatar laoneo
laoneo - comment - 21 Sep 2022

As I said already, do it in the model much earlier in the call chain. This change like in this pr should be considered for 5.0 at earliest.

avatar alikon
alikon - comment - 12 Nov 2022

so can we have a final agreement this ?
i.e decision

avatar alikon alikon - change - 26 Nov 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-11-26 10:34:52
Closed_By alikon
Labels Added: ?
avatar alikon alikon - close - 26 Nov 2022
avatar alikon
alikon - comment - 26 Nov 2022

please reopen the original issue #8244

Add a Comment

Login with GitHub to post a comment