diff options
31 files changed, 579 insertions, 155 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/update-schema b/bin/update-schema index f728f8c56..81c9ff4ab 100644 --- a/bin/update-schema +++ b/bin/update-schema @@ -84,6 +84,7 @@ print "Nothing to do\n" if $nothing; # By querying the database schema, we can see where we're currently at # (assuming schema change files are never half-applied, which should be the case) sub get_db_version { + return '0029' if column_exists('body', 'deleted'); return '0028' if table_exists('body'); return '0027' if column_exists('problem', 'subcategory'); return '0026' if column_exists('open311conf', 'send_extended_statuses'); 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/db/schema.sql b/db/schema.sql index 5e4bc4a3e..5c54e2d88 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -64,7 +64,8 @@ create table body ( comment_user_id int references users(id), suppress_alerts boolean not null default 'f', can_be_devolved boolean not null default 'f', - send_extended_statuses boolean not null default 'f' + send_extended_statuses boolean not null default 'f', + deleted boolean not null default 'f' ); create table body_areas ( diff --git a/db/schema_0029-add_deleted_flag_to_body.sql b/db/schema_0029-add_deleted_flag_to_body.sql new file mode 100644 index 000000000..3a8be7890 --- /dev/null +++ b/db/schema_0029-add_deleted_flag_to_body.sql @@ -0,0 +1,6 @@ +BEGIN; + +ALTER table body + ADD column deleted BOOL NOT NULL DEFAULT 'f'; + +COMMIT; 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/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index ba8f22a05..4973b7c4e 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -413,7 +413,7 @@ sub update_contacts : Private { sub body_params : Private { my ( $self, $c ) = @_; - my @fields = qw/name endpoint jurisdiction api_key send_method send_comments suppress_alerts send_extended_statuses comment_user_id can_be_devolved parent/; + my @fields = qw/name endpoint jurisdiction api_key send_method send_comments suppress_alerts send_extended_statuses comment_user_id can_be_devolved parent deleted/; my %defaults = map { $_ => '' } @fields; %defaults = ( %defaults, send_comments => 0, @@ -422,6 +422,7 @@ sub body_params : Private { send_extended_statuses => 0, can_be_devolved => 0, parent => undef, + deleted => 0, ); my %params = map { $_ => $c->req->param($_) || $defaults{$_} } @fields; return \%params; 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 8e8b79d07..e15170721 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,29 +397,51 @@ 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, + 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}; + # Some changes will be accompanied by an internal note, which if needed + # should be stored in this variable. + my $internal_note_text = ""; + # Workflow things my $redirect = 0; my $new_cat = $c->req->params->{category}; if ( $new_cat && $new_cat ne $problem->category ) { my $cat = $c->model('DB::Contact')->search( { category => $c->req->params->{category} } )->first; + my $old_cat = $problem->category; $problem->category( $new_cat ); $problem->external_body( undef ); $problem->bodies_str( $cat->body_id ); $problem->whensent( undef ); $extra->{changed_category} = 1; + $internal_note_text = "Weitergeleitet von $old_cat an $new_cat"; $redirect = 1 if $cat->body_id ne $body->id; } elsif ( my $subdiv = $c->req->params->{body_subdivision} ) { $extra->{moderated_overdue} = $self->overdue( $problem ); @@ -401,7 +464,7 @@ sub admin_report_edit { } } - $problem->extra( { %$extra } ); + $problem->extra( $extra ); $problem->title( $c->req->param('title') ); $problem->detail( $c->req->param('detail') ); $problem->latitude( $c->req->param('latitude') ); @@ -410,7 +473,7 @@ sub admin_report_edit { # Final, public, Update from DM if (my $update = $c->req->param('status_update')) { $extra->{public_response} = $update; - $problem->extra( { %$extra } ); + $problem->extra( $extra ); if ($c->req->params->{publish_response}) { $problem->state( 'fixed - council' ); _admin_send_email( $c, 'problem-closed.txt', $problem ); @@ -424,9 +487,22 @@ sub admin_report_edit { '<p><em>' . _('Updated!') . '</em></p>'; # do this here otherwise lastupdate and confirmed times - # do not display correctly + # do not display correctly (reloads problem from database, including + # fields modified by the database when saving) $problem->discard_changes; + # Create an internal note if required + if ($internal_note_text) { + $problem->add_to_comments( { + text => $internal_note_text, + user => $c->user->obj, + state => 'hidden', # seems best fit, should not be shown publicly + mark_fixed => 0, + anonymous => 1, + extra => { is_internal_note => 1 }, + } ); + } + if ( $redirect ) { $c->detach('index'); } @@ -462,14 +538,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 @@ -491,7 +559,7 @@ sub admin_report_edit { if ($c->req->param('no_more_updates')) { my $extra = $problem->extra || {}; $extra->{subdiv_overdue} = $self->overdue( $problem ); - $problem->extra( { %$extra } ); + $problem->extra( $extra ); $problem->bodies_str( $body->parent->id ); $problem->whensent( undef ); $problem->state( 'planned' ); 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/Body.pm b/perllib/FixMyStreet/DB/Result/Body.pm index b6a044b04..c2b0555fb 100644 --- a/perllib/FixMyStreet/DB/Result/Body.pm +++ b/perllib/FixMyStreet/DB/Result/Body.pm @@ -40,6 +40,8 @@ __PACKAGE__->add_columns( { data_type => "boolean", default_value => \"false", is_nullable => 0 }, "send_extended_statuses", { data_type => "boolean", default_value => \"false", is_nullable => 0 }, + "deleted", + { data_type => "boolean", default_value => \"false", is_nullable => 0 }, ); __PACKAGE__->set_primary_key("id"); __PACKAGE__->has_many( @@ -90,8 +92,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07035 @ 2013-09-10 17:11:54 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:JT0w76BWaDpjAV61WVSYHg +# Created by DBIx::Class::Schema::Loader v0.07035 @ 2013-09-10 18:11:23 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:hTOxxiiHmC8nmQK/p8dXhQ sub url { my ( $self, $c ) = @_; 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 875a4262c..6039017e9 100644 --- a/t/cobrand/zurich.t +++ b/t/cobrand/zurich.t @@ -1,19 +1,69 @@ # TODO -# Overdue alerts +# Overdue alerts 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 ); @@ -65,6 +115,48 @@ $mech->content_contains( 'report_edit/' . $report->id ); $mech->content_contains( DateTime->now->strftime("%d.%m.%Y") ); $mech->content_contains( 'Erfasst' ); + +subtest "changing of categories" => sub { + # create a few categories (which are actually contacts) + foreach my $name ( qw/Cat1 Cat2/ ) { + FixMyStreet::App->model('DB::Contact')->find_or_create({ + body => $division, + category => $name, + email => "$name\@example.org", + confirmed => 1, + deleted => 0, + editor => "editor", + whenedited => DateTime->now(), + note => "note for $name", + }); + } + + # put report into known category + my $original_category = $report->category; + $report->update({ category => 'Cat1' }); + is( $report->category, "Cat1", "Category set to Cat1" ); + + # get the latest comment + my $comments_rs = $report->comments->search({},{ order_by => { -desc => "created" } }); + ok ( !$comments_rs->first, "There are no comments yet" ); + + # change the category via the web interface + $mech->get_ok( '/admin/report_edit/' . $report->id ); + $mech->submit_form_ok( { with_fields => { category => 'Cat2' } } ); + + # check changes correctly saved + $report->discard_changes(); + is( $report->category, "Cat2", "Category changed to Cat2 as expected" ); + + # Check that a new comment has been created. + my $new_comment = $comments_rs->first(); + is( $new_comment->text, "Weitergeleitet von Cat1 an Cat2", "category change comment created" ); + + # restore report to original state. + $report->update({category => $original_category }); +}; + + $mech->get_ok( '/admin/report_edit/' . $report->id ); $mech->content_contains( 'Unbestätigt' ); # Unconfirmed email $mech->submit_form_ok( { with_fields => { state => 'confirmed' } } ); @@ -82,8 +174,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 +191,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 +200,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 +214,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 +227,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 +306,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 +325,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,20 +335,41 @@ $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 ); +$user->update; +$report->state( 'confirmed' ); +$report->update; +$mech->get_ok( '/admin/body/' . $external_body->id ); +$mech->submit_form_ok( { with_fields => { deleted => 1 } } ); +$mech->get_ok( '/admin/report_edit/' . $report->id ); +$mech->content_lacks( $external_body->name ); +$user->from_body( 2 ); +$user->update; +$mech->log_out_ok; + # Test hidden report email are only sent when requested $user = $mech->log_in_ok( 'dm1@example.org') ; -my $extra = $report->extra; +$extra = $report->extra; $extra->{email_confirmed} = 1; $report->extra ( { %$extra } ); $report->update; diff --git a/templates/web/default/admin/bodies.html b/templates/web/default/admin/bodies.html index fabaf8923..c0824521e 100644 --- a/templates/web/default/admin/bodies.html +++ b/templates/web/default/admin/bodies.html @@ -16,6 +16,7 @@ <th>[% loc('Name') %]</th> [% IF c.cobrand.moniker == 'zurich' %] <th>[% loc('Email') %]</th> + <th>[% loc('Deleted') %]</th> [% ELSE %] <th>[% loc('Category') %]</th> [% END %] @@ -23,7 +24,7 @@ [%- FOREACH body IN bodies %] [%- SET id = body.id %] [% NEXT IF c.cobrand.moniker == 'zurich' AND admin_type == 'dm' AND (body.parent OR body.bodies) %] - <tr> + <tr[% IF c.cobrand.moniker == 'zurich' AND body.deleted %] class="muted"[% END %]> <td> [% IF c.cobrand.moniker == 'zurich' %] [% FILTER repeat(4*body.api_key) %] [% END %] @@ -39,6 +40,7 @@ </td> [% IF c.cobrand.moniker == 'zurich' %] <td>[% body.endpoint %]</td> + <td>[% IF body.deleted %]Yes[% END %]</td> [% ELSE %] <td> [% IF counts.$id %] 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/body-form.html b/templates/web/zurich/admin/body-form.html index 5ae8eb8a6..4e570a058 100644 --- a/templates/web/zurich/admin/body-form.html +++ b/templates/web/zurich/admin/body-form.html @@ -10,7 +10,6 @@ <input type="text" name="endpoint" id="email" value="[% body.endpoint %]" size="50"> </p> -[% IF admin_type == 'super' %] <p> <label for="parent">[% loc('Parent') %]</label> <select name="parent" id="parent"> @@ -31,7 +30,11 @@ [% END %] </select> </p> -[% END %] + + <p> + <label for="deleted">[% loc('Flag as deleted') %]</label> + <input type="checkbox" name="deleted" id="deleted" value="1"[% ' checked' IF body.deleted %] /> + </p> <input type="hidden" name="send_method" value="Email"> <input type="hidden" name="jurisdiction" id="jurisdiction" value="[% body.jurisdiction %]"> 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 d92d5c661..648b91b08 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> @@ -154,7 +154,7 @@ $(function(){ <select name="body_external" id="body_external"> <option value="">--</option> [% FOR body IN bodies %] - [% NEXT IF body.parent OR body.bodies %] + [% NEXT IF body.parent OR body.bodies OR body.deleted %] <option value="[% body.id %]"[% IF body.id == problem.bodies_str %] selected[% END %]>[% body.name %]</option> [% END %] </select> 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"> diff --git a/web/cobrands/zurich/_zurich.scss b/web/cobrands/zurich/_zurich.scss index c9eb7a080..f9ea29d9a 100644 --- a/web/cobrands/zurich/_zurich.scss +++ b/web/cobrands/zurich/_zurich.scss @@ -35,3 +35,7 @@ body.frontpage #zurich-footer-wrapper { #zurich-footer a:hover { color: #3c3c3c; } + +.muted, .muted a { + color: #aaa; +} |