Convert form validation of terms to entity validation

Tagging.

Hello.
DrupalSib team (Russia, Novosibirsk) want to implements this on today code sprint.

Sorry.
We do not have time to solve this problem today. Anybody can take this task.

First patch with test case - not much to validate here except for the term name?

OK, converting the parent field to entity reference breaks a lot of things and is confusing, so that should be splitted out to a separate issue.

Opened #2048555: Convert term parent to entity reference for the term parent reference conversion.

OK, since the automated tests actually pass we can move the parent entity reference back in here and close #2048555: Convert term parent to entity reference as duplicate.

I opened #2049121: Regression: Moving out term children on term listing page is broken to deal with the problem I experienced during my manual testing, which is independent of this issue.

#8: term-validation-2002168-8.patch queued for re-testing.

Related issue #2068287: Support bundle names provided in the request arguments in EntityCreateAccessCheck

I'm a little surprised to see a new attachLoad() method with no corresponding deletion. I thought this issue just moved around existing code?

Yeah, not too happy about front-loading the parents, we didn't do that for a reason. Also related is the issue that attempts to add methods to TermInterface.

I guess we'd rather have to make this a computed field, we already made terms slow enough...

Ok, so I implemented a custom field class for the term parent field that lazy loads the parents.

The tests will fail because the parent is a entity reference field and the validation should flag invalid references, but I have no idea why that does not work right now.

It all makes a lot more sense if the parent field is not computed, because we are just loading it on demand.

We could even add a generic abstract LazyLoadField to core, so that the taxonomy module would only have to implement one lazyLoad() method that returns the field values suitable for the setValues() method.

Does not apply anymore.

Rerolled. I also removed a @todo about integrating the description format with the description field, but that would require the creation of a TextFormattedItem (text_formatted_field) class to handle that, and that would be a bit too much for this issue. There is already a @todo in the code about that.

Patch does not apply anymore rerolled. Also fixed a module dependency in the test.

#18: term-validation-2002168-18.patch queued for re-testing.

So the class Field is now FieldItemList, fixed that.

Straight reroll for HEAD changes. No changes to real lines of code.

The test failure is due to parent=0 being treated as an empty value and being filtered out before getting saved to {taxonomy_term_hierarchy}. There were other nasty hacks in the patch working around other difficulties in making parent into an ER field, so let's defer that to #2048555: Convert term parent to entity reference, and proceed with this, in order to move us closer to #2002180: Entity forms skip validation of fields that are edited without widgets.

Looks good to me.

1. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Entity/Term.php
   @@ -215,6 +215,8 @@ public static function baseFieldDefinitions($entity_type) {
        $properties['vid'] = array(
          'label' => t('Vocabulary ID'),
          'description' => t('The ID of the vocabulary to which the term is assigned.'),
   +      // @todo Convert this to an entity_reference_field once that works with
   +      // config entities.
          'type' => 'string_field',
        );
   

   This should be possible now.

   However, doing it here is only partially related to this issue, not sure. The @todo would have to be reworded at least.

   As it's the bundle, wondering if that needs special validation (could be separate issue, for all entities with bundles) as it's basically read-only, you can't just change the bundle of an entity that has fields.

2. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Entity/Term.php
   @@ -247,10 +253,11 @@ public static function baseFieldDefinitions($entity_type) {
          'description' => t('The parents of this term.'),
   +      // @todo Convert this to an entity_reference_field: #2048555: Convert term parent to entity reference
   

   I think we reference the URL in code comments, not just the ID like this?

Child issue of #1696648: [META] Untie content entity validation from form validation, therefore critical.

Rerolled.

A new pull request was opened by klausi for this issue.

Some commits were pushed to the pull request.

Converting the bundle field is a separate issue: #2181593: Convert entity bundle base fields to entity reference.

Converting the parent field is a separate issue: #1915056: Use entity reference for taxonomy parents.

Synced over the changes to StringItem.php from #2002158: Convert form validation of comments to entity validation.

Some commits were pushed to the pull request.

You can't stop with this Github sync shit, can you? https://drupal.org/node/2172149 reopened, raised to critical.

Looks RTBC except minor

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/StringItem.php
@@ -62,4 +62,18 @@ public static function schema(FieldDefinitionInterface $field_definition) {
+    $constraint_manager = \Drupal::typedDataManager()->getValidationConstraintManager();
+    $constraints[] = $constraint_manager->create('ComplexData', array(
+      'value' => array('Length' => array('max' => $this->getFieldSetting('max_length')))
+    ));

I'd just suggest to wrap this in if ($this->getFieldSetting('max_length')) see related #2150511: [meta] Deduplicate the set of available field types

klausi pushed some commits to the pull request.

@andypost: sure, why not.

Interdiff: https://github.com/klausi/drupal/commit/75079dcf5b2c03d2206ceb07d23aa71a...

So I see no nitpicks and like to see this commited asap

Committed/pushed to 8.x, thanks!