Front image must respect the sort order

I'll rewrite this if you really want to have it that way, but do you realize that the same pieces go into two queries, i.e. there would be two bulky and almost identical switch statements?

No, this doesn't work well.

The query that selects the "latest" picture is inside a loop. It needs to be re-assembled for every loop iteration (IN (". implode(',', $descendant_tids) .")). So, that switch would need to go inside the loop.

However, the loop body is not guaranteed to be executed, so the other switch needs to go outside the loop (and you wouldn't want to assemble the second query umpteen times for nothing, would you?).

@drewish: Please review this again in the light of my comment above.

Yes, this works fine for the first step. Thanks!

- the image that satisfies the sort condition out of the pool of all images in A, B, C, D

No! Throwing all directly and indirectly contained images into one bucket and picking, say, the most recent one, will result in seemingly random behavior — it could be the top image of any of A, B, C, or D that would get selected.

- the image that satisfies the sort condition from A, and if no images in A, out of the first gallery that has images

Yes, that's what I meant. "First gallery" is determined by weight and alphabetical sort order (if the weight is the same), not by age or any of the other criteria used to sort the images.

And yes, it should definitely work recursively. If gallery X has no images, but it contains galleries Y and Z then it would be nice to see Y's front image as the front image of X (or Z's if Y has no images). If a gallery says "There are 123 images in this gallery" it ought to be able to display a front image, no?

My use case is a multi-level gallery of events. It contains a gallery for 2007, one for 2008, and one for 2009. Those in turn contain one gallery per event. Something like

Events
-- 2009
-- -- Spring event (images)
-- 2008
-- -- Fall event (images)
-- -- Summer event (images)
-- -- Spring event (images)
-- 2007
-- -- Fall event (images)
Members
Club House

I'd like to see the Fall 2008 front image as the front image of the 2008 gallery. We add events at the top of their year (using weights), so the 2009 front image should be the front image of the latest event throughout 2009, and the front image of Events should always be the front image of the latest event overall, so that it changes after every event. Having no front image for Events and 2009/2008/2007 would be a pity...

In reality there are not three events per year but about 80. Having to manually duplicate front images into 2009 and Events would be quite a chore, and requiring a lone image just to have a front image would mess up the clean look of the gallery of galleries pages.

(Because we have about 80 events, we actually have to introduce another intermediary level, because the weight is limited to the -10..10 range, which is not enough to enforce the correct order.)

BTW, sun, there was no ASC on the original f.filepath case.

Sure. But when I say 'gimme the most recent thing in this folder', I expect that to include contents of subfolders too. That's not just limited to technically-minded people -- most people would agree that the most recent piece in the museum wing is the most recent piece in all galleries in that wing ;)

Yes, that works reasonably well if your sort order happens to be most-recent-first. However, you're offering three other sort orders, and it won't work so well with those others. For example, I use the oldest-first order. The people who fill my galleries can't manage to load pictures in reverse order. They load the first picture first, then the second, etc. So we want oldest-first in each gallery, but the most recent gallery at the top and on the containing gallery.

This means in my example in #10, that the Events gallery will keep displaying the top image of the Fall 2007 gallery as its front image forever! You can't seriously claim that this is how it should work...

Ah, yes, sticky brings the desired picture into the front position and onto the outer gallery pages. However, it has two disadvantages:

1. it puts the picture into a box inside its gallery, messing up the table layout of the gallery (is this a bug or a feature?), and

2. it requires a manual intervention to achieve what should be normal behavior.

@joachim: I'm a bit confused about "galleries as views". Is that intended to officially replace the current image_gallery.module? I've had a very very bad experience with galleries as views under D5. Obviously, Views2 is completely different, and it's certainly an interesting project, but we're talking about fixing image_gallery.module here (unless it's already doomed to be replaced). Just as a side note: I'm using Thickbox, and there are a number of similar modules in use. It works very well with image_gallery.module, and "galleries as views" cannot fly until it's supported by those modules.

@sun: What do you mean with CNR? Your patch is fine as far as it goes and it's definitely an improvement. Are you looking for more reviews (which will not extend the functionality), or did you mean to set it CNW to work on the recursive front image?

Yes, sticky is only available with administer nodes, which I'm not willing to give to the people who maintain the galleries. That'd be objection #3, a killer...

Getting *box modules to work is simply a matter of giving them the right classes to work on, no? That's just theming the view.

No idea...

What problems did you have with gallery views on D5? Have you filed issue reports?

Yes, of course. There are a bunch of issues about that in the queue and I don't think any have been solved. But I have moved on to D6 and lost interest in D5.

No, let's not drop image_gallery.module — people use it. As long as #405456: Galleries made with views isn't completed and hasn't been shown to do everything that image_gallery.module does and more, we must not drop image_gallery.module.

Also, quoting http://drupal.org/patch/review:

No patch can save the world, not even a Drupal core patch. If it works and does something useful on its own, then it is good to go. One of the worst things you can do is elaborate on other, equivalent approaches and suggest complex extensions to the patch. Additional features can be added later. A scalpel is often better than a Swiss Army Knife.

I've just verified that the patch in #6 still works. It represents a definitive improvement, even if we may wish to pursue this further and discuss recursion in a followup issue. Please commit the patch in #6.

I'm aware that it needs testing, but interest seems limited at best. You're not trying to force people into testing/adapting a new unknown module by killing a well-known one that has been working fine for years, are you?

Views is not exactly a slim module. I run gallery sites that don't have Views installed, but I do have image_gallery_access and thickbox, and they work just fine with image_gallery. If you don't want to support image_gallery any longer, then let's move it out of image.