Updated: Comment #N

Problem/Motivation

FieldTypePluginManager has 3 methods with no interface:
getDefaultSettings()
getDefaultInstanceSettings()
getConfigurableDefinitions()

Proposed resolution

Add an interface that extends PluginManagerInterface

Remaining tasks

Write interface
Replace any usage of FieldTypePluginManager in core

User interface changes

N/A

API changes

N/A

CommentFileSizeAuthor
#8 interdiff.txt2.19 KBswentel
#8 field-type-plugin-2175415-8.patch17.48 KBswentel
#6 field-type-plugin-2175415-6.patch15.78 KBtim.plunkett
#4 interdiff.txt770 bytesjibran
#4 field-type-plugin-mgr-if-2175415.4.patch13.4 KBjibran
#2 field-type-plugin-mgr-if-2175415.1.patch13.31 KBlarowlan

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan

why not

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
Status: Active » Needs review
StatusFileSize
new field-type-plugin-mgr-if-2175415.1.patch13.31 KB

something like this?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

I think FieldTypePluginManagerInterface should extend PluginManagerInterface.

Other than that, just whitespace nitpicks:

  1. +++ b/core/lib/Drupal/Core/Field/FieldTypePluginManagerInterface.php
@@ -0,0 +1,45 @@
+<?php
+/**
...
+ */
+namespace Drupal\Core\Field;
...
+interface FieldTypePluginManagerInterface {
+  /**

    Missing blank lines 

  2. +++ b/core/lib/Drupal/Core/Field/FieldTypePluginManagerInterface.php
@@ -0,0 +1,45 @@
+
+
+/**

    Extra line

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new field-type-plugin-mgr-if-2175415.4.patch13.4 KB
new interdiff.txt770 bytes

Fixed #3.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community
Related issues: +#2175017: FieldDefinition::create() doesn't populate default 'settings' for the field type

Thanks!
I opened this issue due to #2175017: FieldDefinition::create() doesn't populate default 'settings' for the field type, so if that is committed first, this needs a reroll to use it in the unit test. If this goes in first, that can be rerolled too (but that is major).

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new field-type-plugin-2175415-6.patch15.78 KB

Rerolled after the other issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

I think we can use the new interface to improve the PluginManagerInterface typehint in \Drupal\Core\Field\Plugin\DataType\Deriver\FieldItemDeriver

swentel’s picture

swentel commented
Status: Needs work » Needs review
StatusFileSize
new field-type-plugin-2175415-8.patch17.48 KB
new interdiff.txt2.19 KB

Status: Needs review » Needs work

The last submitted patch, 8: field-type-plugin-2175415-8.patch, failed testing.

swentel’s picture

swentel commented

8: field-type-plugin-2175415-8.patch queued for re-testing.

yched’s picture

yched commented
Status: Needs work » Reviewed & tested by the community

Yup.

xjm’s picture

xjm
she/her
Primary language English
commented

I guess https://drupal.org/node/2064123 should reference the interface then once this goes in?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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