Move 'panels_page_allowed_types' variable to a custom table.

Here is a full patch based on joelpittet's initial version, plus interdiff. Could you guys review and let me know if anything is missing? Also, do we want to move "_default" variables to a table as well?

Hey guys,

Just wanted to check if anyone had a chance to review this patch. I'm eager to get it into one of the upcoming module releases. Thanks!

The patch looks good and passed all tests for me. @joelpittet was there anything else you wanted to add before this patch can get RTBC?

Thank you for a detailed review, @joelpittet. Here is a new patch that addresses those issues. I had to make more changes since the data scheme version has changed. Regarding your comments:

1. 'pat'/'pta' - made it 'pat' to match 'panels_allowed_types' table name
2. Replaced fetchAllKeyed(1,2) with fetchAllKeyed() since fields are explicitly set in ->fields('pat', array('type', 'allowed')) statement now
3. Fixed double quotes in my code and a couple of other places. hope it's alright
4. I inherited index naming from the piece that you created originally. No doubt you like it ;-)
5. Fixed all the extra line breaks and trailing spaces

Not sure why test bot didn't pick the previous patch. Trying one more time.

Pre drupalci patches had a hard time understanding that they are still patches. trying some more things.

It looks like tests are broken on D7 branch - https://www.drupal.org/node/74958/qa. Most likely they were disabled for automatic issue verification because of that.

Actually, looks like *panels doesnt have any tests*, so, doesnt much matter whether it runs or not.

+++ b/includes/common.inc
@@ -196,7 +196,7 @@ class panels_allowed_layouts {
     if (!is_null($this->module_name)) {
-      variable_set($this->module_name . "_allowed_layouts", serialize($this));

Shouldn't this be updated to the new db?

That's a good question. Right now the issue is specific to "_allowed_types" variable. On my site, that variable is 20 times bigger than "_allowed_layouts" and it has a potential to grow even bigger, while with layouts I'd say it won't grow much. Therefore it was decided to address only "_allowed_types" for now, unless "_allowed_layouts" is going to become an issue as well. Switching back to RTBC.

Okay, we can tackle the allowed layouts later. This one is good. Committed.

>Find out if we can just do the panel types and how this will affect panelizer
Just tested. Panelizer allowed types also go into this table.

> Needs to be exportable (export/import hook).
This seems to have been forgotten here so that stuff is not deployable anymore. Reopening to discuss how and where to tackle this.

Hello,

This update deleted some variables added by other modules, for example media_wysiwyg_wysiwyg_allowed_types (from media_wysiwyg module).

Perhaps check if the variables start by panels_ or panelizer_ in the query?

@joelpittet - I think a discovery method is best. What we need as a starting point is a list of panelized entity types. Take a look at the 'panelizer_entity_view_alter' function. It calls 'panelizer_entity_plugin_get_handler' to check if there's a handler. If not, then it returns without doing anything.

For each panelized node bundle, I'm seeings two variables of interest. Take the 'page' node bundle for example: 'panelizer_node:page_allowed_types' and 'panelizer_node:page_allowed_types_default'.

Also, +1 on Features integration. Previously, this configuration was deployable through Strongarm. Currently, it's no longer possible to deploy or manage 'allowed types' configuration with Features.

Changing category and priority per https://www.drupal.org/node/45111#major-bug.

for the upgrade hook deleting too many variables: i think we should have a fast solution for this because there are still some people out there that do not have their config in code thus lose work!

so while we seek the silver bullet we should either
a) remove the update hook immediately or
b) keep the pattern matching, but respect the namespace

My vote is that b) is enuff and we don't need more complex discovery. The main point is imho to get that information killer out immediately.

> @axel.rutz oh because it was in the variables table before it was deployable through strongarm I'm guessing?

Yes indeed. This is a serious regression which seems to break our site after every deployment (which is strange and i did not have time to figure out why).

same issue here

+1 on features exporting. I feel that this commit should never have been released because it broke existing features integration, which is a huge deal. This has made me unable to deploy new changes to my content type features, and new sites that have been spun up since have no restrictions on allowed content.

The current recommended release of Panels is documented to break sites. It 1) deletes configuration of other modules, and 2) breaks Features-based workflows. Either of these should trigger a release.

I see two options: 1) Fix this issue and create a new release or 2) Revert the commit and create a new release.

Here's a patch to revert the new functionality while leaving the developed schema changes in tact. This should be enough for most people until the new stuff is actually fully complete.

#32
> The current recommended release of Panels is documented to break sites.

Indeed. so now qualifying as critical.

We're going to include the patch to revert in Panopoly

The patch to revert in #33 leaves the update hook intact - so the allowed_types variables provided by other modules are still being deleted. It seems weird to change an update hook after the code has been released, but that seems like the only way to prevent data loss on sites that haven't upgraded to 3.8 yet.

So for the benefit of sites that have not updated to 3.8 yet, it doesn't make sense to revert/remove the code that uses the custom table instead of the variables, and leave in place the code that deletes the variables.

Also, if these sites are using Features or any other exportables, they won't want to start using the new custom table until there's a way to export the configuration in the table. So if the code is reverted to still use variables, there's not much point in running the variable-to-table conversion in update hook 7307. The data in the table will not be updated if configuration changes, the variables will be updated. When sites are ready and able to export configuration from the table, they'd have to run the conversion again.

So I think the revert should not only avoid deleting the variables, it shouldn't bother to run the data conversion either. Just create the table, and leave it empty until everything's ready. The data conversion + variable deletion can happen in an update hook that's released along with the ability to export this configuration.

I've attached a patch (and interdiff from #33) that removes that code from the update hook, leaving only the schema change, and adds a note about what happened.

The latest patch makes sense to me -- we're using it in Panopoly now. Marking RTBC!

Makes sense, +1 for committing this soon.

Yah, I agree. we need to revert this. Reverted. I've also marked this 'needs work' to the original issue.

Did you 'git revert' or apply the patch? The patch is probably better for bringing this back later, since some pepole have already updated

Yah I applied the patch, that included the note from cboyden in the update hook. Since it went out with 3.8 and had data changes, it seemed best to go that way.

Great, thanks!

Just discovered on a site, that people were seeing all possible blocks (allowed_types) when adding content, while checking the variables all were on default and all values where still available in {panels_allowed_types}. All updates were ran before installing 3.9

drush vget _allowed_types

panelizer_node:hr_space_allowed_types_default: 0
panelizer_node:hr_sector_allowed_types_default: 0
panelizer_node:hr_page_allowed_types_default: 0
panelizer_node:hr_operation_allowed_types_default: 0
panelizer_node:hr_bundle_allowed_types_default: 0
panelizer_node:webform_allowed_types_default: 0
panelizer_node:hr_dataset_allowed_types_default: 0
panelizer_node:hr_contact_allowed_types_default: 0
panelizer_node:hr_office_allowed_types_default: 0
panelizer_node:hr_toolbox_item_allowed_types_default: 0
panelizer_node:hr_funding_allowed_types_default: 0
panelizer_node:hr_disaster_allowed_types_default: 0
panelizer_node:acc_incident_allowed_types_default: 0