diff options
34 files changed, 162 insertions, 140 deletions
diff --git a/perllib/Catalyst/Plugin/Session/State/Cookie.pm b/perllib/Catalyst/Plugin/Session/State/Cookie.pm index bef0c1e66..c4b61123b 100644 --- a/perllib/Catalyst/Plugin/Session/State/Cookie.pm +++ b/perllib/Catalyst/Plugin/Session/State/Cookie.pm @@ -50,13 +50,6 @@ sub update_session_cookie { sub cookie_is_rejecting { my ( $c, $cookie ) = @_; - # Prevent infinite loop in request->path. mySociety addition - return 0 if $c->request->has_uri && !$c->request->_has_path; - - # Don't output cookie for JS or JPEG files. mySociety addition - return 1 if substr($c->request->path, -3) eq '.js' - || substr($c->request->path, -5) eq '.jpeg'; - if ( $cookie->{path} ) { return 1 if index '/'.$c->request->path, $cookie->{path}; } diff --git a/perllib/FixMyStreet/App.pm b/perllib/FixMyStreet/App.pm index af9dc1f9d..79ca7f9ee 100644 --- a/perllib/FixMyStreet/App.pm +++ b/perllib/FixMyStreet/App.pm @@ -246,7 +246,7 @@ sub setup_dev_overrides { delete $params{$_} for grep { !m{^_override_} } keys %params; # stop if there is nothing to add - return 1 unless scalar keys %params; + return unless scalar keys %params; # Check to see if we should clear all if ( $params{_override_clear_all} ) { @@ -270,14 +270,14 @@ sub setup_dev_overrides { Checks the overrides for the value given and returns it if found, undef if not. -Always returns undef unless on a staging site (avoids autovivifying overrides -hash in session and so creating a session for all users). +Always returns undef unless on a staging site and we already have a session +(avoids autovivifying overrides hash and so creating a session for all users). =cut sub get_override { my ( $c, $key ) = @_; - return unless $c->config->{STAGING_SITE}; + return unless $c->config->{STAGING_SITE} && $c->sessionid; return $c->session->{overrides}->{$key}; } diff --git a/perllib/FixMyStreet/App/Controller/Open311.pm b/perllib/FixMyStreet/App/Controller/Open311.pm index f35dc64a5..4f1727b1a 100644 --- a/perllib/FixMyStreet/App/Controller/Open311.pm +++ b/perllib/FixMyStreet/App/Controller/Open311.pm @@ -284,7 +284,7 @@ sub output_requests : Private { my $display_photos = $c->cobrand->allow_photo_display($problem); if ($display_photos && $problem->photo) { my $url = $c->cobrand->base_url(); - my $imgurl = $url . "/photo/$id.full.jpeg"; + my $imgurl = $url . $problem->photos->[0]->{url_full}; $request->{'media_url'} = [ $imgurl ]; } push(@problemlist, $request); diff --git a/perllib/FixMyStreet/App/Controller/Photo.pm b/perllib/FixMyStreet/App/Controller/Photo.pm index bfb1c5535..d24d3ff71 100644 --- a/perllib/FixMyStreet/App/Controller/Photo.pm +++ b/perllib/FixMyStreet/App/Controller/Photo.pm @@ -29,12 +29,12 @@ Display a photo =cut -sub during :LocalRegex('^([0-9a-f]{40})\.(temp|fulltemp)\.jpeg$') { +sub during :LocalRegex('^(temp|fulltemp)\.([0-9a-f]{40}\.(?:jpeg|png|gif|tiff))$') { my ( $self, $c ) = @_; - my ( $hash, $size ) = @{ $c->req->captures }; + my ( $size, $filename ) = @{ $c->req->captures }; my $photoset = FixMyStreet::App::Model::PhotoSet->new({ - data_items => [ $hash ] + data_items => [ $filename ] }); $size = $size eq 'temp' ? 'default' : 'full'; @@ -43,7 +43,7 @@ sub during :LocalRegex('^([0-9a-f]{40})\.(temp|fulltemp)\.jpeg$') { $c->forward( 'output', [ $photo ] ); } -sub index :LocalRegex('^(c/)?(\d+)(?:\.(\d+))?(?:\.(full|tn|fp))?\.jpeg$') { +sub index :LocalRegex('^(c/)?(\d+)(?:\.(\d+))?(?:\.(full|tn|fp))?\.(?:jpeg|png|gif|tiff)$') { my ( $self, $c ) = @_; my ( $is_update, $id, $photo_number, $size ) = @{ $c->req->captures }; @@ -79,10 +79,10 @@ sub output : Private { # Save to file File::Path::make_path( FixMyStreet->path_to( 'web', 'photo', 'c' )->stringify ); - File::Slurp::write_file( FixMyStreet->path_to( 'web', $c->req->path )->stringify, \$photo ); + File::Slurp::write_file( FixMyStreet->path_to( 'web', $c->req->path )->stringify, \$photo->{data} ); - $c->res->content_type( 'image/jpeg' ); - $c->res->body( $photo ); + $c->res->content_type( $photo->{content_type} ); + $c->res->body( $photo->{data} ); } sub no_photo : Private { @@ -140,7 +140,7 @@ sub process_photo_upload_or_cache : Private { /^photo/ ? # photo, photo1, photo2 etc. ($c->req->upload($_)) : () } sort $c->req->upload), - split /,/, ($c->get_param('upload_fileid') || '') + grep { $_ } split /,/, ($c->get_param('upload_fileid') || '') ); my $photoset = FixMyStreet::App::Model::PhotoSet->new({ diff --git a/perllib/FixMyStreet/App/Controller/Reports.pm b/perllib/FixMyStreet/App/Controller/Reports.pm index 027b0d5a4..02dc4dc11 100644 --- a/perllib/FixMyStreet/App/Controller/Reports.pm +++ b/perllib/FixMyStreet/App/Controller/Reports.pm @@ -188,15 +188,6 @@ sub rss_area_ward : Path('/rss/area') : Args(2) { # We're checking an area here, but this function is currently doing that. return if $c->cobrand->reports_body_check( $c, $area ); - # If we're passed an ID number (don't think this is used anywhere, it - # certainly shouldn't be), just look that up on mapit and redirect - if ($area =~ /^\d+$/) { - my $council = mySociety::MaPit::call('area', $area); - $c->detach( 'redirect_index') if $council->{error}; - $c->stash->{body} = $council; - $c->detach( 'redirect_body' ); - } - # We must now have a string to check on mapit my $areas = mySociety::MaPit::call( 'areas', $area, type => $c->cobrand->area_types, @@ -296,15 +287,6 @@ sub body_check : Private { # Oslo/ kommunes sharing a name in Norway return if $c->cobrand->reports_body_check( $c, $q_body ); - # If we're passed an ID number (don't think this is used anywhere, it - # certainly shouldn't be), just look that up on MaPit and redirect - if ($q_body =~ /^\d+$/) { - my $area = mySociety::MaPit::call('area', $q_body); - $c->detach( 'redirect_index') if $area->{error}; - $c->stash->{body} = $area; - $c->detach( 'redirect_body' ); - } - # We must now have a string to check my @bodies = $c->model('DB::Body')->search( { name => { -like => "$q_body%" } } )->all; diff --git a/perllib/FixMyStreet/App/Controller/Rss.pm b/perllib/FixMyStreet/App/Controller/Rss.pm index 6047f063b..d708aa71c 100755 --- a/perllib/FixMyStreet/App/Controller/Rss.pm +++ b/perllib/FixMyStreet/App/Controller/Rss.pm @@ -6,6 +6,8 @@ use POSIX qw(strftime); use URI::Escape; use XML::RSS; +use FixMyStreet::App::Model::PhotoSet; + use mySociety::Gaze; use mySociety::Locale; use mySociety::MaPit; @@ -277,8 +279,13 @@ sub add_row : Private { $item{category} = ent($row->{category}) if $row->{category}; if ($c->cobrand->allow_photo_display($row) && $row->{photo}) { + # Bit yucky as we don't have full objects here + my $photoset = FixMyStreet::App::Model::PhotoSet->new({ db_data => $row->{photo} }); + my $first_fn = $photoset->get_id(0); + my ($hash, $format) = split /\./, $first_fn; + my $cachebust = substr($hash, 0, 8); my $key = $alert_type->item_table eq 'comment' ? 'c/' : ''; - $item{description} .= ent("\n<br><img src=\"". $base_url . "/photo/$key$row->{id}.jpeg\">"); + $item{description} .= ent("\n<br><img src=\"". $base_url . "/photo/$key$row->{id}.0.$format?$cachebust\">"); } if ( $row->{used_map} ) { diff --git a/perllib/FixMyStreet/App/Model/PhotoSet.pm b/perllib/FixMyStreet/App/Model/PhotoSet.pm index 54457bae9..b783a20e6 100644 --- a/perllib/FixMyStreet/App/Model/PhotoSet.pm +++ b/perllib/FixMyStreet/App/Model/PhotoSet.pm @@ -49,7 +49,7 @@ has data_items => ( # either a) split from db_data or b) provided by photo uploa my $self = shift; my $data = $self->db_data or return []; - return [$data] if (_jpeg_magic($data)); + return [$data] if (detect_type($data)); return [ split ',' => $data ]; }, @@ -70,10 +70,12 @@ has upload_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) ? +sub detect_type { + return 'jpeg' if $_[0] =~ /^\x{ff}\x{d8}/; + return 'png' if $_[0] =~ /^\x{89}\x{50}/; + return 'tiff' if $_[0] =~ /^II/; + return 'gif' if $_[0] =~ /^GIF/; + return ''; } =head2 C<ids>, C<num_images>, C<get_id>, C<all_ids> @@ -106,15 +108,17 @@ has ids => ( # Arrayref of $fileid tuples (always, so post upload/raw data proc 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 + my ($type) = $ct =~ m{image/(jpeg|pjpeg|gif|tiff|png)}; + $type = 'jpeg' if $type && $type eq 'pjpeg'; # 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' ) { + $type = 'jpeg' if !$type && $ct eq 'application/octet-stream'; + unless ( $type ) { my $c = $self->c; $c->log->info('Bad photo tried to upload, type=' . $ct); - $c->stash->{photo_error} = _('Please upload a JPEG image only'); + $c->stash->{photo_error} = _('Please upload an image only'); return (); } @@ -139,12 +143,13 @@ has ids => ( # Arrayref of $fileid tuples (always, so post upload/raw data proc # get the photo into a variable my $photo_blob = eval { my $filename = $upload->tempname; - my $out = `jhead -se -autorot $filename 2>&1`; + my $out; + $out = `jhead -se -autorot $filename 2>&1` if $type eq 'jpeg'; 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 an image only") . "\n" if !defined $w || $err !~ /JPG|GIF|PNG|TIF/; } - die _("Please upload a JPEG image only") . "\n" if $out && $out =~ /Not JPEG:/; + die _("Please upload an image only") . "\n" if $out && $out =~ /Not JPEG:/; my $photo = $upload->slurp; }; if ( my $error = $@ ) { @@ -157,29 +162,30 @@ has ids => ( # Arrayref of $fileid tuples (always, so post upload/raw data proc # 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); + my $file = $self->get_file($fileid, $type); $upload->copy_to( $file ); - return $fileid; + return $file->basename; } - if (_jpeg_magic($part)) { + if (my $type = detect_type($part)) { my $photo_blob = $part; my $fileid = $self->get_fileid($photo_blob); - my $file = $self->get_file($fileid); + my $file = $self->get_file($fileid, $type); $file->spew_raw($photo_blob); - return $fileid; + return $file->basename; } - if (length($part) == 40) { - my $fileid = $part; - my $file = $self->get_file($fileid); + my ($fileid, $type) = split /\./, $part; + $type ||= 'jpeg'; + if (length($fileid) == 40) { + my $file = $self->get_file($fileid, $type); if ($file->exists) { - $fileid; + $file->basename; } else { - warn "File $fileid doesn't exist"; + warn "File $part doesn't exist"; (); } } else { - warn sprintf "Received bad photo hash of length %d", length($part); + warn sprintf "Received bad photo hash of length %d", length($fileid); (); } }); @@ -193,18 +199,23 @@ sub get_fileid { } sub get_file { - my ($self, $fileid) = @_; + my ($self, $fileid, $type) = @_; my $cache_dir = $self->upload_dir; - return path( $cache_dir, "$fileid.jpeg" ); + return path( $cache_dir, "$fileid.$type" ); } -sub get_raw_image_data { +sub get_raw_image { my ($self, $index) = @_; - my $fileid = $self->get_id($index); - my $file = $self->get_file($fileid); + my $filename = $self->get_id($index); + my ($fileid, $type) = split /\./, $filename; + my $file = $self->get_file($fileid, $type); if ($file->exists) { my $photo = $file->slurp_raw; - return $photo; + return { + data => $photo, + content_type => "image/$type", + extension => $type, + }; } } @@ -212,8 +223,9 @@ sub get_image_data { my ($self, %args) = @_; my $num = $args{num} || 0; - my $photo = $self->get_raw_image_data( $num ) + my $image = $self->get_raw_image( $num ) or return; + my $photo = $image->{data}; my $size = $args{size}; if ( $size eq 'tn' ) { @@ -226,7 +238,10 @@ sub get_image_data { $photo = _shrink( $photo, $args{default} || '250x250' ); } - return $photo; + return { + data => $photo, + content_type => $image->{content_type}, + }; } sub delete_cached { @@ -250,13 +265,15 @@ sub remove_images { --$dec; } + $self->delete_cached(); + + return undef if !@images; + my $new_set = (ref $self)->new({ data_items => \@images, object => $self->object, }); - $self->delete_cached(); - return $new_set->data; # e.g. new comma-separated fileid } @@ -266,8 +283,8 @@ sub rotate_image { my @images = $self->all_ids; return if $index > $#images; - my $image_data = $self->get_raw_image_data($index); - $images[$index] = _rotate_image( $image_data, $direction ); + my $image = $self->get_raw_image($index); + $images[$index] = _rotate_image( $image->{data}, $direction ); my $new_set = (ref $self)->new({ data_items => \@images, diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index c76ee0f7d..6ba144f0d 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -625,6 +625,7 @@ sub short_name { my ($area) = @_; my $name = $area->{name} || $area->name; + $name =~ tr{/}{_}; $name = URI::Escape::uri_escape_utf8($name); $name =~ s/%20/+/g; return $name; diff --git a/perllib/FixMyStreet/Cobrand/EmptyHomes.pm b/perllib/FixMyStreet/Cobrand/EmptyHomes.pm index 995c39c85..70c3c9469 100644 --- a/perllib/FixMyStreet/Cobrand/EmptyHomes.pm +++ b/perllib/FixMyStreet/Cobrand/EmptyHomes.pm @@ -81,7 +81,7 @@ sub short_name { $name =~ s/ (Borough|City|District|County) Council$//; $name =~ s/ Council$//; $name =~ s/ & / and /; - $name =~ s{/}{_}g; + $name =~ tr{/}{_}; $name = URI::Escape::uri_escape_utf8($name); $name =~ s/%20/-/g; return $name; diff --git a/perllib/FixMyStreet/Cobrand/UK.pm b/perllib/FixMyStreet/Cobrand/UK.pm index c33b39aac..0eff48b00 100644 --- a/perllib/FixMyStreet/Cobrand/UK.pm +++ b/perllib/FixMyStreet/Cobrand/UK.pm @@ -109,7 +109,7 @@ sub short_name { $name =~ s/ (Borough|City|District|County) Council$//; $name =~ s/ Council$//; $name =~ s/ & / and /; - $name =~ s{/}{_}g; + $name =~ tr{/}{_}; $name = URI::Escape::uri_escape_utf8($name); $name =~ s/%20/+/g; return $name; diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index 596ef2dc6..d31b1c84e 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -1018,14 +1018,15 @@ sub munge_sendreport_params { or return; my $id = $row->id; my @attachments = map { + my $image = $photoset->get_raw_image($_); { - body => $photoset->get_raw_image_data($_), + body => $image->{data}, attributes => { - filename => "$id.$_.jpeg", - content_type => 'image/jpeg', + filename => "$id.$_." . $image->{extension}, + content_type => $image->{content_type}, encoding => 'base64', # quoted-printable ends up with newlines corrupting binary data - name => "$id.$_.jpeg", + name => "$id.$_." . $image->{extension}, }, } } (0..$num-1); diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm index e5929ca57..85cdb29f0 100644 --- a/perllib/FixMyStreet/DB/Result/Comment.pm +++ b/perllib/FixMyStreet/DB/Result/Comment.pm @@ -173,12 +173,12 @@ sub photos { my $i = 0; my $id = $self->id; my @photos = map { - my $format = 'jpeg'; my $cachebust = substr($_, 0, 8); + my ($hash, $format) = split /\./, $_; { - id => $_, - url_temp => "/photo/$_.temp.$format", - url_temp_full => "/photo/$_.fulltemp.$format", + id => $hash, + url_temp => "/photo/temp.$hash.$format", + url_temp_full => "/photo/fulltemp.$hash.$format", url => "/photo/c/$id.$i.$format?$cachebust", url_full => "/photo/c/$id.$i.full.$format?$cachebust", idx => $i++, diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index 780a30e69..764f381a2 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -875,12 +875,12 @@ sub photos { my $i = 0; my $id = $self->id; my @photos = map { - my $format = 'jpeg'; my $cachebust = substr($_, 0, 8); + my ($hash, $format) = split /\./, $_; { - id => $_, - url_temp => "/photo/$_.temp.$format", - url_temp_full => "/photo/$_.fulltemp.$format", + id => $hash, + url_temp => "/photo/temp.$hash.$format", + url_temp_full => "/photo/fulltemp.$hash.$format", url => "/photo/$id.$i.$format?$cachebust", url_full => "/photo/$id.$i.full.$format?$cachebust", url_tn => "/photo/$id.$i.tn.$format?$cachebust", diff --git a/perllib/FixMyStreet/Script/Reports.pm b/perllib/FixMyStreet/Script/Reports.pm index e1eb5e2c5..8b1ce6759 100644 --- a/perllib/FixMyStreet/Script/Reports.pm +++ b/perllib/FixMyStreet/Script/Reports.pm @@ -83,7 +83,7 @@ sub send(;$) { $h{phone_line} = $h{phone} ? _('Phone:') . " $h{phone}\n\n" : ''; if ($row->photo) { $h{has_photo} = _("This web page also contains a photo of the problem, provided by the user.") . "\n\n"; - $h{image_url} = $email_base_url . '/photo/' . $row->id . '.full.jpeg'; + $h{image_url} = $email_base_url . $row->photos->[0]->{url_full}; } else { $h{has_photo} = ''; $h{image_url} = ''; @@ -145,7 +145,7 @@ sub send(;$) { $skip = 1; debug_print("skipped by sender " . $sender_info->{method} . " (might be due to previous failed attempts?)", $row->id) if $debug_mode; } else { - debug_print("OK, adding recipient body " . $body->id . ":" . $body->name . ", " . $body->send_method, $row->id) if $debug_mode; + debug_print("OK, adding recipient body " . $body->id . ":" . $body->name . ", " . $sender_info->{method}, $row->id) if $debug_mode; push @dear, $body->name; $reporters{ $sender }->add_body( $body, $sender_info->{config} ); } diff --git a/perllib/Open311.pm b/perllib/Open311.pm index e500219bc..fb793b027 100644 --- a/perllib/Open311.pm +++ b/perllib/Open311.pm @@ -345,7 +345,7 @@ sub _populate_service_request_update_params { if ( $comment->photo ) { my $cobrand = FixMyStreet::Cobrand->get_class_for_moniker($comment->cobrand)->new(); my $email_base_url = $cobrand->base_url($comment->cobrand_data); - my $url = $email_base_url . '/photo/c/' . $comment->id . '.full.jpeg'; + my $url = $email_base_url . $comment->photos->[0]->{url_full}; $params->{media_url} = $url; } diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t index ec41dce77..bfb6017e2 100644 --- a/t/app/controller/auth_social.t +++ b/t/app/controller/auth_social.t @@ -33,7 +33,10 @@ my $fb_uid = 123456789; for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { for my $page ( 'my', 'report', 'update' ) { subtest "test FB '$fb_state' login for page '$page'" => sub { + # Lots of user changes happening here, make sure we don't confuse + # Catalyst with a cookie session user that no longer exists $mech->log_out_ok; + $mech->cookie_jar({}); if ($fb_state eq 'existing UID') { my $user = $mech->create_user_ok($fb_email); $user->update({ facebook_id => $fb_uid }); diff --git a/t/app/controller/photo.t b/t/app/controller/photo.t index 39380e769..425e3c4df 100644 --- a/t/app/controller/photo.t +++ b/t/app/controller/photo.t @@ -65,10 +65,10 @@ subtest "Check multiple upload worked" => sub { ok $mech->success, 'Made request with multiple photo upload'; $mech->base_is('http://localhost/report/new'); $mech->content_like( - qr[(<img align="right" src="/photo/1cdd4329ceee2234bd4e89cb33b42061a0724687.temp.jpeg" alt="">\s*){3}], + qr[(<img align="right" src="/photo/temp.1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg" alt="">\s*){3}], 'Three uploaded pictures are all shown, safe'); $mech->content_contains( - 'name="upload_fileid" value="1cdd4329ceee2234bd4e89cb33b42061a0724687,1cdd4329ceee2234bd4e89cb33b42061a0724687,1cdd4329ceee2234bd4e89cb33b42061a0724687"', + 'name="upload_fileid" value="1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg,1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg,1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg"', 'Returned upload_fileid contains expected hash, 3 times'); my $image_file = path($UPLOAD_DIR, '1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg'); ok $image_file->exists, 'File uploaded to temp'; diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t index cf72221b4..eb29d37da 100644 --- a/t/app/controller/report_new.t +++ b/t/app/controller/report_new.t @@ -420,7 +420,7 @@ foreach my $test ( changes => { photo1 => '', }, - errors => [ "Please upload a JPEG image only" ], + errors => [ "Please upload an image only" ], }, { msg => 'bad photo upload gives error', @@ -443,7 +443,7 @@ foreach my $test ( changes => { photo1 => '', }, - errors => [ "That image doesn't appear to have uploaded correctly (Please upload a JPEG image only ), please try again." ], + errors => [ "That image doesn't appear to have uploaded correctly (Please upload an image only ), please try again." ], }, { msg => 'photo with octet-stream gets through okay', diff --git a/t/app/controller/reports.t b/t/app/controller/reports.t index 02625fcc7..9b446174d 100644 --- a/t/app/controller/reports.t +++ b/t/app/controller/reports.t @@ -12,6 +12,7 @@ $mech->create_body_ok(2514, 'Birmingham City Council'); my $body_edin_id = $mech->create_body_ok(2651, 'City of Edinburgh Council')->id; my $body_west_id = $mech->create_body_ok(2504, 'Westminster City Council')->id; my $body_fife_id = $mech->create_body_ok(2649, 'Fife Council')->id; +my $body_slash_id = $mech->create_body_ok(10000, 'Electricity/Gas Council')->id; $mech->delete_problems_for_body( $body_west_id ); $mech->delete_problems_for_body( $body_edin_id ); @@ -122,6 +123,10 @@ is scalar @$problems, 5, 'correct number of problems displayed'; FixMyStreet::override_config { MAPIT_URL => 'http://mapit.mysociety.org/', }, sub { + $mech->get_ok('/reports'); + $mech->follow_link_ok({ url_regex => qr{/reports/Electricity_Gas\+Council} }); + is $mech->uri->path, '/reports/Electricity_Gas+Council', 'Path is correct'; + $mech->get_ok('/reports/City+of+Edinburgh?t=new'); }; $problems = $mech->extract_problem_list; diff --git a/t/app/model/photoset.t b/t/app/model/photoset.t index cfb5236a8..577e39eb1 100644 --- a/t/app/model/photoset.t +++ b/t/app/model/photoset.t @@ -58,7 +58,7 @@ subtest 'Photoset with photo inline in DB' => sub { my $report = make_report( $image_path->slurp ); my $photoset = $report->get_photoset(); is $photoset->num_images, 1, 'Found just 1 image'; - is $photoset->data, '1cdd4329ceee2234bd4e89cb33b42061a0724687'; + is $photoset->data, '1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg'; }; $image_path->copy( path( $UPLOAD_DIR, '0123456789012345678901234567890123456789.jpeg' ) ); diff --git a/t/open311.t b/t/open311.t index 6333355e8..42d09b29c 100644 --- a/t/open311.t +++ b/t/open311.t @@ -3,6 +3,8 @@ use utf8; use strict; use warnings; +use File::Temp 'tempdir'; +use Path::Tiny; use Test::More; use Test::Warn; use FixMyStreet::DB; @@ -24,7 +26,7 @@ EOT is $o->_process_error( $err_text ), "400: Service Code cannot be null -- can't proceed with the request.\n", 'error text parsing'; is $o->_process_error( '503 - service unavailable' ), 'unknown error', 'error text parsing of bad error'; -my $o2 = Open311->new( endpoint => 'http://192.168.50.1/open311/', jurisdiction => 'example.org' ); +my $o2 = Open311->new( endpoint => 'http://127.0.0.1/open311/', jurisdiction => 'example.org' ); my $u = FixMyStreet::DB->resultset('User')->new( { email => 'test@example.org', name => 'A User' } ); @@ -258,16 +260,26 @@ subtest 'extended request update post parameters' => sub { }; subtest 'check media url set' => sub { - $comment->photo(1); + my $UPLOAD_DIR = tempdir( CLEANUP => 1 ); + + my $image_path = path('t/app/controller/sample.jpg'); + $image_path->copy( path( $UPLOAD_DIR, '0123456789012345678901234567890123456789.jpeg' ) ); + + $comment->photo("0123456789012345678901234567890123456789"); $comment->cobrand('fixmystreet'); - my $results = make_update_req( $comment, '<?xml version="1.0" encoding="utf-8"?><service_request_updates><request_update><update_id>248</update_id></request_update></service_request_updates>' ); + FixMyStreet::override_config { + UPLOAD_DIR => $UPLOAD_DIR, + }, sub { + my $results = make_update_req( $comment, '<?xml version="1.0" encoding="utf-8"?><service_request_updates><request_update><update_id>248</update_id></request_update></service_request_updates>' ); - is $results->{ res }, 248, 'got update id'; + is $results->{ res }, 248, 'got update id'; - my $c = CGI::Simple->new( $results->{ req }->content ); - my $expected_path = '/c/' . $comment->id . '.full.jpeg'; - like $c->param('media_url'), qr/$expected_path/, 'image url included'; + my $c = CGI::Simple->new( $results->{ req }->content ); + my $expected_path = '/c/' . $comment->id . '.0.full.jpeg'; + like $c->param('media_url'), qr/$expected_path/, 'image url included'; + }; + $comment->photo(undef); }; foreach my $test ( diff --git a/t/open311/getservicerequestupdates.t b/t/open311/getservicerequestupdates.t index 18a5802bb..a57d2d0e6 100644 --- a/t/open311/getservicerequestupdates.t +++ b/t/open311/getservicerequestupdates.t @@ -388,7 +388,7 @@ subtest 'Update with media_url includes image in update' => sub { is $problem->comments->count, 1, 'comment count'; my $c = $problem->comments->first; is $c->external_id, 638344; - is $c->photo, '1cdd4329ceee2234bd4e89cb33b42061a0724687', 'photo exists'; + is $c->photo, '1cdd4329ceee2234bd4e89cb33b42061a0724687.jpeg', 'photo exists'; }; foreach my $test ( diff --git a/templates/web/base/admin/body.html b/templates/web/base/admin/body.html index b46d411c2..be84c6415 100644 --- a/templates/web/base/admin/body.html +++ b/templates/web/base/admin/body.html @@ -24,7 +24,7 @@ [% END %] [% END %] <br> - <a href="[% c.uri_for_email( '/reports/' _ body_id ) %]" class="admin-offsite-link">[% loc('List all reported problems' ) %]</a> | + <a href="[% c.uri_for_email(body.url(c)) %]" class="admin-offsite-link">[% loc('List all reported problems' ) %]</a> | <a href="[% c.uri_for( 'body', body_id, { text => 1 } ) %]">[% loc('Text only version') %]</a> </p> diff --git a/templates/web/base/around/_error_multiple.html b/templates/web/base/around/_error_multiple.html index 15089ba6b..b47a91ca9 100644 --- a/templates/web/base/around/_error_multiple.html +++ b/templates/web/base/around/_error_multiple.html @@ -13,7 +13,7 @@ [% IF partial_token %] <p style="margin-top: 0; color: #cc0000;"> - <img align="right" src="/photo/[% partial_report.id %].jpeg" hspace="5"> + <img align="right" src="[% partial_report.photos.first.url_temp %]" hspace="5"> [% loc("Thanks for uploading your photo. We now need to locate your problem, so please enter a nearby street name or postcode in the box above :") %] </p> [% END %] diff --git a/templates/web/base/questionnaire/index.html b/templates/web/base/questionnaire/index.html index 86887c163..a523cd439 100644 --- a/templates/web/base/questionnaire/index.html +++ b/templates/web/base/questionnaire/index.html @@ -82,7 +82,7 @@ [% IF upload_fileid %] <p>[% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]</p> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/base/report/new/form_report.html b/templates/web/base/report/new/form_report.html index 68fb2c796..3fbf7c160 100644 --- a/templates/web/base/report/new/form_report.html +++ b/templates/web/base/report/new/form_report.html @@ -31,7 +31,7 @@ [% IF upload_fileid %] <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> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/base/report/update/form_update.html b/templates/web/base/report/update/form_update.html index fb977e00e..007bd68d8 100644 --- a/templates/web/base/report/update/form_update.html +++ b/templates/web/base/report/update/form_update.html @@ -13,7 +13,7 @@ [% IF upload_fileid %] <p>[% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]</p> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/bromley/report/display.html b/templates/web/bromley/report/display.html index 1484bc489..da83e005f 100644 --- a/templates/web/bromley/report/display.html +++ b/templates/web/bromley/report/display.html @@ -72,7 +72,7 @@ [% IF upload_fileid %] <p>[% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]</p> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/bromley/report/new/fill_in_details_form.html b/templates/web/bromley/report/new/fill_in_details_form.html index 858da1a07..96f23d1fb 100644 --- a/templates/web/bromley/report/new/fill_in_details_form.html +++ b/templates/web/bromley/report/new/fill_in_details_form.html @@ -60,7 +60,7 @@ [% IF upload_fileid %] <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> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/eastsussex/report/update-form.html b/templates/web/eastsussex/report/update-form.html index ae9cceac4..abcc2f532 100644 --- a/templates/web/eastsussex/report/update-form.html +++ b/templates/web/eastsussex/report/update-form.html @@ -72,7 +72,7 @@ [% IF upload_fileid %] <p>[% loc('You have already attached photos to this update. Note that you can attach a maximum of 3 to this update (if you try to upload more, the oldest will be removed).') %]</p> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/templates/web/emptyhomes/report/new/fill_in_details_form.html b/templates/web/emptyhomes/report/new/fill_in_details_form.html index 20b0b6842..049ffa076 100644 --- a/templates/web/emptyhomes/report/new/fill_in_details_form.html +++ b/templates/web/emptyhomes/report/new/fill_in_details_form.html @@ -66,23 +66,24 @@ </div> [% IF c.cobrand.allow_photo_upload %] + <input type="hidden" name="upload_fileid" value="[% upload_fileid %]" /> + <label for="form_photo">[% loc('Photo') %]</label> [% IF field_errors.photo %] <div class='form-error'>[% field_errors.photo %]</div> [% END %] - <div class='form-field'> - [% IF upload_fileid || report.photo %] - <p>[% loc('You have already attached a photo to this report, attaching another one will replace it.') %]</p> - [% IF upload_fileid %] - <input type="hidden" name="upload_fileid" value="[% upload_fileid %]" /> + <div id="form_photos"> + [% IF upload_fileid %] + <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> + [% FOREACH id IN upload_fileid.split(',') %] + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] - [% IF report.photo %] - <img align="right" src="/photo/[% report.id %].jpeg" hspace="5"> - [% END %] - [% END %] - - <label for="form_photo">[% loc('Photo:') %]</label> - <input type="file" name="photo" id="form_photo" style="width:20em"> + [% END %] + <input type="file" name="photo1" id="form_photo"> + <label for="form_photo2">[% loc('Photo') %]</label> + <input type="file" name="photo2" id="form_photo2"> + <label for="form_photo3">[% loc('Photo') %]</label> + <input type="file" name="photo3" id="form_photo3"> </div> [% END %] diff --git a/templates/web/seesomething/report/new/fill_in_details_form.html b/templates/web/seesomething/report/new/fill_in_details_form.html index 659fadd04..728b27e41 100644 --- a/templates/web/seesomething/report/new/fill_in_details_form.html +++ b/templates/web/seesomething/report/new/fill_in_details_form.html @@ -49,24 +49,24 @@ </div> [% IF c.cobrand.allow_photo_upload %] + <input type="hidden" name="upload_fileid" value="[% upload_fileid %]"> <label for="form_photo">[% loc('Photo') %]</label> - [% IF upload_fileid || report.photo %] - [% IF upload_fileid %] - <img align="right" src="/photo/[% upload_fileid %].temp.jpeg" alt=""> - <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> - - [% IF report.photo %] - <img align="right" src="/photo/[% report.id %].jpeg"> - [% END %] - [% END %] - [% IF field_errors.photo %] <p class='form-error'>[% field_errors.photo %]</p> [% END %] - <input type="file" name="photo" id="form_photo"> + <div id="form_photos"> + [% IF upload_fileid %] + <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> + [% FOREACH id IN upload_fileid.split(',') %] + <img align="right" src="/photo/temp.[% id %]" alt=""> + [% END %] + [% END %] + <input type="file" name="photo1" id="form_photo"> + <label for="form_photo2">[% loc('Photo') %]</label> + <input type="file" name="photo2" id="form_photo2"> + <label for="form_photo3">[% loc('Photo') %]</label> + <input type="file" name="photo3" id="form_photo3"> + </div> [% END %] <h2>Personal Details:</h2> 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 7628bf23f..1e9db6b31 100644 --- a/templates/web/zurich/report/new/fill_in_details_form.html +++ b/templates/web/zurich/report/new/fill_in_details_form.html @@ -54,7 +54,7 @@ [% IF upload_fileid %] <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> [% FOREACH id IN upload_fileid.split(',') %] - <img align="right" src="/photo/[% id %].temp.jpeg" alt=""> + <img align="right" src="/photo/temp.[% id %]" alt=""> [% END %] [% END %] <input type="file" name="photo1" id="form_photo"> diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 4de18be9a..5935d2bae 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -206,7 +206,7 @@ $(function(){ addRemoveLinks: true, thumbnailHeight: 150, thumbnailWidth: 150, - acceptedFiles: 'image/jpeg,image/pjpeg', + acceptedFiles: 'image/jpeg,image/pjpeg,image/gif,image/tiff,image/png', dictDefaultMessage: translation_strings.upload_default_message, dictCancelUploadConfirmation: translation_strings.upload_cancel_confirmation, dictInvalidFileType: translation_strings.upload_invalid_file_type, @@ -258,7 +258,7 @@ $(function(){ } var mockFile = { name: f, server_id: f }; photodrop.emit("addedfile", mockFile); - photodrop.createThumbnailFromUrl(mockFile, '/photo/' + f + '.temp.jpeg'); + photodrop.createThumbnailFromUrl(mockFile, '/photo/temp.' + f); photodrop.emit("complete", mockFile); photodrop.options.maxFiles -= 1; }); |