Update pathauto patch

Moving this to the main Panopoly queue.
Looks good!

Patch been updated

Need to make sure te latest pathauto update function has run (safe to rerun - for those running the earlier patch).

Here's a re-roll to remove https in the .make, which is discouraged.

As an aside, the above patch to pathauto cannot be used as-is for existing Panopoly users. The previous patch that Panopoly appled (https://drupal.org/files/936222-pathauto-persist.patch) to pathauto is not compatable with this patch. Specifically, this patch defines the pathauto database table as 'pathauto_state' while the previous patch used 'pathauto'. Similarly, the update hook in the original patch uses the same update number (pathauto_update_7006).

I'm still looking into what's needed here, but I suspect that the cleanest option is to disable and uninstall pathauto and reinstall it with the new patch, nevertheless, that has it's own drawbacks.

brandenlhamilton: look at patch 4 o.O

@hefox, my apologies, my last post should have had the link to the patch in the fourth post. My intention was to bring attention to the fact that if you're already using Panopoly, you are using an alternative patch that already defines a Pathauto schema that isn't necessarily compatible with the patch in #4.

In my case, I was running into PDO exceptions expecting the new schema defined in #4, pathauto_state, when attempting either run update.php or drush updb. My suspicion is that pathauto_entity_state_load_multiple() is called before hook_update_N has had a chance to update {pathauto} to {pathauto_state}.

I agree that this patch is super important. Thanks to @Xen, @hefox, @beeradb and @brandenlhamilton for working on getting it into Panopoly!

I have a number of existing production sites on Panopoly and I share @brandenlhamilton's concerns about the upgrade path, so I just did the following test:

1. Installed a new unpatched Panopoly site
2. Created a single "Content page" with a manually alias set
3. Manually applied the patch to panopoly_core in #4 and pathauto (I had to first revert the previous patch, and then apply the new one)
4. Ran drush updatedb

It runs fine, meaning there are no errors. However, the state data that was previously saved to the {pathauto} table is gone! It no longer knows that I manually set the alias on the node I created.

I'll try to put together a patch that preserves the data...

New patch attached which will preserve existing state that was stored on the {pathauto} table. Please let me know what you think!

Thanks so much everyone for the work here. That issue #936222: Merge in pathauto_persist module functionality to prevent losing manual aliases with node_save() calls is such a pain and glad its almost committed and the upgrade path for Panopoly folks is clear.

I did some basic tests and stuff was good. Committing now and will test the upgrade path on a few sites.

Awesome, thanks!

The patch in #9 causes a fatal error on updb:

Missing argument 1 for pathauto_update_7006(), called in /Users/robertson/Web                                                    [warning]
Sites/example/profiles/panopoly/modules/panopoly/panopoly_core/panopoly_core.install on line 90 and defined
pathauto.install:207

The actual signature of that function is pathauto_update_7006(&$sandbox) - so we need to pass in a $sandbox variable. Even after fixing that, however, it still doesn't find the needed table and throws a SQL error:

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'example.pathauto_state' doesn't exist

So what went in is very broken and needs to be revisited.

Hmm. Looking at the patch:

https://drupal.org/files/pathauto-persist-936222-130-pathauto-state.patch

The signature of the function is NOT pathauth_update_7006(&$sandbox).

@@ -169,6 +220,61 @@ function pathauto_update_7005() {
 }
 
 /**
+ * Create pathauto table, copying data from pathauto_persist if it exists
+ */
+function pathauto_update_7006() {

Also, I used this update on two live production sites some time ago and it worked fine. I was able to verify in the database that the entries got moved to the new table and everything. I'm not sure how our pathauth modules could be so different...

Actually, looking at the old patch - the signature is pathauth_update_7006(&$sandbox)! Is it possible that you're using the new panopoly_core but didn't run panopoly_core.make to get the new dependencies with the new patches? If the new patch gets applied to the pathauto module, this will work!

I have seen these errors as well and this def should be fixed. That pathauto issue is just ridiculous and hope we can get it solved soon.

dsnopek, you'll encounter this if you upgrade via drush up rather than doing a whole rebuild.

seanr: I believe what people are saying that that is not not a supported workflow for modules provided by a distro, as it means distros are unable to apply their patches to said modules, etc.

Sounds like this can be set to fixed again?

@seanr: What @hefox said ;-) - you shouldn't be upgrading distros with drush up. A newer version of a distro can include new patches and/or may require you to stay on a specific version of a module (not go to the newest available like drush up will do). All of this is defined in the *.make files which you need to build from. So, I'm going to mark this as fixed again.

hi
gave up and uninstalled panopoly core for the time being.

Added a link to the original issue in the pathauto queue.

Experiencing similar issues as mentioned in #13. In our case, the scenario is that we are updating from Panopoly 7.x-1.0-rc3 to the rc5.

It appears that Panopoly rc3 has the patch http://drupal.org/files/936222-pathauto-persist.patch applied to the Pathauto whereas the latest rc5 is applying http://drupal.org/files/pathauto-persist-936222-130-pathauto-state.patch.

This causes the errors on updb because the installer file has a different database schema but no update path between them, in the first patch it has $schema['pathauto'] and in the second one $schema['pathauto_state'].

The issue here the pathauto_update_7006() function checks for pathauto_persists table and moves data from it to the pathauto_state table but it does not check if there is a table "pathauto" and move data from that to the new table, this causes the errors on drush updb.

Will provide a patch for this before the xmas if I have time.

Appears that the patch at http://drupal.org/files/pathauto-persist-936222-130-pathauto-state.patch has at least one "weirdness"; it is setting entity_id to varchar (255) whereas entity id is always numeric, means that column should be numeric as well, in the older patch it was correctly set to unsigned int. Investigating this further tomorrow.

Switching discussion to the original issue queue at https://drupal.org/node/936222.