Problem/Motivation

Standardize a create and wrap new entities. This would allow for easy implementation of default values.

Proposed resolution

Have a createEntity() (maybe wrapNew()) function in the TypedRepositoryInterface.
An added bonus is the TypedRepositoryInterface already knows the bundle and this can be autofilled.

Issue fork typed_entity-3213360

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Birk created an issue. See original summary.

Birk’s picture

Birk CreditAttribution: Birk at B14 commented

I've added a proposed code solution in the issue fork. Only consisting of logic and no tests.

This is my first tumble with the forking approach, so let me know if I'm missing something.

e0ipso’s picture

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

I do not see a merge request in GitLab. I could access the code changes, in this link https://git.drupalcode.org/issue/typed_entity-3213360/-/compare/4.x...32... but I cannot merge them.

---

Feedback:

This is a fantastic idea. A couple of thoughts:

  1. Do you think adding a WrappedEntityStorageTrait and WrappedEntityStorageInterface would help achieve the same goal? That way we can provide a solution to a common use case, yet keep the base repository a bit cleaner. Maybe we can end up moving some stuff out from the base repository in the future.
  2. I see that the return type is WrappedEntityInterface|null. When do you think you will get null in this case?
e0ipso’s picture

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

Birk CreditAttribution: Birk at B14 commented

I thought you only opened the merge request once the code was complete, but I'll open it up straight away.

---

Not sure a trait would work, the new createEntity() function depends on multiple properties of the TypedRepository class (bundle, entityTypeManager and entityType).
I haven't been able to find anything official on traits in Drupal, but with a few random samples from core it seems all traits are self contained, and make no assumption of the classes using them (which makes sense).

This doesn't rule out splitting the repository class into traits, but it seems like a refactoring issue worthy of it's own ticket.

Good catch with the null, that's an oversight, I also switch between getEntityType and entityType, I'll fix it.

Birk opened merge request !10

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot for IBM commented
Status: Needs work » Reviewed & tested by the community

This looks good. Despite the interface being updates, since there is a base class for it we can merge this withoug BC breaks.

  • e0ipso committed f397704 on 4.x authored by Birk 
    Issue #3213360 by Birk, e0ipso: Create new entity via TypedRepository
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot for IBM commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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