ensure images have unique displayOrder on reorder

diff --git a/site/cgi-bin/modules/BSE/TB/SiteCommon.pm b/site/cgi-bin/modules/BSE/TB/SiteCommon.pm
index d65453356607fbea2f6ceff6c335f07d487bcbdf..1d6f13647a9ac3ac7143637bb1000eb8fed57763 100644 (file)
--- a/site/cgi-bin/modules/BSE/TB/SiteCommon.pm
+++ b/site/cgi-bin/modules/BSE/TB/SiteCommon.pm
@@ -2,7 +2,7 @@ package BSE::TB::SiteCommon;
 use strict;
 use Carp qw(confess);
 
-our $VERSION = "1.015";
+our $VERSION = "1.016";
 
 =head1 NAME
 
@@ -534,10 +534,10 @@ sub reorder_child {
 }
 
 sub set_image_order {
-  my ($self, $order) = @_;
+  my ($self, $order, $images) = @_;
 
-  my @images = $self->images;
-  my %images = map { $_->{id} => $_ } @images;
+  $images ||= [ $self->images ];
+  my %images = map { $_->{id} => $_ } @$images;
 
   my @new_order;
   for my $id (@$order) {
@@ -545,14 +545,20 @@ sub set_image_order {
       push @new_order, delete $images{$id};
     }
   }
-  for my $id (map $_->id, @images) {
+  for my $id (map $_->id, @$images) {
     if ($images{$id}) {
       push @new_order, delete $images{$id};
     }
   }
 
-  my @display_order = map $_->{displayOrder}, @images;
-  for my $index (0 .. $#images) {
+  my @display_order = map $_->{displayOrder}, @$images;
+  my %seen;
+  if (grep $seen{$_}++, @display_order) {
+    # have a duplicate somewhere
+    my ($max) = $display_order[0];
+    @display_order = reverse( ($max - $#$images) .. $max );
+  }
+  for my $index (0 .. $#$images) {
     $new_order[$index]->set_displayOrder($display_order[$index]);
     $new_order[$index]->save;
   }

diff --git a/t/080-remote/010-save.t b/t/080-remote/010-save.t
index 76bc7c4e5e724fde4bbc93098261cabb1c236583..b658939b85864cfa8c50f3ca5cc343c13d65e84a 100644 (file)
--- a/t/080-remote/010-save.t
+++ b/t/080-remote/010-save.t
@@ -5,10 +5,11 @@ use JSON;
 use DevHelp::HTML;
 use Test::More;
 use Article;
+use Time::HiRes qw(time sleep);
 
 my @cols = Article->columns;
 
-my $base = 119;
+my $base = 130;
 
 my $count = $base + (@cols - 13) * 4;
 
@@ -261,6 +262,46 @@ SKIP:
              "check the tags");
   }
 
+ SKIP:
+  { # add some images
+    my %image_req =
+      (
+       id => $art->{id},
+       addimg => 1,
+       image => [ "t/data/govhouse.jpg" ],
+      );
+    my $data = do_multi_req($add_url, \%image_req, "add first image")
+      or skip("response failed", 9);
+    ok($data->{success}, "success")
+      or skip("response failed", 8);
+    ok($data->{image}, "has an image");
+    my $im1 = $data->{image};
+
+    $image_req{image} = [ "t/data/t101.jpg" ];
+    $data = do_multi_req($add_url, \%image_req, "add second image")
+      or skip("response failed", 6);
+    ok($data->{success}, "success")
+      or skip("response failed: $data->{message}", 5);
+    ok($data->{image}, "has an image");
+    my $im2 = $data->{image};
+
+    # set their order
+    $data = do_req
+      ($add_url,
+       {
+       a_order_images => 1,
+       id => $art->{id},
+       order => $im1->{id} . "," . $im2->{id},
+       },
+       "set image order")
+       or skip("set image order", 3);
+    ok($data->{success}, "successfully ordered")
+      or skip("failed ordering", 2);
+    ok($data->{images}, "had images")
+      or skip("no images", 1);
+    is($data->{images}[0]{id}, $im1->{id}, "ordering worked");
+  }
+
   # error handling on save
  SKIP:
   { # bad title
@@ -528,6 +569,24 @@ EOS
   return $data;
 }
 
+sub do_multi_req {
+  my ($url, $req, $comment) = @_;
+
+  my $res = $ua->post($url,
+                     @ajax_hdr,
+                     Content_Type => "form-data",
+                     Content => $req);
+  unless ($res->is_success) {
+    fail("$comment: http request " . $res->status_line);
+    return;
+  }
+  my $data = eval { from_json($res->decoded_content) };
+  ok($data, "$comment: response decoded as json")
+    or note $@;
+
+  return $data;
+}
+
 sub do_add {
   my ($url, $req, $comment) = @_;