From d917430aec662272d70a9db73fbfda1bd2ec6bc9 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 3 Aug 2010 23:13:40 -0700 Subject: [PATCH] Did a pass over the ecard code. It still doesn't work, but it's a lot closer to our regular style now. And a fair bit simpler. Structural - Added an installer and moved all the default messages into it so that they're part of the install process. - Got rid of the model; you don't need to add an ecard entry in the database to send an ecard. You just need to send a mail. - Renamed the form and controller methods to reflect the fact that we're sending an ecard, not adding an ecard to the db. - Stopped inlining all of the code into the page like we do in comments, and instead put it into a dialog box with a button in the sidebar. This lets us ditch the extra .js and .css and greatly simplifies the code. General - Added rules to the send_ecard form so that the validation code has something to chew on. - Renamed ecard::can_ecard() to ecard::can_send_ecard() cause you should have a verb in there. - Moved the can_send_ecard() check into ecard_theme::sidebar_bottom and out of ecards.html.php Style cleanup - Changed "Title Casing" to "Sentence casing" everywhere for consistency with our style. - Cleaned up some indentation - Got rid of carriage returns LEFT TO DO: 1) Rename Ecards_Controller to Ecard_Controller for consistency (and put it in ecard.php) 2) Actually have it send the ecard :-) --- modules/ecard/controllers/admin_ecards.php | 40 ++++--- modules/ecard/controllers/ecards.php | 48 ++------ modules/ecard/css/ecard.css | 45 -------- modules/ecard/helpers/ecard.php | 25 +++-- modules/ecard/helpers/ecard_installer.php | 29 +++++ modules/ecard/helpers/ecard_theme.php | 29 ++--- modules/ecard/js/ecard.js | 45 -------- modules/ecard/models/ecard.php | 125 --------------------- modules/ecard/views/admin_ecards.html.php | 2 +- modules/ecard/views/ecards.html.php | 10 +- 10 files changed, 86 insertions(+), 312 deletions(-) delete mode 100644 modules/ecard/css/ecard.css create mode 100644 modules/ecard/helpers/ecard_installer.php delete mode 100644 modules/ecard/js/ecard.js delete mode 100644 modules/ecard/models/ecard.php diff --git a/modules/ecard/controllers/admin_ecards.php b/modules/ecard/controllers/admin_ecards.php index ac802b60..b2bde957 100644 --- a/modules/ecard/controllers/admin_ecards.php +++ b/modules/ecard/controllers/admin_ecards.php @@ -29,31 +29,29 @@ class Admin_ecards_Controller extends Admin_Controller { public function save() { access::verify_csrf(); $form = $this->_get_admin_form(); - $form->validate(); - module::set_var( - "ecard", "sender", $form->ecard->sender->value); - module::set_var( - "ecard", "subject", $form->ecard->subject->value); - module::set_var( - "ecard", "message", $form->ecard->message->value); - module::set_var( - "ecard", "access_permissions", - $form->ecard->access_permissions->value); - message::success(t("eCard settings updated")); - url::redirect("admin/ecards"); + if ($form->validate()) { + module::set_var("ecard", "sender", $form->ecard->sender->value); + module::set_var("ecard", "subject", $form->ecard->subject->value); + module::set_var("ecard", "message", $form->ecard->message->value); + module::set_var("ecard", "access_permissions", $form->ecard->access_permissions->value); + message::success(t("eCard settings updated")); + url::redirect("admin/ecards"); + } else { + print $form; + } } private function _get_admin_form() { - $form = new Forge("admin/ecards/save", "", "post", - array("id" => "g-ecards-admin-form")); + $form = new Forge("admin/ecards/save", "", "post", array("id" => "g-ecards-admin-form")); $ecard_settings = $form->group("ecard")->label(t("eCard settings")); - $ecard_settings->input("sender")->label(t('E-mail Sender (leave blank for a user-defined address)')) - ->value(module::get_var("ecard", "sender", "")); - $ecard_settings->input("subject")->label(t('E-mail Subject')) - ->value(module::get_var("ecard", "subject", "You have been sent an eCard")); - $ecard_settings->textarea("message")->label(t('E-mail Message')) - ->value(module::get_var("ecard", "message", "Hello %toname%, \r\n%fromname% has sent you an eCard. Click the image to be taken to the gallery.")); - $ecard_settings->dropdown("access_permissions") + $ecard_settings->input("sender") + ->label(t("E-mail sender (leave blank for a user-defined address)")) + ->value(module::get_var("ecard", "sender", "")); + $ecard_settings->input("subject")->label(t("E-mail subject")) + ->value(module::get_var("ecard", "subject")); + $ecard_settings->textarea("message")->label(t("E-mail message")) + ->value(module::get_var("ecard", "message")); + $ecard_settings->dropdown("access_permissions") ->label(t("Who can send eCards?")) ->options(array("everybody" => t("Everybody"), "registered_users" => t("Only registered users"))) diff --git a/modules/ecard/controllers/ecards.php b/modules/ecard/controllers/ecards.php index 546570a2..6ea77a00 100644 --- a/modules/ecard/controllers/ecards.php +++ b/modules/ecard/controllers/ecards.php @@ -19,59 +19,35 @@ */ class ecards_Controller extends Controller { /** - * Add a new ecard to the collection. + * Send the ecard. */ - public function create($id) { + public function send($id) { $item = ORM::factory("item", $id); access::required("view", $item); - if (!ecard::can_ecard()) { + if (!ecard::can_send_ecard()) { access::forbidden(); } - $form = ecard::get_add_form($item); - try { - $valid = $form->validate(); - $form->item_id = $id; - $form->author_id = identity::active_user()->id; - $form->text = $form->add_ecard->text->value; - $form->to_name = $form->add_ecard->inputs["to_name"]->value; - $form->to_email = $form->add_ecard->to_email->value; - $form->validate(); - } catch (ORM_Validation_Exception $e) { - // Translate ORM validation errors into form error messages - foreach ($e->validation->errors() as $key => $error) { - switch ($key) { - case "to_name": $key = "name"; break; - case "to_email": $key = "email"; break; - } - $form->add_ecard->inputs[$key]->add_error($error, 1); - } - $valid = false; - } - - if ($valid) { - ecard::save(); - - print json_encode( - array("result" => "success", - "view" => (string) $view, - "form" => (string) ecard::get_add_form($item))); + $form = ecard::get_send_form($item); + if ($form->validate()) { + Kohana_Log::add("error",print_r($form,1)); + // Send the ecard here, based on the form data + json::reply(array("result" => "success")); } else { - $form = ecard::prefill_add_form($form); - print json_encode(array("result" => "error", "form" => (string) $form)); + json::reply(array("result" => "error", "html" => (string)$form)); } } /** * Present a form for adding a new ecard to this item or editing an existing ecard. */ - public function form_add($item_id) { + public function form_send($item_id) { $item = ORM::factory("item", $item_id); access::required("view", $item); - if (!ecard::can_ecard()) { + if (!ecard::can_send_ecard()) { access::forbidden(); } - print ecard::prefill_add_form(ecard::get_add_form($item)); + print ecard::prefill_send_form(ecard::get_send_form($item)); } } diff --git a/modules/ecard/css/ecard.css b/modules/ecard/css/ecard.css deleted file mode 100644 index 992367ae..00000000 --- a/modules/ecard/css/ecard.css +++ /dev/null @@ -1,45 +0,0 @@ -#g-content #g-ecard-form { - margin-top: 2em; -} - -#g-content #g-ecards { - margin-top: 2em; - position: relative; -} - -#g-content #g-ecards ul li { - margin: 1em 0; -} - -#g-content #g-ecards .g-author { - border-bottom: 1px solid #ccc; - color: #999; - height: 32px; - line-height: 32px; -} - -#g-content #g-ecards ul li div { - padding: 0 8px 8px 43px; -} - -#g-content #g-ecards .g-avatar { - height: 32px; - margin-right: .4em; - width: 32px; -} - -#g-add-ecard { - position: absolute; - right: 0; - top: 2px; -} - -#g-admin-ecards-menu { - margin: 1em 0; -} - -#g-admin-ecards-menu a { - margin: 0; - padding: .2em .6em; -} - diff --git a/modules/ecard/helpers/ecard.php b/modules/ecard/helpers/ecard.php index 02671671..0b4d63db 100644 --- a/modules/ecard/helpers/ecard.php +++ b/modules/ecard/helpers/ecard.php @@ -24,49 +24,54 @@ * Note: by design, this class does not do any permission checking. */ class ecard_Core { - static function get_add_form($item) { - $form = new Forge("ecards/create/{$item->id}", "", "post", array("id" => "g-ecard-form")); - $group = $form->group("add_ecard")->label(t("Send eCard")); + static function get_send_form($item) { + $form = new Forge("ecards/send/{$item->id}", "", "post", array("id" => "g-ecard-form")); + $group = $form->group("send_ecard")->label(t("Send eCard")); $group->input("from_name") - ->label(t("Your Name")) + ->label(t("Your name")) ->id("g-author") + ->rules("required") ->error_messages("required", t("You must enter a name for yourself")); $group->input("from_email") ->label(t("Your e-mail")) ->id("g-email") + ->rules("required|valid_email") ->error_messages("required", t("You must enter a valid email address")) ->error_messages("invalid", t("You must enter a valid email address")); $group->input("to_name") ->label(t("Recipient's Name")) ->id("g-recipient") + ->rules("required") ->error_messages("required", t("You must enter a recipient's name")); $group->input("to_email") ->label(t("Recipient's e-mail")) ->id("g-recip-email") + ->rules("required|valid_email") ->error_messages("required", t("You must enter a valid email address")) ->error_messages("invalid", t("You must enter a valid email address")); $group->textarea("text") ->label(t("Message")) ->id("g-text") + ->rules("required") ->error_messages("required", t("You must enter a message")); $group->hidden("item_id")->value($item->id); - module::event("ecard_add_form", $form); + module::event("ecard_send_form", $form); $group->submit("")->value(t("Send"))->class("ui-state-default ui-corner-all"); return $form; } - static function prefill_add_form($form) { + static function prefill_send_form($form) { $active = identity::active_user(); if (!$active->guest) { - $group = $form->add_ecard; - $group->inputs["from_name"]->value($active->full_name)->disabled("disabled"); - $group->from_email->value($active->email)->disabled("disabled"); + $group = $form->send_ecard; + $group->inputs["from_name"]->value($active->full_name); + $group->from_email->value($active->email); } return $form; } - static function can_ecard() { + static function can_send_ecard() { return !identity::active_user()->guest || module::get_var("ecard", "access_permissions") == "everybody"; } diff --git a/modules/ecard/helpers/ecard_installer.php b/modules/ecard/helpers/ecard_installer.php new file mode 100644 index 00000000..2f514b09 --- /dev/null +++ b/modules/ecard/helpers/ecard_installer.php @@ -0,0 +1,29 @@ +css("ecard.css"); - $theme->script("ecard.js"); - return ""; - } - - static function admin_head($theme) { - $theme->css("ecard.css"); - return ""; - } - - static function photo_bottom($theme) { - $block = new Block; - $block->css_id = "g-ecards"; - $block->title = t("ecards"); - $block->anchor = "ecards"; - - $view = new View("ecards.html"); - - $block->content = $view; - return $block; + static function sidebar_bottom($theme) { + if (ecard::can_send_ecard()) { + $block = new Block; + $block->css_id = "g-ecards"; + $block->title = t("eCards"); + $block->content = new View("ecards.html"); + return $block; + } } } \ No newline at end of file diff --git a/modules/ecard/js/ecard.js b/modules/ecard/js/ecard.js deleted file mode 100644 index b0377168..00000000 --- a/modules/ecard/js/ecard.js +++ /dev/null @@ -1,45 +0,0 @@ -$("document").ready(function() { - $("#g-add-ecard").click(function(event) { - event.preventDefault(); - if (!$("#g-ecard-form").length) { - $.get($(this).attr("href"), - {}, - function(data) { - $("#g-ecard-detail").append(data); - ajaxify_ecard_form(); - $.scrollTo("#g-ecard-form-anchor", 800); - }); - } - }); - $(".g-no-ecards a").click(function(event) { - event.preventDefault(); - if (!$("#g-ecard-form").length) { - $.get($(this).attr("href"), - {}, - function(data) { - $("#g-ecard-detail").append(data); - ajaxify_ecard_form(); - }); - $(".g-no-ecards").remove(); - } - }); -}); - -function ajaxify_ecard_form() { - $("#g-ecards form").ajaxForm({ - dataType: "json", - success: function(data) { - if (data.result == "success") { - $("#g-ecards #g-ecard-detail ul").append(data.view); - $("#g-ecards #g-ecard-detail ul li:last").effect("highlight", {color: "#cfc"}, 8000); - $("#g-ecard-form").hide(2000).remove(); - $("#g-no-ecards").hide(2000); - } else { - if (data.form) { - $("#g-ecards form").replaceWith(data.form); - ajaxify_ecard_form(); - } - } - } - }); -} diff --git a/modules/ecard/models/ecard.php b/modules/ecard/models/ecard.php deleted file mode 100644 index eee343be..00000000 --- a/modules/ecard/models/ecard.php +++ /dev/null @@ -1,125 +0,0 @@ -item_id); - } - - function author() { - return identity::lookup_user($this->author_id); - } - - function author_name() { - $author = $this->author(); - if ($author->guest) { - return $this->from_name; - } else { - return $author->display_name(); - } - } - - function author_email() { - $author = $this->author(); - if ($author->guest) { - return $this->from_email; - } else { - return $author->email; - } - } - - /** - * Add some custom per-instance rules. - */ - public function validate(Validation $array=null) { - // validate() is recursive, only modify the rules on the outermost call. - if (!$array) { - $this->rules = array( - "from_name" => array("callbacks" => array(array($this, "valid_author"))), - "from_email" => array("callbacks" => array(array($this, "valid_email"))), - "to_name" => array("rules" => array("required")), - "item_id" => array("callbacks" => array(array($this, "valid_item"))), - "to_email" => array("rules" => array("required")), - "message" => array("rules" => array("required")), - ); - } - - parent::validate($array); - } - - /** - * @see ORM::save() - */ - public function save() { - return $this; - } - - /** - * Add a set of restrictions to any following queries to restrict access only to items - * viewable by the active user. - * @chainable - */ - public function viewable() { - $this->join("items", "items.id", "ecards.item_id"); - return item::viewable($this); - } - - /** - * Make sure we have an appropriate author id set, or a guest name. - */ - public function valid_author(Validation $v, $field) { - if (empty($this->author_id)) { - $v->add_error("author_id", "required"); - } else if ($this->author_id == identity::guest()->id && empty($this->from_name)) { - $v->add_error("from_name", "required"); - } - } - - /** - * Make sure that the email address is legal. - */ - public function valid_email(Validation $v, $field) { - if ($this->author_id == identity::guest()->id) { - if (empty($v->from_email)) { - $v->add_error("from_email", "required"); - } else if (!valid::email($v->from_email)) { - $v->add_error("from_email", "invalid"); - } - } - } - - /** - * Make sure we have a valid associated item id. - */ - public function valid_item(Validation $v, $field) { - if (db::build() - ->from("items") - ->where("id", "=", $this->item_id) - ->count_records() != 1) { - $v->add_error("item_id", "invalid"); - } - } - - /** - * Make sure that the state is legal. - */ - static function valid_state($value) { - return in_array($value, array("published", "unpublished", "spam", "deleted")); - } -} diff --git a/modules/ecard/views/admin_ecards.html.php b/modules/ecard/views/admin_ecards.html.php index 8609fe1d..0dd6b926 100644 --- a/modules/ecard/views/admin_ecards.html.php +++ b/modules/ecard/views/admin_ecards.html.php @@ -1,6 +1,6 @@
-

+

diff --git a/modules/ecard/views/ecards.html.php b/modules/ecard/views/ecards.html.php index d8227e2d..e655debb 100644 --- a/modules/ecard/views/ecards.html.php +++ b/modules/ecard/views/ecards.html.php @@ -1,12 +1,6 @@ - -id}") ?>#ecard-form" id="g-add-ecard" - class="g-button ui-corner-all ui-icon-left ui-state-default"> +id}") ?>" id="g-add-ecard" + class="g-button ui-corner-all ui-icon-left ui-state-default g-dialog-link"> - - -
- -