Closed (fixed)
Project:
Wishlist Module
Version:
4.6.x-1.x-dev
Component:
Miscellaneous
Priority:
Minor
Category:
Bug report
Assigned:
Reporter:
Created:
1 Apr 2006 at 23:25 UTC
Updated:
6 Apr 2006 at 14:59 UTC
Hi,
The current wishlist.mysql schema sets the nid field to auto_increment. I believe this shouldn't be here, as wishlist is referencing the nid value from {node}, not creating its own nid.
Checking out the code, I think it's safe to simply remove auto_increment from the mysql file. As each wishlist entry is unique per node, the foreign key will still be unique and therefore acceptable as primary key in the wishlist table.
Nice work, btw!
Comments
Comment #1
scott.mclewin commentedYou are correct. That harkens back to the original design where I misunderstood how to extend a node and designed the wishlist entries as their own stand-alone type. Oops. I don't see any negative impact from this and have reset it to a minor priority.
I've checked in a fixed version to HEAD. I'll bundle it up into the 4.7 branch the next time I incorporate changes. Thanks for taking the time to report this, and I'm glad you find my module useful.
Comment #2
chromeyellow commentedAgree it's minor priority and figured it was residual. If you're working on 4.7/cvs, I'll try to pitch in on backporting improvements/features to 4.6, if you'd like.
Comment #3
scott.mclewin commentedI've made a handful of improvements in 4.7 (which matches HEAD with the exception of this wishlist.pgsql change). If you back-port them into 4.6 and test I'd certainly commit the version you give me. Send it as the entire wishlist.module file rather than as a patch.
Since my own site runs 4.6 at the moment, even I'm not using my latest work. Here again, just keeping my time contained. I figured setting limits and sharing the module was a better choice than keeping it to myself.
There really are not too many functional changes from 4.6 to 4.7/head. The biggest is that the module can now be translated. I'd not used the t() function correctly and my module simply could not be internationalized. There are a few cosmetic changes, better descriptions and some reworking of the list display.