Follow-up from #2010930: [META] Apply formatters and widgets to rendered entity base fields (all besides node.title).

See the other issues from the parent for instructions.

Start with just the name and then fix the test failures, the user_id reference will be a bit more complicated and require a dependency on entity_reference.module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
19.41 KB

Added widgets for user_id and name. Made sure that they have default values, removing the values from the test forms which are all just using it to test specific field widgets, so we have enough coverage of those. Would be annoying to update them all, especially the user_id autocomplete widget.

Status: Needs review » Needs work

The last submitted patch, 1: entity-test-form-2238149-1.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
26.87 KB
7.46 KB

Fixing those tests. The missing user table fails are because of the default value for the user_id field in entity_test. I think that's a small price to pay for all the user_id's in the forms that we can just remove instead of having to build a valid "username (id)" string for an entity reference widget.

Wim Leers’s picture

Status: Needs review » Needs work

Looking good :)

+++ b/core/modules/system/src/Tests/KeyValueStore/KeyValueContentEntityStorageTest.php
@@ -10,13 +10,14 @@
 use Drupal\simpletest\DrupalUnitTestBase;

This should be removed.

  • +++ b/core/modules/system/src/Tests/KeyValueStore/KeyValueContentEntityStorageTest.php
    @@ -26,6 +27,15 @@ class KeyValueContentEntityStorageTest extends DrupalUnitTestBase {
    +  }
    +
    +
    +  /**
    

    2 newlines instead of 1.

  • +++ b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTest.php
    @@ -99,8 +103,20 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    +      ->setDefaultValue(array(0 => 1))
    

    Why array(0 => 1) and not just array(1)?

    Because you want to stress that it's a single-valued field?

    Also, I think this could benefit from a comment along the lines of "Default EntityTest entities to have the root user as the owner" — that then clearly explains why you are able to remove all those 'user_id' => 1, lines in so many tests.

  • +++ b/core/modules/system/tests/modules/entity_test/src/EntityTestForm.php
    @@ -18,30 +19,20 @@ class EntityTestForm extends ContentEntityForm {
    +    if (empty($this->entity->name->value)) {
    +      $random = new Random();
    +      $this->entity->name->value = $random->name();
    +    }
    

    Add a comment like "Assign a random name to new EntityTest entities, to avoid repetition in tests."?

  • Berdir’s picture

    Status: Needs work » Needs review
    FileSize
    28.55 KB
    2.17 KB

    Fixed KeyValueContentEntityStorageTest.

    The default value needs to be like that to comply with the config schema if I remember correctly.

    Status: Needs review » Needs work

    The last submitted patch, 7: entity-test-form-2238149-7.patch, failed testing.

    Berdir’s picture

    Status: Needs work » Needs review
    FileSize
    30.35 KB

    Fixed merge conflict in EntityReferenceIntegrationTest. There were also som left-overs from the node formatters/widget patch that are no longe required, so cleaned that up. Most of the difference there is now reverting changes from that issue, almost back to how it was before.

    Wim Leers’s picture

    Status: Needs review » Reviewed & tested by the community
    catch’s picture

    Status: Reviewed & tested by the community » Fixed

    Committed/pushed to 8.0.x, thanks!

    • catch committed 3ac426f on 8.0.x
      Issue #2238149 by Berdir: Apply formatters and widgets in...

    • catch committed f749eac on 8.0.x
      Revert "Issue #2238149 by Berdir: Apply formatters and widgets in...
    catch’s picture

    Status: Fixed » Needs work

    Not quite.

    Berdir’s picture

    Status: Needs work » Needs review
    FileSize
    28.87 KB
    1.49 KB

    Yeah, sorry, I have no idea where those FormCacheTest changes are coming from.

    Wim Leers’s picture

    Status: Needs review » Reviewed & tested by the community

    And again.

    Wim Leers’s picture

    Assigned: Wim Leers » Unassigned

    IDK why this was assigned to me.

    Status: Reviewed & tested by the community » Needs work

    The last submitted patch, 16: entity-test-form-2238149-16.patch, failed testing.

    Wim Leers’s picture

    Looks like HEAD has changed again, causing this patch to fail now (it was green when I RTBC'd it).

    Berdir’s picture

    Yeah, entityValidateAndSave() is new, from the generate field values issue.

    swentel’s picture

    Status: Needs work » Needs review
    FileSize
    29.57 KB
    718 bytes

    Wim Leers’s picture

    Status: Needs review » Reviewed & tested by the community
    alexpott’s picture

    Status: Reviewed & tested by the community » Fixed

    Committed 22d79dc and pushed to 8.0.x. Thanks!

    • alexpott committed 22d79dc on 8.0.x
      Issue #2238149 by Berdir, swentel: Apply formatters and widgets in...
    Wim Leers’s picture

    Issue tags: -sprint

    Status: Fixed » Closed (fixed)

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