Add support for multiple photos per report.

For Zurich, see mysociety/FixMyStreet-Commercial#664.

This commit includes a new PhotoSet class (NB: called Model:: though not
really a model), should handle binary data (e.g. old style photos in
database), fileids (40-char hash), and Catalyst::Upload objects.

15 files changed, 567 insertions, 166 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm
index f9ea383f8..d7bca05a7 100644
--- a/perllib/FixMyStreet/App/Controller/Admin.pm
+++ b/perllib/FixMyStreet/App/Controller/Admin.pm
@@ -10,6 +10,7 @@ use Digest::SHA qw(sha1_hex);
 use mySociety::EmailUtil qw(is_valid_email);
 use if !$ENV{TRAVIS}, 'Image::Magick';
 use DateTime::Format::Strptime;
+use List::Util 'first';
 
 
 use FixMyStreet::SendReport;
@@ -672,8 +673,8 @@ sub report_edit : Path('report_edit') : Args(1) {
         );
     }
 
-    if ( $c->get_param('rotate_photo') ) {
-        $c->forward('rotate_photo');
+    if (my $rotate_photo_param = $self->_get_rotate_photo_param($c)) {
+        $self->rotate_photo($c,  @$rotate_photo_param);
         return 1;
     }
 
@@ -1370,36 +1371,28 @@ Rotate a photo 90 degrees left or right
 
 =cut
 
+# returns index of photo to rotate, if any
+sub _get_rotate_photo_param {
+    my ($self, $c) = @_;
+    my $params = $c->req->parameters;
+    my $key = first { /^rotate_photo/ } $c->req->param or return;
+    my ($index) = $key =~ /(\d+)$/;
+    my $direction = $c->req->param($key);
+    return [ $index || 0, $key, $direction ];
+}
+
 sub rotate_photo : Private {
-    my ( $self, $c ) =@_;
+    my ( $self, $c, $index, $key, $direction ) = @_;
 
-    my $direction = $c->get_param('rotate_photo');
     return unless $direction eq _('Rotate Left') or $direction eq _('Rotate Right');
 
-    my $photo = $c->stash->{problem}->photo;
-    my $file;
-
-    # If photo field contains a hash
-    if ( length($photo) == 40 ) {
-        $file = file( $c->config->{UPLOAD_DIR}, "$photo.jpeg" );
-        $photo = $file->slurp;
-    }
-
-    $photo = _rotate_image( $photo, $direction eq _('Rotate Left') ? -90 : 90 );
-    return unless $photo;
+    my $problem = $c->stash->{problem};
+    my $fileid = $problem->get_photoset($c)->rotate_image(
+        $index,
+        $direction eq _('Rotate Left') ? -90 : 90
+    ) or return;
 
-    # Write out to new location
-    my $fileid = sha1_hex($photo);
-    $file = file( $c->config->{UPLOAD_DIR}, "$fileid.jpeg" );
-
-    my $fh = $file->open('w');
-    print $fh $photo;
-    close $fh;
-
-    unlink glob FixMyStreet->path_to( 'web', 'photo', $c->stash->{problem}->id . '.*' );
-
-    $c->stash->{problem}->photo( $fileid );
-    $c->stash->{problem}->update();
+    $problem->update({ photo => $fileid });
 
     return 1;
 }
@@ -1449,18 +1442,6 @@ sub trim {
     return $e;
 }
 
-sub _rotate_image {
-    my ($photo, $direction) = @_;
-    my $image = Image::Magick->new;
-    $image->BlobToImage($photo);
-    my $err = $image->Rotate($direction);
-    return 0 if $err;
-    my @blobs = $image->ImageToBlob();
-    undef $image;
-    return $blobs[0];
-}
-
-
 =head1 AUTHOR
 
 Struan Donald
diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm
index a2ec7d4c8..7cf78cb2f 100644
--- a/perllib/FixMyStreet/App/Controller/Photo.pm
+++ b/perllib/FixMyStreet/App/Controller/Photo.pm
@@ -8,8 +8,8 @@ use DateTime::Format::HTTP;
 use Digest::SHA qw(sha1_hex);
 use File::Path;
 use File::Slurp;
-use Image::Size;
 use Path::Class;
+use FixMyStreet::App::Model::PhotoSet;
 use if !$ENV{TRAVIS}, 'Image::Magick';
 
 =head1 NAME
@@ -49,13 +49,13 @@ sub during :LocalRegex('^([0-9a-f]{40})\.(temp|fulltemp)\.jpeg$') {
     $c->forward( 'output', [ $photo ] );
 }
 
-sub index :LocalRegex('^(c/)?(\d+)(?:\.(full|tn|fp))?\.jpeg$') {
+sub index :LocalRegex('^(c/)?(\d+)(?:\.(\d+))?(?:\.(full|tn|fp))?\.jpeg$') {
     my ( $self, $c ) = @_;
-    my ( $is_update, $id, $size ) = @{ $c->req->captures };
+    my ( $is_update, $id, $photo_number, $size ) = @{ $c->req->captures };
 
-    my @photo;
+    my $item;
     if ( $is_update ) {
-        @photo = $c->model('DB::Comment')->search( {
+        ($item) = $c->model('DB::Comment')->search( {
             id => $id,
             state => 'confirmed',
             photo => { '!=', undef },
@@ -67,35 +67,21 @@ sub index :LocalRegex('^(c/)?(\d+)(?:\.(full|tn|fp))?\.jpeg$') {
         }
 
         $c->detach( 'no_photo' ) if $id =~ /\D/;
-        @photo = $c->cobrand->problems->search( {
+        ($item) = $c->cobrand->problems->search( {
             id => $id,
             state => [ FixMyStreet::DB::Result::Problem->visible_states(), 'partial' ],
             photo => { '!=', undef },
         } );
     }
 
-    $c->detach( 'no_photo' ) unless @photo;
+    $c->detach( 'no_photo' ) unless $item;
 
-    my $item = $photo[0];
     $c->detach( 'no_photo' ) unless $c->cobrand->allow_photo_display($item); # Should only be for reports, not updates
-    my $photo = $item->photo;
 
-    # If photo field contains a hash
-    if (length($photo) == 40) {
-        my $file = file( $c->config->{UPLOAD_DIR}, "$photo.jpeg" );
-        $photo = $file->slurp;
-    }
-
-    if ( $size eq 'tn' ) {
-        $photo = _shrink( $photo, 'x100' );
-    } elsif ( $size eq 'fp' ) {
-        $photo = _crop( $photo );
-    } elsif ( $size eq 'full' ) {
-    } elsif ( $c->cobrand->default_photo_resize ) {
-        $photo = _shrink( $photo, $c->cobrand->default_photo_resize );
-    } else {
-        $photo = _shrink( $photo, '250x250' );
-    }
+    my $photo = $item->get_photoset( $c )
+        ->get_image_data( num => $photo_number, size => $size )
+        
+    or $c->detach( 'no_photo' );
 
     $c->forward( 'output', [ $photo ] );
 }
@@ -156,84 +142,28 @@ sub process_photo : Private {
     my ( $self, $c ) = @_;
 
     return
-         $c->forward('process_photo_upload')
-      || $c->forward('process_photo_cache')
+         $c->forward('process_photo_upload_or_cache')
       || 1;    # always return true
 }
 
-sub process_photo_upload : Private {
+sub process_photo_upload_or_cache : Private {
     my ( $self, $c ) = @_;
-
-    # check for upload or return
-    my $upload = $c->req->upload('photo')
-      || return;
-
-    # check that the photo is a jpeg
-    my $ct = $upload->type;
-    $ct =~ s/x-citrix-//; # Thanks, Citrix
-    # Had a report of a JPEG from an Android 2.1 coming through as a byte stream
-    unless ( $ct eq 'image/jpeg' || $ct eq 'image/pjpeg' || $ct eq 'application/octet-stream' ) {
-        $c->log->info('Bad photo tried to upload, type=' . $ct);
-        $c->stash->{photo_error} = _('Please upload a JPEG image only');
-        return;
-    }
-
-    # get the photo into a variable
-    my $photo_blob = eval {
-        my $filename = $upload->tempname;
-        my $out = `jhead -se -autorot $filename 2>&1`;
-        unless (defined $out) {
-            my ($w, $h, $err) = Image::Size::imgsize($filename);
-            die _("Please upload a JPEG image only") . "\n" if !defined $w || $err ne 'JPG';
-        }
-        die _("Please upload a JPEG image only") . "\n" if $out && $out =~ /Not JPEG:/;
-        my $photo = $upload->slurp;
-        return $photo;
-    };
-    if ( my $error = $@ ) {
-        my $format = _(
-"That image doesn't appear to have uploaded correctly (%s), please try again."
-        );
-        $c->stash->{photo_error} = sprintf( $format, $error );
-        return;
-    }
-
-    # we have an image we can use - save it to the upload dir for storage
-    my $cache_dir = dir( $c->config->{UPLOAD_DIR} );
-    $cache_dir->mkpath;
-    unless ( -d $cache_dir && -w $cache_dir ) {
-        warn "Can't find/write to photo cache directory '$cache_dir'";
-        return;
-    }
-
-    my $fileid = sha1_hex($photo_blob);
-    $upload->copy_to( file($cache_dir, $fileid . '.jpeg') );
-
-    # stick the hash on the stash, so don't have to reupload in case of error
-    $c->stash->{upload_fileid} = $fileid;
-
-    return 1;
-}
-
-=head2 process_photo_cache
-
-Look for the upload_fileid parameter and check it matches a file on disk. If it
-does return true and put fileid on stash, otherwise false.
-
-=cut
-
-sub process_photo_cache : Private {
-    my ( $self, $c ) = @_;
-
-    # get the fileid and make sure it is just a hex number
-    my $fileid = $c->get_param('upload_fileid') || '';
-    $fileid =~ s{[^0-9a-f]}{}gi;
-    return unless $fileid;
-
-    my $file = file( $c->config->{UPLOAD_DIR}, "$fileid.jpeg" );
-    return unless -e $file;
-
-    $c->stash->{upload_fileid} = $fileid;
+    my @items = (
+        ( map {
+            /^photo/ ? # photo, photo1, photo2 etc.
+                ($c->req->upload($_)) : ()
+        } sort $c->req->upload),
+        split /,/, ($c->req->param('upload_fileid') || '')
+    );
+
+    my $photoset = FixMyStreet::App::Model::PhotoSet->new({
+        c => $c,
+        data_items => \@items,
+    });
+
+    my $fileid = $photoset->data;
+
+    $c->stash->{upload_fileid} = $fileid or return;
     return 1;
 }
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index b55152693..6a884db7f 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -93,6 +93,7 @@ sub report_new : Path : Args(0) {
     $c->forward('check_for_category');
 
     # deal with the user and report and check both are happy
+
     return unless $c->forward('check_form_submitted');
     $c->forward('process_user');
     $c->forward('process_report');
@@ -290,7 +291,7 @@ sub report_import : Path('/import') {
     }
 
     # handle the photo upload
-    $c->forward( '/photo/process_photo_upload' );
+    $c->forward( '/photo/process_photo' );
     my $fileid = $c->stash->{upload_fileid};
     if ( my $error = $c->stash->{photo_error} ) {
         push @errors, $error;
diff --git a/perllib/FixMyStreet/App/Model/PhotoSet.pm b/perllib/FixMyStreet/App/Model/PhotoSet.pm
new file mode 100644
index 000000000..fa6eb060b
--- /dev/null
+++ b/perllib/FixMyStreet/App/Model/PhotoSet.pm
@@ -0,0 +1,269 @@
+package FixMyStreet::App::Model::PhotoSet;
+
+# TODO this isn't a Cat model, rename to something else
+
+use Moose;
+use Path::Tiny 'path';
+use if !$ENV{TRAVIS}, 'Image::Magick';
+use Scalar::Util 'openhandle', 'blessed';
+use Digest::SHA qw(sha1_hex);
+use Image::Size;
+
+has c => (
+    is => 'ro',
+);
+
+has object => (
+    is => 'ro',
+);
+
+has data => ( # generic data from DB field
+    is => 'ro',
+    lazy => 1,
+    default => sub {
+        # yes, this is a little circular: data -> data_items -> items -> data
+        # e.g. if not provided, then we're presumably uploading/etc., so calculate from 
+        # the stored cached fileids
+        # (obviously if you provide none of these, then you'll get an infinite loop)
+        my $self = shift;
+        my $data = join ',', map { $_->[0] } $self->all_images;
+        return $data;
+    }
+);
+
+has data_items => ( # either a) split from data or b) provided by photo upload
+    isa => 'ArrayRef',
+    is => 'rw',
+    traits => ['Array'],
+    lazy => 1,
+    handles => {
+        map_data_items => 'map',
+    },
+    default => sub {
+        my $self = shift;
+        my $data = $self->data
+            or return [];
+
+        return [$data] if (_jpeg_magic($data));
+
+        return [ split ',' => $data ];
+    },
+);
+
+has upload_dir => (
+    is => 'ro',
+    lazy => 1,
+    default => sub {
+        my $self = shift;
+        my $cache_dir = path( $self->c->config->{UPLOAD_DIR} );
+        $cache_dir->mkpath;
+        unless ( -d $cache_dir && -w $cache_dir ) {
+            warn "Can't find/write to photo cache directory '$cache_dir'";
+            return;
+        }
+        $cache_dir;
+    },
+);
+
+sub _jpeg_magic {
+    $_[0] =~ /^\x{ff}\x{d8}/; # JPEG
+    # NB: should we also handle \x{89}\x{50} (PNG, 15 results in live DB) ?
+    #     and \x{49}\x{49} (Tiff, 3 results in live DB) ?
+}
+
+has images => ( # jpeg data for actual image
+    isa => 'ArrayRef',
+    is => 'rw',
+    traits => ['Array'],
+    lazy => 1,
+    handles => {
+        num_images => 'count',
+        get_raw_image_data => 'get',
+        all_images => 'elements',
+    },
+    default => sub {
+        my $self = shift;
+        my @photos = $self->map_data_items( sub {
+            my $part = $_;
+
+            if (blessed $part and $part->isa('Catalyst::Request::Upload')) {
+                # check that the photo is a jpeg
+                my $upload = $part;
+                my $ct = $upload->type;
+                $ct =~ s/x-citrix-//; # Thanks, Citrix
+                # Had a report of a JPEG from an Android 2.1 coming through as a byte stream
+                unless ( $ct eq 'image/jpeg' || $ct eq 'image/pjpeg' || $ct eq 'application/octet-stream' ) {
+                    my $c = $self->c;
+                    $c->log->info('Bad photo tried to upload, type=' . $ct);
+                    $c->stash->{photo_error} = _('Please upload a JPEG image only');
+                    return ();
+                }
+
+                # get the photo into a variable
+                my $photo_blob = eval {
+                    my $filename = $upload->tempname;
+                    my $out = `jhead -se -autorot $filename 2>&1`;
+                    unless (defined $out) {
+                        my ($w, $h, $err) = Image::Size::imgsize($filename);
+                        die _("Please upload a JPEG image only") . "\n" if !defined $w || $err ne 'JPG';
+                    }
+                    die _("Please upload a JPEG image only") . "\n" if $out && $out =~ /Not JPEG:/;
+                    my $photo = $upload->slurp;
+                };
+                if ( my $error = $@ ) {
+                    my $format = _(
+            "That image doesn't appear to have uploaded correctly (%s), please try again."
+                    );
+                    $self->c->stash->{photo_error} = sprintf( $format, $error );
+                    return ();
+                }
+
+                # we have an image we can use - save it to the upload dir for storage
+                my $fileid = $self->get_fileid($photo_blob);
+                my $file = $self->get_file($fileid);
+                $upload->copy_to( $file );
+                return [$fileid, $photo_blob];
+
+            }
+            if (_jpeg_magic($part)) {
+                my $photo_blob = $part;
+                my $fileid = $self->get_fileid($photo_blob);
+                my $file = $self->get_file($fileid);
+                $file->spew_raw($photo_blob);
+                return [$fileid, $photo_blob];
+            }
+            if (length($part) == 40) {
+                my $fileid = $part;
+                my $file = $self->get_file($fileid);
+                if ($file->exists) {
+                    my $photo = $file->slurp_raw;
+                    [$fileid, $photo];
+                }
+                else {
+                    warn "File $fileid doesn't exist";
+                    ();
+                }
+            }
+            else {
+                warn sprintf "Received bad photo hash of length %d", length($part);
+                ();
+            }
+        });
+        return \@photos;
+    },
+);
+
+sub get_fileid {
+    my ($self, $photo_blob) = @_;
+    return sha1_hex($photo_blob);
+}
+
+sub get_file {
+    my ($self, $fileid) = @_;
+    my $cache_dir = $self->upload_dir;
+    return path( $cache_dir, "$fileid.jpeg" );
+}
+
+sub get_image_data {
+    my ($self, %args) = @_;
+    my $num = $args{num} || 0;
+
+    my $data = $self->get_raw_image_data( $num )
+        or return;
+
+    my ($fileid, $photo) = @$data;
+
+    my $size = $args{size};
+    if ( $size eq 'tn' ) {
+        $photo = _shrink( $photo, 'x100' );
+    } elsif ( $size eq 'fp' ) {
+        $photo = _crop( $photo );
+    } elsif ( $size eq 'full' ) {
+        # do nothing
+    } else {
+        $photo = _shrink( $photo, $self->c->cobrand->default_photo_resize || '250x250' );
+    }
+
+    return $photo;
+}
+
+sub delete_cached {
+    my ($self) = @_;
+    my $object = $self->object or return;
+
+    unlink glob FixMyStreet->path_to(
+        'web',
+        'photo',
+        $object->id . '.*'
+    );
+}
+
+sub rotate_image {
+    my ($self, $index, $direction) = @_;
+
+    my @images = $self->all_images;
+    return if $index > $#images;
+
+    my @items = map $_->[0], @images;
+    $items[$index] = _rotate_image( $images[$index][1], $direction );
+
+    my $new_set = (ref $self)->new({
+        data_items => \@items,
+        c => $self->c,
+        object => $self->object,
+    });
+
+    $self->delete_cached();
+
+    return $new_set->data; # e.g. new comma-separated fileid
+}
+
+sub _rotate_image {
+    my ($photo, $direction) = @_;
+    return $photo unless $Image::Magick::VERSION;
+    my $image = Image::Magick->new;
+    $image->BlobToImage($photo);
+    my $err = $image->Rotate($direction);
+    return 0 if $err;
+    my @blobs = $image->ImageToBlob();
+    undef $image;
+    return $blobs[0];
+}
+
+
+
+
+
+# NB: These 2 subs stolen from A::C::Photo, should be purged from there!
+#
+# Shrinks a picture to the specified size, but keeping in proportion.
+sub _shrink {
+    my ($photo, $size) = @_;
+    return $photo unless $Image::Magick::VERSION;
+    my $image = Image::Magick->new;
+    $image->BlobToImage($photo);
+    my $err = $image->Scale(geometry => "$size>");
+    throw Error::Simple("resize failed: $err") if "$err";
+    $image->Strip();
+    my @blobs = $image->ImageToBlob();
+    undef $image;
+    return $blobs[0];
+}
+
+# Shrinks a picture to 90x60, cropping so that it is exactly that.
+sub _crop {
+    my ($photo) = @_;
+    return $photo unless $Image::Magick::VERSION;
+    my $image = Image::Magick->new;
+    $image->BlobToImage($photo);
+    my $err = $image->Resize( geometry => "90x60^" );
+    throw Error::Simple("resize failed: $err") if "$err";
+    $err = $image->Extent( geometry => '90x60', gravity => 'Center' );
+    throw Error::Simple("resize failed: $err") if "$err";
+    $image->Strip();
+    my @blobs = $image->ImageToBlob();
+    undef $image;
+    return $blobs[0];
+}
+
+1;
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index 9706087aa..635c41382 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -483,11 +483,17 @@ sub url {
 
 =head2 get_photo_params
 
-Returns a hashref of details of any attached photo for use in templates.
+Returns a hashref of details of the attached photo, if any, for use in templates.
+
+NB: this method doesn't currently support multiple photos gracefully.  
+
+Use get_photoset($c) instead to do the right thing with reports with 0, 1, or more photos.
 
 =cut
 
 sub get_photo_params {
+    # use Carp 'cluck';
+    # cluck "get_photo_params called"; # TEMPORARY die to make sure I've done right thing with Zurich templates
     my $self = shift;
     return FixMyStreet::App::get_photo_params($self, 'id');
 }
@@ -828,6 +834,27 @@ sub latest_moderation_log_entry {
     return $self->admin_log_entries->search({ action => 'moderation' }, { order_by => 'id desc' })->first;
 }
 
+=head2 get_photoset
+
+Return a PhotoSet object for all photos attached to this field
+
+    my $photoset = $obj->get_photoset( $c );
+    print $photoset->num_images;
+    return $photoset->get_image_data(num => 0, size => 'full');
+
+=cut
+
+sub get_photoset {
+    my ($self, $c) = @_;
+    my $class = 'FixMyStreet::App::Model::PhotoSet';
+    eval "use $class";
+    return $class->new({
+        c => $c,
+        data => $self->photo,
+        object => $self,
+    });
+}
+
 __PACKAGE__->has_many(
   "admin_log_entries",
   "FixMyStreet::DB::Result::AdminLog",
diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm
index bd2ca4096..ef60b0d36 100644
--- a/perllib/FixMyStreet/TestMech.pm
+++ b/perllib/FixMyStreet/TestMech.pm
@@ -631,7 +631,7 @@ sub create_problems_for_body {
             latitude           => '51.5016605453401',
             longitude          => '-0.142497580865087',
             user_id            => $user->id,
-            photo              => 1,
+            photo              => $mech->get_photo_data,
         };
 
         my %report_params = ( %$default_params, %$params );
@@ -646,4 +646,13 @@ sub create_problems_for_body {
     return @problems;
 }
 
+sub get_photo_data {
+    my $mech = shift;
+    return $mech->{sample_photo} ||= do {
+        my $sample_file = FixMyStreet->path_to( 't/app/controller/sample.jpg' );
+        $mech->builder->ok( -f "$sample_file", "sample file $sample_file exists" );
+        $sample_file->slurp(iomode => '<:raw');
+    };
+}
+
 1;
diff --git a/t/app/controller/moderate.t b/t/app/controller/moderate.t
index cd4c742bb..b79f50e73 100644
--- a/t/app/controller/moderate.t
+++ b/t/app/controller/moderate.t
@@ -42,10 +42,7 @@ sub create_report {
         latitude           => '51.4129',
         longitude          => '0.007831',
         user_id            => $user->id,
-        photo              => 'DUMMY DATA', # this obv fake data would not be
-                                            # accepted by front-end but is
-                                            # enough to trigger "I have a
-                                            # photo" behaviour
+        photo              => $mech->get_photo_data,
     });
 }
 my $report = create_report();
@@ -216,7 +213,7 @@ sub create_update {
         user      => $user,
         name      => 'Test User',
         anonymous => 'f',
-        photo     => 'DUMMY DATA', # as above
+        photo     => $mech->get_photo_data,
         text      => 'update good good bad good',
         state     => 'confirmed',
         mark_fixed => 0,
@@ -283,7 +280,8 @@ subtest 'updates' => sub {
 
         $mech->get_ok($REPORT_URL);
 
-        $mech->content_contains('Photo of this report');
+        $mech->content_contains('Photo of this report')
+            or die $mech->content;
 
         $mech->post_ok( $MODERATE_UPDATE_URL, {
             %update_prepopulated,
diff --git a/t/app/controller/photo.t b/t/app/controller/photo.t
new file mode 100644
index 000000000..6e61ebb32
--- /dev/null
+++ b/t/app/controller/photo.t
@@ -0,0 +1,75 @@
+use strict;
+use utf8; # sign in error message has &ndash; in it
+use warnings;
+use feature 'say';
+use Test::More;
+use utf8;
+
+use FixMyStreet::TestMech;
+use FixMyStreet::App;
+use Web::Scraper;
+use Path::Tiny;
+use File::Temp 'tempdir';
+
+# disable info logs for this test run
+FixMyStreet::App->log->disable('info');
+END { FixMyStreet::App->log->enable('info'); }
+
+my $mech = FixMyStreet::TestMech->new;
+
+my $sample_file = path(__FILE__)->parent->child("sample.jpg");
+ok $sample_file->exists, "sample file $sample_file exists";
+
+my $westminster = $mech->create_body_ok(2527, 'Liverpool City Council');
+
+subtest "Check multiple upload worked" => sub {
+    $mech->get_ok('/around');
+
+    my $UPLOAD_DIR = tempdir( CLEANUP => 1 );
+
+    # submit initial pc form
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => [ { fixmystreet => '.' } ],
+        MAPIT_URL => 'http://mapit.mysociety.org/',
+        UPLOAD_DIR => $UPLOAD_DIR,
+    }, sub {
+
+        $mech->log_in_ok('test@example.com');
+
+
+        # submit the main form
+        # can't post_ok as we lose the Content_Type header
+        # (TODO rewrite with HTTP::Request::Common and request_ok)
+        $mech->post( '/report/new',
+            Content_Type => 'form-data',
+            Content =>
+            {
+            submit_problem => 1,
+            title         => 'Test',
+            lat => 53.4031156, lon => -2.9840579, # in Liverpool
+            pc            => 'L1 4LN',
+            detail        => 'Detail',
+            photo1         => [ $sample_file, undef, Content_Type => 'application/octet-stream' ],
+            photo2         => [ $sample_file, undef, Content_Type => 'application/octet-stream' ],
+            photo3         => [ $sample_file, undef, Content_Type => 'application/octet-stream' ],
+            name          => 'Bob Jones',
+            may_show_name => '1',
+            email         => 'test@example.com',
+            phone         => '',
+            category      => 'Street lighting',
+            #password_sign_in => '',
+            #password_register => '',
+            #remember_me => undef,
+            }
+        );
+        ok $mech->success, 'Made request with multiple photo upload';
+        $mech->base_is('http://localhost/report/new');
+        $mech->content_contains(
+            'name="upload_fileid" value="1cdd4329ceee2234bd4e89cb33b42061a0724687,1cdd4329ceee2234bd4e89cb33b42061a0724687,1cdd4329ceee2234bd4e89cb33b42061a0724687"',
+            'Returned upload_fileid contains expected hash, 3 times');
+        my $image_file = path($UPLOAD_DIR, '1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg');
+        ok $image_file->exists, 'File uploaded to temp';
+    };
+};
+
+done_testing();
diff --git a/t/app/controller/report_import.t b/t/app/controller/report_import.t
index 16874ac3c..480fdd89f 100644
--- a/t/app/controller/report_import.t
+++ b/t/app/controller/report_import.t
@@ -364,11 +364,14 @@ subtest "Submit a correct entry (with location) to cobrand" => sub {
       {
         name          => 'Test User ll',
         detail        => 'This is a test report ll',
-        photo         => '',
+        photo1         => '',
+        photo2         => '',
+        photo3         => '',
         phone         => '',
         email => 'test-ll@example.com',
       },
-      "check imported fields are shown";
+      "check imported fields are shown"
+          or diag Dumper( $mech->visible_form_values ); use Data::Dumper;
 
     my $user =
       FixMyStreet::App->model('DB::User')
diff --git a/t/app/model/photoset.t b/t/app/model/photoset.t
new file mode 100644
index 000000000..9e566f873
--- /dev/null
+++ b/t/app/model/photoset.t
@@ -0,0 +1,76 @@
+use strict;
+use warnings;
+use Test::More;
+use Test::Exception;
+use utf8;
+
+use FixMyStreet::App;
+use Data::Dumper;
+use DateTime;
+use Path::Tiny 'path';
+use File::Temp 'tempdir';
+
+my $dt = DateTime->now;
+
+my $c = FixMyStreet::App->new;
+my $UPLOAD_DIR = tempdir( CLEANUP => 1 );
+local $c->config->{UPLOAD_DIR} = $UPLOAD_DIR;
+
+my $user = $c->model('DB::User')->find_or_create({
+        name => 'Bob', email => 'bob@example.com',
+});
+
+my $image_path = path('t/app/controller/sample.jpg');
+
+my $db = FixMyStreet::App->model('DB')->schema;
+$db->txn_begin;
+
+sub make_report {
+    my $photo_data = shift;
+    return $db->resultset('Problem')->create({
+        postcode           => 'BR1 3SB',
+        bodies_str         => '',
+        areas              => ",,",
+        category           => 'Other',
+        title              => 'test',
+        detail             => 'test',
+        used_map           => 't',
+        name               => 'Anon',
+        anonymous          => 't',
+        state              => 'confirmed',
+        confirmed          => $dt,
+        lang               => 'en-gb',
+        service            => '',
+        cobrand            => 'default',
+        cobrand_data       => '',
+        send_questionnaire => 't',
+        latitude           => '51.4129',
+        longitude          => '0.007831',
+        user => $user,
+        photo => $photo_data,
+    });
+}
+
+
+subtest 'Photoset with photo inline in DB' => sub {
+    my $report = make_report( $image_path->slurp );
+    my $photoset = $report->get_photoset($c);
+    is $photoset->num_images, 1, 'Found just 1 image';
+};
+
+$image_path->copy( path( $UPLOAD_DIR, '0123456789012345678901234567890123456789.jpeg' ) );
+subtest 'Photoset with 1 referenced photo' => sub {
+    my $report = make_report( '0123456789012345678901234567890123456789' );
+    my $photoset = $report->get_photoset($c);
+    is $photoset->num_images, 1, 'Found just 1 image';
+};
+
+subtest 'Photoset with 1 referenced photo' => sub {
+    my $report = make_report( '0123456789012345678901234567890123456789,0123456789012345678901234567890123456789,0123456789012345678901234567890123456789' );
+    my $photoset = $report->get_photoset($c);
+    is $photoset->num_images, 3, 'Found 3 images';
+};
+
+$db->txn_rollback;
+
+done_testing();
diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t
index 6394c8942..e1cb55b16 100644
--- a/t/cobrand/zurich.t
+++ b/t/cobrand/zurich.t
@@ -6,6 +6,7 @@ use warnings;
 use DateTime;
 use Test::More;
 use JSON;
+use Path::Tiny;
 
 # Check that you have the required locale installed - the following
 # should return a line with de_CH.utf8 in. If not install that locale.
@@ -21,6 +22,10 @@ my $c = FixMyStreet::App->new();
 my $cobrand = FixMyStreet::Cobrand::Zurich->new({ c => $c });
 $c->stash->{cobrand} = $cobrand;
 
+my $sample_file = path(__FILE__)->parent->parent->child("app/controller/sample.jpg");
+ok $sample_file->exists, "sample file $sample_file exists";
+my $sample_photo = $sample_file->slurp_raw;
+
 # This is a helper method that will send the reports but with the config
 # correctly set - notably SEND_REPORTS_ON_STAGING needs to be true.
 sub send_reports_for_zurich {
@@ -103,6 +108,7 @@ my @reports = $mech->create_problems_for_body( 1, $division->id, 'Test', {
     state              => 'unconfirmed',
     confirmed          => undef,
     cobrand            => 'zurich',
+    photo         => $sample_photo,
 });
 my $report = $reports[0];
 
@@ -136,7 +142,6 @@ $mech->content_contains( 'report_edit/' . $report->id );
 $mech->content_contains( DateTime->now->strftime("%d.%m.%Y") );
 $mech->content_contains( 'Erfasst' );
 
-
 subtest "changing of categories" => sub {
     # create a few categories (which are actually contacts)
     foreach my $name ( qw/Cat1 Cat2/ ) {
@@ -213,11 +218,14 @@ subtest "report_edit" => sub {
         $mech->content_contains( 'Unbest&auml;tigt' ); # Unconfirmed email
         $mech->submit_form_ok( { with_fields => { state => 'confirmed' } } );
         $mech->get_ok( '/report/' . $report->id );
+
+        $report->discard_changes();
+        is $report->state, 'confirmed', 'state has been updated to confirmed';
     };
 
     $mech->content_contains('Aufgenommen');
     $mech->content_contains('Test Test');
-    $mech->content_lacks('photo/' . $report->id . '.jpeg');
+    $mech->content_lacks('photo/' . $report->id . '.0.jpeg');
     $mech->email_count_is(0);
 
     $report->discard_changes;
@@ -302,7 +310,7 @@ FixMyStreet::override_config {
     $mech->get_ok( '/admin/report_edit/' . $report->id );
     $mech->submit_form_ok( { with_fields => { state => 'confirmed', publish_photo => 1 } } );
     $mech->get_ok( '/report/' . $report->id );
-    $mech->content_contains('photo/' . $report->id . '.jpeg');
+    $mech->content_contains('photo/' . $report->id . '.0.jpeg');
 
     # Internal notes
     $mech->get_ok( '/admin/report_edit/' . $report->id );
@@ -424,6 +432,7 @@ $mech->clear_emails_ok;
     state              => 'unconfirmed',
     confirmed          => undef,
     cobrand            => 'zurich',
+    photo         => $sample_photo,
 });
 $report = $reports[0];
 
@@ -458,6 +467,7 @@ $mech->email_count_is(0);
     state              => 'unconfirmed',
     confirmed          => undef,
     cobrand            => 'zurich',
+    photo         => $sample_photo,
 });
 $report = $reports[0];
 
@@ -679,3 +689,5 @@ END {
     ok $mech->host("www.fixmystreet.com"), "change host back";
     done_testing();
 }
+
+1;
diff --git a/templates/web/base/report/photo.html b/templates/web/base/report/photo.html
index c463c34c4..094f677d8 100644
--- a/templates/web/base/report/photo.html
+++ b/templates/web/base/report/photo.html
@@ -1,8 +1,21 @@
 [% IF c.cobrand.allow_photo_display(object) && object.photo %]
-[% photo = object.get_photo_params %]
-<div class="update-img">
-    [% IF photo.url_full %]<a href="[% photo.url_full %]" rel="fancy">[% END
-    %]<img alt="Photo of this report" [% IF photo.height %]height="[% photo.height %]" width="[% photo.width %]"[% END %] src="[% photo.url %]">
-    [%- IF photo.url_full %]<span>zoom</span></a>[% END %]
-</div>
+    [% IF object.can('get_photoset') %]
+    [% FOR photo IN object.get_photoset(c).images %]
+    <div class="update-img">
+        <a href="[% c.cobrand.base_url %]/photo/[% object.id %].[% loop.index %].full.jpeg?[% photo.0 %]" rel="fancy">
+            <img alt="Photo of this report" src="[% c.cobrand.base_url %]/photo/[% object.id %].[% loop.index %].jpeg?[% photo.0 %]">
+        <span>zoom</span></a>
+    </div>
+    [% END %]
+    [% ELSE %]
+    [%# e.g. comments %]
+    [% photo = object.get_photo_params %]
+    <div class="update-img">
+        [% IF photo.url_full %]<a href="[% photo.url_full %]" rel="fancy">[% END %]
+        <img alt="Photo of this report"
+        [%- IF photo.height %]height="[% photo.height %]" width="[% photo.width %]"[% END -%]
+        src="[% photo.url %]">
+        [%- IF photo.url_full %]<span>zoom</span></a>[% END %]
+    </div>
+    [% END %]
 [% END %]
diff --git a/templates/web/zurich/admin/problem_row.html b/templates/web/zurich/admin/problem_row.html
index 9b395a1ac..123ff5469 100644
--- a/templates/web/zurich/admin/problem_row.html
+++ b/templates/web/zurich/admin/problem_row.html
@@ -34,7 +34,11 @@
 
         <td>
         [% IF problem.photo %]
-            <img class="img" height="60" width="90" src="[% c.cobrand.base_url %]/photo/[% problem.photo %].temp.jpeg" alt="">
+            [% FOR photo IN problem.get_photoset(c).images %]
+            <div class="update-img">
+                <img height="60" width="90" alt="" src="[% c.cobrand.base_url %]/photo/[% photo.0 %].temp.jpeg">
+            </div>
+            [% END %]
         [% END %]
         </td>
 
diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html
index f8da9f211..02c396e1f 100644
--- a/templates/web/zurich/admin/report_edit.html
+++ b/templates/web/zurich/admin/report_edit.html
@@ -63,15 +63,16 @@
 
 [% IF problem.photo %]
 <li>
-[% photo = problem.get_photo_params %]
+[% FOR photo IN problem.get_photoset(c).images %]
 <div class="update-img">
-    <a href="[% c.cobrand.base_url %]/photo/[% problem.photo %].fulltemp.jpeg" rel="fancy">
-        <img alt="Photo of this report" src="[% c.cobrand.base_url %]/photo/[% problem.photo %].temp.jpeg">
+    <a href="[% c.cobrand.base_url %]/photo/[% photo.0 %].fulltemp.jpeg" rel="fancy">
+        <img alt="Photo of this report" src="[% c.cobrand.base_url %]/photo/[% photo.0 %].temp.jpeg">
     <span>zoom</span></a>
 </div>
+<input type="submit" name="rotate_photo_[% loop.index %]" value="[% loc('Rotate Left') %]">
+<input type="submit" name="rotate_photo_[% loop.index %]" value="[% loc('Rotate Right') %]">
+[% END %]
 <br>
-<input type="submit" name="rotate_photo" value="[% loc('Rotate Left') %]">
-<input type="submit" name="rotate_photo" value="[% loc('Rotate Right') %]">
 <br>
 <input type="checkbox" id="publish_photo" name="publish_photo" value="1"[% ' checked' IF problem.extra.publish_photo %]>
 <label class="inline" for="publish_photo">[% loc("Publish photo") %]</label></li>
diff --git a/templates/web/zurich/report/new/fill_in_details_form.html b/templates/web/zurich/report/new/fill_in_details_form.html
index 56096c9bd..c8c567786 100644
--- a/templates/web/zurich/report/new/fill_in_details_form.html
+++ b/templates/web/zurich/report/new/fill_in_details_form.html
@@ -58,7 +58,7 @@
                         <input type="hidden" name="upload_fileid" value="[% upload_fileid %]">
                     [% END %]
 
-                    <p>[% loc('You have already attached a photo to this report, attaching another one will replace it.') %]</p>
+                    <p>[% loc('You have already attached photos to this report.  Note that you can attach a maximum of 3 to this report (if you try to upload more, the oldest will be removed).') %]</p>
 
                     [% IF report.photo %]
                         <img align="right" src="/photo/[% report.id %].jpeg">
@@ -68,7 +68,9 @@
                 [% IF field_errors.photo %]
                     <p class='form-error'>[% field_errors.photo %]</p>
                 [% END %]
-                <input type="file" name="photo" id="form_photo">
+                <input type="file" name="photo1" id="form_photo">
+                <input type="file" name="photo2" id="form_photo2">
+                <input type="file" name="photo3" id="form_photo3">
             [% END %]
 
                 <label for="form_email">[% loc('Your email') %]</label>