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

@@ -269,6 +269,11 @@ class Notice extends Managed_DataObject
         return $title;
     }
     
+    public function getContent()
+    {
+        return $this->content;
+    }
+
     /*
      * Get the original representation URL of this notice.
      */

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;
+    }
 }

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,14 +99,14 @@ abstract class MicroAppPlugin extends ActivityHandlerPlugin
 
         $adapter = $this->adaptNoticeListItem($nli);
 
-        if (!empty($adapter)) {
+        if (empty($adapter)) {
+            throw new ServerException('Could not adapt NoticeListItem');
+        }
+
         $adapter->showNotice();
         $adapter->showNoticeAttachments();
         $adapter->showNoticeInfo();
         $adapter->showNoticeOptions();
-        } else {
-            $this->oldShowNotice($nli);
-        }
 
         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().'));
-    }
 }

lib/noticelistitem.php

@@ -273,6 +273,7 @@ class NoticeListItem extends Widget
     {
         // FIXME: URL, image, video, audio
         $this->out->elementStart('div', array('class' => 'e-content'));
+        if (Event::handle('StartShowNoticeContent', array($this->notice, $this->out, $this->out->getScoped()))) {
             if ($this->notice->rendered) {
                 $this->out->raw($this->notice->rendered);
             } else {
@@ -281,6 +282,8 @@ class NoticeListItem extends Widget
                 // 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');
     }
 

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
      *

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');
     }
 }

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)

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();

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) {

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) {

plugins/Poll/PollPlugin.php

@@ -376,12 +376,7 @@ class PollPlugin extends MicroAppPlugin
     }
 
 
-    /**
+    function showNoticeContent(Notice $notice, HTMLOutputter $out)
-     * @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)
     {
         switch ($notice->object_type) {
         case self::POLL_OBJECT:

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: