Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

Status: Active » Needs review
StatusFileSize
new17.65 KB

Patch

Status: Needs review » Needs work

The last submitted patch, 2: 3198498-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

claudiu.cristea’s picture

Status: Needs work » Needs review
StatusFileSize
new4.16 KB
new4.16 KB

Fixing tests.

claudiu.cristea’s picture

StatusFileSize
new1.49 KB
new19.93 KB

More tests fixes.

pfrenssen’s picture

Status: Needs review » Needs work
  1. diff --git a/src/MetaEntityRepositoryInterface.php b/src/MetaEntityRepositoryInterface.php
    index f3ec3d9..ca8a531 100644
    --- a/src/MetaEntityRepositoryInterface.php
    +++ b/src/MetaEntityRepositoryInterface.php
    @@ -63,4 +63,15 @@ interface MetaEntityRepositoryInterface {
    +  /**
    +   * Returns a list of meta entity types that are configured with auto-creation.
    +   *
    +   * @return string[]
    +   *   A list o meta entity type IDs.
    +   */
    

    Small typo "A list _of_ meta entity type IDs".

  2. --- a/src/MetaEntityRepository.php
    +++ b/src/MetaEntityRepository.php
    @@ -175,4 +175,41 @@ class MetaEntityRepository implements MetaEntityRepositoryInterface {
    +  public function getTypesWithAutoCreation(ContentEntityInterface $entity): array {
    +    $cid = 'auto_create';
    +    $bundle = $entity->bundle() ?: $entity->getEntityTypeId();
    

    What motivated you to distrust the value returned by $entity->bundle() and provide a fallback to the entity type ID? Did you see a case where a certain entity type returns NULL for the bundle?

    The method as described in EntityInterface::bundle() already specifies that the entity type ID is returned as a fallback, and returning NULL is not allowed according to the interface:

      /**
       * Gets the bundle of the entity.
       *
       * @return string
       *   The bundle of the entity. Defaults to the entity type ID if the entity
       *   type does not make use of different bundles.
       */
      public function bundle();
    

    Of course in Drupal core this is not enforced yet (it lacks strict return types), but it seems to me if you found a case where this returns NULL then this is probably a bug in the entity type, and not strictly needed for us to provide a fallback.

  3. diff --git a/src/Form/MetaEntityTypeForm.php b/src/Form/MetaEntityTypeForm.php
    index 614b0b9..6f37629 100644
    --- a/src/Form/MetaEntityTypeForm.php
    +++ b/src/Form/MetaEntityTypeForm.php
    @@ -158,11 +158,12 @@ class MetaEntityTypeForm extends BundleEntityFormBase {
    +        $form['mapping'][$entity_type_id]['bundles'][$bundle_id]['settings'] = [
    ...
    +          '#title' => $this->t('Settings for  @bundle @entity_type', [
    

    Small typo: double space after "Settings for ".

ankithashetty’s picture

StatusFileSize
new19.93 KB
new1015 bytes

Updated the patch in #5 addressing #6.1 and #6.3. Retaining status as "Needs work" to address #6.2... Thanks!

claudiu.cristea’s picture

Status: Needs work » Needs review
StatusFileSize
new1.27 KB
new19.91 KB

@pfrenssen #6.2, very good observation. Fixed.

@ankithashetty, thank you for fixing.

Status: Needs review » Needs work

The last submitted patch, 8: 3198498-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

claudiu.cristea’s picture

Status: Needs work » Needs review
StatusFileSize
new2.77 KB
new20.65 KB

Fixing test failure and coding standards. Adding credits.

claudiu.cristea’s picture

StatusFileSize
new20.56 KB

Reroll.

claudiu.cristea’s picture

StatusFileSize
new20.56 KB
new430 bytes

CS fix.

claudiu.cristea’s picture

StatusFileSize
new20.56 KB

The interdiff was correct in #12 but not the patch.

pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the fast fixes @ankithashetty and @claudiu.cristea!

It's looking good now, thanks a lot!

pfrenssen’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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