Upgrade function does not add snid column to {simplenews_mail_spool}

Hi,

From what version did you migrate ?
To migrate from 6.x to 7.x, you first have to update the modules to the last 6.x version available (currently the 6.x-2.0-alpha2).

No that's not really the problem. I have this and I just updated from 7.x-1.x-dev-Mar-28 to Oct-17, triggering it. The problem is that the update functions are incomplete. The logic is this, the 'snid' field wasn't part of the original schema of 7.x-dev and when it was added to the 7.x schema there should also have been an update function adding the field - but that never happened.

I don't know exactly when it was added as I've been away for a while over the summer and some more, so haven't updated anything until now. But I had a quick look in my private svn repo where I keep track of my stuff and I can clearly see this one is missing in the gap.

As for migration from D6 to D7 I'm not sure but think it shouldn't really matter as update functions for D6 up to the point where D7 takes over should be in the D7 install file, and drupal will pick up doing its stuff from the point where the schema version is in the db. But here a create field call for 'snid' is missing in a D7 update function.

The workaround is to create the field manually and rerun update.php

Ok, I was able to confirm this issue.

The attached patch fixes the reported issue, removes old (unported and therefore useless) 6xxx update functions, defines hook_update_last_removed() and hook_update_dependencies().

I also added a upgrade test which makes this patch so huge.

I basically set up a trivial 6.x site with just Simplenews installed, used the dump-database-d6.sh script from Drupal 7, which generates a file which creates the structure and inserts the content of that using the D7 DB API. That is then run the by the UpgradePathTest class and I'm just verifying that the upgrade works and I'm still able to see the two newsletter categories which I've added. This can easily be extended later on.

Tagging

Accidently search & replaced too much. tests++.

I also noticed that there are a bunch of additional update functions in 6.x-2.x, we need to make sure that the 6.x-2.x -> 7.x-1.x upgrade works too. But I'd like to get this patch commited first, the second will add another db dump and will be equally big.

And now with patch.

Commited, created a follow-up task for 6.x-2.x at #1360806: Check and add tests for upgrade path from 6.x-2.x

I migrated a site from D6 to D7, applied the patch fix_upgrade_path_including_tests2.patch, and still get the 'snid' doesn't exist in table error. When applying the patch, simplenews/tests/simplenews.test did not update properly. Not an issue, since the diff was only adding to this file I just added the code manually in this case. Other files updated properly.

Ran the update script, same error. I was thinking of just uninstalling the module, then reinstalling - but in that case I'll need to re-enter all subscribed emails. Is uninstalling and reinstalling a necessary step? Thanks.

Please provide us EXACT versions you have / had.
Did you first upgrade the 6.x version to the most recent one?
The upgrade path is always: update in D6 to the latest (6.x-2.x) and then upgrade to the D7 version.

BTW: You can add the snid column via phpMyAdmin

Also, you don't need to apply any patches when using alpha2, that's should already be part of that release.

I just upgraded to alpha2, still get the errors on running the update script. I was running 6.x-1.3 on my D6 site when I upgrade to D7 a few months ago. Updating my D6 site to 6.x-2.x, then upgrading again to D7 isn't really an option as I've done a good bit of development to the site since the upgrade. 6.x-1.3 was still green on the status reports page when I upgraded, didn't realize I needed to go to 6.x-2.0-alpha2 at the time.

I think I'll go the phpadmin route, sounds like the best solution given what I've done.

snid - int, default 0, not null=true. Anything else I should be aware of before adding manually?

It shouldn't be necessary to go to 6.x-2.x, upgrading from 6.x-1.x is supported.

I think your problem is elsewhere, namely that you already executed the initial 7000 update function, which was fixed by the above patch. Now Drupal just tries to re-run 7001 without knowing that 7000 has changed.

That's why it's actually wrong to change existing update functions but we need to get it in somewhere before 7001 is run. The only other option is moving that bit into 7001, before adding the index.

I don't know anything about 7000 vs. 7001. I'm trying to add via phpmyadmin:

I ran this:
ALTER TABLE `simplenews_mail_spool` ADD `snid` INT NOT NULL DEFAULT '0' COMMENT 'Foreign key for subscriber table ({simplenews_subscriptions}.snid)'

Then this:
ALTER TABLE simplenews_mail_spool ADD INDEX `snid_tid` (`snid`, `tid`)

Is there a command I need to run to add a foreign key? It appears so from simplenews.install. Thanks in advance, and apologies for asking basic questions. I don't completely understand all this.

No, foreign keys are currently just a logical information within drupal, they aren't actually added on the database. You also don't need to do the index manually, that should be added automatically once you have a snid column.

The update logic is split into multiple, numbered functions, starting with 7000 (for Drupal 7). This error happens in the 7001 update function. The snid column is added in 7000. You probably already executed the 7000 update function before applying this fix (e.g. trying with alpha1) and ended up in an inconsistent state.

The attached patch should fix the error you're seeing by moving code around.

If you've made a backup before adding the snid column manually, please revert and try with this patch (can be on a separate database, just to confirm that the patch works, you can continue working with your current database).

Fixed this after some manual tests, including column deletion and schema version manipulation / retrigger updates.
Worked perfect.