We need this before release.

CommentFileSizeAuthor
#4 2860816-3.patch500 bytesswentel
#3 2860816-2.patch500 bytesswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

swentel created an issue. See original summary.

swentel’s picture

Title: Create update hook to uninstall layout_plugin and install layout_discover » Create update hook to uninstall layout_plugin and install layout_discovery
swentel’s picture

Status: Active » Needs review
FileSize
500 bytes
swentel’s picture

FileSize
500 bytes

The last submitted patch, 3: 2860816-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2860816-3.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review

Tested this locally, seems to work fine. Not sure if we'd want an upgrade path test for this ...

swentel’s picture

So we probably should also test the branch version of panels which should be 8.x-4.x ?

aspilicious’s picture

Status: Needs review » Needs work

We shouldn't do this line, as we are not the only module using this.
+ \Drupal::service('module_installer')->uninstall(['layout_plugin'], FALSE);

Instead we should throw a message during update.

swentel’s picture

Status: Needs work » Needs review

Well you can't install both (layout_discovery has a requirement check) so there needs to be an uninstall anyway.
Panels 8.x-4.x uses layout_discovery, so we need to align anyway.
setting back to to get more eyes on it :)

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Yeah seems fine to commit in that case.

swentel’s picture

Status: Reviewed & tested by the community » Needs review
Related issues: +#2833976: How Panels, Display suite, Layout Plugin, Layout Discovery and core get to a stable layout plugin in core

Keeping it at needs review to get more reviews, pinging more people.
Adding #2833976: How Panels, Display suite, Layout Plugin, Layout Discovery and core get to a stable layout plugin in core as related.

goldlilys’s picture

Not sure if this is compatible with Bootstrap Layouts https://www.drupal.org/project/bootstrap_layouts either to use for Display Suite since it's causing this error when I enable some Bootstrap classes:

The website encountered an unexpected error. Please try again later.
Error: Call to undefined method Drupal\bootstrap_layouts\Plugin\Layout\BootstrapLayoutsBase::getRegionNames() in Drupal\bootstrap_layouts\Plugin\Layout\BootstrapLayoutsBase->submitConfigurationForm() (line 231 of modules/contrib/bootstrap_layouts/src/Plugin/Layout/BootstrapLayoutsBase.php).

I'm not fully sure what is causing this error, but somehow the Bootstrap Layout is using this new Layout Discovery with Display Suite. It worked on the previous version of Core, DS and Bootstrap Layout, but after needing to upgrade the core to 8.3 for the Calendars Event Date Range to work, this Display Suite started to not work and causing errors.

aspilicious’s picture

Goldlilys, can you tell use what branch you're on (for display suite?)

swentel’s picture

You probably also want to have the 8.x-5.x branch of bootstrap layouts.

swentel’s picture

You probably also want to have the 8.x-5.x branch of bootstrap layouts.

So given that, checking for other modules is kind of pointless, so will commit this soon.

  • swentel committed 1478d1c on 8.x-3.x
    Issue #2860816 by swentel: Create update hook to uninstall layout_plugin...
swentel’s picture

Status: Needs review » Fixed

pushed, will release alpha 2 now.

Status: Fixed » Closed (fixed)

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

EdPhillis’s picture

I upgraded core to 8.3 which broke my layouts. Im using Bootstrap layouts so upgrading that module to 5-dev as suggested which brought me to this issue as I have display suite installed as well. DS requires Layout Plugin so Im guessing I will need to uninstall DS, apply this patch and re-install. Is that correct? At the moment I cant get to uninstall screen because of this issue but i think i should be able to do it with drush..

karenann’s picture

Running drush cim in my env results in:

>> Drupal\Core\Config\ConfigImporterException: There were errors validating the config synchronization. in                                             [error]
Drupal\Core\Config\ConfigImporter->validate() (line 728 of /usr/share/nginx/html/docroot/core/lib/Drupal/Core/Config/ConfigImporter.php).
.usr.share.nginx.html.docroot#default >> The import failed due for the following reasons:                                                                                                    [error]
Unable to uninstall the <em class="placeholder">Layout Discovery</em> module since the <em class="placeholder">Display Suite</em> module is installed.
Unable to uninstall the <em class="placeholder">Layout Discovery</em> module since the <em class="placeholder">Display Suite Extras</em> module is installed.
borisson_’s picture

@#22

Layout Discovery is the core module that's needed for layouts, you shouldn't try to uninstall that one. Are you sure that you did a cex after running updb? It looks like you didn't.

To make sure that installing works again, edit config/core.extension.yml and add layout_discovery: 0 to the modules before importing again.