When trying to load a translation for a language not provided in the current revision of an entity you are met with the following fatal error.

Uncaught exception 'Error' with message 'Call to a member function wasDefaultRevision() on null'
in /docroot/core/modules/content_moderation/src/ModerationInformation.php

$latest_revision_id is null because $storage->getLatestTranslationAffectedRevisionId returns null if no revision affecting the specified translation could be found

The attached patch only changes the following:
if ($latest_revision_id != $default_revision_id) {
to
if ($latest_revision_id != $default_revision_id && $latest_revision_id != '') {
in core/modules/content_moderation/src/ModerationInformation.php

CommentFileSizeAuthor
#11 3001299-10.patch3.05 KBjoseph.olstad
#11 interdiff_6_to_10_issue-3001299.txt1.92 KBjoseph.olstad
#6 3001299-6.patch2.79 KBseanB
#6 3001299-6-fail.patch1.7 KBseanB
core-translation-pending-revision.patch1.43 KBdamondt
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damondt created an issue. See original summary.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Status: Active » Needs work
Issue tags: +Needs tests

Can you reproduce this in a kernel test?

damondt’s picture

damondt CreditAttribution: damondt commented

@timmillwood I don't know how to write a kernel test, but may be able to learn. Would it be intended to test the error or the patch? How would I submit it?

damondt’s picture

damondt CreditAttribution: damondt commented

I got some advice on writing tests, I'll see what I can do.

seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented

Just had the same error! Since there are several reasons why getLatestTranslationAffectedRevisionId() can return NULL (it is even documented on the method), checking for it makes sense.

seanB’s picture

seanB
Primary language Dutch
Location Netherlands
CreditAttribution: seanB commented
Version: 8.5.6 » 8.7.x-dev
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
3001299-6-fail.patch1.7 KB
3001299-6.patch2.79 KB

Patch and fail patch attached to test ModerationInformation::hasPendingRevision().

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

Looks great to me, I only found a small nitpick:

+++ b/core/modules/content_moderation/tests/src/Kernel/ModerationInformationTest.php
@@ -154,4 +154,35 @@ public function testIsDefaultRevisionPublishedMultilingual() {
+    $this->assertEquals(FALSE, $this->moderationInformation->hasPendingRevision($translated));
...
+    $this->assertEquals(FALSE, $this->moderationInformation->hasPendingRevision($translated));
...
+    $this->assertEquals(TRUE, $this->moderationInformation->hasPendingRevision($translated));

We could use assertTrue() and assertFalse() here :)

The last submitted patch, 6: 3001299-6-fail.patch, failed testing. View results

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

we're also observing this issue in a bilingual site (two languages), applied the core patch.

The above patch looks great, tests prove it.

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
FileSize
interdiff_6_to_10_issue-3001299.txt1.92 KB
3001299-10.patch3.05 KB

as requested
with the minor change from assertEquals to assertTrue
patch was rolled against 8.7.x

casaran’s picture

casaran CreditAttribution: casaran commented
Status: Needs review » Reviewed & tested by the community

The path in #11 worked fine for me.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 5cbd58e6a3 to 8.8.x and 2a9029d981 to 8.7.x. Thanks!

Credited @amateescu for issue review.

  • alexpott committed 5cbd58e on 8.8.x 
    Issue #3001299 by seanB, joseph.olstad, damondt, amateescu: Error...

  • alexpott committed 2a9029d on 8.7.x 
    Issue #3001299 by seanB, joseph.olstad, damondt, amateescu: Error...

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

@alexpott thanks, great to see this fixed for 8.8.x AND 8.7.x, it was not easy to find this.