diff options
-rw-r--r-- | CHANGELOG.md | 1 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/Buckinghamshire.pm | 19 | ||||
-rw-r--r-- | perllib/FixMyStreet/Cobrand/FixMyStreet.pm | 3 | ||||
-rw-r--r-- | t/app/controller/contact.t | 144 | ||||
-rw-r--r-- | templates/web/base/contact/index.html | 19 | ||||
-rw-r--r-- | templates/web/bathnes/contact/index.html | 12 | ||||
-rw-r--r-- | templates/web/buckinghamshire/contact/who.html | 48 | ||||
-rw-r--r-- | templates/web/fixmystreet.com/contact/who.html | 82 | ||||
-rw-r--r-- | web/cobrands/sass/_base.scss | 6 | ||||
-rw-r--r-- | web/js/contact.js | 14 |
10 files changed, 221 insertions, 127 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba77e48c..cdd389ca9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Don't require two taps on reports list on touchscreens. #2294 - Allow moderation to work without JavaScript. #2339 - More prominent display of "state" on report page #2350 + - Improved report/update display on contact form. #2351 - Admin improvements: - Allow moderation to potentially change category. #2320 - Add Mark/View private reports permission #2306 diff --git a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm index 1b437b2c5..055e481d9 100644 --- a/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm +++ b/perllib/FixMyStreet/Cobrand/Buckinghamshire.pm @@ -358,5 +358,24 @@ sub lookup_site_code_config { { } } } +sub extra_contact_validation { + my $self = shift; + my $c = shift; + + # Don't care about dest unless reporting abuse + return () unless $c->stash->{problem}; + + my %errors; + + $c->stash->{dest} = $c->get_param('dest'); + + if (!$c->get_param('dest')) { + $errors{dest} = "Please enter a topic of your message"; + } elsif ( $c->get_param('dest') eq 'council' || $c->get_param('dest') eq 'update' ) { + $errors{not_for_us} = 1; + } + + return %errors; +} 1; diff --git a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm index b2fc5ba83..fb454f495 100644 --- a/perllib/FixMyStreet/Cobrand/FixMyStreet.pm +++ b/perllib/FixMyStreet/Cobrand/FixMyStreet.pm @@ -52,9 +52,6 @@ sub extra_contact_validation { my $self = shift; my $c = shift; - # Don't care about dest if reporting abuse - return () if $c->stash->{problem}; - my %errors; $c->stash->{dest} = $c->get_param('dest'); diff --git a/t/app/controller/contact.t b/t/app/controller/contact.t index 254522b92..fe67e89ec 100644 --- a/t/app/controller/contact.t +++ b/t/app/controller/contact.t @@ -279,25 +279,16 @@ for my $test ( }; } -for my $test ( - { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - }, - }, - { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - id => $problem_main->id, - }, - }, +my %common = ( + em => 'test@example.com', + name => 'A name', + subject => 'A subject', + message => 'A message', +); +for my $test ( + { fields => \%common }, + { fields => { %common, id => $problem_main->id } }, ) { subtest 'check email sent correctly' => sub { @@ -336,39 +327,22 @@ for my $test ( for my $test ( { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => undef, - }, + fields => { %common, dest => undef }, page_errors => [ 'There were problems with your report. Please see below.', 'Please enter who your message is for', + 'You can only contact the team behind FixMyStreet using our contact form', # The JS-hidden one ] }, { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => 'council', - }, + fields => { %common, dest => 'council' }, page_errors => [ 'There were problems with your report. Please see below.', 'You can only contact the team behind FixMyStreet using our contact form', ] }, { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => 'update', - }, + fields => { %common, dest => 'update' }, page_errors => [ 'There were problems with your report. Please see below.', 'You can only contact the team behind FixMyStreet using our contact form', @@ -391,44 +365,21 @@ for my $test ( $test->{fields}->{'extra.phone'} = ''; is_deeply $mech->visible_form_values, $test->{fields}, 'form values'; + # Ugh, but checking div not hidden; text always shown and hidden with CSS if ( $test->{fields}->{dest} and $test->{fields}->{dest} eq 'update' ) { - $mech->content_contains( 'www.writetothem.com', 'includes link to WTT if trying to update report' ); + $mech->content_contains('<div class="form-error__box form-error--update">'); } elsif ( $test->{fields}->{dest} and $test->{fields}->{dest} eq 'council' ) { - $mech->content_lacks( 'www.writetothem.com', 'does not include link to WTT if trying to contact council' ); - $mech->content_contains( 'should find contact details', 'mentions checking council website for contact details' ); + # Ugh, but checking div not hidden + $mech->content_contains('<div class="form-error__box form-error--council">'); } } }; } for my $test ( - { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => 'help', - }, - }, - { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => 'feedback', - }, - }, - { - fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', - dest => 'from_council', - }, - }, + { fields => { %common, dest => 'help' } }, + { fields => { %common, dest => 'feedback' } }, + { fields => { %common, dest => 'from_council' } }, ) { subtest 'check email sent correctly with dest field set to us' => sub { @@ -446,11 +397,53 @@ for my $test ( for my $test ( { + fields => { %common, dest => undef }, + page_errors => + [ 'There were problems with your report. Please see below.', + 'Please enter a topic of your message', + 'You can only use this form to report inappropriate content', # The JS-hidden one + ] + }, + { + fields => { %common, dest => 'council' }, + page_errors => + [ 'There were problems with your report. Please see below.', + 'You can only use this form to report inappropriate content', + ] + }, + { + fields => { %common, dest => 'update' }, + page_errors => + [ 'There were problems with your report. Please see below.', + 'You can only use this form to report inappropriate content', + ] + }, + ) +{ + subtest 'check Bucks submit page incorrect destination handling' => sub { + FixMyStreet::override_config { + ALLOWED_COBRANDS => [ 'buckinghamshire' ], + }, sub { + $mech->get_ok( '/contact?id=' . $problem_main->id, 'can visit for abuse report' ); + $mech->submit_form_ok( { with_fields => $test->{fields} } ); + is_deeply $mech->page_errors, $test->{page_errors}, 'page errors'; + + $test->{fields}->{'extra.phone'} = ''; + is_deeply $mech->visible_form_values, $test->{fields}, 'form values'; + + if ( $test->{fields}->{dest} and $test->{fields}->{dest} eq 'update' ) { + $mech->content_contains('please leave an update'); + } elsif ( $test->{fields}->{dest} and $test->{fields}->{dest} eq 'council' ) { + $mech->content_contains('should find other contact details'); + } + } + }; +} + +for my $test ( + { fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', + %common, dest => 'from_council', success_url => '/faq', }, @@ -458,10 +451,7 @@ for my $test ( }, { fields => { - em => 'test@example.com', - name => 'A name', - subject => 'A subject', - message => 'A message', + %common, dest => 'from_council', success_url => 'http://www.example.com', }, diff --git a/templates/web/base/contact/index.html b/templates/web/base/contact/index.html index 326c26ce8..f71c36fb1 100644 --- a/templates/web/base/contact/index.html +++ b/templates/web/base/contact/index.html @@ -1,3 +1,6 @@ +[% extra_js = [ + version('/js/contact.js') +] -%] [% INCLUDE 'header.html', title = loc('Contact Us') robots = 'noindex,nofollow' @@ -23,16 +26,16 @@ </p> <blockquote> - <p> + <cite> [% IF update.anonymous %] [% tprintf( loc('Update below added anonymously at %s'), prettify_dt( update.confirmed ) ) %] [% ELSE %] [% tprintf( loc('Update below added by %s at %s'), update.name, prettify_dt( update.confirmed ) ) | html %] [% END %] - </p> + </cite> <p> - [% update.text | html %] + [%~ update.text | html ~%] </p> </blockquote> @@ -50,16 +53,16 @@ <blockquote> <h2>[% problem.title_safe | html %]</h2> - <p> + <cite> [% IF problem.anonymous %] [% tprintf( loc('Reported anonymously at %s'), prettify_dt( problem.confirmed ) ) %] [% ELSE %] [% tprintf( loc('Reported by %s at %s'), problem.user.name, prettify_dt( problem.confirmed ) ) | html %] [% END %] - </p> + </cite> <p> - [% problem.detail | html %] + [%~ problem.detail | html ~%] </p> </blockquote> @@ -71,6 +74,8 @@ [% END %] + [% INCLUDE 'contact/who.html' %] + <label for="form_name">[% loc('Your name') %]</label> [% IF field_errors.name %] <div class="form-error">[% field_errors.name %]</div> @@ -95,8 +100,6 @@ [% END %] <input type="text" class="form-control required" name="subject" id="form_subject" value="[% subject | html %]" size="30"> - [% INCLUDE 'contact/who.html' %] - <label for="form_message">[% loc('Message') %]</label> [% IF field_errors.message %] <div class="form-error">[% field_errors.message %]</div> diff --git a/templates/web/bathnes/contact/index.html b/templates/web/bathnes/contact/index.html index d9947cbec..c6bca0350 100644 --- a/templates/web/bathnes/contact/index.html +++ b/templates/web/bathnes/contact/index.html @@ -25,16 +25,16 @@ </p> <blockquote> - <p> + <cite> [% IF update.anonymous %] [% tprintf( loc('Update below added anonymously at %s'), prettify_dt( update.confirmed ) ) %] [% ELSE %] [% tprintf( loc('Update below added by %s at %s'), update.name, prettify_dt( update.confirmed ) ) | html %] [% END %] - </p> + </cite> <p> - [% update.text | html %] + [%~ update.text | html ~%] </p> </blockquote> @@ -54,16 +54,16 @@ <blockquote> <h2>[% problem.title_safe | html %]</h2> - <p> + <cite> [% IF problem.anonymous %] [% tprintf( loc('Reported anonymously at %s'), prettify_dt( problem.confirmed ) ) %] [% ELSE %] [% tprintf( loc('Reported by %s at %s'), problem.user.name, prettify_dt( problem.confirmed ) ) | html %] [% END %] - </p> + </cite> <p> - [% problem.detail | html %] + [%~ problem.detail | html ~%] </p> </blockquote> diff --git a/templates/web/buckinghamshire/contact/who.html b/templates/web/buckinghamshire/contact/who.html new file mode 100644 index 000000000..e3b045521 --- /dev/null +++ b/templates/web/buckinghamshire/contact/who.html @@ -0,0 +1,48 @@ +[% IF problem %] +<h4>Topic:</h4> + +[% IF field_errors.dest %] +<div class="form-error">[% field_errors.dest %]</div> +[% END %] + +<div class="checkbox-group"> + <input name="dest" id="dest_rules" type="radio" value="rules" class="required"[% IF dest AND dest == 'rules' %] checked[% END %]> + <label class="inline" for="dest_rules">This [% update ? 'update' : 'report' %] is abusive, contains personal information, or similar</label> +</div> + +<div class="checkbox-group"> + <input name="dest" id="dest_update" type="radio" value="update" class="required"[% IF dest AND dest == 'update' %] checked[% END %]> + <label class="inline" for="dest_update">I wish to report it has not been fixed yet</label> +</div> + +<div class="checkbox-group"> + <input name="dest" id="dest_council" type="radio" value="council" class="required"[% IF dest AND dest == 'council' %] checked[% END %]> + <label class="inline" for="dest_council">I wish to make a new report</label> +</div> + +<div id="dest-error"[% IF NOT field_errors.not_for_us %] class="hidden"[% END %]> + <div class="form-error">You can only use this form to report inappropriate content</div> + + <div class="form-error__box form-error--council[% IF dest != 'council' %] hidden[% END %]"> + <p> + <strong>If you want to report a street problem</strong>, return to the + <a href="/">homepage</a> and enter your postcode. You can then make a report. + We'll send it to your council, and publish it on the site. + </p> + <p> + <strong>If your problem is not a street issue</strong>, or is <strong>not + suitable for publication on the site</strong>, then this isn't the + right place for it. You should find other contact details on the + <a href="https://www.buckscc.gov.uk/">council's website</a>. + </p> + </div> + + <div class="form-error__box form-error--update[% IF dest != 'update' %] hidden[% END %]"> + <p> + <strong>If you'd like to update a report</strong>, please leave an update + on the <a href="/report/[% problem.id %]">report’s page</a>, underneath the report details. + </p> + </div> +</div> + +[% END %] diff --git a/templates/web/fixmystreet.com/contact/who.html b/templates/web/fixmystreet.com/contact/who.html index e16809e48..996ceeca7 100644 --- a/templates/web/fixmystreet.com/contact/who.html +++ b/templates/web/fixmystreet.com/contact/who.html @@ -1,12 +1,53 @@ -[% IF NOT problem %] <h4>Topic:</h4> [% IF field_errors.dest %] <div class="form-error">[% field_errors.dest %]</div> -[% ELSIF field_errors.not_for_us %] - <div class="form-error">You can only contact the team behind FixMyStreet using our contact form</div> +[% END %] + +[% IF problem %] +<div class="checkbox-group"> + <input name="dest" id="dest_rules" type="radio" value="rules" class="required"[% IF dest AND dest == 'rules' %] checked[% END %]> + <label class="inline" for="dest_rules">This [% update ? 'update' : 'report' %] breaks the <a href="/about/house-rules">House Rules</a></label> +</div> +<div class="checkbox-group"> + <input name="dest" id="dest_update" type="radio" value="update" class="required"[% IF dest AND dest == 'update' %] checked[% END %]> + <label class="inline" for="dest_update">This report has not been fixed</label> +</div> +<div class="checkbox-group"> + <input name="dest" id="dest_council" type="radio" value="council" class="required"[% IF dest AND dest == 'council' %] checked[% END %]> + <label class="inline" for="dest_council">I want to make a new report about a street problem</label> +</div> +[% ELSE %] +<div class="checkbox-group"> + <input name="dest" id="dest_help" type="radio" value="help" class="required"[% IF dest AND dest == 'help' %] checked[% END %]> + <label class="inline" for="dest_help">I need help using the site</label> +</div> + +<div class="checkbox-group"> + <input name="dest" id="dest_feedback" type="radio" value="feedback" class="required"[% IF dest AND dest == 'feedback' %] checked[% END %]> + <label class="inline" for="dest_feedback">I have feedback about the site</label> +</div> + +<div class="checkbox-group"> + <input name="dest" id="dest_from_council" type="radio" value="from_council" class="required"[% IF dest AND dest == 'from_council' %] checked[% END %]> + <label class="inline" for="dest_from_council">I am from a council and I have a question for the FixMyStreet team</label> +</div> + +<div class="checkbox-group"> + <input name="dest" id="dest_council" type="radio" value="council" class="required"[% IF dest AND dest == 'council' %] checked[% END %]> + <label class="inline" for="dest_council">I want to report a street problem</label> +</div> - [% IF dest == 'council' %] +<div class="checkbox-group"> + <input name="dest" id="dest_update" type="radio" value="update"[% IF dest AND dest == 'update' %] checked[% END %]> + <label class="inline" for="dest_update">My street problem hasn't been fixed</label> +</div> +[% END %] + +<div id="dest-error"[% IF NOT field_errors.not_for_us %] class="hidden"[% END %]> + <div class="form-error">You can only contact the team behind FixMyStreet using our contact form</div> + + <div class="form-error__box form-error--council[% IF dest != 'council' %] hidden[% END %]"> <p> We’re not the council: we just run this website which helps you report issues to them. @@ -24,7 +65,9 @@ right place for it. You should find contact details on your council's own website. </p> - [% ELSIF dest == 'update' %] + </div> + + <div class="form-error__box form-error--update[% IF dest != 'update' %] hidden[% END %]"> <p> FixMyStreet is great for reporting problems, but we don't fix them - your council oversees that. @@ -46,32 +89,5 @@ you could try contacting your local councillor, using another useful mySociety site: <a href="https://www.writetothem.com/?utm_source=fixmystreet.com&utm_campaign=contact_workflow_links&utm_medium=link&utm_content=contact+not_fixed">WriteToThem</a>. </p> - - [% END %] -[% END %] - -<div class="checkbox-group"> - <input name="dest" id="dest_help" type="radio" value="help" class="required"[% IF dest AND dest == 'help' %] checked[% END %]> - <label class="inline" for="dest_help">I need help using the site</label> -</div> - -<div class="checkbox-group"> - <input name="dest" id="dest_feedback" type="radio" value="feedback" class="required"[% IF dest AND dest == 'feedback' %] checked[% END %]> - <label class="inline" for="dest_feedback">I have feedback about the site</label> + </div> </div> - -<div class="checkbox-group"> - <input name="dest" id="dest_from_council" type="radio" value="from_council" class="required"[% IF dest AND dest == 'from_council' %] checked[% END %]> - <label class="inline" for="dest_from_council">I am from a council and I have a question for the FixMyStreet team</label> -</div> - -<div class="checkbox-group"> - <input name="dest" id="dest_council" type="radio" value="council" class="required"[% IF dest AND dest == 'council' %] checked[% END %]> - <label class="inline" for="dest_council">I want to report a street problem</label> -</div> - -<div class="checkbox-group"> - <input name="dest" id="dest_update" type="radio" value="update"[% IF dest AND dest == 'update' %] checked[% END %]> - <label class="inline" for="dest_update">My street problem hasn't been fixed</label> -</div> -[% END %] diff --git a/web/cobrands/sass/_base.scss b/web/cobrands/sass/_base.scss index 15b42cc3e..a2da14280 100644 --- a/web/cobrands/sass/_base.scss +++ b/web/cobrands/sass/_base.scss @@ -510,6 +510,12 @@ textarea.form-error { @include border-radius(0 0.25em 0.25em 0.25em); } +.form-error__box { + border: solid 2px #ff0000; + padding: 0.5em; + margin-bottom: 0.5em; +} + ul.error { background:#ff0000; color:#fff; diff --git a/web/js/contact.js b/web/js/contact.js new file mode 100644 index 000000000..9529ede16 --- /dev/null +++ b/web/js/contact.js @@ -0,0 +1,14 @@ +$('[name=dest]').change(function() { + var err = $('.form-error--' + this.value), + inputs = $(this).closest('form').find('input[type=text], input[type=submit]'); + $('.form-error__box').addClass('hidden'); + if (err.length) { + $('#dest-error').removeClass('hidden'); + $('#dest-error .form-error').show(); // might have been hidden by normal validate + inputs.prop('disabled', true); + $('.form-error--' + this.value).removeClass('hidden'); + } else { + $('#dest-error').addClass('hidden'); + inputs.prop('disabled', false); + } +}); |