The presence of a did on panelizer defaults exported via Features causes displays to be incorrectly added to defaults other than the intended default on subsequent exports.

For example if you have several defaults already exported in a feature, you do a fresh install of drupal (this means a fresh panels_display table and serial ids for the did that start from 1), find an exported default that has a did of 1, update the content of a different default, that will create a display in the db with a did of 1, now if you export your feature again both the default you intended to update and the default that previously used the same did will have the same display.

The issue appears to be resolved if the did is set to NULL before exporting. Displays are part of the exported panelizer default already so there don't seem to be any other consequences from doing this.

CommentFileSizeAuthor
#8 1555194-better-did-checking-on-subrecords.patch484 bytespopulist
#5 1555194-panelizer-did-should-not-export.patch330 bytesmerlinofchaos
panelizer_null-did-export.patch558 bytesgilgabar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Priority: Normal » Major

Hm. Interesting. panels_export_display() should be leaving a did on the export, I thought. That's very interesting. Need to research that a little more.

populist’s picture

populist CreditAttribution: populist commented
Priority: Major » Normal
Status: Needs review » Reviewed & tested by the community

I was taking a look at #1387634: Proper Features Support for Panelizer and believe that this patch was the solution here. I was seeing hardcoded DIDs in my Panelizer exports which caused the trouble in that issue and this issue fixed it (and the related problems).

pirog’s picture

pirog CreditAttribution: pirog commented

i think these are two separate issues.

#1387634: Proper Features Support for Panelizer addresses panelizer incorrectly telling features that some panelizers are in code when they are in fact not. This limits the exportability of many panelizers because features thinks they've already been exported.

#1555194: Remove did from panelizer exports addresses panelizer incorrectly attaching displays to panelizers on export.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Priority: Normal » Major
Status: Reviewed & tested by the community » Needs review

Okay, looking at panels_export_display() there is definitely a bug, possibly on the CTools side. The did field is set to 'no export' so it should NOT be showing up in the export ever. Going to have to dig some more into this one.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
1555194-panelizer-did-should-not-export.patch330 bytes

Aha. It's the panelizer DID that's exporting, not the display's. That definitely should be marked no export in the schema. THis patch should work (will require a cache clear. Sometimes 2 as schema caching is annoying).

populist’s picture

populist CreditAttribution: populist commented
Status: Needs review » Needs work

I am using the patch in #5, but am seeing this error when saving nodes:

Notice: Undefined property: stdClass::$did in panelizer_export_delete_callback_subrecords() (line 850 of /usr/local/var/www/panopoly/sites/all/modules/contrib/panelizer/panelizer.module).

I looked at the file and it seems that panelizer_export_delete_callback_subrecords() is doing some deleting based on did. Not sure if this is necessary (maybe it can be removed) or maybe a different ID would need to be used.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Hm. Why is saving a node calling a delete callback? That's odd.

populist’s picture

populist CreditAttribution: populist commented
Status: Needs work » Needs review
FileSize
1555194-better-did-checking-on-subrecords.patch484 bytes

That function is being defined as the 'subrecords callback' as part of the $scheme definition which handles (according to some googling) random one to many relationships - http://drupal.org/node/1365282 as part of the export process.

Since we are no longer exporting the did as part of the exporting process, I am not sure this function is even necessary. However, I could see some legacy sites which are assuming the did or someone who is clever with the hook_schema_alter() needing to use the did.

In any case, I made a patch (http://drupal.org/files/1555194-better-did-checking-on-subrecords.patch) which should ignore the did if it is not present. This patch plus the one from #5 should be our ticket to ride here so marking "needs review".

populist’s picture

populist CreditAttribution: populist commented
Status: Needs review » Closed (duplicate)

I took a number of the fixes here (#8, #5, #1) and unified them with work in #1387634: Proper Features Support for Panelizer. I am closing this as a duplicate, but check other issue for latest details.