Problem/Motivation

Now that we added the minimum age to delete and the when to delete variables in #2911131: Add a config variable to configure the time for the 'when_to_delete' and 'minimun_age_to_delete' variables we need to recalculate the candidates nodes.

Proposed resolution

Update the _node_revision_delete_candidates() function to show the real candidates nodes that have revisions to delete.

Notes about how revisions works

  • Each time a new revision is created in one language, a revision is created for each of the other active languages, therefore in the table node, the vid field always refers to the id of the current revision for all the active languages on the site.
  • Regardless of the language, the creation time of the revision is always in the node_revision table in the revision_timestamp field.

Remaining tasks

#3075824: Identify the candidate nodes using only the minimum number of revisions to keep
#3076227: Identify the candidate nodes using the minimum age of revisions to delete
#3076228: Identify the candidate nodes using the when to delete configuration

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#14 2911844.patch17.83 KBlarowlan
#12 node_revision_delete-identify_candidate_nodes-2911844-12.patch.patch19.29 KBnplowman
#8 node_revision_delete-identify_candidate_nodes-2911844-8-D8.patch16.21 KBkamkejj
#7 interdiff.txt16.65 KBadriancid
#7 node_revision_delete-identify_candidate_nodes-2911844-7-D8.patch15.98 KBadriancid
#6 node_revision_delete-count_candidate_nodes-2911844-6.patch17.83 KBec0g
#3 node_revision_delete-count_candidate_nodes-2911844-0.patch16.52 KBec0g

Comments

adriancid created an issue. See original summary.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Parent issue: » #2808383: Node Revision Delete Drupal 8 port
ec0g’s picture

ec0g commented
StatusFileSize
new node_revision_delete-count_candidate_nodes-2911844-0.patch16.52 KB

Hi @adriancid,

I have some code that should resolve this issue. We have a need to use this functionality so I figured we can contribute some code back to help with the effort.

I have a branch ready with the code to push to the repo (just need access). I added a patch file here so you can take a look at the code.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Active » Needs work

Thanks @ec0g for the patch.

I'm returning from holidays, there is something that I see very quick, you use:

' months ago'

But the time can be weeks and days too, you need to check the configuration for this.

Read more at: https://www.drupal.org/project/node_revision_delete/issues/2892807#comme...

ec0g’s picture

ec0g commented

Thanks for the feedback @adriancid. Hope your holidays were great.

I will check the configuration and update the code. I should have an updated patch in the next few days.

ec0g’s picture

ec0g commented
StatusFileSize
new node_revision_delete-count_candidate_nodes-2911844-6.patch17.83 KB

Translate the time interval selected by the user into a \DateTime object.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
StatusFileSize
new node_revision_delete-identify_candidate_nodes-2911844-7-D8.patch15.98 KB
new interdiff.txt16.65 KB

Hi @ec0g, thanks again for the patch, I don't have time yet to check if the nodes candidates are correctly identified (one reason is that I don't have a database with revisions, so if you can provide me one this will help me) but I have some remarks (I fixed some of them) that should be fixed in the code concerning the coding standards.

FILE: ...ode_revision_delete/src/Form/NodeRevisionDeleteAdminSettings.php
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
 317 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 317 | ERROR | Missing parameter type
     |       | (Drupal.Commenting.FunctionComment.MissingParamType)
 318 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 318 | ERROR | Missing parameter type
     |       | (Drupal.Commenting.FunctionComment.MissingParamType)
 320 | ERROR | Description for the @return value is missing
     |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
----------------------------------------------------------------------


FILE: ...s/drupal.org/node_revision_delete/src/RevisionQueryInterface.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
  5 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 13 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 20 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 27 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
----------------------------------------------------------------------


FILE: ...pal.org/node_revision_delete/src/WorkQuery/RevisionWorkQuery.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
  7 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 12 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 19 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 26 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
----------------------------------------------------------------------


FILE: ...al.org/node_revision_delete/src/WorkQuery/WorkQueryInterface.php
----------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  7 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 12 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 13 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 17 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 18 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 22 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 23 | ERROR | Missing parameter comment
    |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 24 | ERROR | Missing parameter comment
    |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
----------------------------------------------------------------------


FILE: .../drupal.org/node_revision_delete/src/WorkQuery/NodeWorkQuery.php
----------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 67 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 85 | ERROR | Missing parameter comment
    |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 85 | ERROR | Missing parameter type
    |       | (Drupal.Commenting.FunctionComment.MissingParamType)
 86 | ERROR | Missing parameter comment
    |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 86 | ERROR | Missing parameter type
    |       | (Drupal.Commenting.FunctionComment.MissingParamType)
 87 | ERROR | Missing parameter comment
    |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 89 | ERROR | Description for the @return value is missing
    |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
----------------------------------------------------------------------


FILE: .../drupal.org/node_revision_delete/src/WorkQuery/BaseWorkQuery.php
----------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  5 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 10 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 15 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 20 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 25 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 30 | ERROR | Missing short description in doc comment
    |       | (Drupal.Commenting.DocComment.MissingShort)
 35 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
 42 | ERROR | Doc comment is empty
    |       | (Drupal.Commenting.DocComment.Empty)
----------------------------------------------------------------------


FILE: ...st/modules/drupal.org/node_revision_delete/src/RevisionQuery.php
----------------------------------------------------------------------
FOUND 18 ERRORS AFFECTING 18 LINES
----------------------------------------------------------------------
   9 | ERROR | Doc comment is empty
     |       | (Drupal.Commenting.DocComment.Empty)
  14 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  19 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  24 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  29 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  34 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  39 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
  51 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
  52 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
  53 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
  54 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
  77 | ERROR | Description for the @return value is missing
     |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
  85 | ERROR | Doc comment is empty
     |       | (Drupal.Commenting.DocComment.Empty)
 100 | ERROR | Description for the @return value is missing
     |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 119 | ERROR | Description for the @return value is missing
     |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
 150 | ERROR | Missing short description in doc comment
     |       | (Drupal.Commenting.DocComment.MissingShort)
 151 | ERROR | Missing parameter comment
     |       | (Drupal.Commenting.FunctionComment.MissingParamComment)
 153 | ERROR | Description for the @return value is missing
     |       | (Drupal.Commenting.FunctionComment.MissingReturnComment)
----------------------------------------------------------------------
kamkejj’s picture

kamkejj commented
StatusFileSize
new node_revision_delete-identify_candidate_nodes-2911844-8-D8.patch16.21 KB

Here's a re-work of patch 7 with the formatting fixed. Not all comments are necessarily meaningful.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

Hi @kamkejj thanks for the patch, I don't have time to test this in the next days, do you test it and it really has shown the candidates nodes?

kamkejj’s picture

kamkejj commented

Didn't get a chance to test today, but will tomorrow.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

I tried to apply your patch but it doesn't work, should I need to apply the patch in #7 or in #6 before?

nplowman’s picture

nplowman commented
StatusFileSize
new node_revision_delete-identify_candidate_nodes-2911844-12.patch.patch19.29 KB

I went back through the previous patches and tried to piece things together so the patch would apply cleanly.
I don't have the full context to know how to test it right now, but I'm interested in helping get this D8 port complete, so I'll likely continue to investigate and contribute more in the following weeks.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

@nplowman thanks, will be great if you can help with this. We need to identify the candidate nodes with revisions to delete, one problem that I have is that I need a database with revisions or a drush command or something to create many revisions to test this functionality.

I'm creating now some tests for the module because I'm don't have enough time to develop this functionality and the other that will delete the revisions.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review
StatusFileSize
new 2911844.patch17.83 KB

re-roll

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Issue summary: View changes
volkswagenchick’s picture

volkswagenchick
she/her
Primary language English
Location San Francisco Bay Area
commented
Issue tags: +drupalnorth2019

Tagging for DrupalNorth 2019

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Assigned: Unassigned » adriancid
Category: Task » Plan
Issue summary: View changes
Status: Needs review » Active
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Active » Needs work
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Issue summary: View changes
Issue tags: -drupalnorth2019
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Issue summary: View changes
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.