Problem/Motivation

Re-installing the media library doesn't work, because of the following error:

Unable to install Media library, core.entity_form_mode.media.media_library already exists in active configuration.

Proposed resolution

Add an enforced dependency of media_library to core.entity_form_mode.media.media_library

CommentFileSizeAuthor
#9 2989884-9.patch2.18 KBalexpott
#9 6-9-interdiff.txt3.66 KBalexpott
#9 2989884-9.test-only.patch1.62 KBalexpott
#6 interdiff-2989884-5-6.txt795 byteschr.fritsch
#6 2989884-6.patch2.59 KBchr.fritsch
#6 2989884-6-FAIL.patch2.03 KBchr.fritsch
#5 interdiff-2989884-2-5.txt1.74 KBchr.fritsch
#5 2989884-5.patch2.58 KBchr.fritsch
#2 2989884.patch2.13 KBchr.fritsch
#2 2989884-FAIL.patch1.58 KBchr.fritsch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chr.fritsch created an issue. See original summary.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Active » Needs review
FileSize
2989884-FAIL.patch1.58 KB
2989884.patch2.13 KB

Here are a fail patch and a patch with the fix.

The last submitted patch, 2: 2989884-FAIL.patch, failed testing. View results

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Needs work

Looks good to me, only found a few issues...

  1. +++ b/core/modules/media_library/tests/src/Functional/MediaLibraryInstallTest.php
@@ -0,0 +1,45 @@
+ * Tests media_library Install / Uninstall logic.

    Should be "install" and "uninstall" (no capitalization). 

  2. +++ b/core/modules/media_library/tests/src/Functional/MediaLibraryInstallTest.php
@@ -0,0 +1,45 @@
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['media_library'];

    Should be protected. 

  3. +++ b/core/modules/media_library/tests/src/Functional/MediaLibraryInstallTest.php
@@ -0,0 +1,45 @@
+    // Uninstall the media_library module.
+    $this->container->get('module_installer')->uninstall(['media_library'], FALSE);

    Why not use the UI to uninstall the module? 

  4. +++ b/core/modules/media_library/tests/src/Functional/MediaLibraryInstallTest.php
@@ -0,0 +1,45 @@
+    $this->assertSession()->pageTextNotContains('Unable to install Media library, core.entity_form_mode.media.media_library already exists in active configuration.');

    Should use existing $assert_session variable.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
Status: Needs work » Needs review
FileSize
2989884-5.patch2.58 KB
interdiff-2989884-2-5.txt1.74 KB

Thanks for reviewing, @phenaproxima

Here are the fixes.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
CreditAttribution: chr.fritsch at Thunder commented
FileSize
2989884-6-FAIL.patch2.03 KB
2989884-6.patch2.59 KB
interdiff-2989884-5-6.txt795 bytes

Lost the changes for #4.1 and #4.2 during patch-creation...

So here is a new one and also uploaded a fail patch again.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks great. RTBC once the true patch is green on all backends.

The last submitted patch, 6: 2989884-6-FAIL.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
FileSize
2989884-9.test-only.patch1.62 KB
6-9-interdiff.txt3.66 KB
2989884-9.patch2.18 KB

Here's a better test that means this won't happen in core again.

The last submitted patch, 9: 2989884-9.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

That makes much more sense 👍

@alexpott++

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Priority: Normal » Major
Status: Reviewed & tested by the community » Fixed

Committed and pushed d712f83e2c to 8.7.x and 0fc35d3efc to 8.6.x. Thanks!

Backported to 8.6.x because media library is an experimental module and this fix puts it in better shape for 8.6.0.

  • alexpott committed d712f83 on 8.7.x 
    Issue #2989884 by chr.fritsch, alexpott, phenaproxima: Re-installing the...

  • alexpott committed 0fc35d3 on 8.6.x 
    Issue #2989884 by chr.fritsch, alexpott, phenaproxima: Re-installing the...

Status: Fixed » Closed (fixed)

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