Follow-up from #2083803: Convert field type to typed data plugin for field_test module

This hook was added in #565480: TF #2: Multilingual field handling and no longer needed as parent issue shows.
Also field_test.install file could be removed

The reason to preserve this hook was @berdir's suggestion about scope #2083803-40: Convert field type to typed data plugin for field_test module

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

Issue tags: +Novice

properly tagging

Ivan Zugec’s picture

This patch removes field_test_install() from field_test.install.

However, based on this line, "Also field_test.install file could be removed", should we just remove "field_test.install" file?

Ivan Zugec’s picture

Status: Active » Needs review
Berdir’s picture

Status: Needs review » Needs work

The issue will conflict which the referenced field_test issue, let's get that in first. If we don't need this anymore then we should also remove the actual implementation of the referenced hook, field_test_entity_info_alter().

Berdir’s picture

Issue tags: +Needs reroll

The referenced issue went in, so this can be re-rolled. Remember to also remove the referenced hook from field_test.module.

andypost’s picture

This hook is needed and used in tests.
There's just no more need in weight because hook_entity_info_alter in field_test.entity.inc have no collisions with content_translation module that operates on bundle level via content_translation_entity_bundle_info_alter

PS: it's really interesting how content translation content_translation_entity_info_alter() works because it always fires before field_test_entity_info_alter()... so 'translatable; entity property is not changed

andypost’s picture

Issue tags: +D8MI, +language-content

seems we need tag this, because field_test_entity_info_translatable() used in TranslationTest.php & TranslationWebTest.php

swentel’s picture

Status: Needs work » Needs review
FileSize
1.74 KB

Let's see what this gives now.

Status: Needs review » Needs work

The last submitted patch, 2106501-8.patch, failed testing.

andypost’s picture

+++ b/core/modules/field/tests/modules/field_test/field_test.entity.inc
@@ -20,15 +20,6 @@ function field_test_entity_info() {
-function field_test_entity_info_alter(&$entity_info) {
-  foreach (field_test_entity_info_translatable() as $entity_type => $translatable) {
-    $entity_info[$entity_type]['translatable'] = $translatable;
...
 function field_test_entity_info_translatable($entity_type = NULL, $translatable = NULL) {

as I said this needed because this code initializes 'translatable' for entities

swentel’s picture

Status: Needs work » Needs review
FileSize
1.04 KB

Ok, let's see what this gives then :)

Berdir’s picture

Hm, ok. #2019055: Switch from field-level language fallback to entity-level language fallback will change how that stuff works and replaces those tests with new ones I think. So by then, it will probably not be used anymore. We should then either simply remove it there or postpone it on that issue, I think?

andypost’s picture

Status: Needs review » Reviewed & tested by the community

No reason to postpone. The scope of the issue to clean-up install

Xano’s picture

#11: 2106501-11.patch queued for re-testing.

webchick’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs reroll

Nice find!

Committed and pushed to 8.x. Thanks!

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