Problem/Motivation

media_library_update_8703() was added in #3039829: Remove link to media item from media library view.. After commit, @catch said that it should have been a post-update hook.

Proposed resolution

Since no hook_update_N's have been added to media_library since that issue, we can convert the update to a post-update. Let's do that. This does not need additional test coverage, as the update is already covered.

Remaining tasks

Do it.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

None.

CommentFileSizeAuthor
#7 interdiff-3084043-5-7.txt2.41 KBphenaproxima
#7 3084043-7.patch14.95 KBphenaproxima
#5 3084043-5.patch14.62 KBphenaproxima
#2 3084043-2.patch14.73 KBphenaproxima

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review
StatusFileSize
new 3084043-2.patch14.73 KB
seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community

Tests are green!

alexpott’s picture

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

Patch doesn't apply.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new 3084043-5.patch14.62 KB

Rerolled! Preemptively restoring RTBC on the assumption that tests will pass on all backends.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/modules/media_library/media_library.install
@@ -223,213 +223,5 @@ function media_library_update_8702() {
  * Add edit and delete button to media library view page display.

    If we do this we ned to change this comment as this appears in the UI. And it does not do that anymore. 

  2. +++ b/core/modules/media_library/media_library.post_update.php
@@ -307,3 +307,218 @@ function media_library_post_update_add_status_extra_filter() {
+  $view = \Drupal::configFactory()->getEditable('views.view.media_library');
+  if (!$view->isNew() && $view->get('display.page')) {

    If we doing a post update hook then we can use the full Views API and load the view here rather than do config manipulation. Which would one of the reasons to move this to a post update hook. I'm not sure why we'd just move the hook_update_N into a post update.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 3084043-7.patch14.95 KB
new interdiff-3084043-5-7.txt2.41 KB

How's this look?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
larowlan’s picture

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

Committed 9cf1553 and pushed to 8.8.x. Thanks!

  • larowlan committed 9cf1553 on 8.8.x 
    Issue #3084043 by phenaproxima, alexpott: Convert...
xjm’s picture

xjm
she/her
Primary language English
commented

Are either of these update functions valid following #2981105: Media Library should not modify the media view?

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented

@xjm I believe so, the changes to the media library view in this issue are useful and needed, regardless of #2981105: Media Library should not modify the media view. They do separate things.

Status: Fixed » Closed (fixed)

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