We've run into a problem when pictures are uploaded to one gallery, set as cover image, and then moved to another via the gallery select box. The cover images in both the "sending" and "receiving" galleries are incorrect after the "move". The sending gallery's cover remains set to the picture that was moved out. The receiving gallery has no cover. None of the "cover" radio buttons are selected in the "manage images" section of either gallery (the sender or receiver). The following error is an example of what appears in the logs:

Duplicate entry '63' for key 'cover_image' query: UPDATE node_gallery_galleries SET gid = 114, cover_image = 63 WHERE gid = 114 in /path/to/drupal/includes/common.inc on line 3538.

As a workaround, client can set a cover image in each gallery manually after the move.

I can see in node_gallery.inc function node_gallery_set_gallery_cover_image() that the intended design is to set a cover as NULL. Specifically, this query:
db_query("UPDATE {node_gallery_galleries} SET cover_image = NULL WHERE gid = %d", $gallery->gid);
But, should a cover image be set to NULL where one previously existed? Wouldn't a random choice, or any choice, be better? This would provide a fallback for clients whose gallery listings use cover images and are broken without them.

I'm guessing that the duplicate key error is a result of the system trying to set the cover image ID of the image in the receiving gallery before NULLing the ID in the sending gallery. However, after a brief search, I can't find a query that specifically sets the cover_image ID.

CommentFileSizeAuthor
#1 ng3-move-or-delete-cover.patch4.88 KBscroogie

Comments

scroogie’s picture

scroogie commented
Version: 6.x-3.0-alpha2 » 6.x-3.x-dev
Status: Active » Needs review
StatusFileSize
new ng3-move-or-delete-cover.patch4.88 KB

Good catch, thanks for reportnig. Here is a patch vs HEAD.

tomws’s picture

tomws commented

I'll need to figure out how to set up a test environment based on the HEAD version before I can test the patch. I'm assuming I can't just drop that version on top of a cloned live site, anyway. Can you suggest some documentation?

dddave’s picture

dddave commented

Ok, applying against dev from Feb. 9th fails to the most part. ;) Waiting for a new dev...

tomws’s picture

tomws commented

I *think* I've applied the patch to a HEAD version (not accustomed to CVS, sorry). Here's an error:

patching file node_gallery.inc
Hunk #1 succeeded at 779 (offset -2 lines).
Hunk #2 succeeded at 881 (offset -2 lines).
patching file node_gallery.pages.inc
Hunk #1 FAILED at 357.
Hunk #2 succeeded at 359 with fuzz 2 (offset -5 lines).
1 out of 2 hunks FAILED -- saving rejects to file node_gallery.pages.inc.rej

Testing after the broken patch, I still get the same duplicate key error. However, the receiving gallery does maintain its existing cover image now. The sending gallery keeps the old image as cover, though. I'm guessing this is a result of the duplicate key error (and order of update/delete operation).

EDIT: Er, no. That wasn't head - it was alpha2, I think. HEAD has a completely different function, unless I'm just not picking up on what's going on. Line 260 @:
http://drupalcode.org/viewvc/drupal/contributions/modules/node_gallery/n...

scroogie’s picture

scroogie commented

Yes, my fault. The 3.x code was not yet moved to head. You need to check out from the branch DRUPAL-6--3

tomws’s picture

tomws commented
Status: Needs review » Reviewed & tested by the community

I don't know if I qualify as "community" all by myself, but the patch applied correctly to DRUPAL-6--3 and works correctly there.

That rotation feature could be a nice addition. Really, you guys are putting together a package that's taking all of the "hard work" out of image galleries for the non-technical client, which makes the dev's job easier, too. Awesome work.

dddave’s picture

dddave commented

I guess we don't see a new dev before the GIT migration has happened, do we?

justintime’s picture

justintime commented
Status: Reviewed & tested by the community » Fixed

OMG ponies!

I made a git add/commit/push, and the project is still here :)

With any luck, I hope to roll all outstanding patches into an alpha3 release this week. Ironically, I only have dev time at Drupalcon :)

Committed to dev.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.