Module modifies UC core DB tables, doesn't reverse mods on uninstall
TR - July 29, 2008 - 05:16
| Project: | Ubercart Save for Later |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
Description
I really like the functioning of this module, but Dude! You've modified the core Ubercart uc_cart_products table in the DB, and you don't reverse the DB table changes on uninstall. That's a nasty surprise which is not mentioned anywhere. You also don't delete the three variables you create in the variable table: uc_continue_shopping_type, uc_continue_shopping_url, and uc_continue_shopping_text. A hook_uninstall() function should be added to put the database back the way it was.
#1
Agreed, I overlooked the uninstall.
I think I'm going to leave the bump to varchar(255) for cart_id since varchar(32) seems like a bit of a bug in the first place and might eventually get changed and there should not be any harm in that field being varchar(255) instead.
http://www.ubercart.org/forum/development/5111/cart_id_varchar
The variables are actually coming from the uc_cart module, so I won't want to delete those in the uninstall.
I will take care of the extra column on the uc_cart_products table though.
#2
Added hook_uninstall in a D5 commit, http://drupal.org/cvs?commit=130367
I'm still not sure about the Drupalishness of leaving that varchar(255) field in there. Something doesn't seem right about that, but it also doesn't seem right to put a revert back to varchar(32) in the uninstall. The way I've done it doesn't leave the potential to break anything as far as I know, whereas setting that column to varchar(32) on uninstall could potentially break the anonymous cart functionality.
#3
#4
Automatically closed -- issue fixed for two weeks with no activity.