Overview

Looking at #3488368: Also convert metadata (page data) fields in ClientDataToEntityConverter where we will be updating ClientDataToEntityConverter and needing to add more tests

I realized that we have logic in ApiContentUpdateForDemoControllerTest that really tests, ClientDataToEntityConverter and ClientServerConversionTrait. We will need to change these test cases in #3488368

I had mentioned in #3489994-16: Create an endpoint to publish all auto-saved entities that would move the tests coverage out of ApiContentUpdateForDemoControllerTest because this a temporary class needed for demo logic.

In #3488368: Also convert metadata (page data) fields in ClientDataToEntityConverter I would really like to not update ApiContentUpdateForDemoControllerTest more metadata test cases.

Proposed resolution

Move test cases out of ApiContentUpdateForDemoControllerTest and into their own tests of ClientDataToEntityConverter and ClientServerConversionTrait

User interface changes

None

Issue fork experience_builder-3494273

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tedbow created an issue. See original summary.

tedbow opened merge request !476

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue summary: View changes
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review

Asking for @wim leers review again. The test cases are being moved to

  1. \Drupal\Tests\experience_builder\Kernel\ClientServerConversionTraitTest::testConvertClientToServerErrors
  2. or \Drupal\Tests\experience_builder\Kernel\ClientDataToEntityConverterTest::testConvert if the errors are not caught in \Drupal\experience_builder\Controller\ClientServerConversionTrait::convertClientToServer but only caught in \Drupal\experience_builder\ClientDataToEntityConverter::convert

\Drupal\Tests\experience_builder\Functional\ApiContentUpdateForDemoControllerTest::testSave is still left with test cases that prove that if the call to
$violations = $this->clientDataToEntityConverter->convert(json_decode($request->getContent(), TRUE), $entity);

gets an error entity will not be saved

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » larowlan
Status: Needs review » Reviewed & tested by the community

I'd love to see @larowlan approve this too, or suggest an alternative 🤞

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned

I think this controller is temporary code so this unblocks more work

  • wim leers committed 7de4064f on 0.x authored by tedbow 
    Issue #3494273 by tedbow, wim leers, larowlan: Move logic out of...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

Thanks @wim leers and @larowlan!

Status: Fixed » Closed (fixed)

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