Support from Acquia helps fund testing for Drupal Acquia logo

Comments

CTaPByK created an issue. See original summary.

CTaPByK’s picture

Issue summary: View changes
CTaPByK’s picture

Assigned: Unassigned » CTaPByK
FileSize
1.46 KB

I'm not do so much here, i try to make this method to be non recursive, but i stuck with translation->get() and offset, i cant get right field to set translation. I will continue with this later.

thenchev’s picture

Status: Active » Needs review
FileSize
3.09 KB

Here is some test coverage.
Maybe i didn't understand correctly about the test. In testEmbeddedReferences i used reference field to test if they are working but seems they are. Should I add paragraph as dependency to test it directly? I can redo the test...

Berdir’s picture

Title: Integration with paragraph module not working in preview. » Add tests for embedded fields/translatiosn
Category: Bug report » Task
FileSize
4.03 KB
5.01 KB

Fun. The test isn't failing because there's nothing to fail. This works perfectly fine with nodes and normal entity references.

Took me a while to debug, but what's special about paragraphs is that they're not in entity reference fields. They're in entity reference *revision* fields. And those have some copy & pasted code that breaks what we are doing: #2672206: EntityReferenceRevisionsFormatterBase::prepareView() is blindly copied and lies

With that patch, it works perfectly... For entity revision reference fields and any entity type that has static caching enabled. See the additional comments and @todo. We should probably open a follow-up for that.

I also made some improvements to the test. We already have the fields and embedded field configuration, so we don't need that. We just need the entity view display configuration (which I switched to entity view so that we can test the body too) and creating the content + job + preview. With this setup, we could even test a nested child as well.

Changing this to a task for writing test coverage as there is no bug here.

Berdir’s picture

Title: Add tests for embedded fields/translatiosn » Add tests for embedded fields/translations
slashrsm’s picture

Status: Needs review » Reviewed & tested by the community

Test looks good to me and patch in entity reference revision makes sense. Should we get this in and open a follow-up for @todo?

Berdir’s picture

  • Berdir committed e67ec0e on 8.x-1.x
    Issue #2667802 by CTaPByK, Berdir, Denchev: Add tests for embedded...
Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Still applied with git apply -3, even though the cache part already got in with #2682589: Preview not invalidated on change

Status: Fixed » Closed (fixed)

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