A variety of cleanups:

* Allow for the "movie" type in all of our text
* Try to follow the pattern of mainly only passing ORM objects
  to the view and let it generate its own text (this becomes
  even more important when 3rd parties want to customize notification
  messages)
* Rename _send_message to be _notify_subscribers to be more acccurate
  and have it explicitly take a subject in the API
* Use Item_Model::url() in the views instead of hand crafting URLs
* Reformat HTML in views
* Use $comment->author_xxx() functions instead of replicating that code
* Fix several places where we were encoding data by doing ucfirst($item->type)
  with conditionals where we form the text properly.  We should *never*
  be showing data types to the end user!  This is not localizable!

Note that this probably breaks the existing batch processing code.  I
am going to redo that in a subsequent pass.

6 files changed, 144 insertions, 140 deletions

diff --git a/modules/notification/helpers/notification.php b/modules/notification/helpers/notification.php
index 4a644812..4d095888 100644
--- a/modules/notification/helpers/notification.php
+++ b/modules/notification/helpers/notification.php
@@ -84,93 +84,85 @@ class notification {
 
   static function send_item_updated($old, $new) {
     $body = new View("item_updated.html");
+    $body->old = $old;
+    $body->new = $new;
     $body->subject = $old->is_album() ?
       t("Album %title updated", array("title" => $old->title)) :
-      t("Photo %title updated", array("title" => $old->title));
-    $body->type = ucfirst($old->type);
-    $body->item_title = $old->title;
-    $body->description = $item->description;
-    $body->new_title = $old->title != $new->title ? $new->title : null;
-    $body->new_description = $old->title != $new->description ? $new->description : null;
-    $body->url = url::site("{$old->type}s/$old->id", "http");
-
-    self::_send_message($old, $body);
+      ($old->is_photo() ?
+       t("Photo %title updated", array("title" => $old->title))
+       : t("Movie %title updated", array("title" => $old->title)));
+
+    self::_notify_subscribers($old, $body, $body->subject);
   }
 
   static function send_item_add($item) {
     $body = new View("item_added.html");
-    $body->subject = $item->is_album() ?
+    $body->item = $item;
+
+    $parent = $item->parent();
+    $subject = $item->is_album() ?
       t("Album %title added to %parent_title",
-        array("title" => $item->title, "parent_title" => $item->parent()->title)) :
-      t("Photo %title added to %parent_title",
-        array("title" => $item->title, "parent_title" => $item->parent()->title));
-    $body->parent_title = $item->parent()->title;
-    $body->type = $item->type;
-    $body->item_title = $item->title;
-    $body->description = $item->description;
-    $body->url = url::site("{$item->type}s/$item->id", "http");
-
-    self::_send_message($item, $body);
+        array("title" => $item->title, "parent_title" => $parent->title)) :
+      ($item->is_photo() ?
+       t("Photo %title added to %parent_title",
+         array("title" => $item->title, "parent_title" => $parent->title))
+       : t("Movie %title added to %parent_title",
+           array("title" => $item->title, "parent_title" => $parent->title)));
+
+    self::_notify_subscribers($item, $body, $subject);
   }
 
   static function send_batch_add($parent_id) {
     $parent = ORM::factory("item", $parent_id);
     if ($parent->loaded) {
       $body = new View("batch_add.html");
-      $body->subject = t("%parent_title Updated",
-          array("parent_title" => $parent->title));
-      $body->parent_title = $parent->title;
-      $body->url = url::site("{$parent->type}s/$parent->id", "http");
+      $body->item = $parent;
 
-      self::_send_message($parent, $body);
+      $subject = t("Album %title updated", array("title" => $parent->title));
+      self::_notify_subscribers($parent, $body, $subject);
     }
   }
 
   static function send_item_deleted($item) {
-    $parent = $item->parent();
     $body = new View("item_deleted.html");
-    $body->subject =
-      $item->is_album() ?
+    $body->item = $item;
+    $parent = $item->parent();
+    $subject = $item->is_album() ?
       t("Album %title removed from %parent_title",
-        array("title" => $item->title, "parent_title" => $item->parent()->title)) :
-      t("Photo %title removed from %parent_title",
-        array("title" => $item->title, "parent_title" => $item->parent()->title));
-    $body->parent_title = $parent->title;
-    $body->type = $item->type;
-    $body->item_title = $item->title;
-    $body->url = url::site("albums/$parent->id", "http");
-
-    self::_send_message($item, $body);
+        array("title" => $item->title, "parent_title" => $parent->title)) :
+      ($item->is_photo() ?
+       t("Photo %title removed from %parent_title",
+         array("title" => $item->title, "parent_title" => $parent->title))
+       : t("Movie %title removed from %parent_title",
+           array("title" => $item->title, "parent_title" => $parent->title)));
+
+    self::_notify_subscribers($item, $body, $subject);
   }
 
   static function send_comment_published($comment) {
-    $item = $comment->item();
     $body = new View("comment_published.html");
-    $body->subject = $item->is_album() ?
+    $body->comment = $comment;
+
+    $item = $comment->item();
+    $subject = $item->is_album() ?
       t("A new comment was published for album %title", array("title" => $item->title)) :
-      t("A new comment was published for photo %title", array("title" => $item->title));
-    $body->text = $comment->text;
-    if (!empty($comment->author_id)) {
-      $author = ORM::factory("user", $comment->author_id);
-      $body->author = empty($author->full_name) ? $author->name : $author->full_name;
-    } else {
-      $body->author = $comment->guest_name;
-    }
-    $body->url = url::site("albums/$item->id#comments", "http");
+      ($item->is_photo() ?
+       t("A new comment was published for photo %title", array("title" => $item->title))
+       : t("A new comment was published for movie %title", array("title" => $item->title)));
 
-    self::_send_message($item, $body);
+    self::_notify_subscribers($item, $body, $subject);
   }
 
   static function process_notifications() {
     Kohana::log("error", "processing notifications in shutdown");
   }
-  
-  private static function _send_message($item, $body) {
+
+  private static function _notify_subscribers($item, $body, $subject) {
     $users = self::get_subscribers($item);
     if (!empty($users)) {
       Sendmail::factory()
         ->to($users)
-        ->subject($body->subject)
+        ->subject($subject)
         ->header("Mime-Version", "1.0")
         ->header("Content-type", "text/html; charset=utf-8")
         ->message($body->render())
diff --git a/modules/notification/helpers/notification_event.php b/modules/notification/helpers/notification_event.php
index df06a2f1..dde328d8 100644
--- a/modules/notification/helpers/notification_event.php
+++ b/modules/notification/helpers/notification_event.php
@@ -23,9 +23,7 @@ class notification_event_Core {
   }
 
   static function item_created($item) {
-    if (!batch::in_progress("add")) {
-      notification::send_item_add($item);
-    }
+    notification::send_item_add($item);
   }
 
   static function item_before_delete($item) {
diff --git a/modules/notification/views/comment_published.html.php b/modules/notification/views/comment_published.html.php
index 4a7936e6..23588c72 100644
--- a/modules/notification/views/comment_published.html.php
+++ b/modules/notification/views/comment_published.html.php
@@ -1,19 +1,31 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
 <html>
-<head>
-  <title><?= $subject ?> </title>
-</head>
-<body>
-  <h2><?= sprintf(t("A new comment was added by %s"), $author); ?></h2>
-  <table>
-    <tr>
-      <td><?= t("Comment:") ?></td>
-      <td><?= $text ?></td>
-    </tr>
-    <tr>
-      <td><?= t("Url:") ?></td>
-      <td><a href="<?= $url ?>"><?= $url ?></a></td>
-    </tr>
-  </table>
-</body>
+  <head>
+    <title><?= $subject ?> </title>
+  </head>
+  <body>
+    <h2><?= $subject ?></h2>
+    <table>
+      <tr>
+        <td><?= t("Comment:") ?></td>
+        <td><?= $comment->text ?></td>
+      </tr>
+      <tr>
+        <td><?= t("Author Name:") ?></td>
+        <td><?= $comment->author_name() ?></td>
+      </tr>
+      <tr>
+        <td><?= t("Author Email:") ?></td>
+        <td><?= $comment->author_email() ?></td>
+      </tr>
+      <tr>
+        <td><?= t("Author URL:") ?></td>
+        <td><?= $comment->author_url() ?></td>
+      </tr>
+      <tr>
+        <td><?= t("Url:") ?></td>
+        <td><a href="<?= $comment->item()->url(array(), true) ?>#comments"><?= $comment->item()->url(array(), true) ?>#comments</a></td>
+      </tr>
+    </table>
+  </body>
 </html>
diff --git a/modules/notification/views/item_added.html.php b/modules/notification/views/item_added.html.php
index 1832fb39..b67b9f38 100644
--- a/modules/notification/views/item_added.html.php
+++ b/modules/notification/views/item_added.html.php
@@ -1,25 +1,25 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
 <html>
-<head>
-  <title><?= $subject ?> </title>
-</head>
-<body>
-  <h2><?= sprintf(t("A new %s was added to %s"), $type, $parent_title); ?></h2>
-  <table>
-    <tr>
-      <td><?= t("Title:") ?></td>
-      <td><?= $item_title ?></td>
-    </tr>
-    <tr>
-      <td><?= t("Url:") ?></td>
-      <td><a href="<?= $url ?>"><?= $url ?></a></td>
-    </tr>
-    <? if (!empty($description)): ?>
-    <tr>
-      <td><?= t("Description:") ?></td>
-      <td><?= $description ?></td>
-    </tr>
-    <? endif ?>
-  </table>
-</body>
+  <head>
+    <title><?= $subject ?> </title>
+  </head>
+  <body>
+    <h2><?= $subject ?></h2>
+    <table>
+      <tr>
+        <td><?= t("Title:") ?></td>
+        <td><?= $item->title ?></td>
+      </tr>
+      <tr>
+        <td><?= t("Url:") ?></td>
+        <td><a href="<?= $item->url(array(), true) ?>"><?= $item->url(array(), true) ?></a></td>
+      </tr>
+      <? if ($item->description): ?>
+      <tr>
+        <td><?= t("Description:") ?></td>
+        <td><?= $item->description ?></td>
+      </tr>
+      <? endif ?>
+    </table>
+  </body>
 </html>
diff --git a/modules/notification/views/item_deleted.html.php b/modules/notification/views/item_deleted.html.php
index f3e098da..ac9ab594 100644
--- a/modules/notification/views/item_deleted.html.php
+++ b/modules/notification/views/item_deleted.html.php
@@ -1,18 +1,21 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
 <html>
-<head>
-  <title><?= $subject ?> </title>
-</head>
-<body>
-  <h2><?= sprintf(t("%s: %s was removed from %s"), $type, $item_title, $parent_title) ?></h2>
-  <table>
-    <tr>
-    <td colspan="2"><?= sprintf(t("To view the changed album %s use the link below."), $parent_title) ?></td>
-    </tr>
-    <tr>
-      <td><?= t("Url:") ?></td>
-      <td><a href="<?= $url ?>"><?= $url ?></a></td>
-    </tr>
-  </table>
-</body>
+  <head>
+    <title><?= $subject ?> </title>
+  </head>
+  <body>
+    <h2><?= $subject ?></h2>
+    <table>
+      <tr>
+        <td colspan="2">
+          <?= t("To view the changed album %title use the link below.",
+              array("title" => $item->parent()->title)) ?>
+        </td>
+      </tr>
+      <tr>
+        <td><?= t("Url:") ?></td>
+        <td><a href="<?= $item->parent()->url(array(), true) ?>"><?= $item->parent()->url(array(), true) ?></a></td>
+      </tr>
+    </table>
+  </body>
 </html>
diff --git a/modules/notification/views/item_updated.html.php b/modules/notification/views/item_updated.html.php
index 126bd5cc..cba522e8 100644
--- a/modules/notification/views/item_updated.html.php
+++ b/modules/notification/views/item_updated.html.php
@@ -1,36 +1,35 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
 <html>
-<head>
-  <title><?= $subject ?> </title>
-</head>
-<body>
-  <h2><?= sprintf(t("%s %s was updated"), ucfirst($type), $item_title); ?></h2>
-  <table>
-    <tr>
-      <? if (!empty($new_title)): ?>
-      <td><?= t("New Title:") ?></td>
-      <td><?= $new_title ?></td>
-      <? else: ?>
-      <td><?= t("Title:") ?></td>
-      <td><?= $item_title ?></td>
+  <head>
+    <title><?= $subject ?> </title>
+  </head>
+  <body>
+    <h2> <?= $subject ?> </h2>
+    <table>
+      <tr>
+        <? if ($old->title != $new->title): ?>
+        <td><?= t("New Title:") ?></td>
+        <td><?= $new->title ?></td>
+        <? else: ?>
+        <td><?= t("Title:") ?></td>
+        <td><?= $new->title ?></td>
+        <? endif ?>
+      </tr>
+      <tr>
+        <td><?= t("Url:") ?></td>
+        <td><a href="<?= $new->url(array(), true) ?>"><?= $new->url(array(), true) ?></a></td>
+      </tr>
+      <? if ($old->description != $new->description): ?>
+      <tr>
+        <td><?= t("New Description:") ?></td>
+        <td><?= $new->description ?></td>
+      </tr>
+      <? elseif (!empty($new->description)): ?>
+      <tr>
+        <td><?= t("Description:") ?></td>
+        <td><?= $new->description ?></td>
+      </tr>
       <? endif ?>
-    </tr>
-    <tr>
-      <td><?= t("Url:") ?></td>
-      <td><a href="<?= $url ?>"><?= $url ?></a></td>
-    </tr>
-    <? if (!empty($new_description)): ?>
-    <tr>
-      <td><?= t("New Description:") ?></td>
-      <td><?= $new_description ?></td>
-    </tr>
-       <? else: if (!empty($description)): ?>
-    <tr>
-      <td><?= t("Description:") ?></td>
-      <td><?= $description ?></td>
-    </tr>
-    <? endif ?>
-    <? endif ?>
-  </table>
-</body>
+    </table>
+  </body>
 </html>