Use new selection rules in auto-delete (Cron) code
FrankT - August 21, 2009 - 14:28
| Project: | Revision Deletion |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
| Issue tags: | Roadmap |
Jump to:
Description
The module is a good thing to get rid of aged revisions, but one thing is missing in my eyes. If a new revision is created for an aged node so that the older revision immediately matches the criteria for deleting, the older revision can be deleted very quickly.
In other words: there may be circumstances where old revisions are deleted before they should and there is no time to revert to the revision that has just been replaced.
This could be resolved by another setting to define a minimum age of the CURRENT version, too (just as an age for the revision is defined, too).

#1
Good point. I will look at adding that check.
#2
committed
#3
Well, now the new dev was available so I could test it. First of all it looks to me like the age of the current version is not regarded correctly yet - although I had set the current version age to 8 weeks, versions of yesterday were shown. Of course that's the most important thing about this issue.
I also suggest to move the setting for the current revision age further up behind or before the setting for the revision age.
Finally, the setting '0 Sek.' seems to be untranslatable.
#4
That setting is in the "List" fieldset because that is the code that uses it, at least for now. I wasn't sure what to call the 0 value, so I just left it as 0; I am open to suggestions. However, that is done by format_interval, so I don't understand why it is not translatable.
#5
First, I didn't like '0 Sek.', too. But I didn't have a better (short) thing. Immediately doesn't fit, too, 'Don't regard' also. Finally I don't mind to leave it, maybe as '0 seconds' But I would really like to have it with the revision age setting, because both settings belong togehter in my eyes.
I also extracted a translation file once again, there is no '0 Sek.' within!?
#6
I made a change, so check the next -dev. BTW, there is now a template available in the module.
#7
The date of the current version is still not regarded - I set the age of the current version to 8 weeks but also revisions are listed where the current revision is some days old. I tested if it depends on cron (aothough I thnik when set to manual it should not), but running cron didn't change the behaviour.
I suggest to modify the dates offered for the current version as follows: 1/2/4/12 hours 1/4 days, 1/2/4/8/26 weeks (I think especially the longest option of 26 weeks may be needed, currently 8 weeks as a maximum might be too short).
I also still suggest to move this setting up because the two settings would belong together in the eyes of a user.
#8
Yes, the List tab will show items that do not follow the new rules - and the auto delete will delete them too. I have not gotten to the Cron stuff yet, although it is on my plan to do so. On the site that got me interested in this module, I more interested in the manual ("list revisions") technique until I have experimented enough with the rules to see if automating it meets my needs (and, BTW, I think this aging request fits right in with that site). If / when I perform those set-off rules on the automated deletion, then I will be happy to move that setting. For the moment, however, I am trying to make it clear that those setting do not apply to the automated deletion choices.
BTW, I have not back-ported these changes to the 5.x-1.x-dev version.#9
Adding Roadmap tag and changing the title to reflect what needs to be done.
#10
There seems to be a slight misunderstanding (maybe on my side): currently I only work with the manual method and the list page shows more revisions than are defined by the settings. I only ran cron to be sure that running cron does not solve the problem.
#11
This is not hard to happen, because we have two "list" pages - we need to find a better description to at least one of them. There is the list of revisions that eligible to be automatically deleted (Cron) on the tab that says "List." And then there is the page that can be brought up with the "list revisions" operation or through the "node/xxx/revisions" tab. I've been trying to say "list revisions" when I mean the latter.
I think maybe the "List" tab should be changed to either "Overview" or "Eligible."
On the manual "list revisions" page, you will see ALL revisions for the node, but only some (per the rules) will be pre-checked for that page's delete button. I have to show all because there is a "revert" operation there too. Only those revisions that are checked will be deleted when you click the button.
I am working on the "List" tab page (and consequently Cron) to apply the selection rules. When I am done, it will be more like the "list revisions" page, possibly eliminating the need for the "list revisions" operation.
#12
Well, this turned out to be a much larger change than I expected, but future changes should be much easier. Oh, Frank, you will be happy to know that the "Current age" setting has been moved.
Committed to both branches.
#13
Good job, Nancy, thanks!
Just one last thing: there is a string 'It will delete revisions that are older than !age_interval.' on the list page (/admin/content/revision_deletion). This string could be completed by a something like 'if the current version is older than ...'. This part does not need to be displayed if current version age is set to always.
I also suggest to put 'It was last run !last_update_time (!last_update_ago ago).' on a new line for a better overview.
#14
I think my new patch clarifies the messages. Please check it out when it rolls up.
#15
Included in the -rc1 versions.