Problem/Motivation

Media entities currently use the default entity reference selection plugin (\Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection)
This doesn't take into account the 'published' flag.

Proposed resolution

Add a new selection plugin for media.

Remaining tasks

Reviews

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 2995405-8.patch6.89 KBamateescu
#8 2995405-8-test-only.patch4.84 KBamateescu
fix_media_access_bypass.patch5.63 KBlarowlan
fix_media_access_bypass-test-only.patch3.59 KBlarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan created an issue. See original summary.

larowlan credited amateescu.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Active » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Status: Needs review » Needs work

The last submitted patch, fix_media_access_bypass.patch, failed testing. View results

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented

I am wondering if this could be built as a PublishableEntitySelection handler, because it looks to me that there is nothing really specific media code in there. Probably Terms would like to inherit from that then as well.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

That's exactly what I started to write when I found the problem in Media and reported it privately :) However, in the meantime I realized that we can't have a generic PublishableEntitySelection implementation at least until #2809177: Introduce entity permission providers, which should provide a unified way of getting the permissions needed for the entity query condition.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs work » Needs review
FileSize
2995405-8-test-only.patch4.84 KB
2995405-8.patch6.89 KB

Rerolled and updated the patch to current HEAD. Also, isn't this at least major?

The last submitted patch, 8: 2995405-8-test-only.patch, failed testing. View results

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Needs review » Reviewed & tested by the community

This is looking good.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

If that's the case does Block need a custom selection handler as well? In both these cases, the published entity key and admin_permission annotation key would satisfy both use cases. It might be a good enough default implementation?

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented

@Sam152, a custom selection handler for custom blocks is being added in #2987159: Create an entity reference selection plugin for custom blocks that filters out non-reusable blocks, and I don't think we should try to generalize things based on the admin_permission annotation key because that is too restrictive IMO.

Sam152’s picture

Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commented

Gotcha, that makes sense. +1 RTBC.

  • larowlan committed da2a2d2 on 8.7.x 
    Issue #2995405 by amateescu: Media entity needs its own entity reference...

  • larowlan committed 8b23c65 on 8.6.x 
    Issue #2995405 by amateescu: Media entity needs its own entity reference...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Reviewed & tested by the community » Fixed

fixed on commit 

diff --git a/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
index e8dd6b15d8..5bfe1ba611 100644
--- a/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
+++ b/core/modules/media/src/Plugin/EntityReferenceSelection/MediaSelection.php
@@ -23,7 +23,7 @@ class MediaSelection extends DefaultSelection {
   protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS') {
     $query = parent::buildEntityQuery($match, $match_operator);

-    // Ensure that users with insufficient permission can not see unpublished
+    // Ensure that users with insufficient permission cannot see unpublished
     // entities.
     if (!$this->currentUser->hasPermission('administer media')) {
       $query->condition('status', 1);

Committed da2a2d2 and pushed to 8.7.x.
Cherry-picked as 8b23c65 and pushed to 8.6.x.

Status: Fixed » Closed (fixed)

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