As #2840538: If parent-child entities are referenced and have the same field name, then nested includes don't work and #2840337: "Invalid nested filtering" with Field Collection entities and more preceding issues show: \Drupal\jsonapi\Context\FieldResolver is not as reliable as it should be. I suspect bugs in this area are the source of most common frustrations/"WTF moments" for JSON API users. So lets' fix that.

CommentFileSizeAuthor
#12 interdiff-2842145.txt2.78 KBdawehner
#12 2842145-12.patch7.19 KBdawehner
#9 2842145-9.patch5.85 KBdawehner
#9 interdiff-2842145.txt4.16 KBdawehner
#4 2842145-4.patch4.95 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Add comprehensive unit test coverage for FieldResolver » Add comprehensive unit test coverage for \Drupal\jsonapi\Context\FieldResolver
dremy’s picture

dremy CreditAttribution: dremy commented

+1 Thanks Wim

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Active » Needs review
FileSize
2842145-4.patch4.95 KB

There we go.

Status: Needs review » Needs work

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented 
+++ b/tests/src/Kernel/Context/FieldResolverTest.php
@@ -0,0 +1,86 @@
+    \Drupal::requestStack()->pu

PHPLint failure looks legit.

Also WOOHOOO PHPLINTING

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented

Well yeah, this is just a start and I got distracted.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented

I believe that the patch provided contains some code from another patch (the one with listings and access).

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs work » Needs review
FileSize
interdiff-2842145.txt4.16 KB
2842145-9.patch5.85 KB

Here is a bit more of a start.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented

We should add a field with multiple properties so we can assert:

$this->assertEquals('field_test_ref1.entity.body.format', $this->sut->resolveInternal('field_test_ref1.body.format'));
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Status: Needs review » Needs work
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs work » Needs review
FileSize
2842145-12.patch7.19 KB
interdiff-2842145.txt2.78 KB

Good idea!

  • e0ipso committed 1819338 on 8.x-1.x authored by dawehner 
    test(Query Builder) Add comprehensive unit test coverage for \Drupal\...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Status: Needs review » Fixed

This looks great now!

Status: Fixed » Closed (fixed)

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