Problem/Motivation

We still have a fatal errors with JSON:API module.

#3570382: Fatal: Compatibility with JSON API was not useless, because a simple page refresh is "fixing" it after, but was not doing 100% of the job.

Proposed resolution

InstanceStorage must implements ContentEntityStorageInterface like Instance is implementing ContentEntityInterface

Issue fork display_builder-3575913

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

pdureau created an issue. See original summary.

mogtofu33’s picture

mogtofu33 commented

Looks like we either have to go full contentEntity with contentEntityStorage or push the json:api patch to core?

pdureau’s picture

pdureau commented

Looks like we either have to go full contentEntity with contentEntityStorage or push the json:api patch to core?

Exactly. Both are good:

pdureau’s picture

pdureau commented
Status: Active » Needs work

Making progress, will push soon

pdureau opened merge request !236

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Hello Jean,

I have pushed a proposal.

Good news, the change "forced" me to remove the fake "src/Entity/ContentEntityBase.php" added in beta2, so the MR is removing 300 lines of code :)

Main changes:

  1. Instance now have #[ContentEntityType( as attributes (this is the part expected by JSON API)
  2. ContentEntityType attributes is expecting ContentEntityStorageInterface so it extends ContentEntityStorageBase now, with the addition of a few useless but necessary methods.
  3. Because of ContentEntityStorageBase::initFieldValues(). and similar mechanisms, some of Instance class properties must now be public

Playwright is green.

PHPunit has a fail, but I don't know how to fix:

1) Drupal\Tests\display_builder\Kernel\InstanceTest::testIsNew
Error: Typed property Drupal\display_builder\Entity\Instance::$id must not be accessed before initialization
mogtofu33’s picture

mogtofu33 commented
Status: Needs review » Needs work

Seems like we can have a bit more cleanup, but I have issues with existing instances (with saved states), I will check for a hook update.

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs work » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • mogtofu33 committed b145fb62 on 1.0.x authored by pdureau 
    fix: #3575913 Compatibility with JSON API, part 2

By: pdureau
By:...

Status: Fixed » Closed (fixed)

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