Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Prior to the 5.x-2.0 release, Comment Notify used the comments table to store database information. Now, the comment_notify table is used. However, the information stored in the comments table by Comment Notify was never cleaned up. Specifically, the "notify" column should be removed.
This is particularly important when upgrading from 5.x-1.x to a DRUPAL-6 release, as the comments table schema will be incorrect.
Comment | File | Size | Author |
---|---|---|---|
#14 | 317640_5x_drop_notify_column_13.patch | 1.92 KB | aclight |
#13 | 317640_5x_drop_notify_column_13.patch | 1.92 KB | aclight |
#13 | 317640_6x_drop_notify_column_13.patch | 1.06 KB | aclight |
#9 | 317640_6x_drop_notify_column_8.patch | 1.01 KB | greggles |
#4 | 317640_5x_drop_notify_column.patch | 838 bytes | greggles |
Comments
Comment #1
gregglesDoes it cause bugs during the upgrade to 6? If so, then yes, we should remove it.
I've only upgraded one site that uses comment_notify from 5.x to 6.x and didn't notice any problems, but...
Comment #2
gregglesEDIT: "If so, then yes, we should remove it soon. Otherwise we can leave it around a little longer until I get some more comfort that the migration to the new comment_notify table doesn't accidentally drop subscriptions."
Comment #3
Junyor CreditAttribution: Junyor commentedI don't know if it causes bugs, but wouldn't it make Drupal 6 report the incorrect schema on the Status page? Or maybe I'm just making that up and Schema module is required for that.
Comment #4
gregglesHere's a patch for 5.x and 6.x
Comment #5
gregglesI should add that nobody has mentioned issues related to the upgrade so I feel like it's relatively safe to do this.
Comment #6
Junyor CreditAttribution: Junyor commented@greggles: It looks like some unrelated stuff jumped into the 6x patch.
Comment #7
Junyor CreditAttribution: Junyor commentedComment #8
gregglesIndeedy do - thanks for noticing that!
Comment #9
gregglesComment #10
Junyor CreditAttribution: Junyor commentedThe Drupal 5.x patch works as advertised.
Comment #11
gregglesThanks for the review. I like this - I just want to wait on this a little more to see if anyone reports problems with the 5.x-1.x -to- 5.x-2.x upgrade process.
Comment #12
Dave ReidShould actually be using this for D6:
Comment #13
aclight CreditAttribution: aclight commentedAs Dave Reid suggests, we need to check to make sure that the {comments}.notify column exists before we try to drop it, or otherwise we'll get DB error messages. The attached two patches check for the column first and only try to drop it if it exists.
I've tested the attached patch on Drupal 5 with mysql. I haven't tested the pgsql case, nor have I tested this on D6.
Comment #14
aclight CreditAttribution: aclight commentedThe name of the update function needs to be comment_notify_update_5200, not comment_notify_update_5001.
Comment #15
Freso CreditAttribution: Freso commentedThe patch looks good, with a sound logic. Not tested though.
Comment #16
gregglesheh. There is no comment table - only comments, so the 6x patch was broken. Easy fix, obviously.
Committed to 6x http://drupal.org/cvs?commit=169154 and 5x http://drupal.org/cvs?commit=169156