diff options
24 files changed, 498 insertions, 144 deletions
diff --git a/.travis.yml b/.travis.yml index 709342dc8..32e8ae28c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,7 +24,7 @@ before_install: - cpanm -q DIME::Tools --force # And let's install the same version the carton.lock file currently has. - cpanm -q MKUTTER/SOAP-Lite-0.715.tar.gz - - sudo locale-gen cy_GB.UTF-8 en_GB.UTF-8 nb_NO.UTF-8 + - sudo locale-gen cy_GB.UTF-8 en_GB.UTF-8 nb_NO.UTF-8 de_CH.UTF-8 install: - carton install --deployment before_script: @@ -36,6 +36,7 @@ before_script: sed -r -e "s,(FMS_DB_USER:) 'fms',\\1 'postgres'," -e "s,cobrand_one,fixmystreet," -e "s,cobrand_two: 'hostname_substring2',fixmystreet: 'localhost'," + -e "s,cobrand_three,zurich," -e "s,(BASE_URL:) 'http://www.example.org',\\1 'http://localhost'," -e "s,(MAPIT_URL:) '',\\1 'http://mapit.mysociety.org/'," conf/general.yml-example > conf/general.yml diff --git a/bin/zurich/convert_internal_notes_to_comments b/bin/zurich/convert_internal_notes_to_comments new file mode 100755 index 000000000..e92be40b3 --- /dev/null +++ b/bin/zurich/convert_internal_notes_to_comments @@ -0,0 +1,74 @@ +#!/usr/bin/env perl + +=head1 DESCRIPTION + +This is a Zurich specific migration script. + +It converts the internal notes that used to be stored on the problem in the +extra hash into comments instead. + + ./bin/zurich/convert_internal_notes_to_comments user_id + +You need to select a user to have the internal notes be associated with, perhaps +the superuser, or one created just for this purpose? + +=cut + +use strict; +use warnings; + +use FixMyStreet::App; + +# Because it is not possible to determine the user that last edited the +# internal_notes we need require a user to assign all the comments to. +my $comment_user_id = $ARGV[0] + || die "Usage: $0 id_of_user_for_comments"; +my $comment_user = FixMyStreet::App # + ->model('DB::User') # + ->find($comment_user_id) + || die "Could not find user with id '$comment_user_id'"; + +# We use now as the time for the internal note. This is not the time it was +# actually created, but I don't think that can be extracted from the problem. +my $comment_timestamp = DateTime->now(); + +# The state of 'hidden' seems most appropriate for these internal_notes - as +# they should not be shown to the public. Certainly more suitable than +# 'unconfirmed' or 'confirmed'. +my $comment_state = 'hidden'; + +# Load all the comments, more reliable than trying to search on the contents of +# the extra field. +my $problems = FixMyStreet::App->model('DB::Problem')->search(); + +while ( my $problem = $problems->next() ) { + + my $problem_id = $problem->id; + + # Extract the bits we are interested in. May not exist, in which case + # skip on. + my $extra = $problem->extra || {}; + next unless exists $extra->{internal_notes}; + + # If there is something to save create a comment with the notes in them + if ( my $internal_notes = $extra->{internal_notes} ) { + print "Creating internal note comment for problem '$problem_id'\n"; + $problem->add_to_comments( + { + text => $internal_notes, + created => $comment_timestamp, + user => $comment_user, + state => $comment_state, + mark_fixed => 0, + problem_state => $problem->state, + anonymous => 1, + extra => { is_internal_note => 1 }, + } + ); + } + + # Remove the notes from extra and save back to the problem + delete $extra->{internal_notes}; + $problem->update({ extra => $extra }); +} + diff --git a/conf/general.yml-example b/conf/general.yml-example index e35bb4979..2f78de986 100644 --- a/conf/general.yml-example +++ b/conf/general.yml-example @@ -130,6 +130,7 @@ MAP_TYPE: 'OSM' ALLOWED_COBRANDS: - cobrand_one - cobrand_two: 'hostname_substring2' + - cobrand_three # This is only used in "offensive report" emails to provide a link directly to # the admin interface. If wanted, set to the full URL of your admin interface. diff --git a/notes/trouble_shooting.md b/notes/trouble_shooting.md new file mode 100644 index 000000000..7a982ff4b --- /dev/null +++ b/notes/trouble_shooting.md @@ -0,0 +1,42 @@ +# Trouble shooting + +## Empty datetime object + + Couldn't render template "index.html: undef error - Can't call method "strftime" without a package or object reference at /var/www/fixmystreet.127.0.0.1.xip.io/fixmystreet/perllib/Utils.pm line 232 + +- You might have a problem with a datefield that has been left empty by one cobrand that another expects to have a value. Inspert the problem table in the database. +- You may have problems being returned by memcached that your database does not have. Restart memcached to rule this out. + +## Wrong cobrand is displaying + +- Make sure that your hostname does not contain anything that another cobrand is matching on. For example if your config is + +``` yaml +ALLOWED_COBRANDS: + - fixmystreet + - zurich +```` + +Then a domain like `zurich.fixmystreet.com` will match `fixmystreet` first and that is the cobrand that will be served. + +## Account creation emails not arriving + +Your receiving email servers may be rejecting them because: + +* your VM IP address has been blacklisted +* your ISP blocks outgoing connections to port 25 (mobile broadband providers often do this) +* sender verification has failed (applies to `@mysociety.org` servers) - check that your `DO_NOT_REPLY_EMAIL` conf setting passes sender verification (using your own email address works well). + +Perhaps check the entries in `/var/log/mail.log` to check that the message has been sent by the app, and if it has been possible to send them on. + +## Translations not being used + +The locale needs to be installed too or the translations will not be used. Use +`locale -a` to list them all and ensure the one your translation uses is in the +list. + + +## Database connection errors trying to run update-schema + +Make sure that you specify a database host and password in `general.yml`. You +may need to explicitly give your user a password. diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 3d3ddce1e..6018dfa80 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -956,6 +956,9 @@ sub check_for_errors : Private { delete $field_errors{name}; my $report = $c->stash->{report}; $report->title( Utils::cleanup_text( substr($report->detail, 0, 25) ) ); + if ( ! $c->req->param('phone') ) { + $field_errors{phone} = _("This information is required"); + } } # FIXME: need to check for required bromley fields here diff --git a/perllib/FixMyStreet/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm index ffdc1feab..450786c88 100644 --- a/perllib/FixMyStreet/Cobrand/Zurich.pm +++ b/perllib/FixMyStreet/Cobrand/Zurich.pm @@ -8,6 +8,47 @@ use RABX; use strict; use warnings; +=head1 NAME + +Zurich FixMyStreet cobrand + +=head1 DESCRIPTION + +This module provides the specific functionality for the Zurich FMS cobrand. + +=head1 DEVELOPMENT NOTES + +The admin for Zurich is different to the other cobrands. To access it you need +to be logged in as a user associated with an appropriate body. + +You can create the bodies needed to develop by running the 't/cobrand/zurich.t' +test script with the three C<$mech->delete...> lines at the end commented out. +This should leave you with the bodies and users correctly set up. + +The entries will be something like this (but with different ids). + + Bodies: + id | name | parent | endpoint + ----+---------------+--------+--------------------------- + 1 | Zurich | | + 2 | Division 1 | 1 | division@example.org + 3 | Subdivision A | 2 | subdivision@example.org + 4 | External Body | | external_body@example.org + + Users: + id | email | from_body + ----+------------------+----------- + 2 | dm1@example.org | 2 + 3 | sdm1@example.org | 3 + +The passwords for the users is 'secret'. + +Note: the password hashes are salted with the user's id so cannot be easily +changed. High ids have been used so that it should not conflict with anything +you already have, and the countres set so that they shouldn't in future. + +=cut + sub shorten_recency_if_new_greater_than_fixed { return 0; } @@ -356,19 +397,38 @@ sub admin_report_edit { } - # Problem updates upon submission + # If super or sdm check that the token is correct before proceeding if ( ($type eq 'super' || $type eq 'dm') && $c->req->param('submit') ) { $c->forward('check_token'); + } + + # All types of users can add internal notes + if ( ($type eq 'super' || $type eq 'dm' || $type eq 'sdm') && $c->req->param('submit') ) { + # If there is a new note add it as a comment to the problem (with is_internal_note set true in extra). + if ( my $new_internal_note = $c->req->params->{new_internal_note} ) { + $problem->add_to_comments( { + text => $new_internal_note, + user => $c->user->obj, + state => 'hidden', # seems best fit, should not be shown publicly + mark_fixed => 0, + problem_state => $problem->state, + anonymous => 1, + extra => { is_internal_note => 1 }, + } ); + } + } + # Problem updates upon submission + if ( ($type eq 'super' || $type eq 'dm') && $c->req->param('submit') ) { # Predefine the hash so it's there for lookups # XXX Note you need to shallow copy each time you set it, due to a bug? in FilterColumn. my $extra = $problem->extra || {}; - $extra->{internal_notes} = $c->req->param('internal_notes'); $extra->{publish_photo} = $c->req->params->{publish_photo} || 0; $extra->{third_personal} = $c->req->params->{third_personal} || 0; # Make sure we have a copy of the original detail field $extra->{original_detail} = $problem->detail if !$extra->{original_detail} && $c->req->params->{detail} && $problem->detail ne $c->req->params->{detail}; + # Workflow things my $redirect = 0; my $new_cat = $c->req->params->{category}; @@ -462,14 +522,6 @@ sub admin_report_edit { $db_update = 1; } - my $extra = $problem->extra || {}; - $extra->{internal_notes} ||= ''; - if ($c->req->param('internal_notes') && $c->req->param('internal_notes') ne $extra->{internal_notes}) { - $extra->{internal_notes} = $c->req->param('internal_notes'); - $problem->extra( { %$extra } ); - $db_update = 1; - } - $problem->update if $db_update; # Add new update from status_update @@ -592,10 +644,31 @@ sub admin_stats { ); if ( $c->req->params->{export} ) { - my $problems = $c->model('DB::Problem')->search( { %params }, { columns => [ 'id', 'created', 'latitude', 'longitude', 'cobrand', 'category' ] } ); - my $body = "ID,Created,E,N,Category\n"; - while (my $report = $problems->next) { - $body .= join( ',', $report->id, $report->created, $report->local_coords, $report->category ) . "\n"; + my $problems = $c->model('DB::Problem')->search( + {%params}, + { + columns => [ + 'id', 'created', + 'latitude', 'longitude', + 'cobrand', 'category', + 'state', 'user_id', + 'external_body' + ] + } + ); + my $body = "ID,Created,E,N,Category,Status,UserID,External Body\n"; + while ( my $report = $problems->next ) { + my $external_body; + my $body_name = ""; + if ( $external_body = $report->body($c) ) { + $body_name = $external_body->name; + } + $body .= join( ',', + $report->id, $report->created, + $report->local_coords, $report->category, + $report->state, $report->user_id, + "\"$body_name\"" ) + . "\n"; } $c->res->content_type('text/csv; charset=utf-8'); $c->res->body($body); diff --git a/perllib/FixMyStreet/DB/RABXColumn.pm b/perllib/FixMyStreet/DB/RABXColumn.pm new file mode 100644 index 000000000..5f1583018 --- /dev/null +++ b/perllib/FixMyStreet/DB/RABXColumn.pm @@ -0,0 +1,98 @@ +package FixMyStreet::DB::RABXColumn; + +use strict; +use warnings; + +use IO::String; +use RABX; + +=head1 NAME + +FixMyStreet::DB::RABXColumn + +=head2 DESCRIPTION + +This is a helper component that will setup the RABX serialisation for some +fields. This is useful for when you want to persist some data structure such as +hashrefs etc. + +This code will also change the default FilterColumn behaviour so that whenever +your set a column, or specify a RABX'd column in an ->update the value is saved +to the database. The default behaviour is to check if the value is already set, +and for hashrefs this means that changes to the contents are missed as it is +still the same hashref. + +By putting all this code in one place there is also much less repetition. + +=cut + +# Store which columns are RABX cols. +# $RABX_COLUMNS{$class}{$col} = 1 +my %RABX_COLUMNS = (); + +sub _get_class_identifier { + my $class = ref $_[0] || $_[0]; + $class =~ s/.*?(\w+)$/$1/; + return $class; +} + +=head1 METHODS + +=head2 rabx_column + + # In one of your ::Result:: modules + __PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); + __PACKAGE__->rabx_column('data'); + +This sets up the filtering to and from the database, and also changes the +set_filtered_column behaviour to not trust the cache. + +=cut + +sub rabx_column { + my ($class, $col) = @_; + + # Apply the filtering for this column + $class->filter_column( + $col => { + filter_from_storage => sub { + my $self = shift; + my $ser = shift; + return undef unless defined $ser; + utf8::encode($ser) if utf8::is_utf8($ser); + my $h = new IO::String($ser); + return RABX::wire_rd($h); + }, + filter_to_storage => sub { + my $self = shift; + my $data = shift; + my $ser = ''; + my $h = new IO::String($ser); + RABX::wire_wr( $data, $h ); + return $ser; + }, + } + ); + + # store that this column is a RABX column. + $RABX_COLUMNS{ _get_class_identifier($class) }{$col} = 1; +} + + +sub set_filtered_column { + my ($self, $col, $val) = @_; + + my $class = ref $self; + + # because filtered objects may be expensive to marshall for storage there + # is a cache that attempts to detect if they have changed or not. For us + # this cache breaks things and our marshalling is cheap, so clear it when + # trying set a column. + delete $self->{_filtered_column}{$col} + if $RABX_COLUMNS{ _get_class_identifier($class) }{$col}; + + return $self->next::method($col, $val); +} + + +1; diff --git a/perllib/FixMyStreet/DB/Result/Comment.pm b/perllib/FixMyStreet/DB/Result/Comment.pm index c712ad4e1..2d5b6b2c3 100644 --- a/perllib/FixMyStreet/DB/Result/Comment.pm +++ b/perllib/FixMyStreet/DB/Result/Comment.pm @@ -85,32 +85,13 @@ __PACKAGE__->belongs_to( # Created by DBIx::Class::Schema::Loader v0.07035 @ 2013-09-10 17:11:54 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:D/+UWcF7JO/EkCiJaAHUOw -__PACKAGE__->filter_column( - extra => { - filter_from_storage => sub { - my $self = shift; - my $ser = shift; - return undef unless defined $ser; - utf8::encode($ser) if utf8::is_utf8($ser); - my $h = new IO::String($ser); - return RABX::wire_rd($h); - }, - filter_to_storage => sub { - my $self = shift; - my $data = shift; - my $ser = ''; - my $h = new IO::String($ser); - RABX::wire_wr( $data, $h ); - return $ser; - }, - } -); +__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); +__PACKAGE__->rabx_column('extra'); use DateTime::TimeZone; use Image::Size; use Moose; use namespace::clean -except => [ 'meta' ]; -use RABX; with 'FixMyStreet::Roles::Abuser'; diff --git a/perllib/FixMyStreet/DB/Result/Contact.pm b/perllib/FixMyStreet/DB/Result/Contact.pm index 2e1287a21..eca028c9b 100644 --- a/perllib/FixMyStreet/DB/Result/Contact.pm +++ b/perllib/FixMyStreet/DB/Result/Contact.pm @@ -60,25 +60,7 @@ __PACKAGE__->belongs_to( # Created by DBIx::Class::Schema::Loader v0.07035 @ 2013-09-10 17:11:54 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:hq/BFHDEu4OUI4MSy3OyHg -__PACKAGE__->filter_column( - extra => { - filter_from_storage => sub { - my $self = shift; - my $ser = shift; - return undef unless defined $ser; - utf8::encode($ser) if utf8::is_utf8($ser); - my $h = new IO::String($ser); - return RABX::wire_rd($h); - }, - filter_to_storage => sub { - my $self = shift; - my $data = shift; - my $ser = ''; - my $h = new IO::String($ser); - RABX::wire_wr( $data, $h ); - return $ser; - }, - } -); +__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); +__PACKAGE__->rabx_column('extra'); 1; diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm index c8b53e2d1..f14a29f56 100644 --- a/perllib/FixMyStreet/DB/Result/Problem.pm +++ b/perllib/FixMyStreet/DB/Result/Problem.pm @@ -135,54 +135,15 @@ __PACKAGE__->has_one( { cascade_copy => 0, cascade_delete => 0 }, ); -__PACKAGE__->filter_column( - extra => { - filter_from_storage => sub { - my $self = shift; - my $ser = shift; - return undef unless defined $ser; - utf8::encode($ser) if utf8::is_utf8($ser); - my $h = new IO::String($ser); - return RABX::wire_rd($h); - }, - filter_to_storage => sub { - my $self = shift; - my $data = shift; - my $ser = ''; - my $h = new IO::String($ser); - RABX::wire_wr( $data, $h ); - return $ser; - }, - } -); - -__PACKAGE__->filter_column( - geocode => { - filter_from_storage => sub { - my $self = shift; - my $ser = shift; - return undef unless defined $ser; - utf8::encode($ser) if utf8::is_utf8($ser); - my $h = new IO::String($ser); - return RABX::wire_rd($h); - }, - filter_to_storage => sub { - my $self = shift; - my $data = shift; - my $ser = ''; - my $h = new IO::String($ser); - RABX::wire_wr( $data, $h ); - return $ser; - }, - } -); +__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); +__PACKAGE__->rabx_column('extra'); +__PACKAGE__->rabx_column('geocode'); use DateTime::TimeZone; use Image::Size; use Moose; use namespace::clean -except => [ 'meta' ]; use Utils; -use RABX; with 'FixMyStreet::Roles::Abuser'; diff --git a/perllib/FixMyStreet/DB/Result/Token.pm b/perllib/FixMyStreet/DB/Result/Token.pm index 028300842..5525fe7a5 100644 --- a/perllib/FixMyStreet/DB/Result/Token.pm +++ b/perllib/FixMyStreet/DB/Result/Token.pm @@ -34,8 +34,6 @@ __PACKAGE__->set_primary_key("scope", "token"); # use mySociety::DBHandle qw(dbh); use mySociety::AuthToken; -use IO::String; -use RABX; =head1 NAME @@ -54,26 +52,9 @@ ms_current_timestamp. =cut -__PACKAGE__->filter_column( - data => { - filter_from_storage => sub { - my $self = shift; - my $ser = shift; - return undef unless defined $ser; - utf8::encode($ser) if utf8::is_utf8($ser); - my $h = new IO::String($ser); - return RABX::wire_rd($h); - }, - filter_to_storage => sub { - my $self = shift; - my $data = shift; - my $ser = ''; - my $h = new IO::String($ser); - RABX::wire_wr( $data, $h ); - return $ser; - }, - } -); +__PACKAGE__->load_components("+FixMyStreet::DB::RABXColumn"); +__PACKAGE__->rabx_column('data'); + sub new { my ( $class, $attrs ) = @_; diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm index e91c6a1d6..be8f004a5 100644 --- a/perllib/FixMyStreet/TestMech.pm +++ b/perllib/FixMyStreet/TestMech.pm @@ -87,8 +87,8 @@ sub log_in_ok { my $user = $mech->create_user_ok($email); - # store the old password and then change it - my $old_password = $user->password; + # remember the old password and then change it to a known one + my $old_password = $user->password || ''; $user->update( { password => 'secret' } ); # log in @@ -99,7 +99,19 @@ sub log_in_ok { $mech->logged_in_ok; # restore the password (if there was one) - $user->update( { password => $old_password } ) if $old_password; + if ($old_password) { + + # Use store_column and then make_column_dirty to bypass the filters that + # would hash the password, otherwise the password required ito log in + # would be the hash of the previous one. + $user->store_column("password", $old_password); + $user->make_column_dirty("password"); + $user->update(); + + # Belt and braces, check that the password has been correctly saved. + die "password not correctly restored after log_in_ok" + if $user->password ne $old_password; + } return $user; } @@ -296,7 +308,7 @@ sub extract_location { $meta = $mech->extract_problem_meta; -Returns the problem meta information ( submitted by, at etc ) from a +Returns the problem meta information ( submitted by, at etc ) from a problem report page =cut diff --git a/t/app/controller/report_display.t b/t/app/controller/report_display.t index 7904b6736..62a5b3667 100644 --- a/t/app/controller/report_display.t +++ b/t/app/controller/report_display.t @@ -100,7 +100,7 @@ subtest "Zurich unconfirmeds are 200" => sub { if ( !FixMyStreet::Cobrand->exists('zurich') ) { plan skip_all => 'Skipping Zurich test without Zurich cobrand'; } - $mech->host( 'zurich.fixmystreet.com' ); + $mech->host( 'zurich.example.com' ); ok $report->update( { state => 'unconfirmed' } ), 'unconfirm report'; $mech->get_ok("/report/$report_id"); $mech->content_contains( 'Überprüfung ausstehend' ); @@ -403,7 +403,7 @@ subtest "Zurich banners are displayed correctly" => sub { if ( !FixMyStreet::Cobrand->exists('zurich') ) { plan skip_all => 'Skipping Zurich test without Zurich cobrand'; } - $mech->host( 'zurich.fixmystreet.com' ); + $mech->host( 'zurich.example.com' ); for my $test ( { diff --git a/t/app/model/rabx_column.t b/t/app/model/rabx_column.t new file mode 100644 index 000000000..607d578ce --- /dev/null +++ b/t/app/model/rabx_column.t @@ -0,0 +1,23 @@ +use strict; +use warnings; + +use Test::More; + +use_ok "FixMyStreet::DB::RABXColumn"; + +# Test that the class names are correctly normalised +my @tests = ( + ["FixMyStreet::DB::Result::Token", "Token"], + ["FixMyStreet::App::Model::DB::Token", "Token"], +); + +foreach my $test (@tests) { + my ($input, $expected) = @$test; + is( + FixMyStreet::DB::RABXColumn::_get_class_identifier($input), + $expected, + "$input -> $expected" + ); +} + +done_testing(); diff --git a/t/app/model/token.t b/t/app/model/token.t index 12945975e..637477fa3 100644 --- a/t/app/model/token.t +++ b/t/app/model/token.t @@ -3,7 +3,7 @@ use strict; use warnings; -use Test::More tests => 45; +use Test::More; use FixMyStreet; use FixMyStreet::App; @@ -94,3 +94,47 @@ foreach my $test_data_name ( sort keys %tests ) { undef, "token gone with m::AT"; } + + + +# Test that the inflation and deflation works as expected +{ + my $token = + $token_rs->create( { scope => 'testing', data => {} } ); + END { $token->delete() }; + + # Add in temporary check to test that the data is updated as expected. + is_deeply($token->data, {}, "data is empty"); + + # store something in it + $token->update({ data => { foo => 'bar' } }); + $token->discard_changes(); + is_deeply($token->data, { foo => 'bar' }, "data has content"); + + # change the hash stored + $token->update({ data => { baz => 'bundy' } }); + $token->discard_changes(); + is_deeply($token->data, { baz => 'bundy' }, "data has new content"); + + # change the hashref in place + { + my $data = $token->data; + $data->{baz} = 'new'; + $token->data( $data ); + $token->update(); + $token->discard_changes(); + is_deeply($token->data, { baz => 'new' }, "data has been updated"); + } + + # change the hashref in place + { + my $data = $token->data; + $data->{baz} = 'new'; + $token->update({ data => $data }); + $token->discard_changes(); + is_deeply($token->data, { baz => 'new' }, "data has been updated"); + } + +} + +done_testing(); diff --git a/t/cobrand/zurich.t b/t/cobrand/zurich.t index be94d4112..869e5d460 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -5,15 +5,65 @@ use strict; use warnings; use DateTime; use Test::More; +use Sub::Override; plan skip_all => 'Skipping Zurich test without Zurich cobrand' unless FixMyStreet::Cobrand->exists('zurich'); +# To run this test ensure that you have the following in general.yml: +# +# BASE_URL: 'http://zurich.127.0.0.1.xip.io' +# +# ALLOWED_COBRANDS: +# - zurich +# +# Check that you have the required locale installed - the following +# should return a line with de_CH.utf8 in. If not install that locale. +# +# locale -a | grep de_CH +# +# To generate the translations use: +# +# commonlib/bin/gettext-makemo FixMyStreet + + +# 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 { + + # Capture the bits of the original config that the following code will use + my %config = + map {$_ => mySociety::Config::get($_)} + qw(BASE_URL STAGING_SITE CONTACT_EMAIL SEND_REPORTS_ON_STAGING); + + # Change the SEND_REPORTS_ON_STAGING value to true for this test + $config{SEND_REPORTS_ON_STAGING} = 1; + + # Override the get function to return values from our captured config. This + # override will be cleared at the end of this block when the $override guard + # falls out of scope. + my $override_guard = Sub::Override->new( + "mySociety::Config::get", + sub ($;$) { + my ($key, $default) = @_; + exists $config{$key} + ? return $config{$key} + : die "Need to cache config key '$key' here"; + } + ); + + # Actually send the report + FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); + + # tidy up explicitly + $override_guard->restore(); +} + use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; # Front page test -ok $mech->host("zurich.fixmystreet.com"), "change host to Zurich"; +ok $mech->host("zurich.example.com"), "change host to Zurich"; $mech->get_ok('/'); $mech->content_like( qr/zurich/i ); @@ -82,8 +132,10 @@ $mech->content_contains('photo/' . $report->id . '.jpeg'); # Internal notes $mech->get_ok( '/admin/report_edit/' . $report->id ); -$mech->submit_form_ok( { with_fields => { internal_notes => 'Some internal notes.' } } ); -$mech->content_contains( 'Some internal notes' ); +$mech->submit_form_ok( { with_fields => { new_internal_note => 'Initial internal note.' } } ); +$mech->submit_form_ok( { with_fields => { new_internal_note => 'Another internal note.' } } ); +$mech->content_contains( 'Initial internal note.' ); +$mech->content_contains( 'Another internal note.' ); # Original description $mech->submit_form_ok( { with_fields => { detail => 'Edited details text.' } } ); @@ -97,7 +149,7 @@ $mech->get_ok( '/report/' . $report->id ); $mech->content_contains('In Bearbeitung'); $mech->content_contains('Test Test'); -FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); +send_reports_for_zurich(); my $email = $mech->get_email; like $email->header('Subject'), qr/Neue Meldung/, 'subject looks okay'; like $email->header('To'), qr/subdivision\@example.org/, 'to line looks correct'; @@ -106,6 +158,7 @@ $mech->clear_emails_ok; $mech->log_out_ok; $user = $mech->log_in_ok( 'sdm1@example.org') ; +$user->update({ from_body => undef }); $mech->get_ok( '/admin' ); is $mech->uri->path, '/my', "got sent to /my"; $user->from_body( 3 ); @@ -119,7 +172,7 @@ $mech->content_contains( DateTime->now->strftime("%d.%m.%Y") ); $mech->content_contains( 'In Bearbeitung' ); $mech->get_ok( '/admin/report_edit/' . $report->id ); -$mech->content_contains( 'Some internal notes' ); +$mech->content_contains( 'Initial internal note' ); $mech->submit_form_ok( { with_fields => { status_update => 'This is an update.' } } ); is $mech->uri->path, '/admin/report_edit/' . $report->id, "still on edit page"; @@ -132,7 +185,7 @@ $mech->get_ok( '/report/' . $report->id ); $mech->content_contains('In Bearbeitung'); $mech->content_contains('Test Test'); -FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); +send_reports_for_zurich(); $email = $mech->get_email; like $email->header('Subject'), qr/Feedback/, 'subject looks okay'; like $email->header('To'), qr/division\@example.org/, 'to line looks correct'; @@ -211,7 +264,7 @@ $mech->get_ok( '/report/' . $report->id ); $mech->content_contains('Beantwortet'); $mech->content_contains('Third Test'); $mech->content_contains('Wir haben Ihr Anliegen an External Body weitergeleitet'); -FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); +send_reports_for_zurich(); $email = $mech->get_email; like $email->header('Subject'), qr/Weitergeleitete Meldung/, 'subject looks okay'; like $email->header('To'), qr/external_body\@example.org/, 'to line looks correct'; @@ -230,7 +283,7 @@ $mech->get_ok( '/report/' . $report->id ); $mech->content_contains('Beantwortet'); $mech->content_contains('Third Test'); $mech->content_contains('Wir haben Ihr Anliegen an External Body weitergeleitet'); -FixMyStreet::App->model('DB::Problem')->send_reports('zurich'); +send_reports_for_zurich(); $email = $mech->get_email; like $email->header('Subject'), qr/Weitergeleitete Meldung/, 'subject looks okay'; like $email->header('To'), qr/external_body\@example.org/, 'to line looks correct'; @@ -240,17 +293,24 @@ $mech->clear_emails_ok; $mech->log_out_ok; # Test only superuser can edit bodies -$user = $mech->log_in_ok( 'dm1@example.org') ; +$user = $mech->log_in_ok( 'dm1@example.org' ); $mech->get( '/admin/body/' . $zurich->id ); is $mech->res->code, 404, "only superuser should be able to edit bodies"; $mech->log_out_ok; # Test only superuser can see "Add body" form -$user = $mech->log_in_ok( 'dm1@example.org') ; +$user = $mech->log_in_ok( 'dm1@example.org' ); $mech->get_ok( '/admin/bodies' ); $mech->content_lacks( '<form method="post" action="bodies"' ); $mech->log_out_ok; +# Test phone number is mandatory +$user = $mech->log_in_ok( 'dm1@example.org' ); +$mech->get_ok( '/report/new?latitude=51.500802;longitude=-0.143005' ); +$mech->submit_form( with_fields => { phone => "" } ); +$mech->content_contains( 'Diese Information wird benötigt' ); +$mech->log_out_ok; + # Test problems can't be assigned to deleted bodies $user = $mech->log_in_ok( 'dm1@example.org' ); $user->from_body( 1 ); diff --git a/templates/web/default/js/translation_strings.html b/templates/web/default/js/translation_strings.html index f6c4f7d41..8f834a81c 100644 --- a/templates/web/default/js/translation_strings.html +++ b/templates/web/default/js/translation_strings.html @@ -15,6 +15,9 @@ required: '[% loc('Please enter your email') | replace("'", "\\'") %]', email: '[% loc('Please enter a valid email') | replace("'", "\\'") %]' }, + phone: { + required: '[% loc('Please enter your phone number') | replace("'", "\\'") %]' + }, fms_extra_title: '[% loc('Please enter your title') | replace("'", "\\'") %]', first_name: '[% loc('Please enter your first name') | replace("'", "\\'") %]', last_name: '[% loc('Please enter your second name') | replace("'", "\\'") %]', diff --git a/templates/web/zurich/admin/header.html b/templates/web/zurich/admin/header.html index be146f0e1..281b1de23 100644 --- a/templates/web/zurich/admin/header.html +++ b/templates/web/zurich/admin/header.html @@ -14,6 +14,7 @@ %] <style type="text/css"> .adminhidden { color: #666666; } + .admininternal { background-color: #eeeeff; } .active { background-color: #ffffee; cursor: pointer; } .error { color: red; } .overdue { background-color: #ffcccc; } diff --git a/templates/web/zurich/admin/list_updates.html b/templates/web/zurich/admin/list_updates.html index c475d839e..80029462b 100644 --- a/templates/web/zurich/admin/list_updates.html +++ b/templates/web/zurich/admin/list_updates.html @@ -5,12 +5,14 @@ <tr> <th>[% loc('ID') %]</th> <th>[% loc('Created') %]</th> + <th>[% loc('User') %]</th> <th>[% loc('Text') %]</th> </tr> [% FOREACH update IN updates -%] - <tr[% ' class="adminhidden"' IF update.state == 'hidden' || update.problem.state == 'hidden' %]> - <td>[%- update.id %]</td> + <tr class="[% 'adminhidden' IF update.state == 'hidden' || update.problem.state == 'hidden' %] [% 'admininternal' IF update.extra.is_internal_note %]"> + <td>[% update.id %]</td> <td>[% PROCESS format_date this_date=update.created %] [% update.created.hms %]</td> + <td><a href="mailto:[% update.user.email %]">[% update.user.name || update.user.email %]</a></td> <td>[% update.text | html %]</td> </tr> [% END -%] diff --git a/templates/web/zurich/admin/report_edit-sdm.html b/templates/web/zurich/admin/report_edit-sdm.html index 94e8c6c0a..599c60b77 100644 --- a/templates/web/zurich/admin/report_edit-sdm.html +++ b/templates/web/zurich/admin/report_edit-sdm.html @@ -55,8 +55,8 @@ <li><span class="mock-label">[% loc('State:') %]</span> [% states.${problem.state} %]</li> -<li><label for="internal_notes">[% loc('Internal notes:') %]</label> -<textarea name='internal_notes' id='internal_notes' cols=60 rows=5>[% problem.extra.internal_notes | html %]</textarea></li> +<li><label for="new_internal_note">[% loc('New internal note:') %]</label> +<textarea name='new_internal_note' id='new_internal_note' cols=60 rows=5></textarea></li> <li><label for="status_update">[% loc('New update:') %]</label> <textarea name='status_update' id='status_update' cols=60 rows=5></textarea></li> diff --git a/templates/web/zurich/admin/report_edit.html b/templates/web/zurich/admin/report_edit.html index c068158cd..723086c46 100644 --- a/templates/web/zurich/admin/report_edit.html +++ b/templates/web/zurich/admin/report_edit.html @@ -81,8 +81,8 @@ [% END %] </ul> -<p><label for="internal_notes">[% loc('Internal notes:') %]</label> -<textarea name='internal_notes' id='internal_notes' cols=60 rows=5>[% problem.extra.internal_notes | html %]</textarea></p> +<p><label for="new_internal_note">[% loc('New internal note:') %]</label> +<textarea name='new_internal_note' id='new_internal_note' cols=60 rows=5>[% new_internal_note | html %]</textarea></p> <p><span class="mock-label">[% loc('State:') %]</span> <select name="state" id="state"> <option value="">--</option> diff --git a/templates/web/zurich/admin/reports.html b/templates/web/zurich/admin/reports.html index b0bc733c4..68f98c44a 100644 --- a/templates/web/zurich/admin/reports.html +++ b/templates/web/zurich/admin/reports.html @@ -13,6 +13,7 @@ [% FOREACH col IN [ [ 'category', loc('Category') ], [ 'created', loc('Submitted') ], [ 'lastupdate', loc('Updated') ], [ 'state', loc('Status') ] ] %] <th><a href="[% INCLUDE sort_link choice = col.0 %]">[% col.1 %] [% INCLUDE sort_arrow choice = col.0 %]</a></th> [% END %] + <th>[% loc('Photo') %]</th> <th class='edit'>*</th> </tr> [% INCLUDE 'admin/problem_row.html' %] diff --git a/templates/web/zurich/js/validation_rules.html b/templates/web/zurich/js/validation_rules.html new file mode 100644 index 000000000..d98bc1118 --- /dev/null +++ b/templates/web/zurich/js/validation_rules.html @@ -0,0 +1,8 @@ + validation_rules = { + title: { required: true }, + detail: { required: true }, + email: { required: true }, + update: { required: true }, + phone: { required: true }, + rznvy: { required: true } + }; 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 1cecf036d..076536601 100644 --- a/templates/web/zurich/report/new/fill_in_details_form.html +++ b/templates/web/zurich/report/new/fill_in_details_form.html @@ -103,7 +103,10 @@ [% END %] <input type="text" value="[% report.name | html %]" name="name" id="form_name" placeholder="[% loc('Your name') %]"> - <label for="form_phone">[% loc('Phone number (optional)') %]</label> + <label for="form_phone">[% loc('Phone number') %]</label> + [% IF field_errors.phone %] + <p class='form-error'>[% field_errors.phone %]</p> + [% END %] <input type="text" value="[% report.user.phone | html %]" name="phone" id="form_phone" placeholder="[% loc('Your phone number') %]"> <div class="form-txt-submit-box"> |