Problem/Motivation

\Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::testPost() calls getNormalizedPostEntity() twice in order to get two valid representations of an entity for POST testing:

$parseable_valid_request_body   = $this->serializer->encode($this->getNormalizedPostEntity(), static::$format);
$parseable_valid_request_body_2 = $this->serializer->encode($this->getNormalizedPostEntity(), static::$format);

and there are two class variables $firstCreatedEntityId and $secondCreatedEntityId where we can specify the IDs for those two entities.

The problem is that calling getNormalizedPostEntity() twice without any possibility of returning different IDs per invocation assumes that the IDs are automatically generated by the DB layer. However, that's not the case for entity types with string IDs, where they are actually specified by the code which saves the entity (e.g. the entity form).

Proposed resolution

Add a EntityResourceTestBase::getSecondNormalizedPostEntity() method which allows test classes to specify a different, pre-determined ID for the second POST entity.

Remaining tasks

Review.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu created an issue. See original summary.

amateescu’s picture

Status: Active » Needs review
FileSize
1.1 KB
24.56 KB

This patch touches the same code as #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002), which is already RTBC, so I think it's easier if I just make the initial patch depend on it.

Btw, this is a blocker for #2784921: Add Workspaces experimental module.

amateescu’s picture

FileSize
2.08 KB
24.73 KB

Oops, wrong patches, I forgot include a hunk.

The last submitted patch, 2: 2935076-combined.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 3: 2935076-3-combined.patch, failed testing. View results

amateescu’s picture

Status: Needs work » Needs review
FileSize
23.74 KB

This is what happens when you don't check what branch you're working on :/ Maybe it would've been easier to not depend on #2885469: Regression: manually setting the ID field for newly-created content entities is not possible anymore (public follow-up to SA-2017-002)..

Wim Leers’s picture

Status: Needs review » Postponed
Issue tags: +API-First Initiative
amateescu’s picture

Status: Postponed » Reviewed & tested by the community
FileSize
2.08 KB

The blocker is in, re-uploading #3 and setting to RTBC per #7.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

catch’s picture

Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!

Version: 8.5.x-dev » 8.6.x-dev
Status: Fixed » Reviewed & tested by the community
  • catch committed 80aa325 on 8.6.x
    Issue #2935076 by amateescu: EntityResourceTestBase::...

  • catch committed c107021 on 8.5.x
    Issue #2935076 by amateescu: EntityResourceTestBase::...
catch’s picture

Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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