From ffb9d7ad3fb9ad5d9ed0ada9fb9b88fa7ae8e146 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Fri, 4 Jul 2014 14:14:49 +0200 Subject: [PATCH] Rewriting code for notice representation Getting rid of NoticeListItemAdapter, putting more into ActivityHandlerPlugin and relying on plugins to handle rendering code of the content. This gives us a lot more structure and consistency in notice structure and allows activity plugins to stop rendering certain kinds of notices more easily. There should also be a property for an ActivityHandlerPlugin class to avoid rendering notices in the ordinary stream, so we don't have to overload stuff. --- classes/Notice.php | 5 + lib/activityhandlerplugin.php | 97 ++++++++++++++++++- lib/microappplugin.php | 59 ++--------- lib/noticelistitem.php | 17 ++-- plugins/Bookmark/BookmarkPlugin.php | 33 ------- plugins/Bookmark/lib/bookmarklistitem.php | 4 +- plugins/Event/EventPlugin.php | 2 +- plugins/Favorite/FavoritePlugin.php | 13 +++ .../GNUsocialPhoto/GNUsocialPhotoPlugin.php | 2 +- .../GNUsocialVideo/GNUsocialVideoPlugin.php | 2 +- plugins/Poll/PollPlugin.php | 7 +- plugins/QnA/QnAPlugin.php | 2 +- 12 files changed, 137 insertions(+), 106 deletions(-) diff --git a/classes/Notice.php b/classes/Notice.php index 4bd117b837..258652639a 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -268,6 +268,11 @@ class Notice extends Managed_DataObject } return $title; } + + public function getContent() + { + return $this->content; + } /* * Get the original representation URL of this notice. diff --git a/lib/activityhandlerplugin.php b/lib/activityhandlerplugin.php index b94385b447..a6c5117f4e 100644 --- a/lib/activityhandlerplugin.php +++ b/lib/activityhandlerplugin.php @@ -31,6 +31,15 @@ if (!defined('GNUSOCIAL')) { exit(1); } */ abstract class ActivityHandlerPlugin extends Plugin { + /** + * Returns a key string which represents this activity in HTML classes, + * ids etc, as when offering selection of what type of post to make. + * In MicroAppPlugin, this is paired with the user-visible localizable appTitle(). + * + * @return string (compatible with HTML classes) + */ + abstract function tag(); + /** * Return a list of ActivityStreams object type IRIs * which this micro-app handles. Default implementations @@ -40,8 +49,6 @@ abstract class ActivityHandlerPlugin extends Plugin * * An empty list means any type is ok. (Favorite verb etc.) * - * All micro-app classes must override this method. - * * @return array of strings */ abstract function types(); @@ -57,7 +64,7 @@ abstract class ActivityHandlerPlugin extends Plugin * * @return array of strings */ - function verbs() { + public function verbs() { return array(ActivityVerb::POST); } @@ -496,4 +503,88 @@ abstract class ActivityHandlerPlugin extends Plugin } return true; } + + public function onStartOpenNoticeListItemElement(NoticeListItem $nli) + { + if (!$this->isMyNotice($nli->notice)) { + return true; + } + + $this->openNoticeListItemElement($nli); + + Event::handle('EndOpenNoticeListItemElement', array($nli)); + return false; + } + + public function onStartCloseNoticeListItemElement(NoticeListItem $nli) + { + if (!$this->isMyNotice($nli->notice)) { + return true; + } + + $this->closeNoticeListItemElement($nli); + + Event::handle('EndCloseNoticeListItemElement', array($nli)); + return false; + } + + protected function openNoticeListItemElement(NoticeListItem $nli) + { + $id = (empty($nli->repeat)) ? $nli->notice->id : $nli->repeat->id; + $class = 'h-entry notice ' . $this->tag(); + if ($nli->notice->scope != 0 && $nli->notice->scope != 1) { + $class .= ' limited-scope'; + } + $nli->out->elementStart('li', array('class' => $class, + 'id' => 'notice-' . $id)); + } + + protected function closeNoticeListItemElement(NoticeListItem $nli) + { + $nli->out->elementEnd('li'); + } + + + // FIXME: This is overriden in MicroAppPlugin but shouldn't have to be + public function onStartShowNoticeItem(NoticeListItem $nli) + { + if (!$this->isMyNotice($nli->notice)) { + return true; + } + + try { + $this->showNoticeListItem($nli); + } catch (Exception $e) { + $nli->out->element('p', 'error', 'Error showing notice: '.htmlspecialchars($e->getMessage())); + } + + Event::handle('EndShowNoticeItem', array($nli)); + return false; + } + + protected function showNoticeListItem(NoticeListItem $nli) + { + $nli->showNotice(); + $nli->showNoticeAttachments(); + $nli->showNoticeInfo(); + $nli->showNoticeOptions(); + + $nli->showNoticeLink(); + $nli->showNoticeSource(); + $nli->showNoticeLocation(); + $nli->showContext(); + $nli->showRepeat(); + + $nli->showNoticeOptions(); + } + + public function onStartShowNoticeContent(Notice $stored, HTMLOutputter $out, Profile $scoped=null) + { + if (!$this->isMyNotice($stored)) { + return true; + } + + $out->text($stored->getContent()); + return false; + } } diff --git a/lib/microappplugin.php b/lib/microappplugin.php index c6cf4554dd..e034bb9b31 100644 --- a/lib/microappplugin.php +++ b/lib/microappplugin.php @@ -61,15 +61,6 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin */ abstract function appTitle(); - /** - * Returns a key string which represents this micro-app in HTML - * ids etc, as when offering selection of what type of post to make. - * This is paired with the user-visible localizable $this->appTitle(). - * - * All micro-app classes must override this method. - */ - abstract function tag(); - /** * When building the primary notice form, we'll fetch also some * alternate forms for specialized types -- that's you! @@ -99,10 +90,8 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin * @param NoticeListItem $nli The list item being shown. * * @return boolean hook value - * - * @fixme WARNING WARNING WARNING this closes a 'div' that is implicitly opened in BookmarkPlugin's showNotice implementation */ - function onStartShowNoticeItem($nli) + function onStartShowNoticeItem(NoticeListItem $nli) { if (!$this->isMyNotice($nli->notice)) { return true; @@ -110,15 +99,15 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin $adapter = $this->adaptNoticeListItem($nli); - if (!empty($adapter)) { - $adapter->showNotice(); - $adapter->showNoticeAttachments(); - $adapter->showNoticeInfo(); - $adapter->showNoticeOptions(); - } else { - $this->oldShowNotice($nli); + if (empty($adapter)) { + throw new ServerException('Could not adapt NoticeListItem'); } + $adapter->showNotice(); + $adapter->showNoticeAttachments(); + $adapter->showNoticeInfo(); + $adapter->showNoticeOptions(); + return false; } @@ -135,32 +124,6 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin return null; } - function oldShowNotice($nli) - { - $out = $nli->out; - $notice = $nli->notice; - - try { - $this->showNotice($notice, $out); - } catch (Exception $e) { - common_log(LOG_ERR, $e->getMessage()); - // try to fall back - $out->elementStart('div'); - $nli->showAuthor(); - $nli->showContent(); - } - - $nli->showNoticeLink(); - $nli->showNoticeSource(); - $nli->showNoticeLocation(); - $nli->showContext(); - $nli->showRepeat(); - - $out->elementEnd('div'); - - $nli->showNoticeOptions(); - } - function onStartShowEntryForms(&$tabs) { $tabs[$this->tag()] = array('title' => $this->appTitle(), @@ -178,10 +141,4 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin return true; } - - function showNotice($notice, $out) - { - // TRANS: Server exception thrown when a micro app plugin developer has not done his job too well. - throw new ServerException(_('You must implement either adaptNoticeListItem() or showNotice().')); - } } diff --git a/lib/noticelistitem.php b/lib/noticelistitem.php index 485dd14945..0527f0bf0c 100644 --- a/lib/noticelistitem.php +++ b/lib/noticelistitem.php @@ -273,13 +273,16 @@ class NoticeListItem extends Widget { // FIXME: URL, image, video, audio $this->out->elementStart('div', array('class' => 'e-content')); - if ($this->notice->rendered) { - $this->out->raw($this->notice->rendered); - } else { - // XXX: may be some uncooked notices in the DB, - // we cook them right now. This should probably disappear in future - // versions (>> 0.4.x) - $this->out->raw(common_render_content($this->notice->content, $this->notice)); + if (Event::handle('StartShowNoticeContent', array($this->notice, $this->out, $this->out->getScoped()))) { + if ($this->notice->rendered) { + $this->out->raw($this->notice->rendered); + } else { + // XXX: may be some uncooked notices in the DB, + // we cook them right now. This should probably disappear in future + // versions (>> 0.4.x) + $this->out->raw(common_render_content($this->notice->content, $this->notice)); + } + Event::handle('EndShowNoticeContent', array($this->notice, $this->out, $this->out->getScoped())); } $this->out->elementEnd('div'); } diff --git a/plugins/Bookmark/BookmarkPlugin.php b/plugins/Bookmark/BookmarkPlugin.php index 0e3db1ed82..fdd3c359b8 100644 --- a/plugins/Bookmark/BookmarkPlugin.php +++ b/plugins/Bookmark/BookmarkPlugin.php @@ -239,39 +239,6 @@ class BookmarkPlugin extends MicroAppPlugin return true; } - /** - * Output our CSS class for bookmark notice list elements - * - * @param NoticeListItem $nli The item being shown - * - * @return boolean hook value - */ - - function onStartOpenNoticeListItemElement($nli) - { - if (!$this->isMyNotice($nli->notice)) { - return true; - } - - $nb = Bookmark::getByNotice($nli->notice); - - if (empty($nb)) { - $this->log(LOG_INFO, "Notice {$nli->notice->id} has bookmark class but no matching Bookmark record."); - return true; - } - - $id = (empty($nli->repeat)) ? $nli->notice->id : $nli->repeat->id; - $class = 'h-entry notice bookmark'; - if ($nli->notice->scope != 0 && $nli->notice->scope != 1) { - $class .= ' limited-scope'; - } - $nli->out->elementStart('li', array('class' => $class, - 'id' => 'notice-' . $id)); - - Event::handle('EndOpenNoticeListItemElement', array($nli)); - return false; - } - /** * Modify the default menu to link to our custom action * diff --git a/plugins/Bookmark/lib/bookmarklistitem.php b/plugins/Bookmark/lib/bookmarklistitem.php index 3421829a42..118800ad99 100644 --- a/plugins/Bookmark/lib/bookmarklistitem.php +++ b/plugins/Bookmark/lib/bookmarklistitem.php @@ -74,7 +74,7 @@ class BookmarkListItem extends NoticeListItemAdapter $profile = $notice->getProfile(); - $out->elementStart('p', array('class' => 'e-content')); + $out->elementStart('div', array('class' => 'e-content')); // Whether to nofollow @@ -139,6 +139,6 @@ class BookmarkListItem extends NoticeListItemAdapter $nb->description); } - $out->elementEnd('p'); + $out->elementEnd('div'); } } diff --git a/plugins/Event/EventPlugin.php b/plugins/Event/EventPlugin.php index c427278555..9ec709af9a 100644 --- a/plugins/Event/EventPlugin.php +++ b/plugins/Event/EventPlugin.php @@ -44,7 +44,7 @@ if (!defined('STATUSNET')) { * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 * @link http://status.net/ */ -class EventPlugin extends MicroappPlugin +class EventPlugin extends MicroAppPlugin { /** * Set up our tables (event and rsvp) diff --git a/plugins/Favorite/FavoritePlugin.php b/plugins/Favorite/FavoritePlugin.php index 3d27d24a7f..4a0dd806ca 100644 --- a/plugins/Favorite/FavoritePlugin.php +++ b/plugins/Favorite/FavoritePlugin.php @@ -289,6 +289,19 @@ class FavoritePlugin extends ActivityHandlerPlugin } } + public function showNoticeListItem(NoticeListItem $nli) + { + // pass + } + public function openNoticeListItemElement(NoticeListItem $nli) + { + // pass + } + public function closeNoticeListItemElement(NoticeListItem $nli) + { + // pass + } + public function onAppendUserActivityStreamObjects(UserActivityStream $uas, array &$objs) { $faves = array(); diff --git a/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php b/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php index 4f4f4af827..44a6e3fe34 100644 --- a/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php +++ b/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php @@ -119,7 +119,7 @@ class GNUsocialPhotoPlugin extends MicroAppPlugin } - function showNotice($notice, $out) + function showNoticeContent(Notice $notice, HTMLOutputter $out) { $photo = Photo::getByNotice($notice); if ($photo) { diff --git a/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php b/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php index b78ed84c42..3310712ba7 100644 --- a/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php +++ b/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php @@ -112,7 +112,7 @@ class GNUsocialVideoPlugin extends MicroAppPlugin } - function showNotice($notice, $out) + function showNotice(Notice $notice, HTMLOutputter $out) { $vid = Video::getByNotice($notice); if ($vid) { diff --git a/plugins/Poll/PollPlugin.php b/plugins/Poll/PollPlugin.php index d52e69aabe..2a5805015c 100644 --- a/plugins/Poll/PollPlugin.php +++ b/plugins/Poll/PollPlugin.php @@ -376,12 +376,7 @@ class PollPlugin extends MicroAppPlugin } - /** - * @fixme WARNING WARNING WARNING parent class closes the final div that we - * open here, but we probably shouldn't open it here. Check parent class - * and Bookmark plugin for if that's right. - */ - function showNotice(Notice $notice, $out) + function showNoticeContent(Notice $notice, HTMLOutputter $out) { switch ($notice->object_type) { case self::POLL_OBJECT: diff --git a/plugins/QnA/QnAPlugin.php b/plugins/QnA/QnAPlugin.php index 6542e95f98..ec1b16b872 100644 --- a/plugins/QnA/QnAPlugin.php +++ b/plugins/QnA/QnAPlugin.php @@ -322,7 +322,7 @@ class QnAPlugin extends MicroAppPlugin * @param Notice $notice * @param HTMLOutputter $out */ - function showNotice(Notice $notice, $out) + function showNoticeContent(Notice $notice, $out) { switch ($notice->object_type) { case QnA_Question::OBJECT_TYPE: