Revise the user login code.

* Remove user registration link and popup from the theme; this
  shouldn't be done in a popup. Use ajaxform to simplify the way
  that we load the login popup.

* Create form.html.php, this is a template for Forge based forms.

* Move user validation rules into User_Model and let forms
  populate the rules into their forms as useful.

* Undo r18688's changes regarding the REST code.  We should never
  accept a null resource, this breaks the REST abstraction.

* Change login and user controllers to use Forge which lets us delete
  login.html.php and user.html.php since those now are generated by
  the theme-owned form template

13 files changed, 226 insertions, 240 deletions

diff --git a/core/controllers/item.php b/core/controllers/item.php
index 286eb66f..78ee7b0b 100644
--- a/core/controllers/item.php
+++ b/core/controllers/item.php
@@ -21,10 +21,6 @@ class Item_Controller extends REST_Controller {
   protected $resource_type = "item";
 
   public function _get($item) {
-    if (empty($item)) {
-      // A null item is not allowed for albums or photos.
-      return Kohana::show_404();
-    }
     // Redirect to the more specific resource type, since it will render
     // differently.  We could also just delegate here, but it feels more appropriate
     // to have a single canonical resource mapping.
diff --git a/core/controllers/rest.php b/core/controllers/rest.php
index 6e0acbcb..09c85653 100644
--- a/core/controllers/rest.php
+++ b/core/controllers/rest.php
@@ -49,23 +49,14 @@
 abstract class REST_Controller extends Controller {
   protected $resource_type = null;
 
-  public function dispatch($id=null) {
+  public function dispatch($id) {
     if ($this->resource_type == null) {
       throw new Exception("@todo ERROR_MISSING_RESOURCE_TYPE");
     }
 
-    if ($id != null) {
-      // @todo this needs security checks
-      $resource = ORM::factory($this->resource_type, $id);
-      if (!$resource->loaded) {
-        return Kohana::show_404();
-      }
-    } else if (request::method() == "get") {
-      // A null id and a request method of "get" just returns an empty form
-      // @todo figure out how to handle the input without and id
-      // @todo do we use put for create and post for update?
-      $resource = null;
-    } else {
+    // @todo this needs security checks
+    $resource = ORM::factory($this->resource_type, $id);
+    if (!$resource->loaded) {
       return Kohana::show_404();
     }
     /**
diff --git a/modules/user/controllers/login.php b/modules/user/controllers/login.php
index 7c70bb0f..a0e9f403 100644
--- a/modules/user/controllers/login.php
+++ b/modules/user/controllers/login.php
@@ -19,38 +19,29 @@
  */
 class Login_Controller extends Controller {
   public function index() {
-    $form = new Forge();
-    $form->input("username")->rules("required|length[4,32]");
-    $form->password("password")->rules("required|length[5,40]");
-    $form->submit("Login");
-    print $form->render("login.html", true);
-  }
-
-  public function process() {
-    $form = new Forge("login.html", true);
-    $form->input("username")->rules("required|length[4,32]");
-    $form->password("password")->rules("required|length[5,40]");
-    $form->submit("Login");
+    $form = new Forge("login", "", "post", array("id" => "gLogin"));
+    $group = $form->group(_("Login"));
+    $group->input("name")->label(_("Name"))->id("gName")->class(null);
+    $group->password("password")->label(_("Password"))->id("gPassword")->class(null);
+    $group->submit(_("Login"));
+    $form->hidden("continue")->value($this->input->get("continue"));
+    $group->inputs["name"]->error_messages("invalid_login", _("Invalid name or password"));
 
-    $response = array();
     if ($form->validate()) {
-      // Load the user
-      $user = ORM::factory("user")->where("name", $form->username->value)->find();
-      if (!$user->loaded) {
-        $response["error_message"] = _("Invalid username or password");
-      } else {
-        if (user::is_correct_password($user, $form->password->value)) {
-          user::login($user);
-          $response["error_message"] = "";
-        } else {
-          $response["error_message"] = _("Invalid username or password");
+      $user = ORM::factory("user")->where("name", $group->inputs["name"]->value)->find();
+      if ($user->loaded &&
+          user::is_correct_password($user, $group->password->value)) {
+        user::login($user);
+        $continue = $form->hidden["continue"]->value;
+        if ($continue) {
+          url::redirect($form->hidden["continue"]->value);
         }
+        return;
+      } else {
+        $group->inputs["name"]->add_error("invalid_login", 1);
       }
-    } else {
-      $response["error_message"] = _("Invalid username or password");
     }
 
-    print json_encode($response);
+    print $form->render("form.html", false);
   }
-
 }
\ No newline at end of file
diff --git a/modules/user/controllers/logout.php b/modules/user/controllers/logout.php
index 34f27fff..524c79f8 100644
--- a/modules/user/controllers/logout.php
+++ b/modules/user/controllers/logout.php
@@ -24,6 +24,8 @@ class Logout_Controller extends Controller {
     } catch (Exception $e) {
       Kohana::log("error", $e);
     }
-    print json_encode(array("logout" => true));
+    if ($this->input->get("continue")) {
+      url::redirect($this->input->get("continue"));
+    }
   }
 }
\ No newline at end of file
diff --git a/modules/user/controllers/user.php b/modules/user/controllers/user.php
index 41cb9da5..a1085ae5 100644
--- a/modules/user/controllers/user.php
+++ b/modules/user/controllers/user.php
@@ -21,24 +21,58 @@ class User_Controller extends REST_Controller {
   protected $resource_type = "user";
 
   /**
+   * Return the form for creating / modifying users.
+   */
+  private function _get_form($user) {
+    $form = new Forge("user/{$user->id}", "", "post", array("id" => "gUser"));
+    $group = $form->group(_("User Info"));
+    $group->input("name")
+      ->label(_("Name"))
+      ->id("gName")
+      ->class(null)
+      ->value($user->name);
+    $group->input("display_name")
+      ->label(_("Display Name"))
+      ->id("gDisplayName")
+      ->class(null)
+      ->value($user->display_name);
+    $group->password("password")
+      ->label(_("Password"))
+      ->id("gPassword")
+      ->class(null);
+    $group->input("email")
+      ->label(_("Email"))
+      ->id("gEmail")
+      ->class(null)
+      ->value($user->email);
+    $group->submit(_("Modify"));
+    $form->hidden("continue")->value($this->input->get("continue"));
+
+    $this->_add_validation_rules(ORM::factory("user")->validation_rules, $form);
+
+    return $form;
+  }
+
+  /**
+   * @todo Refactor this into a more generic location
+   */
+  private function _add_validation_rules($rules, $form) {
+    foreach ($form->inputs as $name => $input) {
+      if (isset($input->inputs)) {
+        $this->_add_validation_rules($rules, $input);
+      }
+      if (isset($rules[$name])) {
+        $input->rules($rules[$name]);
+      }
+    }
+  }
+
+  /**
    * @see Rest_Controller::_get($resource)
    */
   public function _get($user) {
-    $userView = new View("user.html");
-    if (empty($user)) {
-      // @todo remove this when rest_controller is changed to handle a post with no id
-      $user = ORM::factory("user");
-      $user->save();
-      // @todo remove this when rest_controller is changed to handle a post with no id ^
-      $userView->user_id = $user->id;
-      $userView->action = _("User Registration");
-      $userView->button_text = _("Register");
-    } else {
-      $userView->user_id = $user->id;
-      $userView->action = _("User Modify");
-      $userView->button_text = _("Modify");
-    }
-    print $userView;
+    $form = $this->_get_form($user);
+    print $form->render("form.html", false);
   }
 
   /**
@@ -51,7 +85,17 @@ class User_Controller extends REST_Controller {
   /**
    *  @see Rest_Controller::_post($resource)
    */
-  public function _post($resource) {
+  public function _post($user) {
+    $form = $this->_get_form($user);
+    if ($form->validate()) {
+      // @todo if we use the Validation class here, the ORM can just read the inputs directly.  We
+      // need to investigate that.
+      //
+      // @todo
+      // Verify the user input, store it in the object.
+      // Show errors on validation failure.
+      // On success, redirect if there's a form->continue, else show an empty page.
+    }
     throw new Exception("@todo User_Controller::_post NOT IMPLEMENTED");
   }
 
diff --git a/modules/user/models/user.php b/modules/user/models/user.php
index b09af033..1c56b34e 100644
--- a/modules/user/models/user.php
+++ b/modules/user/models/user.php
@@ -20,6 +20,11 @@
 class User_Model extends ORM {
   protected $has_and_belongs_to_many = array("groups");
 
+  var $validation_rules = array(
+    "name" => "required|length[4,32]",
+    "email" => "valid_email",
+    "password" => "required|length[5,40]");
+
   public function __set($column, $value) {
     switch ($column) {
       case "password":
diff --git a/modules/user/views/login.html.php b/modules/user/views/login.html.php
deleted file mode 100644
index 78e03b46..00000000
--- a/modules/user/views/login.html.php
+++ /dev/null
@@ -1,21 +0,0 @@
-<? defined("SYSPATH") or die("No direct script access."); ?>
-<form id="gLogin" action="<?= url::site("login/process") ?>">
-  <fieldset>
-    <legend><?= _("Login") ?></legend>
-    <ul>
-      <li>
-        <label for="gUsername"><?= _("Username") ?></label>
-        <input type="text" name="username" id="gUsername" />
-      </li>
-      <li>
-        <label for="gPassword"><?= _("Password") ?></label>
-        <input type="password" name="password" id="gPassword" />
-      </li>
-      <li>
-        <input type="submit" value="<?= _("Login")?>" />
-      </li>
-    </ul>
-    <div id="gLoginMessage" class="gStatus gError gDisplayNone">
-    </div>
-  </fieldset>
-</form>
diff --git a/modules/user/views/user.html.php b/modules/user/views/user.html.php
deleted file mode 100644
index 614645e5..00000000
--- a/modules/user/views/user.html.php
+++ /dev/null
@@ -1,34 +0,0 @@
-<? defined("SYSPATH") or die("No direct script access."); ?>
-<form id="gUser" action="<?= url::site("user/dispatch/$user_id") ?>">
-  <fieldset>
-    <legend><?=  $action ?></legend>
-    <ul>
-      <li>
-        <label for="gUsername"><?= _("Username") ?></label>
-        <input type="text" id="gUsername" />
-        <span id="gUsername_error" class="gStatus gError gDisplayNone"></span>
-      </li>
-      <li>
-        <label for="gPassword"><?= _("Password") ?></label>
-        <input type="password" id="gPassword" />
-        <span id="gPassword_error" class="gStatus gError gDisplayNone"></span>
-      </li>
-      <li>
-        <label for="gPassword_confirm"><?= _("Confirm Password") ?></label>
-        <input type="password" id="gPassword_confirm" />
-      </li>
-      <li>
-        <label for="gEmail"><?= _("Password") ?></label>
-        <input type="password" id="gEmail" />
-        <span id="gEmail_error" class="gStatus gError gDisplayNone"></span>
-      </li>
-      <li>
-        <label for="gEmail_confirm"><?= _("Confirm Email") ?></label>
-        <input type="password" id="gEmaild_confirm" />
-      </li>
-      <li>
-        <input type="submit" value="<?=$button_text?>" />
-      </li>
-    </ul>
-  </fieldset>
-</form>
diff --git a/themes/default/css/screen.css b/themes/default/css/screen.css
index 5261706a..eeb4eeb9 100644
--- a/themes/default/css/screen.css
+++ b/themes/default/css/screen.css
@@ -576,7 +576,7 @@ table.gMetadata td.toggle {
 
 /**
  ** *******************************************************************
- * 8) Forms (general and specific) 
+ * 8) Forms (general and specific)
  ** *******************************************************************
  */
 
@@ -596,15 +596,15 @@ label {
   cursor: help;
 }
 
-input[type="text"], 
+input[type="text"],
 input[type="password"],
-textarea, 
-select { 
+textarea,
+select {
 }
 
-input[type="text"], 
-input[type="password"], 
-textarea, 
+input[type="text"],
+input[type="password"],
+textarea,
 .gValidationRule {
   width: 40%;
 }
@@ -645,7 +645,7 @@ optgroup {
 select {
 }
 
-textarea { 
+textarea {
   width: 99%;
   height: 12em;
 }
@@ -655,16 +655,16 @@ button {
 
 /* ~~~~~~~~~ Form layout ~~~~~~~~~~ */
 
-form ul, form li { 
-  list-style: none !important; 
+form ul, form li {
+  list-style: none !important;
 }
-form ul { 
-  margin: 0; 
+form ul {
+  margin: 0;
   padding: 0;
 }
 
-form li { 
-  margin-top: .5em; 
+form li {
+  margin-top: .5em;
   padding: .3em 1.5em .3em 1em;
 }
 
@@ -676,16 +676,16 @@ form ul ul li {
   float: left;
 }
 
-input, 
+input,
 textarea {
-  display: block; 
+  display: block;
   clear: both;
 }
 
 /* ~~~~~~~~~ Inline fieldsets ~~~~~~~~~~ */
 
 .gInline li {
-  float: left; 
+  float: left;
   margin: 0;
   padding: .3em .5em .4em;
   text-align: left;
@@ -694,7 +694,7 @@ textarea {
 .gInline input {
 }
 
-.gInline input[type="Submit"] { 
+.gInline input[type="Submit"] {
   margin-top: 1em;
 }
 
@@ -702,16 +702,16 @@ textarea {
   margin-top: 0;
 }
 
-.gInline input[type="text"], 
-.gInline input[type="password"], 
-.gInline textarea, 
-.gInline .gValidationRule { 
+.gInline input[type="text"],
+.gInline input[type="password"],
+.gInline textarea,
+.gInline .gValidationRule {
   width: 10em;
 }
 
-input:focus, 
-textarea:focus, 
-option:focus { 
+input:focus,
+textarea:focus,
+option:focus {
   background-color: #ffc;
 }
 
@@ -720,13 +720,13 @@ option:focus {
 .gRequired {
 }
 
-ul.gError, 
-li.gError { 
+ul.gError,
+li.gError {
   background-color: #ffdcdc;
 }
 
-.gError label, 
-.gValidationRule { 
+.gError label,
+.gValidationRule {
   color: red;
 }
 
@@ -735,24 +735,24 @@ li.gError {
   margin-top: .5em;
 }
 
-form.gError input[type="text"], 
-li.gError input[type="text"], 
-form.gError input[type="password"], 
-li.gError input[type="password"], 
-form.gError input[type="checkbox"], 
-li.gError input[type="checkbox"], 
-form.gError input[type="radio"], 
-li.gError input[type="radio"], 
-form.gError textarea, 
-li.gError textarea, 
-form.gError select, 
-li.gError select { 
+form.gError input[type="text"],
+li.gError input[type="text"],
+form.gError input[type="password"],
+li.gError input[type="password"],
+form.gError input[type="checkbox"],
+li.gError input[type="checkbox"],
+form.gError input[type="radio"],
+li.gError input[type="radio"],
+form.gError textarea,
+li.gError textarea,
+form.gError select,
+li.gError select {
   border: 2px solid red;
 }
 
 /* ~~~~~~~~ form font size ~~~~~~ */
 
-#gHeader form, #gSidebar form { 
+#gHeader form, #gSidebar form {
   font-size: .9em;
 }
 
diff --git a/themes/default/js/user.js b/themes/default/js/user.js
index b389a67e..42ab1aa7 100644
--- a/themes/default/js/user.js
+++ b/themes/default/js/user.js
@@ -1,66 +1,29 @@
-$(document).ready(function() {
-  $("#gLoginForm").submit(function() {
-    process_login();
-    return false;
-  });
-  $("#gLogoutLink").click(function() {
-    process_logout();
-    return false;
-  });
-});
-
-function show_form(formName) {
-  $(formName + "Link").css({display: "none"});
-  $(formName + "Text").css({display: "inline"});
-  $(formName + "Close").css({display: "inline"});
-  var url = $(formName + "Form").attr("formSrc");
-  $.get(url, null, function(data, textStatus) {
-    $(formName + "Form").html(data);
-    $(formName + "Form").css({display: "block"});  
+function show_login(url) {
+  $("#gLoginLink").hide();
+  $("#gLoginClose").show();
+  $.get(url, function(data) {
+    $("#gLoginFormContainer").html(data);
+    ajaxify_login_form();
   });
 }
 
-function hide_form(formName) {
-  $(formName + "Link").css({display: "inline"});  
-  $(formName + "Form").css({display: "none"});  
-  $(formName + "Form").html("");  
-  $(formName + "Text").css({display: "none"});
-  $(formName + "Close").css({display: "none"});
-}
-
-function process_login() {
-  $.ajax({
-    url: $("#gLogin").attr("action"),
-    type: "POST",
-    data: $("#gLogin").serialize(),
-    dataType: "json",
-    error: function(XMLHttpRequest, textStatus, errorThrown) {
-      alert("textStatus: " + textStatus + "\nerrorThrown: " + errorThrown);
-    },
-    success: function(data, textStatus) {
-      if (data.error_message != "") {
-        $("#gLoginMessage").html(data.error_message);
-        $("#gLoginMessage").css({display: "block"});
-        $("#gLogin").addClass("gError");
-      } else {
+function ajaxify_login_form() {
+  $("form#gLogin").ajaxForm({
+    target: "#gLoginFormContainer",
+    success: function(responseText, statusText) {
+      if (!responseText) {
         window.location.reload();
+      } else {
+        ajaxify_login_form();
       }
-    }
+    },
   });
 }
 
-function process_logout() {
-  $.ajax({
-    url: $("#gLogoutLink").attr("href"),
-    type: "GET",
-    dataType: "json",
-    error: function(XMLHttpRequest, textStatus, errorThrown) {
-      alert("textStatus: " + textStatus + "\nerrorThrown: " + errorThrown);
-    },
-    success: function(data, textStatus) {
-      if (data.logout) {
-        window.location.reload();
-      }
-    }
-  });
-}
\ No newline at end of file
+function close_login() {
+  $("#gLogin").remove();
+  $("#gLoginClose").hide();
+  $("#gLoginLink").show();
+  $("input#gUsername").val("");
+  $("input#gPassword").val("");
+}
diff --git a/themes/default/views/form.html.php b/themes/default/views/form.html.php
new file mode 100644
index 00000000..bc8d1339
--- /dev/null
+++ b/themes/default/views/form.html.php
@@ -0,0 +1,54 @@
+<?
+print($open);
+
+// Not sure what to do with these, but at least show that we received them.
+if ($class) {
+  print "<!-- unused class in form.html.php: $class -->";
+}
+if ($title) {
+  print "<!-- unused title in form.html.php: $title -->";
+}
+
+function DrawForm($inputs, $level=1) {
+  $error_messages = array();
+  $prefix = str_repeat("  ", $level);
+
+  foreach ($inputs as $input) {
+    if ($input->type == 'group') {
+      print "$prefix<fieldset>\n";
+      print "$prefix  <legend>$input->name</legend>\n";
+      print "$prefix  <ul>\n";
+      DrawForm($input->inputs, $level + 2);
+      print "$prefix  </ul>\n";
+      print "$prefix</fieldset>\n";
+    } else {
+      if ($input->error_messages()) {
+        $error_messages = array_merge($error_messages, $input->error_messages());
+        print "$prefix<li class=\"gError\">\n";
+      } else {
+        print "$prefix<li>\n";
+      }
+      if ($input->label()) {
+        print $prefix . "  " . $input->label() . "\n";
+      }
+      print $prefix . "  " . $input->render() . "\n";
+      print "$prefix</li>\n";
+      if ($input->message()) {
+        print "$prefix<li>\n";
+        print $prefix . "  " . $input->message() . "\n";
+        print "$prefix</li>\n";
+      }
+    }
+  }
+  if ($error_messages) {
+    print "$prefix  <div class=\"gStatus gError\">\n";
+    foreach ($error_messages as $message) {
+      print "<p class=\"gError\">$message<p>";
+    }
+    print "$prefix  </div>\n";
+  }
+}
+DrawForm($inputs);
+
+print($close);
+?>
diff --git a/themes/default/views/header.html.php b/themes/default/views/header.html.php
index 24dd9331..8cba82ba 100644
--- a/themes/default/views/header.html.php
+++ b/themes/default/views/header.html.php
@@ -2,31 +2,25 @@
 <img id="gLogo" alt="<?= _("Logo") ?>" src="<?= $theme->url("images/logo.png") ?>" />
 <h1><?= $item->title_edit ?></h1>
 
-<div id="gLoginMenu">
+<ul id="gLoginMenu">
   <? if ($user): ?>
-    <a href="<?= url::site("user/update")?>"><?= _("Modify Profile") ?></a> 
-    | <a href="<?= url::site("logout")?>" id="gLogoutLink"><?= _("Logout") ?></a>
+  <a href="<?= url::site("user/update")?>"><?= _("Modify Profile") ?></a>
+  | <a href="<?= url::site("logout?continue=" . url::current(true)) ?>" id="gLogoutLink">
+    <?= _("Logout") ?>
+  </a>
   <? else: ?>
-    <a href="#"><?=_("Recover password") ?></a>
-    <span id="gUserLink" >
-      | <a href="javascript:show_form('#gUser')"><?= _("Register") ?></a>
-    </span>
-    <span id="gUserLinkText" class="gDisplayNone">
-      | <?= _("Register") ?>
-    </span>
-    <span id="gLoginLink">
-      | <a href="javascript:show_form('#gLogin')"><?= _("Login") ?></a>
-    </span>
-    <span id="gLoginText" class="gDisplayNone">
-      | <?= _("Login") ?> 
-    </span>
-    <span id="gLoginClose" class="gDisplayNone">
-      | <a href="javascript:hide_form('#gLogin')">X</a>
-    </span>
+  <span id="gLoginLink">
+    <a href="javascript:show_login('<?= url::site("login") ?>')">
+      <?= _("Login") ?>
+    </a>
+  </span>
+  <span id="gLoginClose" class="gDisplayNone">
+    <?= _("Login") ?> | <a href="javascript:close_login()">X</a>
+  </span>
+  <div id="gLoginFormContainer"></div>
   <? endif; ?>
-  <span id="gUserForm" class="gDisplayNone" formSrc="<?= url::site("user/dispatch/") ?>"></span>
-  <span id="gLoginForm" class="gDisplayNone" formSrc="<?= url::site("login") ?>"></span>
-</div>
+</ul>
+
 <ul id="gSiteMenu">
   <li><a href="<?= url::base() ?>"><?= _("HOME") ?></a></li>
   <li><a class="active" href="<?= url::site("album/1") ?>"><?= _("BROWSE") ?></a></li>
@@ -38,7 +32,7 @@
 <form id="gSearchForm" class="gInline">
   <ul class="gNoLabels">
     <li><input type="text" value="<?= _("Search Gallery ...") ?>"/></li>
-    <li><input type="submit" value="search" /></li>
+    <li><input type="submit" value="<?= _("search") ?>" /></li>
   </ul>
 </form>
 
diff --git a/themes/default/views/page.html.php b/themes/default/views/page.html.php
index 18dd735c..c6addf98 100644
--- a/themes/default/views/page.html.php
+++ b/themes/default/views/page.html.php
@@ -9,10 +9,11 @@
           media="screen,print,projection" />
     <link rel="stylesheet" type="text/css" href="<?= url::file("lib/yui/base-min.css") ?>"
           media="screen,print,projection" />
-    <link rel="stylesheet" type="text/css" href="<?= $theme->url("css/screen.css") ?>" 
+    <link rel="stylesheet" type="text/css" href="<?= $theme->url("css/screen.css") ?>"
           media="screen,print,projection" />
     <script src="<?= url::file("lib/jquery.js") ?>" type="text/javascript"></script>
     <script src="<?= url::file("lib/jquery.jeditable.js") ?>" type="text/javascript"></script>
+    <script src="<?= url::file("lib/jquery.form.js") ?>" type="text/javascript"></script>
     <script src="<?= $theme->url("js/user.js") ?>" type="text/javascript"></script>
   </head>