Problem/Motivation

In #2924724: Add an API to create a new revision correctly handling multilingual pending revisions we introduced a new API to instantiate new revisions handling translations properly. The logic was partly borrowed from the node revision translation form logic. However this logic is still there when it could now rely on the new API.

Proposed resolution

Replace the old code with new API.

Remaining tasks

  • Validate the proposed solution
  • Write a patch
  • Reviews

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#2 node-revision_revert-2939107-2.patch2.73 KBplach

Comments

plach created an issue. See original summary.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Assigned: plach » Unassigned
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new node-revision_revert-2939107-2.patch2.73 KB
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Title: Clean up the revision translation revert logic to leverage the new API » Clean up the node revision translation revert logic to leverage the new API
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Clean up the node revision translation revert logic to leverage the new API » Follow-up for #2924724: Clean up the node revision translation revert logic to leverage the new API

From 18 LoC to 3? Yes, please!

Just one question:

+++ b/core/modules/node/src/Tests/NodeRevisionsTest.php
@@ -64,6 +64,9 @@ protected function setUp() {
+    // Enable translation for page nodes.
+    \Drupal::service('content_translation.manager')->setEnabled('node', 'page', TRUE);

Why was this not necessary before?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

Why was this not necessary before?

Because the new code explicitly checks whether translation is enabled before applying the multilingual logic (optimization), while the previous one was just checking whether the entity was translated, which of course can only happen if translation was enabled before :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

  • Gábor Hojtsy committed db9c0e6 on 8.5.x 
    Issue #2939107 by plach, Wim Leers: Follow-up for #2924724: Clean up the...

  • Gábor Hojtsy committed 6538805 on 8.6.x 
    Issue #2939107 by plach, Wim Leers: Follow-up for #2924724: Clean up the...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Superb, thanks, much cleaner!

Status: Fixed » Closed (fixed)

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