Surely one of the main reason for moving from 7.x-2.x to 7.x-3.x is the ability to specify minimum age and when when to delete.
It's quite bad that this missing functionality isn't listed as a known issue as it leads to quite significant data loss.
Anyway, here is an updated _node_revision_delete_candidates() that does take these into account, not sure how/if the if (!$minimum_age_to_delete && !$when_to_delete) { idea is meant to work, but this seems like the way it should be done to me:
function _node_revision_delete_candidates($content_type, $minimum_revisions_to_keep, $minimum_age_to_delete, $when_to_delete) {
$params = array(
':content_type' => $content_type,
':max_revisions' => $minimum_revisions_to_keep,
':age' => strtotime($minimum_age_to_delete . ' months ago'),
':when' => strtotime($when_to_delete . ' months ago'),
);
$result = db_query('SELECT r.nid, count(*) as total
FROM {node} n
INNER JOIN {node_revision} r ON r.nid = n.nid
WHERE n.type = :content_type
AND r.timestamp > :age
AND n.changed > :when
GROUP BY r.nid
HAVING count(*) > :max_revisions
ORDER BY total DESC', $params);
return $result->fetchCol();
}
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | node_revision_delete-2892807-11.patch | 1.75 KB | mustanggb |
Comments
Comment #2
adriancid@MustangGB thanks for reporting, would be great if the next time you use the Issue Summary Template to report an issue.
If you go to read the 7.x-3.0-alpha1 release notes you can see:
I will check your code and made some test,
Thanks again.
Comment #3
adriancidComment #4
adriancidComment #5
mustanggb commented@adriancid What I meant was on the project page there is a section called "Known problems", would be nice if the issue was listed here, for the reason previously mentioned.
Anyway here's a patch.
Comment #6
adriancidThanks for the patch, I added the issue to the project page.
You need to consider in your code that month is not the only allowed time, we have days and weeks too.
Check the time configurations in admin/config/content/node_revision_delete
The variables are:
node_revision_delete_when_to_delete_time and node_revision_delete_minimum_age_to_delete_time
Comment #7
mustanggb commentedI did wonder about that, but was going off the function comment, I guess they'll need updating at some point as they all seem to refer to months only, as this is a module wide update that would be needed to fix the comments I'm going to call it out of scope for this issue.
Here is an updated version that checks for days/weeks, note there is no need to pass the value through
NODE_REVISION_DELETE_TIME_OPTIONSasstrtotime()doesn't care if you use the singular or plural form, which is nice because it means we can statically store the result.Comment #8
adriancid@MustangGB thanks for the patch I will test it in the next days.
If you have other ideas about how to improve the module please open new issues ;-)
Comment #9
adriancid@MustangGB I think that we need to work more in this, test this scenario:
Set article to:
Minimum to keep: 1
Minimum age: 2 day
When to delete: After 2 day of inactivity
Then go and create a new article and then a new revision, and you will see this new node as candidate.
Comment #10
adriancid@MustangGB if you can work on this will be great, I don't have time at this moment to add this feature.
Comment #11
mustanggb commentedFixed the typo
<instead of>.Comment #17
adriancidThanks @MustangGB it seems to works, we need more test, so I will release a beta in a few minutes and will wait that more users use the beta to see what happens, meanwhile I will finish the last drush command that I have in mind to manage the content types configurations.
And we need to port this patch to the Drupal 8 version :-)
Comment #18
adriancidComment #19
adriancidI'm closing this issue because I see that we don't delete any revision yet in the Drupal 8 version. I will open a new issue about the revision deletion in Drupal 8.
Comment #20
adriancidComment #21
adriancidComment #22
adriancidComment #23
dmitryl commentedHi @adriancid , is
_node_revision_delete_do_delete($nid, $minimum_revisions_to_keep, $dry_run)function supposed to be updated as well? Right now it doesn't accept$minimum_age_to_deleteparameter and deletes all revisions but$minimum_revisions_to_keep.Comment #24
adriancidHi @dmitryl, good catch, thanks, I think that you're right, we need to update the
_node_revision_delete_do_delete()function adding the$minimum_age_to_deleteparameter, we don't need to use the $when_to_delete because it is only needed to know if we will have candidates nodes or not.I think that is better handle this in #2915230: Use the minimum_age_to_delete variable in the node revision deletion
Feel free to help us doing this :-)