Comments

mariancalinro’s picture

stefan.r’s picture

The last submitted patch, 2: drupal8.bundle-constraint-test-only.1845546-2.patch, failed testing.

fago’s picture

Status: Needs review » Reviewed & tested by the community

Good work!

This shows why we should have these tests :-) This would need a re-roll to move the test to the Entity component tests once #2137301: Move entity constraints under the Entity component is in.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

#2137301: Move entity constraints under the Entity component was just committed so this will need a re-roll.

fago’s picture

Status: Needs work » Needs review
StatusFileSize
new4.1 KB
new1.8 KB

re-rolled and updated the test to work with HEAD.

fago’s picture

6: d8_bundleconstraint_test.patch queued for re-testing.

yched’s picture

Minor stuff, otherwise RTBC.

  1. +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/BundleConstraintValidatorTest.php
    @@ -0,0 +1,82 @@
    +   * Tests the constraint validation BundleConstraintValidator.
    

    Language seems weird ?

  2. +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/BundleConstraintValidatorTest.php
    @@ -0,0 +1,82 @@
    +    $article_node = $this->container->get('entity.manager')->getStorageController('node')->create(array('type' => 'foo'));
    

    var is misnamed ?

berdir’s picture

6: d8_bundleconstraint_test.patch queued for re-testing.

The last submitted patch, 6: d8_bundleconstraint_test.patch, failed testing.

fago’s picture

Re-rolled to account for the typed data manager rename and addressed #8.

yched’s picture

Status: Needs review » Reviewed & tested by the community

Same remark as #2142991-10: Test for ComplexDataConstraintValidator about $this->typedData, but otherwise RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

catch’s picture

Status: Fixed » Needs work

This broke HEAD, not sure if it is consistent or random:

 Drupal\system\Tests\Entity\BundleConstraintValidatorTest (1 pass(es), 1 fail(s), and 0 exception(s))
   - [fail] [Completion check] "The test did not complete due to a fatal error." in BundleConstraintValidatorTest.php on line 44 of Drupal\system\Tests\Entity\BundleConstraintValidatorTest->testValidation().

Rolled back for now.

damiankloip’s picture

Status: Needs work » Needs review
StatusFileSize
new4.08 KB
new721 bytes

Here is patch rerolled with fix. We had the fix but couldn't beat catch to the revert :)

fago’s picture

Status: Needs review » Reviewed & tested by the community

Looks like things changed a bit since when the patch got re-rolled :-/

berdir’s picture

Looks like the following change broke it:

$ git show 814aed core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
commit 814aed203d3db4c5dcab569ae6754bc575558486
Author: Alex Pott <alex.a.pott@googlemail.com>
Date:   Wed Jan 1 11:08:57 2014 +0100

    Issue #2005716 by tim.plunkett, msonnabaum, dawehner, alexpott, effulgentsia: Promote EntityType to a domain object.

diff --git a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
index 655f4fd..40f7a3a 100644
--- a/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
@@ -51,7 +51,7 @@ public function getPropertyDefinitions() {
 
     if (!isset(static::$propertyDefinitions[$key])) {
       $target_type_info = \Drupal::entityManager()->getDefinition($target_type);
-      if (is_subclass_of($target_type_info['class'], '\Drupal\Core\Entity\ContentEntityInterface')) {
+      if ($target_type_info->isSubclassOf('\Drupal\Core\Entity\ContentEntityInterface')) {
         // @todo: Lookup the entity type's ID data type and use it here.
         // https://drupal.org/node/2107249
         static::$propertyDefinitions[$key]['target_id'] = DataDefinition::create('integer')

Which is quite interesting, because this does apparently not cause any php warnings or notices at all:

$target_type_info = FALSE;
var_dump(is_subclass_of($target_type_info['class'], '\Drupal\Core\Entity\ContentEntityInterface'));

It just says bool(false).

Anyway, the fix is correct and should have been there from the beginning.

yched’s picture

Got bitten by this too in #2144919-95: Allow widgets and formatters for base fields to be configured in Field UI

It's a case where $target_type_info is NULL, because the test doesn't enable the module that defines the entity type.

Before #2005716: Promote EntityType to a domain object, the old code did :
is_subclass_of($target_type_info['class'], 'ContentEntityInterface')
i.e is_subclass_of(NULL['class'], 'ContentEntityInterface')
i.e is_subclass_of(NULL, 'ContentEntityInterface')
i.e FALSE

The new code does :
$target_type_info->isSubclassOf('ContentEntityInterface')
i.e NULL->isSubclassOf('ContentEntityInterface')
i.e fatal error

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, let's try this again. :)

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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