12 files changed, 269 insertions, 31 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/Cobrand/Zurich.pm b/perllib/FixMyStreet/Cobrand/Zurich.pm
index d39c804ad..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
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( '&Uuml;berpr&uuml;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/cobrand/zurich.t b/t/cobrand/zurich.t
index 69cf82db9..7a060ae2b 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';
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 30dbcbd4b..128e74766 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>