I use node_gallery to share photos of events in family. So a w-e happen, we take 200 pictures, upload all of them by one click in node_gallery, and then…

I wanted users to be able to actually share the photos, its provided by : https://www.drupal.org/node/2321387

I also need a quick way to sort the images in chronological order… while its in a fancy "uploading race" order if nothing is done. So here comes a patch which provide a "Set chronological order" button in the Sort Items tab. Clicking on it will sort the displayed images.

I reported a missing pager in this tab (in https://www.drupal.org/node/2280221), and the display-all workaround, which is very convinient when using this new feature. I would like to be able to bypass paging issues on this feature, to always sort all the gallery while putting it in chronological order.

To finish, the feature is locked by default in the node_gallery other settings.

CommentFileSizeAuthor
#6 node_gallery-set_chronological_order_2345499-3.patch5.51 KBsiltaar
#3 node_gallery-set_chronological_order_2345499-2.patch5.47 KBsiltaar
#1 node_gallery-set_chronological_order_2345499.patch4.86 KBsiltaar

Comments

siltaar’s picture

siltaar commented
Status: Active » Needs review
StatusFileSize
new node_gallery-set_chronological_order_2345499.patch4.86 KB
zengenuity’s picture

zengenuity commented
Status: Needs review » Needs work

This is a nice feature, but the patch needs work.

Most important thing: You need to use the t() function correctly and not concatenate strings. The way you have it in this patch would open a XSS vulnerability.

Change this:
t("Unable to find creation date in exif data for: ").$file->title

To this:
t("Unable to find creation date in exif data for: @title", array('@title' =>$file->title))

There are some other places with the same issue.

Next thing: why is the chronological_date_sort() function inside another function. Don't do that. Also, this function needs to respect namespace and start with node_gallery_api.

Finally: Drupal coding standards should be used for spacing.

siltaar’s picture

siltaar commented
StatusFileSize
new node_gallery-set_chronological_order_2345499-2.patch5.47 KB

Worked on code convention issues.

siltaar’s picture

siltaar commented
Status: Needs work » Needs review
siltaar’s picture

siltaar commented
Status: Needs review » Needs work

Some fixes to come.

siltaar’s picture

siltaar commented
StatusFileSize
new node_gallery-set_chronological_order_2345499-3.patch5.51 KB
siltaar’s picture

siltaar commented
Status: Needs work » Needs review

  • zengenuity committed 4ce2386 on 7.x-1.x 
    #2355799, #2345499: Allow sorting by EXIF created date.
zengenuity’s picture

zengenuity commented

Thanks. This patch needed a decent amount of rework, but I think I have it working. A couple points:

  • Node Gallery needs to support files that aren't images as well as nodes that don't have any files at all, so there need to be guards in place for that. I've added them so it doesn't crash.
  • You should always make patches clean from the latest git HEAD. It's hard to apply them when they are based on other patches you've submitted. The two exif ones appear to both assume your download patch, which isn't accepted yet, is in place. I had to manually apply several rejected patch stanzas to make these work.
  • Please follow the Drupal coding standards. No space after function name when calling function.
zengenuity’s picture

zengenuity commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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