Sign in over login; tidy CSS.

15 files changed, 114 insertions, 112 deletions

diff --git a/TODO.txt b/TODO.txt
deleted file mode 100644
index ee3007b6d..000000000
--- a/TODO.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-
-Auth:
-  * add 'remember me' option on login.
-  * limit session to this browser session on create account
-  * redirect back to page they came from on login
-
-Problem creation:
-  ? what should the new flow be for not-logged-in (probably unchanged).
-  * after becoming confirmed require user to manually send off 'pending' reports
-
-Users:
-  * create a message to the user if they have reports which have not been
-    confirmed.
-
-Email:
-  * currently don't send email via EvEl or do any of the smarts it does - should
-    we switch to using it (Email::Send::EvEl...)?
-
-Framework:
-  * use Plack to handle all the redirects and also run cgi scripts - get apache
-    out of the picture (what about tile proxy...). Does this makes sense?
-
-
-
-Future ideas:
-  * dashboard for council to put on big screen
-  
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 2277639df..8aed746ec 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -14,14 +14,14 @@ FixMyStreet::App::Controller::Auth - Catalyst Controller
 
 =head1 DESCRIPTION
 
-Controller for all the authentication related pages - create account, login,
-logout.
+Controller for all the authentication related pages - create account, sign in,
+sign out.
 
 =head1 METHODS
 
 =head2 index
 
-Present the user with a login / create account page. 
+Present the user with a sign in / create account page.
 
 =cut
 
@@ -36,25 +36,27 @@ sub general : Path : Args(0) {
     return unless $req->method eq 'POST';
 
     # decide which action to take
-    $c->detach('email_login') if $req->param('email_login');
-    $c->detach('login');    # default
+    $c->detach('email_sign_in') if $req->param('email_sign_in');
+
+       $c->forward( 'sign_in' )
+    && $c->detach( 'redirect_on_signin', [ $req->param('r') ] );
 
 }
 
-=head2 login
+=head2 sign_in
 
-Allow the user to legin with a username and a password.
+Allow the user to sign in with a username and a password.
 
 =cut
 
-sub login : Private {
-    my ( $self, $c ) = @_;
+sub sign_in : Private {
+    my ( $self, $c, $email ) = @_;
 
-    my $email       = $c->req->param('email')          || '';
-    my $password    = $c->req->param('password_login') || '';
-    my $remember_me = $c->req->param('remember_me')    || 0;
+    $email        ||= $c->req->param('email')            || '';
+    my $password    = $c->req->param('password_sign_in') || '';
+    my $remember_me = $c->req->param('remember_me')      || 0;
 
-    # logout just in case
+    # Sign out just in case
     $c->logout();
 
     if (   $email
@@ -66,22 +68,22 @@ sub login : Private {
         $c->set_session_cookie_expire(0)
           unless $remember_me;
 
-        $c->detach( 'redirect_on_signin', [ $c->req->param('r') ] );
+        return 1;
     }
 
-    # could not authenticate - show an error
-    $c->stash->{login_error} = 1;
+    $c->stash->{sign_in_error} = 1;
+    return;
 }
 
-=head2 email_login
+=head2 email_sign_in
 
-Email the user the details they need to log in. Don't check for an account - if
+Email the user the details they need to sign in. Don't check for an account - if
 there isn't one we can create it when they come back with a token (which
 contains the email addresss).
 
 =cut
 
-sub email_login : Private {
+sub email_sign_in : Private {
     my ( $self, $c ) = @_;
 
     # check that the email is valid - otherwise flag an error
@@ -104,7 +106,7 @@ sub email_login : Private {
     my $token_obj = $c->model('DB::Token')    #
       ->create(
         {
-            scope => 'email_login',
+            scope => 'email_sign_in',
             data  => {
                 email => $good_email,
                 r => $c->req->param('r'),
@@ -121,7 +123,7 @@ sub email_login : Private {
 
 =head2 token
 
-Handle the 'email_login' tokens. Find the account for the email address
+Handle the 'email_sign_in' tokens. Find the account for the email address
 (creating if needed), authenticate the user and delete the token.
 
 =cut
@@ -132,7 +134,7 @@ sub token : Path('/M') : Args(1) {
     # retrieve the token or return
     my $token_obj = $url_token
       ? $c->model('DB::Token')->find( {
-          scope => 'email_login', token => $url_token
+          scope => 'email_sign_in', token => $url_token
         } )
       : undef;
 
@@ -141,7 +143,7 @@ sub token : Path('/M') : Args(1) {
         return;
     }
 
-    # logout in case we are another user
+    # Sign out in case we are another user
     $c->logout();
 
     # get the email and scrap the token
@@ -175,7 +177,7 @@ sub redirect_on_signin : Private {
 
 =head2 redirect
 
-Used when trying to view a page that requires login when you're not.
+Used when trying to view a page that requires sign in when you're not.
 
 =cut
 
@@ -228,13 +230,13 @@ sub change_password : Local {
 
 }
 
-=head2 logout
+=head2 sign_out
 
 Log the user out. Tell them we've done so.
 
 =cut
 
-sub logout : Local {
+sub sign_out : Local {
     my ( $self, $c ) = @_;
     $c->logout();
 }
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 44a7fa6bc..a6be6c90c 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -28,8 +28,6 @@ Create a new report, or complete a partial one .
 
 =head2 flow control
 
-submit_map: true if we reached this page by clicking on the map
-
 submit_problem: true if a problem has been submitted
 
 =head2 location (required)
@@ -912,9 +910,12 @@ sub redirect_or_confirm_creation : Private {
     }
 
     # otherwise create a confirm token and email it to them.
-    my $token =
-      $c->model("DB::Token")
-      ->create( { scope => 'problem', data => $report->id } );
+    my $token = $c->model("DB::Token")->create( {
+        scope => 'problem',
+        data => {
+            id => $report->id
+        }
+    } );
     $c->stash->{token_url} = $c->uri_for_email( '/P', $token->token );
     $c->send_email( 'problem-confirm.txt', {
         to => [ [ $report->user->email, $report->name ] ],
diff --git a/perllib/FixMyStreet/App/Controller/Tokens.pm b/perllib/FixMyStreet/App/Controller/Tokens.pm
index 111508e60..c9c9f3ab7 100644
--- a/perllib/FixMyStreet/App/Controller/Tokens.pm
+++ b/perllib/FixMyStreet/App/Controller/Tokens.pm
@@ -32,7 +32,7 @@ sub confirm_problem : Path('/P') {
       $c->forward( 'load_auth_token', [ $token_code, 'problem' ] );
 
     # Load the problem
-    my $problem_id = $auth_token->data;
+    my $problem_id = $auth_token->data->{id};
     my $problem = $c->cobrand->problems->find( { id => $problem_id } )
       || $c->detach('token_error');
     $c->stash->{problem} = $problem;
diff --git a/perllib/FixMyStreet/TestMech.pm b/perllib/FixMyStreet/TestMech.pm
index 13f1b10cf..1391254b6 100644
--- a/perllib/FixMyStreet/TestMech.pm
+++ b/perllib/FixMyStreet/TestMech.pm
@@ -94,8 +94,8 @@ sub log_in_ok {
     # log in
     $mech->get_ok('/auth');
     $mech->submit_form_ok(
-        { with_fields => { email => $email, password_login => 'secret' } },
-        "login using form" );
+        { with_fields => { email => $email, password_sign_in => 'secret' } },
+        "sign in using form" );
     $mech->logged_in_ok;
 
     # restore the password (if there was one)
@@ -114,7 +114,7 @@ Log out the current user
 
 sub log_out_ok {
     my $mech = shift;
-    $mech->get_ok('/auth/logout');
+    $mech->get_ok('/auth/sign_out');
     $mech->not_logged_in_ok;
 }
 
diff --git a/t/app/controller/alert_new.t b/t/app/controller/alert_new.t
index 5bf7e31dd..d0976b4bb 100644
--- a/t/app/controller/alert_new.t
+++ b/t/app/controller/alert_new.t
@@ -193,8 +193,8 @@ foreach my $test (
 foreach my $test (
     {
         desc       => 'logged in user signing up',
-        user       => 'test-login@example.com',
-        email      => 'test-login@example.com',
+        user       => 'test-sign-in@example.com',
+        email      => 'test-sign-in@example.com',
         type       => 'council',
         param1     => 2651,
         param2     => 2651,
@@ -203,7 +203,7 @@ foreach my $test (
     {
         desc       => 'logged in user signing up with different email',
         user       => 'loggedin@example.com',
-        email      => 'test-login@example.com',
+        email      => 'test-sign-in@example.com',
         type       => 'council',
         param1     => 2651,
         param2     => 2651,
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 056dea225..8fbc413ec 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -38,7 +38,7 @@ for my $test (
         {
             form_name => 'general_auth',
             fields    => { email => $email, },
-            button    => 'email_login',
+            button    => 'email_sign_in',
         },
         "try to create an account with email '$email'"
     );
@@ -53,7 +53,7 @@ $mech->submit_form_ok(
     {
         form_name => 'general_auth',
         fields    => { email => $test_email, },
-        button    => 'email_login',
+        button    => 'email_sign_in',
     },
     "create an account for '$test_email'"
 );
@@ -100,7 +100,7 @@ $mech->not_logged_in_ok;
     $mech->not_logged_in_ok;
 }
 
-# get a login email and change password
+# get a sign in email and change password
 {
     $mech->clear_emails_ok;
     $mech->get_ok('/auth');
@@ -111,9 +111,9 @@ $mech->not_logged_in_ok;
                 email => "$test_email",
                 r     => 'faq', # Just as a test
             },
-            button    => 'email_login',
+            button    => 'email_sign_in',
         },
-        "email_login with '$test_email'"
+        "email_sign_in with '$test_email'"
     );
 
     # rest is as before so no need to test
@@ -182,19 +182,19 @@ $mech->not_logged_in_ok;
 }
 
 foreach my $remember_me ( '1', '0' ) {
-    subtest "login using valid details (remember_me => '$remember_me')" => sub {
+    subtest "sign in using valid details (remember_me => '$remember_me')" => sub {
         $mech->get_ok('/auth');
         $mech->submit_form_ok(
             {
                 form_name => 'general_auth',
                 fields    => {
                     email       => $test_email,
-                    password_login => $test_password,
+                    password_sign_in => $test_password,
                     remember_me => ( $remember_me ? 1 : undef ),
                 },
-                button => 'login',
+                button => 'sign_in',
             },
-            "login with '$test_email' & '$test_password"
+            "sign in with '$test_email' & '$test_password"
         );
         is $mech->uri->path, '/my', "redirected to correct page";
 
@@ -210,18 +210,18 @@ foreach my $remember_me ( '1', '0' ) {
     };
 }
 
-# try to login with bad details
+# try to sign in with bad details
 $mech->get_ok('/auth');
 $mech->submit_form_ok(
     {
         form_name => 'general_auth',
         fields    => {
             email    => $test_email,
-            password_login => 'not the password',
+            password_sign_in => 'not the password',
         },
-        button => 'login',
+        button => 'sign_in',
     },
-    "login with '$test_email' & '$test_password"
+    "sign in with '$test_email' & '$test_password"
 );
 is $mech->uri->path, '/auth', "redirected to correct page";
 $mech->content_contains( 'Email or password wrong', 'found error message' );
diff --git a/t/app/controller/my.t b/t/app/controller/my.t
index 1ed6806a4..da509e8ed 100644
--- a/t/app/controller/my.t
+++ b/t/app/controller/my.t
@@ -7,9 +7,9 @@ use FixMyStreet::TestMech;
 my $mech = FixMyStreet::TestMech->new;
 
 $mech->get_ok('/my');
-is $mech->uri->path, '/auth', "got sent to the login page";
+is $mech->uri->path, '/auth', "got sent to the sign in page";
 
-# login
+# sign in
 my $user = $mech->log_in_ok( 'test@example.com' );
 $mech->get_ok('/my');
 is $mech->uri->path, '/my', "stayed on '/my/' page";
diff --git a/t/app/controller/report_updates.t b/t/app/controller/report_updates.t
index 21429c2a4..fb2f7abb7 100644
--- a/t/app/controller/report_updates.t
+++ b/t/app/controller/report_updates.t
@@ -611,7 +611,6 @@ foreach my $test (
         alert     => 1,    # we signed up for alerts before, do not unsign us
         anonymous => 0,
         answered  => 0,
-        login     => 1,
         path => '/report/update',
         content =>
 "Thanks, glad to hear it's been fixed! Could we just ask if you have ever reported a problem to a council before?",
@@ -639,7 +638,6 @@ foreach my $test (
         alert     => 1,    # we signed up for alerts before, do not unsign us
         anonymous => 0,
         answered  => 1,
-        login     => 1,
         path    => '/report/' . $report->id,
         content => $report->title,
     },
diff --git a/templates/web/default/around/display_location.html b/templates/web/default/around/display_location.html
index 88300113b..aa1dc86d6 100755
--- a/templates/web/default/around/display_location.html
+++ b/templates/web/default/around/display_location.html
@@ -23,7 +23,6 @@
             pc         => pc
             latitude   => short_latitude,
             longitude  => short_longitude,
-            submit_map => 1,
             skipped    => 1,
         }
     );
@@ -37,7 +36,6 @@
 %]
 
 <form action="[% c.uri_for('/report/new') %]" method="post" name="mapForm" id="mapForm">
-<input type="hidden" name="submit_map" value="1">
 <input type="hidden" name="map" value="[% c.req.params.map | html %]">
 <input type="hidden" name="pc" value="[% pc | html %]">
 [% c.cobrand.form_elements('mapForm') %]
diff --git a/templates/web/default/auth/general.html b/templates/web/default/auth/general.html
index b5e6bf6e0..d30fefcee 100644
--- a/templates/web/default/auth/general.html
+++ b/templates/web/default/auth/general.html
@@ -17,27 +17,25 @@
         loc_email_error = errors.$email_error || errors.other;
     END %]
     
-    <div id="fieldset">
+    [% IF loc_email_error  %]
+        <div class="form-error">[% loc_email_error %]</div>
+    [% ELSIF sign_in_error %]
+        <div class="form-error">[% loc('Email or password wrong - please try again.') %]</div>
+    [% END %]
 
-        [% IF loc_email_error  %]
-            <div class="form-error">[% loc_email_error %]</div>
-        [% ELSIF login_error %]
-            <div class="form-error">[% loc('Email or password wrong - please try again.') %]</div>
-        [% END %]
-
-
-        <div class="form-field">
+    <div class="form-field">
         <label class="n" for="email">[% loc('Your email address:') %]</label>
         <input type="email" size="30" id="email" name="email" value="[% email | html %]">
-        </div>
+    </div>
 
-        <h3 style="margin-bottom:0">[% loc("Do you have a FixMyStreet password?") %]</h3>
+<div id="form_sign_in">
+    <h3>[% loc("Do you have a FixMyStreet password?") %]</h3>
 
-<div style="float: left; width: 48%; border-right: solid 1px #999999;">
+    <div id="form_sign_in_yes">
 
         <p>
-        <label class="n" for="password_login">[% loc('<strong>Yes</strong>, I have a password:') %]</label>
-        <input type="password" name="password_login" id="password_login" value="">
+        <label class="n" for="password_sign_in">[% loc('<strong>Yes</strong>, I have a password:') %]</label>
+        <input type="password" name="password_sign_in" id="password_sign_in" value="">
         </p>
 
         <p>
@@ -48,22 +46,24 @@
         </p>
         
         <p>
-        <input type="submit" name="login" value="[% loc('Sign in') %]">
+        <input type="submit" name="sign_in" value="[% loc('Sign in') %]">
         </p>
 
-</div>
-<div style="float: right; width: 48%; clear:none;">
+    </div>
+    <div id="form_sign_in_no">
 
         <p>[% loc('<strong>No</strong>, I do not, let me sign in by email:') %]</p>
 
-        <div class="form-field">
-        <label for="name">[% loc('Your name:') %]</label>
-        <input type="text" name="name" value="">
-        </div>
+        <div id="fieldset">
+            <div class="form-field">
+                <label for="name">[% loc('Your name:') %]</label>
+                <input type="text" name="name" value="">
+            </div>
 
-        <div class="form-field">
-        <label for="password_register">[% loc('Enter a new password:') %]</label>
-        <input type="password" name="password_register" id="password_register" value="">
+            <div class="form-field">
+                <label for="password_register">[% loc('Enter a new password:') %]</label>
+                <input type="password" name="password_register" id="password_register" value="">
+            </div>
         </div>
 
         <p><small>Providing a name and password is optional, but doing so
@@ -71,12 +71,12 @@
         manage your reports.</small></p>
 
         <p>
-        <input type="submit" name="email_login" value="[% loc('Sign in by email') %]">
+        <input type="submit" name="email_sign_in" value="[% loc('Sign in by email') %]">
         </p>
 
-</div>
-
     </div>
+
+</div>
     
 </form>
 
diff --git a/templates/web/default/auth/logout.html b/templates/web/default/auth/sign_out.html
index 3d8df60e4..3d8df60e4 100644
--- a/templates/web/default/auth/logout.html
+++ b/templates/web/default/auth/sign_out.html
diff --git a/templates/web/default/report/new/notes.html b/templates/web/default/report/new/notes.html
index 38c8bfb7b..be605ddaa 100644
--- a/templates/web/default/report/new/notes.html
+++ b/templates/web/default/report/new/notes.html
@@ -1,4 +1,4 @@
-<p>[% loc("Please note:") %]</p>
+<p style="clear:both">[% loc("Please note:") %]</p>
 
 <ul>
 
diff --git a/web/css/core.css b/web/css/core.css
index 747b76c47..576437251 100644
--- a/web/css/core.css
+++ b/web/css/core.css
@@ -145,6 +145,20 @@
   padding: 5px;
   text-align: center;
 }
+#mysociety #form_sign_in_yes {
+  float: left;
+  width: 47%;
+  padding-right: 1%;
+  border-right: solid 1px #999999;
+  margin-bottom: 1em;
+}
+#mysociety #form_sign_in_no, #mysociety #fieldset #form_sign_in_no {
+  float: right;
+  width: 47%;
+  padding-left: 1%;
+  clear: none;
+  margin-bottom: 1em;
+}
 #mysociety #watermark {
   background: url("/i/mojwatermark6.png");
   height: 113px;
diff --git a/web/css/core.scss b/web/css/core.scss
index 99d9584e9..246a33167 100644
--- a/web/css/core.scss
+++ b/web/css/core.scss
@@ -189,6 +189,22 @@ $map_width: 500px;
         text-align: center;
     }
 
+    #form_sign_in_yes {
+        float: left;
+        width: 47%;
+        padding-right: 1%;
+        border-right: solid 1px #999999;
+        margin-bottom: 1em;
+    }
+
+    #form_sign_in_no, #fieldset #form_sign_in_no {
+        float: right;
+        width: 47%;
+        padding-left: 1%;
+        clear: none;
+        margin-bottom: 1em;
+    }
+
     // Map
 
     #watermark {