Problem/Motivation

This is postponed on #2831274: Bring Media entity module to core as Media module.

Currently media source plugins can specify which field types should be allowed for their source field via a allowed_field_types key in their annotation. This is used to limit the available field types in the field mapping form for a media type.

However, media sources may want to enforce that certain field settings are configured in a certain way in order for a field to qualify as a "valid" field for this media source. For example, if a media source uses an entity reference field as a source, it will want to make sure that the correct entity type is being referenced.

Proposed resolution

?

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 consider_providing_a-2863648-6.patch2.9 KBchr.fritsch

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Some relevant comments from #2831274: Bring Media entity module to core as Media module:

  • Boobaa in #291

    Tried to jump on this train early by adding a MediaSource plugin to our Brightcove module. We'll need a source field with the entity_reference type (brightcove_video is an entity type in Drupal that stores info about the videos hosted on Brightcove). So I added allowed_field_types = {"entity_reference"} which is the only thing that makes sense here, I guess – but I cannot find any way to provide additional required info: the entity type this field must reference.

    So I think there should be a way to provide this additional info, probably in the annotation, but as an optional thing. Something like settings = { "target_type" = "brightcove_video" } would do in our case, which would mean "field configuration needs to include these configuration values" (per IRC discussion with @slashrsm). Rationale: there are some field types which have required settings, so the MediaSource plugins should be able to provide those settings.

    Not sure if this warrants a "Needs work" status, tho.

  • Gábor Hojtsy in #292:

    Sounds like a needs work yes.

  • seanB in #294

    We just discussed the issue of boobaa in #291 on IRC. The following is a summary of what was discussed. Please correct me if I'm wrong!

    Creating a source field for a default MediaType in a module can be done by providing default config. This is already possible.
    Auto creating a source field for a manually created MediaType can be done by implementing createSourceField() in the MediaSource plugin. This is already possible.
    Reusing existing source field for an manually created MediaType currently allows all fields for a type provided in the annotation of the MediaSource plugin. It would help to allow something like getSourceFieldCandidates() on the MediaSource plugin, to allow plugins to limit the source fields a user can select. Maybe there are alternatives since we want to reduce the surface of MediaSourceInterface?
    We need to lock source fields to make sure users can't change the settings afterwards. I think this can already be done through createSourceField()?
    Boobaa suggested adding settings through annotations. Allowing field settings for the source field through an annotation could remove the need to override createSourceField() in #2. The settings could at the same time be used to filter the list of source fields and remove the need for #3. Locking auto created source fields by default could remove the need for #4.

    The annotation's look like the easier solution for module developers. Any thoughts?

boobaa’s picture

boobaa
he/him
commented
Title: Consider providing a mechanism for media source field validation » [PP-1] Consider providing a mechanism for media source field validation
Parent issue: » #2831274: Bring Media entity module to core as Media module
Related issues: +#2274433: Do not allow to alter Locked field via UI

Thanks for creating this issue.

Based on IRC discussions, I see three things that might need validation.

  1. The most obvious one is validating field value: a Twitter MediaSource plugin should accept only Twitter URLs in its link/text field.
  2. A bit less obvious would be validating the field instance config - but it's already handled by locking it (in the parent issue: #2831274: Bring Media entity module to core as Media module), so it cannot be changed (at least from the UI).
  3. The third one would be validating the field storage config: a Brightcove Video MediaSource plugin should accept only Brightcove Video entities in its entityreference field. (This is mentioned in OP, and is (partially?) covered in the slightly related #2274433: Do not allow to alter Locked field via UI).

Above is the what to validate. Next question would be how to do them.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Title: [PP-1] Consider providing a mechanism for media source field validation » Consider providing a mechanism for media source field validation
Status: Postponed » Active

Not blocked anymore

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new consider_providing_a-2863648-6.patch2.9 KB

I really like the solution with the annotation, because it's really powerful and flexible.

Here is a first patch. The MediaSourceFileTest already proof's that it works.

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Needs review » Closed (won't fix)

We just discussed this in Vienna with chr.fritsch and phenaproxima. This probably is won't fix. The conclusion was the API currently provided enough flexibility for a source to do it's own thing if needed:

  • Creating a source field for a default MediaType in a module can be done by providing default config. This is already possible.
  • Auto creating a source field for a manually created MediaType can be done by implementing createSourceField() in the MediaSource plugin. This is already possible.
  • Reusing existing source field for an manually created MediaType currently allows all fields for a type provided in the annotation of the MediaSource plugin. If a source had very specific requirements, it is possible to implement buildConfigurationForm() in the source.
  • If it is important to lock the source field, this can also be done by the source by implementing createSourceField().

If there are things we missed please feel free to re-open the issue.