Move Panels-specific code out of Page Manager

Needs an integration test (which is the part that didn't exist in page_manager), because this is missing the necessary form_alters, and will probably also pass tests.

Unfortunately, I wasn't able to get this working with a hook_form_alter() at all, because we actually need to set the Panels storage info before the form is generated, because it affects whether or not Panels will give the user the IPE option (if Panels storage isn't supported, then the IPE can't work, so it pre-emptively doesn't let users pick it). So, I had to use hook_entity_create(), but I think that's fine.

The part that is less fine, is that we need to set the Panels storage information again on save because then we'll have the actual entity id (which we don't have in hook_entity_create()). But I couldn't get hook_entity_presave() working, so I had to use hook_entity_insert() and then save again. :-/ If we could come up with a version that doesn't have to save again, that would be ideal!

Also, @tim.plunkett is right - we need tests for this!

Here's a new patch that adds an integration test. The code in hook_entity_insert() got even uglier (an additional load!) to prevent an exception in ConfigEntityBase::preSave() which may be a core bug. That whole function just need to be reworked to not require the additional load/save - maybe using a form alter instead? Anyway, the test will verify if the new code replace it with is still working, so this is a step in the right direction. :-)

So, here's a version that uses a hook_form_alter() and doesn't need an additional load and save. However, we have to abuse a #validate callback and it got me thinking that if we're using hook_entity_create() for the first pass, page variants that are created programmatically will be weirdly broken. :-/ So, I think I'm going to switch this back to using entity hooks, but ideally without having to add additional loads/saves.

Alright! I finally have a version using hook_entity_presave() that actually works WITHOUT needing any additional loads or saves. The key to why I couldn't get it to work before is that the plugin collection magic that copies the configuration of the variant plugin to the page variant entity is run before hook_entity_presave() so we have to manually copy it again.

This should be the last one of these patches I make before getting some more review!

I've removed the panelstest from panels so we can get tests working again. This patch re-adds the test with the page manager features.. but we'll have to wait for the PM release before this can go in.

This issue isn't related to restoring PanelsTest or the branch tests failing (other than that issue also prevented the tests here from running -- but that was true of all patches) so I created a new issue just about restoring PanelsTest:

#2667648: Add the PanelsTest back

Here is a new patch that removes the changes from #13 and removes the dependency on page_manager_ui from this patch. The interdiff is from #8.

Whoops! I totally messed up the patch on #16, but the interdiff was correct. Here's the right patch!

Removed an unused 'use' statement.

I just did some testing of Panels (via Panelizer) on a fresh site that didn't even have Page Manager in the modules directory, just to make double extra sure the way I'm referring to PageVariantInterface won't break things in that case. Everything worked fine! So, as far as I'm concerned this issue is ready, but I'd love some additional review!

Actually, I just realized this needs one more change now that #2659546: Add special 'change layout' op to access checking in PanelsStorage is committed! We need to account for the 'change layout' op, or we won't be able to update layouts anywhere, which would be no good. Here's an updated patch! (Hopefully, the last... :-))

1. +++ b/panels.module
   @@ -24,6 +26,50 @@ function panels_api_version() {
   +    // Since the plugin collection magic has already run and copied the
   +    // configuration from the $panels_display to the $page_variant, we need to
   +    // do it again manually.
   +    $page_variant->set('variant_settings', $panels_display->getConfiguration());
   

   I don't fully understand why we need this.

2. +++ b/src/Plugin/PanelsStorage/PageManagerPanelsStorage.php
   @@ -0,0 +1,124 @@
   +      if (!($variant_plugin instanceof PanelsDisplayVariant)) {
   ...
   +      if (!($panels_display instanceof PanelsDisplayVariant)) {
   

   Should we have an interface here? It seems odd to check for a specific plugin class.

#22.1: So, normally, the configuration from the variant plugin is copied onto the config entity because the variant plugin is in a plugin collection. Unfortunately, the actual copying is done in ConfigEntityBase::preSave() which happens just before hook_entity_presave(). This means if we make any changes to the variant plugin, they won't be saved, because they're only on the plugin and not the config entity -- unless we manually copy them to the config entity.

#22.2: Maybe! We've been type-hinting and using the PanelsDisplayVariant class everywhere, so I think if we did make an interface, it should be a seperate issue that updates everything. It would be kinda nice to have a PanelsDisplayInterface so I can stop typing "Variant" which just doesn't make sense in Panelizer, mini-Panels, etc. :-)

ConfigEntityBase::preSave() which happens just before hook_entity_presave().

Ahhh. I think it should mention that. Maybe that's too technical though? /me shrugs

I have no problem making the comment more specific so it's clearer! Here's a new patch with a (hopefully) improved comment.

Tagging for the 3.0-beta4 release!

+++ b/panels.module
@@ -6,6 +6,8 @@
 
+use \Drupal\page_manager\PageVariantInterface;
+

Forgive my ignorance, but wouldn't this negate #2511576: Remove dependency on Page Manager?? Or will the use statement be ignored if page manager isn't enabled or installed?

I tested this patch on a fresh Drupal 8 site that didn't even have the page_manager module code in the modules/ directory in #20 - it worked! But feel free to re-test this just in case!

For an explanation of why I think this works: use statements don't actually load anything, they just tell PHP specifically what we mean by a short class name. Ex. the use statement you quoted tells PHP that when it encounters PageVariantInterface, we really mean \Drupal\page_manager\PageVariantInterface. But code in PHP is loaded by the autoloader only when we actually use a class constructor or static method (and maybe some other rules). So, nothing in the code in this patch should trigger the autoloader.

I hope that makes sense! But definitely more testing that this is the actually the case would be a good thing! :-)

Ok, cool I didn't have any issues either in my testing. Committed!