diff --git a/perllib/FixMyStreet/App/Form/Claims.pm b/perllib/FixMyStreet/App/Form/Claims.pm index 72ff7b456b6..fbc25ce5bcc 100644 --- a/perllib/FixMyStreet/App/Form/Claims.pm +++ b/perllib/FixMyStreet/App/Form/Claims.pm @@ -17,6 +17,13 @@ has finished_action => ( is => 'ro' ); has '+is_html5' => ( default => 1 ); +has upload_dir => ( is => 'ro', default => sub { + my $cfg = FixMyStreet->config('PHOTO_STORAGE_OPTIONS'); + my $dir = $cfg ? $cfg->{UPLOAD_DIR} : FixMyStreet->config('UPLOAD_DIR'); + $dir = path($dir, "claims_files")->absolute(FixMyStreet->path_to())->mkdir; + return $dir; +}); + before _process_page_array => sub { my ($self, $pages) = @_; foreach my $page (@$pages) { @@ -1145,23 +1152,19 @@ sub file_upload { my $c = $form->{c}; my $saved_data = $form->saved_data; - my $receipts = $c->req->upload($field); - if ( $receipts ) { - my $cfg = FixMyStreet->config('PHOTO_STORAGE_OPTIONS'); - my $dir = $cfg ? $cfg->{UPLOAD_DIR} : FixMyStreet->config('UPLOAD_DIR'); - $dir = path($dir, "claims_files")->absolute(FixMyStreet->path_to())->mkdir; - - FixMyStreet::PhotoStorage::base64_decode_upload($c, $receipts); - my ($p, $n, $ext) = fileparse($receipts->filename, qr/\.[^.]*/); - my $key = sha1_hex($receipts->slurp) . $ext; - my $out = $dir->child($key); - unless (copy($receipts->tempname, $out)) { + my $upload = $c->req->upload($field); + if ( $upload ) { + FixMyStreet::PhotoStorage::base64_decode_upload($c, $upload); + my ($p, $n, $ext) = fileparse($upload->filename, qr/\.[^.]*/); + my $key = sha1_hex($upload->slurp) . $ext; + my $out = $form->upload_dir->child($key); + unless (copy($upload->tempname, $out)) { $c->log->info('Couldn\'t copy temp file to destination: ' . $!); $c->stash->{photo_error} = _("Sorry, we couldn't save your file(s), please try again."); return; } # Then store the file hashes along with the original filenames for display - $saved_data->{$field} = { files => $key, filenames => [ $receipts->raw_basename ] }; + $saved_data->{$field} = { files => $key, filenames => [ $upload->raw_basename ] }; } } diff --git a/perllib/FixMyStreet/App/Form/Field/FileIdUpload.pm b/perllib/FixMyStreet/App/Form/Field/FileIdUpload.pm index 059fc6ee25c..4dc1889efe0 100644 --- a/perllib/FixMyStreet/App/Form/Field/FileIdUpload.pm +++ b/perllib/FixMyStreet/App/Form/Field/FileIdUpload.pm @@ -1,18 +1,53 @@ +=head1 NAME + +FixMyStreet::App::Form::Field::FileIdUpload - file upload field via ID + +=head1 SYNOPSIS + +Our Forms upload files by storing them in the upload directory and then using a +hash of the contents as the ID to pass around in multi-step forms etc. Here we +subclass the Upload field to validate the presence of the data in the various +places it could be, not assuming an Upload object is supplied as the default. + +=head1 DESCRIPTION + +=cut + package FixMyStreet::App::Form::Field::FileIdUpload; use Moose; extends 'HTML::FormHandler::Field::Upload'; +=head2 validate + +If we've explicitly said the field is not required, we don't perform any validation. + +Otherwise we check we've got something saved in either C<< $self->value >> (the field +itself), C<< $self->form->saved_data->{$self->name} >> (saved from the previous step), +or C<< $self->form->params >> (not uploaded, but ID saved in a previous step). + +We also check the file isn't zero in size at this point. + +=cut + sub validate { my ($self) = @_; return if $self->tag_exists('required') && !$self->get_tag('required'); - return $self->add_error($self->get_message('upload_file_not_found')) - unless ( defined $self->value && defined $self->value->{files} + + my $key = ( defined $self->value && defined $self->value->{files} && $self->value->{files} ) || ( defined $self->form->saved_data->{$self->name}->{files} && $self->form->saved_data->{$self->name}->{files} ) || $self->form->params->{$self->name . '_fileid'}; + + return $self->add_error($self->get_message('upload_file_not_found')) + unless $key; + + my $out = $self->form->upload_dir->child($key); + unless ($out->size) { + return $self->add_error($self->get_message('upload_file_too_small')); + } } __PACKAGE__->meta->make_immutable; diff --git a/t/app/controller/claims.t b/t/app/controller/claims.t index 155965a7e9f..7b8db479f3a 100644 --- a/t/app/controller/claims.t +++ b/t/app/controller/claims.t @@ -8,6 +8,7 @@ my $sample_file = path(__FILE__)->parent->child("sample.jpg"); ok $sample_file->exists, "sample file $sample_file exists"; my $sample_pdf = path(__FILE__)->parent->child("sample.pdf"); ok $sample_pdf->exists, "sample file $sample_pdf exists"; +my $sample_blank = path(__FILE__)->parent(3)->child("fixtures", "blank.jpeg"); my $mech = FixMyStreet::TestMech->new; @@ -93,6 +94,10 @@ FixMyStreet::override_config { photos => [ $sample_file, undef, Content_Type => 'image/jpeg' ], photos2 => [ $sample_file, undef, Content_Type => 'image/jpeg' ], } }, "cause screen"); + $mech->submit_form_ok({ with_fields => { registration => 'rego!', mileage => '20', + v5 => [ $sample_blank ], v5_in_name => 'Yes', damage_claim => 'No', vat_reg => 'No', + } }, "bad v5 file"); + $mech->content_contains('File is too small'); $mech->submit_form_ok({ with_fields => { registration => 'rego!', mileage => '20', v5 => [ $sample_pdf, undef, Content_Type => 'application/octet-stream', filename => 'v5.pdf' ], v5_in_name => 'Yes', insurer_address => 'insurer address', damage_claim => 'No', vat_reg => 'No', @@ -105,6 +110,9 @@ FixMyStreet::override_config { tyre_damage => 'Yes', tyre_mileage => 20, } }, "damage details"); $mech->content_contains('Review'); + $mech->submit_form_ok({ form_number => 13 }, "Back to about vehicle page"); + $mech->submit_form_ok({ with_fields => { continue => 'Continue' } }); + $mech->submit_form_ok({ with_fields => { continue => 'Continue' } }); $mech->submit_form_ok({ with_fields => { process => 'summary' } }); $mech->content_contains('Claim submitted'); diff --git a/t/fixtures/blank.jpeg b/t/fixtures/blank.jpeg new file mode 100644 index 00000000000..e69de29bb2d