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

scott.mclewin’s picture

Assigned: Unassigned » scott.mclewin
Priority: Normal » Minor
Status: Active » Fixed

You 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.

chromeyellow’s picture

Agree 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.

scott.mclewin’s picture

Status: Fixed » Closed (fixed)

I'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.