User tests: Successful: Unsuccessful:
Pull Request for Issue #8244 .
update modified field when exists and when publish/unpublish from the list
see #8244 .
the Modified Date is not changed
the Modified Date is changed
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
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.
@coolcat-creations the check is done if the table has a field modified
at all, not if it is visible in the form.
Ok - thanks for clarifying- is the PR ready to test or still to discuss?
@laoneo i've based the change on this #8244 (comment)
I have 2 questions here.
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?)
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.
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.
make sense i'll do it when i'll found some free time
This PR causes a massive b/c break.
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".
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.
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.
Then how to fix the bug without a b/c break?
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.
so can we have a final agreement this ?
i.e decision
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-11-26 10:34:52 |
Closed_By | ⇒ | alikon | |
Labels |
Added:
?
|
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!