This is easily reproducible in Lightning, when the Lightning Preview component is installed: #2817231: Author of node entities is not replicated

The crux of the issue is that EntityReferenceItemNormalizer mistakenly overwrites an entire array when it should be adding an element to it. This patch corrects the error. Should probably have an automated test (a unit test of EntityReferenceItemNormalizer would suffice, IMHO), but I tested it manually in Lightning and it works.

CommentFileSizeAuthor
#10 interdiff.txt452 bytesjeqq
#10 2820105-10.patch4.55 KBjeqq
#9 2820105-7.patch4.25 KBjosephdpurcell
#7 interdiff-2820105-4-7.txt921 bytesphenaproxima
#7 2820105-7.patch4.25 KBphenaproxima
#4 interdiff-2820105-2-4.txt3.2 KBphenaproxima
#4 2820105-4.patch4.01 KBphenaproxima
#2 2820105-2.patch675 bytesphenaproxima
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Active » Needs review
FileSize
2820105-2.patch675 bytes
timmillwood’s picture

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

I'm struggling a little to follow the logic, but it sounds plausible that this is the issue.

Would you be able to add an EntityReferenceItemNormalizerTest? It looks as though we don't even have one https://github.com/relaxedws/drupal-replication/tree/8.x-1.x/tests/src/U...

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
2820105-4.patch4.01 KB
interdiff-2820105-2-4.txt3.2 KB

Okay, here is a test of EntityReferenceItemNormalizer.

josephdpurcell’s picture

josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commented

This looks really good--an important find, obviously. I don't see any required changes, but have some suggestions:

+++ b/tests/src/Unit/Normalizer/EntityReferenceItemNormalizerTest.php
@@ -0,0 +1,102 @@
+  public function testEntityReferenceFieldNormalization() {

Rename this to testUserReferenceFieldNormalization and update the doc comment.

Add a @todo to write a generic testEntityReferenceFieldNormalization.

Reason: the EntityReferenceItemNormalizer treats users as a special case.

Also add a @todo to test the user mapping.

Also add a @todo to test the username in the field info.

josephdpurcell’s picture

josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commented
Priority: Normal » Major
Status: Needs review » Needs work

Escalating to Major since there is no workaround for this.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
2820105-7.patch4.25 KB
interdiff-2820105-4-7.txt921 bytes

Fixed all but the last one, since that is specifically what is being fixed (and tested) by this patch. :)

josephdpurcell’s picture

josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commented

This looks good, assuming tests pass it can be RTBC'd.

josephdpurcell’s picture

josephdpurcell CreditAttribution: josephdpurcell at Digital Bridge Solutions commented
FileSize
2820105-7.patch4.25 KB

Will a re-upload work?

jeqq’s picture

jeqq
Primary language Romanian
CreditAttribution: jeqq at FFW for Pfizer, Inc. commented
FileSize
2820105-10.patch4.55 KB
interdiff.txt452 bytes

A fix for type check.

  • jeqq committed fd81550 on 8.x-1.x authored by phenaproxima 
    Issue #2820105 by phenaproxima, jeqq, josephdpurcell, timmillwood: User...
jeqq’s picture

jeqq
Primary language Romanian
CreditAttribution: jeqq at FFW for Pfizer, Inc. commented
Status: Needs review » Fixed

Thank you!

The last submitted patch, 2: 2820105-2.patch, failed testing.

The last submitted patch, 2: 2820105-2.patch, failed testing.

The last submitted patch, 4: 2820105-4.patch, failed testing.

The last submitted patch, 4: 2820105-4.patch, failed testing.

The last submitted patch, 7: 2820105-7.patch, failed testing.

The last submitted patch, 7: 2820105-7.patch, failed testing.

The last submitted patch, 9: 2820105-7.patch, failed testing.

The last submitted patch, 2: 2820105-2.patch, failed testing.

The last submitted patch, 9: 2820105-7.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 10: 2820105-10.patch, failed testing.

The last submitted patch, 10: 2820105-10.patch, failed testing.

The last submitted patch, 4: 2820105-4.patch, failed testing.

The last submitted patch, 7: 2820105-7.patch, failed testing.

The last submitted patch, 9: 2820105-7.patch, failed testing.

The last submitted patch, 10: 2820105-10.patch, failed testing.

jeqq’s picture

jeqq
Primary language Romanian
CreditAttribution: jeqq at FFW for Pfizer, Inc. commented
Status: Needs work » Closed (works as designed)

Tests started to run after the branch has been fixed and patch commited.