Clean up user editing

src/lib/Hydra/Controller/User.pm

@@ -16,7 +16,7 @@ use HTML::Entities;
 __PACKAGE__->config->{namespace} = '';
 
 
-sub login :Local :Args(0) :ActionClass('REST::ForBrowsers') { }
+sub login :Local :Args(0) :ActionClass('REST') { }
 
 sub login_POST {
     my ($self, $c) = @_;
@@ -30,11 +30,11 @@ sub login_POST {
     accessDenied($c, "Bad username or password.")
         if !$c->authenticate({username => $username, password => $password});
 
-    $self->status_ok($c, entity => { });
+    $self->status_no_content($c);
 }
 
 
-sub logout :Local :Args(0) :ActionClass('REST::ForBrowsers') { }
+sub logout :Local :Args(0) :ActionClass('REST') { }
 
 sub logout_POST {
     my ($self, $c) = @_;
@@ -106,9 +106,13 @@ sub register :Local Args(0) {
 
     accessDenied($c, "User registration is currently not implemented.") unless isAdmin($c);
 
+    if ($c->request->method eq "GET") {
         $c->stash->{template} = 'user.tt';
         $c->stash->{create} = 1;
-    return if $c->request->method ne "POST";
+        return;
+    }
+
+    die unless $c->request->method eq "PUT";
 
     my $userName = trim $c->req->params->{username};
     $c->stash->{username} = $userName;
@@ -138,13 +142,16 @@ sub register :Local Args(0) {
     }
 
     $c->flash->{successMsg} = "User <tt>$userName</tt> has been created.";
-    backToReferer($c);
+    $self->status_no_content($c);
 }
 
 
 sub updatePreferences {
     my ($c, $user) = @_;
 
+    my $fullName = trim($c->req->params->{fullname} // "");
+    error($c, "Your must specify your full name.") if $fullName eq "";
+
     my $password = trim($c->req->params->{password} // "");
     if ($user->type eq "hydra" && ($user->password eq "!" || $password ne "")) {
         error($c, "You must specify a password of at least 6 characters.")
@@ -156,9 +163,6 @@ sub updatePreferences {
         setPassword($user, $password);
     }
 
-    my $fullName = trim($c->req->params->{fullname} // "");
-    error($c, "Your must specify your full name.") if $fullName eq "";
-
     my $emailAddress = trim($c->req->params->{emailaddress} // "");
     # FIXME: validate email address?
 
@@ -185,8 +189,7 @@ sub currentUser_GET {
 
     requireUser($c);
 
-    $self->status_ok(
+    $self->status_ok($c,
-        $c,
         entity => $c->model("DB::Users")->find($c->user->username)
     );
 }
@@ -205,51 +208,56 @@ sub user :Chained('/') PathPart('user') CaptureArgs(1) {
 }
 
 
-sub deleteUser {
+sub edit :Chained('user') :PathPart('') :Args(0) :ActionClass('REST::ForBrowsers') { }
-    my ($self, $c, $user) = @_;
-    my ($project) = $c->model('DB::Projects')->search({ owner => $user->username });
-    error($c, "User " . $user->username . " is still owner of project " . $project->name . ".")
-        if defined $project;
-    $c->logout() if $user->username eq $c->user->username;
-    $user->delete;
-}
-
-
-sub edit :Chained('user') :Args(0) :ActionClass('REST::ForBrowsers') { }
 
 sub edit_GET {
     my ($self, $c) = @_;
-
-    my $user = $c->stash->{user};
-
     $c->stash->{template} = 'user.tt';
-
-    $c->session->{referer} = $c->request->referer if !defined $c->session->{referer};
-
-    $c->stash->{fullname} = $user->fullname;
-
-    $c->stash->{emailonerror} = $user->emailonerror;
 }
 
-sub edit_POST {
+sub edit_PUT {
     my ($self, $c) = @_;
-
     my $user = $c->stash->{user};
 
-    $c->stash->{template} = 'user.tt';
-
-    $c->session->{referer} = $c->request->referer if !defined $c->session->{referer};
-
-    if (($c->stash->{params}->{submit} // "") eq "delete") {
-        deleteUser($self, $c, $user);
-        backToReferer($c);
-    }
-
     if (($c->stash->{params}->{submit} // "") eq "reset-password") {
+        return;
+    }
+
+    txn_do($c->model('DB')->schema, sub {
+        updatePreferences($c, $user);
+    });
+
+    $c->flash->{successMsg} = "Your preferences have been updated.";
+    $self->status_no_content($c);
+}
+
+sub edit_DELETE {
+    my ($self, $c) = @_;
+    my $user = $c->stash->{user};
+
+    my ($project) = $c->model('DB::Projects')->search({ owner => $user->username });
+    error($c, "User " . $user->username . " is still owner of project " . $project->name . ".")
+        if defined $project;
+
+    $c->logout() if $user->username eq $c->user->username;
+
+    $user->delete;
+
+    $c->flash->{successMsg} = "The user has been deleted.";
+    $self->status_no_content($c);
+}
+
+
+sub reset_password :Chained('user') :PathPart('reset-password') :Args(0) {
+    my ($self, $c) = @_;
+    my $user = $c->stash->{user};
+
+    requirePost($c);
+
     error($c, "This user's password cannot be reset.") if $user->type ne "hydra";
-        $c->stash->{json} = {};
     error($c, "No email address is set for this user.")
         unless $user->emailaddress;
+
     my $password = Crypt::RandPasswd->word(8,10);
     setPassword($user, $password);
     sendEmail($c,
@@ -260,20 +268,10 @@ sub edit_POST {
         "You can change your password at " . $c->uri_for($self->action_for('edit'), [$user->username]) . ".\n\n".
         "With regards,\n\nHydra.\n"
     );
-        return;
-    }
 
-    txn_do($c->model('DB')->schema, sub {
+    $c->flash->{successMsg} = "A new password has been sent to ${\$user->emailaddress}.";
-        updatePreferences($c, $user);
-    });
-
-    if ($c->request->looks_like_browser) {
-        $c->flash->{successMsg} = "Your preferences have been updated.";
-        backToReferer($c);
-    } else {
     $self->status_no_content($c);
 }
-}
 
 
 sub dashboard :Chained('user') :Args(0) {

src/root/static/js/common.js

@@ -145,3 +145,9 @@ function redirectJSON(args) {
     };
     return requestJSON(args);
 };
+
+function backToReferrer() {
+    // FIXME: should only do this if the referrer is another Hydra
+    // page.
+    window.location = document.referrer;
+}

src/root/user.tt

@@ -1,4 +1,4 @@
-[% WRAPPER layout.tt title=(create ? "Register new user" : "Editing user $user.username") %]
+[% WRAPPER layout.tt title=(create ? "Add new user" : "Editing user $user.username") %]
 [% PROCESS common.tt %]
 
 [% BLOCK roleoption %]
@@ -14,7 +14,7 @@
     >[% role %]</option>
 [% END %]
 
-<form class="form-horizontal" method="post">
+<form class="form-horizontal">
 
   <fieldset>
 
@@ -30,7 +30,7 @@
     <div class="control-group">
       <label class="control-label">Full name</label>
       <div class="controls">
-        <input type="text" class="span3" name="fullname" [% HTML.attributes(value => fullname) %]/>
+        <input type="text" class="span3" name="fullname" [% HTML.attributes(value => create ? '' : user.fullname) %]/>
       </div>
     </div>
 
@@ -60,7 +60,7 @@
     <div class="control-group">
       <div class="controls">
         <label class="checkbox">
-          <input type="checkbox" name="emailonerror" [% IF emailonerror; 'checked="checked"'; END %]/>Receive evaluation error notifications
+          <input type="checkbox" name="emailonerror" [% IF !create && user.emailonerror; 'checked="checked"'; END %]/>Receive evaluation error notifications
         </label>
       </div>
     </div>
@@ -93,44 +93,21 @@
     [% END %]
 
     <div class="form-actions">
-      <button type="submit" class="btn btn-primary">
+      <button id="submit-user" class="btn btn-primary">
         <i class="icon-ok icon-white"></i>
         [%IF create %]Create[% ELSE %]Apply changes[% END %]
       </button>
       [% IF !create && c.check_user_roles('admin') && user.type == 'hydra' %]
-        <button id="reset-password" type="submit" class="btn btn-warning" name="submit" value="reset-password">
+        <button id="reset-password" class="btn btn-warning">
           <i class="icon-trash icon-white"></i>
           Reset password
         </button>
-        <script type="text/javascript">
-          $("#reset-password").click(function() {
-            bootbox.confirm(
-              'Are you sure you want to reset the password for this user?',
-              function(c) {
-                if (!c) return;
-                $.post("[% c.uri_for(c.controller('User').action_for('edit'), [user.username]) %]", { submit: 'reset-password' })
-                  .done(function(data) {
-                    if (data.error)
-                      bootbox.alert("Unable to reset password: " + data.error);
-                    else
-                      bootbox.alert("An email containing the new password has been sent to the user.");
-                  })
-                  .fail(function() { bootbox.alert("Server request failed!"); });
-              });
-            return false;
-          });
-        </script>
       [% END %]
       [% IF !create %]
-        <button id="delete-user" type="submit" class="btn btn-danger" name="submit" value="delete">
+        <button id="delete-user" class="btn btn-danger">
           <i class="icon-trash icon-white"></i>
           Delete this user
         </button>
-        <script type="text/javascript">
-          $("#delete-user").click(function() {
-            return confirm("Are you sure you want to delete this user?");
-          });
-        </script>
       [% END %]
     </div>
 
@@ -138,4 +115,48 @@
 
 </form>
 
+<script>
+  $("#submit-user").click(function() {
+    requestJSON({
+      [% IF create %]
+        url: "[% c.uri_for(c.controller('User').action_for('register')) %]",
+      [% ELSE %]
+        url: "[% c.uri_for(c.controller('User').action_for('edit'), c.req.captures) %]",
+      [% END %]
+      data: $(this).parents("form").serialize(),
+      type: 'PUT',
+      success: backToReferrer
+    });
+    return false;
+  });
+
+  $("#reset-password").click(function() {
+    bootbox.confirm(
+      'Are you sure you want to reset the password for this user?',
+      function(c) {
+        if (!c) return;
+        requestJSON({
+          url: "[% c.uri_for(c.controller('User').action_for('reset_password'), [user.username]) %]",
+          type: 'POST',
+          success: backToReferrer
+        });
+      });
+    return false;
+  });
+
+  $("#delete-user").click(function() {
+    bootbox.confirm(
+      'Are you sure you want to delete this user?',
+      function(c) {
+        if (!c) return;
+        requestJSON({
+          url: "[% c.uri_for(c.controller('User').action_for('edit'), c.req.captures) %]",
+          type: 'DELETE',
+          success: backToReferrer
+        });
+      });
+    return false;
+  });
+</script>
+
 [% END %]