From e1c030cfd06f2c2791d3811179040d674692a27e Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 11 Dec 2018 16:27:03 +0000 Subject: Improved report/update display on contact form. --- CHANGELOG.md | 1 + templates/web/base/contact/index.html | 12 ++++++------ templates/web/bathnes/contact/index.html | 12 ++++++------ 3 files changed, 13 insertions(+), 12 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/templates/web/base/contact/index.html b/templates/web/base/contact/index.html index 326c26ce8..8e0894312 100644 --- a/templates/web/base/contact/index.html +++ b/templates/web/base/contact/index.html @@ -23,16 +23,16 @@

-

+ [% 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 %] -

+

- [% update.text | html %] + [%~ update.text | html ~%]

@@ -50,16 +50,16 @@

[% problem.title_safe | html %]

-

+ [% 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 %] -

+

- [% problem.detail | html %] + [%~ problem.detail | html ~%]

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 @@

-

+ [% 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 %] -

+

- [% update.text | html %] + [%~ update.text | html ~%]

@@ -54,16 +54,16 @@

[% problem.title_safe | html %]

-

+ [% 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 %] -

+

- [% problem.detail | html %] + [%~ problem.detail | html ~%]

-- cgit v1.2.3 From c4169478a3cd94902b9d4d5ca166dbd85bca09ce Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Wed, 19 Dec 2018 12:36:13 +0000 Subject: Make who question more prominent. --- t/app/controller/contact.t | 8 ++-- templates/web/base/contact/index.html | 7 ++- templates/web/fixmystreet.com/contact/who.html | 64 ++++++++++++++------------ web/cobrands/sass/_base.scss | 6 +++ web/js/contact.js | 14 ++++++ 5 files changed, 64 insertions(+), 35 deletions(-) create mode 100644 web/js/contact.js diff --git a/t/app/controller/contact.t b/t/app/controller/contact.t index 254522b92..3fa83be9b 100644 --- a/t/app/controller/contact.t +++ b/t/app/controller/contact.t @@ -346,6 +346,7 @@ for my $test ( 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 ] }, { @@ -391,11 +392,12 @@ 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('
'); } 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('
'); } } }; diff --git a/templates/web/base/contact/index.html b/templates/web/base/contact/index.html index 8e0894312..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' @@ -71,6 +74,8 @@ [% END %] + [% INCLUDE 'contact/who.html' %] + [% IF field_errors.name %]
[% field_errors.name %]
@@ -95,8 +100,6 @@ [% END %] - [% INCLUDE 'contact/who.html' %] - [% IF field_errors.message %]
[% field_errors.message %]
diff --git a/templates/web/fixmystreet.com/contact/who.html b/templates/web/fixmystreet.com/contact/who.html index e16809e48..a683412b7 100644 --- a/templates/web/fixmystreet.com/contact/who.html +++ b/templates/web/fixmystreet.com/contact/who.html @@ -3,10 +3,37 @@ [% IF field_errors.dest %]
[% field_errors.dest %]
-[% ELSIF field_errors.not_for_us %] -
You can only contact the team behind FixMyStreet using our contact form
+[% END %] + +
+ + +
+ +
+ + +
+ +
+ + +
+ +
+ + +
+ +
+ + +
+ + - -[% END %] -- cgit v1.2.3 From 584b6297f229f5fa2e1c0b44d9078a7b09e83e63 Mon Sep 17 00:00:00 2001 From: Matthew Somerville Date: Tue, 11 Dec 2018 16:26:16 +0000 Subject: [Buckinghamshire] Get topic when reporting abuse. --- perllib/FixMyStreet/Cobrand/Buckinghamshire.pm | 19 ++++ t/app/controller/contact.t | 136 +++++++++++-------------- templates/web/buckinghamshire/contact/who.html | 48 +++++++++ 3 files changed, 129 insertions(+), 74 deletions(-) create mode 100644 templates/web/buckinghamshire/contact/who.html 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/t/app/controller/contact.t b/t/app/controller/contact.t index 3fa83be9b..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,13 +327,7 @@ 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', @@ -350,26 +335,14 @@ for my $test ( ] }, { - 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', @@ -404,33 +377,9 @@ for my $test ( } 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,13 +395,55 @@ 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', }, @@ -460,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/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 %] +

Topic:

+ +[% IF field_errors.dest %] +
[% field_errors.dest %]
+[% END %] + +
+ + +
+ +
+ + +
+ +
+ + +
+ + + +[% END %] -- cgit v1.2.3