Problem/Motivation

To support query logic changes in #3118464: Add support for "Maximum number allowed" revisions per content node translation and #3094354: Delete draft revisions safely, I'm proposing to refactor NodeRevisionDelete:getCandidatesRevisionsByNids() as NodeRevisionDelete:getCandidatesRevisionsByNid() to only accept a single node id so that the query logic found within NodeRevisionDelete:getCandidatesRevisionsByNids() and use a subquery to only find translated revisions that are older than the current translation version.

Proposed resolution

This solution relies upon patch from #3127258-4: Replace CandidateNodesForm::submitForm usage of getCandidatesRevisionsByNids to remove the only usage of NodeRevisionDelete:getCandidatesRevisionsByNids() with an array of node IDs. Once removed, we can refactor NodeRevisionDelete:getCandidatesRevisionsByNids() as NodeRevisionDelete:getCandidatesRevisionsByNid() to accept a single node id.

Remaining tasks

  • Consensus
  • Patch
  • Feedback

User interface changes

None

API changes

Changes to NodeRevisionDelete interface and NodeRevisionDelete:getCandidatesRevisionsByNids().

Data model changes

None

Release notes snippet

TBD

CommentFileSizeAuthor
#3 node_revision_delete-refactor_getCandidatesRevisionsByNids-3127265-3.patch4.47 KBjasonawant

Comments

jasonawant created an issue. See original summary.

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented
Assigned: Unassigned » jasonawant
Related issues: +#3127258: Replace CandidateNodesForm::submitForm usage of getCandidatesRevisionsByNids, +#3118464: Add support for "Maximum number allowed" revisions per content node translation, +#3094354: Delete draft revisions safely

adding related issue links, self-assigned to upload patch

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented
Assigned: jasonawant » Unassigned
Status: Active » Needs review
StatusFileSize
new node_revision_delete-refactor_getCandidatesRevisionsByNids-3127265-3.patch4.47 KB

Here's a patch for review. This patch requires patch from #3127258-4: Replace CandidateNodesForm::submitForm usage of getCandidatesRevisionsByNids.

adriancid’s picture

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

Why we need to remove the use of the array as a parameter when we can make something like:

public function getCandidatesRevisionsByNid($nid) {
  return $this->getCandidatesRevisionsByNids([$nid]);
}
jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented

@adriancid,

The query logic found within the current getCandidatesRevisionsByNids() method relies upon the array here

$sub_query->condition('n.nid', $nids, 'IN');

My attempt to to account for # of revisions per translation in #3118464-13: Add support for "Maximum number allowed" revisions per content node translation relies upon removing the provided nids array and change this query logic. So, to adjust the query logic in 3118464, I had to remove the only usage of the array in #3127258: Replace CandidateNodesForm::submitForm usage of getCandidatesRevisionsByNids and then remove it here in this issue.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

Hi @jasonawant thanks for your patches and for your time, I really appreciate the time you are putting on the module, unfortunately, I'm missing time on these days, I hope in the coming days I will have some hours to check all the patches. Thanks for your contributions.

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented

I totally understand; no worries.

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Active » Closed (outdated)

Closing because the current development is being made in branch 2.x