Merge pull request #8186 from nupplaphil/bug/8182_another_notification_bug

Fix Notification issues

include/api.php

@@ -5892,10 +5892,11 @@ api_register_func('api/friendica/activity/unattendmaybe', 'api_friendica_activit
  * Returns notifications
  *
  * @param string $type Known types are 'atom', 'rss', 'xml' and 'json'
+ *
  * @return string|array
- * @throws BadRequestException
  * @throws ForbiddenException
- * @throws InternalServerErrorException
+ * @throws BadRequestException
+ * @throws Exception
  */
 function api_friendica_notification($type)
 {
@@ -5908,7 +5909,7 @@ function api_friendica_notification($type)
 		throw new BadRequestException("Invalid argument count");
 	}
 
-	$notifications = DI::notify()->select(['uid' => api_user()], ['order' => ['seen' => 'ASC', 'date' => 'DESC'], 'limit' => 50]);
+	$notifications = DI::notification()->getApiList(local_user());
 
 	if ($type == "xml") {
 		$xmlnotes = false;

include/enotify.php

@@ -483,23 +483,24 @@ function notification($params)
 
 	if ($show_in_notification_page) {
 		$notification = DI::notify()->insert([
-			'name'   => $params['source_name'],
+			'name'       => $params['source_name'] ?? '',
-			'url'    => $params['source_link'],
+			'name_cache' => strip_tags(BBCode::convert($params['source_name'] ?? '')),
-			'photo'  => $params['source_photo'],
+			'url'        => $params['source_link'] ?? '',
-			'uid'    => $params['uid'],
+			'photo'      => $params['source_photo'] ?? '',
-			'iid'    => $item_id,
+			'link'       => $itemlink ?? '',
-			'parent' => $parent_id,
+			'uid'        => $params['uid'] ?? 0,
-			'type'   => $params['type'],
+			'iid'        => $item_id ?? 0,
-			'verb'   => $params['verb'],
+			'parent'     => $parent_id ?? 0,
-			'otype'  => $params['otype'],
+			'type'       => $params['type'] ?? '',
+			'verb'       => $params['verb'] ?? '',
+			'otype'      => $params['otype'] ?? '',
 		]);
 
-		$notification->link = DI::baseUrl() . '/notification/view/' . $notification->id;
+		$notification->msg = Renderer::replaceMacros($epreamble, ['$itemlink' => $notification->link]);
-		$notification->msg  = Renderer::replaceMacros($epreamble, ['$itemlink' => $notification->link]);
 
 		DI::notify()->update($notification);
 
-		$itemlink  = $notification->link;
+		$itemlink  = DI::baseUrl() . '/notification/view/' . $notification->id;
 		$notify_id = $notification->id;
 	}
 

src/BaseCollection.php

@@ -17,14 +17,14 @@ abstract class BaseCollection extends \ArrayIterator
 	protected $totalCount = 0;
 
 	/**
-	 * @param BaseModel[] $models
+	 * @param BaseEntity[] $entities
-	 * @param int|null    $totalCount
+	 * @param int|null     $totalCount
 	 */
-	public function __construct(array $models = [], int $totalCount = null)
+	public function __construct(array $entities = [], int $totalCount = null)
 	{
-		parent::__construct($models);
+		parent::__construct($entities);
 
-		$this->totalCount = $totalCount ?? count($models);
+		$this->totalCount = $totalCount ?? count($entities);
 	}
 
 	/**

src/BaseEntity.php

@@ -11,7 +11,22 @@ namespace Friendica;
  */
 abstract class BaseEntity implements \JsonSerializable
 {
+	/**
+	 * Returns the current entity as an json array
+	 *
+	 * @return array
+	 */
 	public function jsonSerialize()
+	{
+		return $this->toArray();
+	}
+
+	/**
+	 * Returns the current entity as an array
+	 *
+	 * @return array
+	 */
+	public function toArray()
 	{
 		return get_object_vars($this);
 	}

src/BaseModel.php

@@ -12,7 +12,7 @@ use Psr\Log\LoggerInterface;
  *
  * @property int id
  */
-abstract class BaseModel
+abstract class BaseModel extends BaseEntity
 {
 	/** @var Database */
 	protected $dba;

src/Collection/Api/Notifications.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Friendica\Collection\Api;
+
+use Friendica\BaseCollection;
+use Friendica\Object\Api\Friendica\Notification;
+
+class Notifications extends BaseCollection
+{
+	/**
+	 * @return Notification
+	 */
+	public function current()
+	{
+		return parent::current();
+	}
+}

src/DI.php

@@ -280,14 +280,6 @@ abstract class DI
 		return self::$dice->create(Model\User\Cookie::class);
 	}
 
-	/**
-	 * @return Repository\Notify
-	 */
-	public static function notify()
-	{
-		return self::$dice->create(Repository\Notify::class);
-	}
-
 	/**
 	 * @return Model\Storage\IStorage
 	 */
@@ -324,6 +316,14 @@ abstract class DI
 		return self::$dice->create(Repository\ProfileField::class);
 	}
 
+	/**
+	 * @return Repository\Notify
+	 */
+	public static function notify()
+	{
+		return self::$dice->create(Repository\Notify::class);
+	}
+
 	//
 	// "Protocol" namespace instances
 	//

src/Factory/Notification/Notification.php

@@ -6,6 +6,7 @@ use Exception;
 use Friendica\App;
 use Friendica\App\BaseURL;
 use Friendica\BaseFactory;
+use Friendica\Collection\Api\Notifications as ApiNotifications;
 use Friendica\Content\Text\BBCode;
 use Friendica\Core\L10n;
 use Friendica\Core\PConfig\IPConfig;
@@ -15,6 +16,7 @@ use Friendica\Database\Database;
 use Friendica\Model\Item;
 use Friendica\Module\BaseNotifications;
 use Friendica\Network\HTTPException\InternalServerErrorException;
+use Friendica\Object\Api\Friendica\Notification as ApiNotification;
 use Friendica\Protocol\Activity;
 use Friendica\Repository;
 use Friendica\Util\DateTimeFormat;
@@ -352,4 +354,26 @@ class Notification extends BaseFactory
 
 		return $formattedNotifications;
 	}
+
+	/**
+	 * @param int   $uid    The user id of the API call
+	 * @param array $params Additional parameters
+	 *
+	 * @return ApiNotifications
+	 *
+	 * @throws Exception
+	 */
+	public function getApiList(int $uid, array $params = ['order' => ['seen' => 'ASC', 'date' => 'DESC'], 'limit' => 50])
+	{
+		$notifies = $this->notification->select(['uid' => $uid], $params);
+
+		/** @var ApiNotification[] $notifications */
+		$notifications = [];
+
+		foreach ($notifies as $notify) {
+			$notifications[] = new ApiNotification($notify);
+		}
+
+		return new ApiNotifications($notifications);
+	}
 }

src/Model/Notify.php

@@ -5,19 +5,12 @@ namespace Friendica\Model;
 use Exception;
 use Friendica\BaseModel;
 use Friendica\Content\Text\BBCode;
-use Friendica\Content\Text\HTML;
 use Friendica\Database\Database;
 use Friendica\Network\HTTPException\InternalServerErrorException;
-use Friendica\Util\DateTimeFormat;
-use Friendica\Util\Temporal;
 use Psr\Log\LoggerInterface;
 
 /**
  * Model for an entry in the notify table
- * - Including additional, calculated properties
- *
- * Is used either for frontend interactions or for API-based interaction
- * @see https://github.com/friendica/friendica/blob/develop/doc/API-Entities.md#notification
  *
  * @property string  hash
  * @property integer type
@@ -36,11 +29,6 @@ use Psr\Log\LoggerInterface;
  *
  * @property-read string name_cache Full name of the contact subject
  * @property-read string msg_cache  Plaintext version of the notification text with a placeholder (`{0}`) for the subject contact's name.
- *
- * @property-read integer timestamp  Unix timestamp
- * @property-read string  dateRel  	 Time since the note was posted, eg "1 hour ago"
- * @property-read string  $msg_html
- * @property-read string  $msg_plain
  */
 class Notify extends BaseModel
 {
@@ -59,8 +47,7 @@ class Notify extends BaseModel
 		$this->repo = $repo;
 
 		$this->setNameCache();
-		$this->setTimestamp();
+		$this->setMsgCache();
-		$this->setMsg();
 	}
 
 	/**
@@ -81,41 +68,23 @@ class Notify extends BaseModel
 		}
 	}
 
-	/**
-	 * Set some extra properties to the notification from db:
-	 *  - timestamp as int in default TZ
-	 *  - date_rel : relative date string
-	 */
-	private function setTimestamp()
-	{
-		try {
-			$this->timestamp = strtotime(DateTimeFormat::local($this->date));
-		} catch (Exception $e) {
-		}
-		$this->dateRel = Temporal::getRelativeDate($this->date);
-	}
-
 	/**
 	 * Sets the pre-formatted name (caching)
-	 *
-	 * @throws InternalServerErrorException
 	 */
 	private function setNameCache()
 	{
-		$this->name_cache = strip_tags(BBCode::convert($this->source_name ?? ''));
+		try {
+			$this->name_cache = strip_tags(BBCode::convert($this->source_name ?? ''));
+		} catch (InternalServerErrorException $e) {
+		}
 	}
 
 	/**
-	 * Set some extra properties to the notification from db:
+	 * Sets the pre-formatted msg (caching)
-	 *  - msg_html: message as html string
-	 *  - msg_plain: message as plain text string
-	 *  - msg_cache: The pre-formatted message (caching)
 	 */
-	private function setMsg()
+	private function setMsgCache()
 	{
 		try {
-			$this->msg_html  = BBCode::convert($this->msg, false);
-			$this->msg_plain = explode("\n", trim(HTML::toPlaintext($this->msg_html, 0)))[0];
 			$this->msg_cache = self::formatMessage($this->name_cache, strip_tags(BBCode::convert($this->msg)));
 		} catch (InternalServerErrorException $e) {
 		}
@@ -125,12 +94,8 @@ class Notify extends BaseModel
 	{
 		parent::__set($name, $value);
 
-		if ($name == 'date') {
-			$this->setTimestamp();
-		}
-
 		if ($name == 'msg') {
-			$this->setMsg();
+			$this->setMsgCache();
 		}
 
 		if ($name == 'source_name') {

src/Object/Api/Friendica/Notification.php

@@ -0,0 +1,79 @@
+<?php
+
+namespace Friendica\Object\Api\Friendica;
+
+use Friendica\BaseEntity;
+use Friendica\Content\Text\BBCode;
+use Friendica\Content\Text\HTML;
+use Friendica\Model\Notify;
+use Friendica\Util\DateTimeFormat;
+use Friendica\Util\Temporal;
+
+/**
+ * Friendica Notification
+ *
+ * @see https://github.com/friendica/friendica/blob/develop/doc/API-Entities.md#notification
+ */
+class Notification extends BaseEntity
+{
+	/** @var integer */
+	protected $id;
+	/** @var string */
+	protected $hash;
+	/** @var integer */
+	protected $type;
+	/** @var string Full name of the contact subject */
+	protected $name;
+	/** @var string Profile page URL of the contact subject */
+	protected $url;
+	/** @var string Profile photo URL of the contact subject */
+	protected $photo;
+	/** @var string YYYY-MM-DD hh:mm:ss local server time */
+	protected $date;
+	/** @var string The message (BBCode) */
+	protected $msg;
+	/** @var integer Owner User Id */
+	protected $uid;
+	/** @var string Notification URL */
+	protected $link;
+	/** @var integer Item Id */
+	protected $iid;
+	/** @var integer Parent Item Id */
+	protected $parent;
+	/** @var boolean  Whether the notification was read or not. */
+	protected $seen;
+	/** @var string Verb URL @see http://activitystrea.ms */
+	protected $verb;
+	/** @var string Subject type (`item`, `intro` or `mail`) */
+	protected $otype;
+	/** @var string Full name of the contact subject (HTML) */
+	protected $name_cache;
+	/** @var string Plaintext version of the notification text with a placeholder (`{0}`) for the subject contact's name. (Plaintext) */
+	protected $msg_cache;
+	/** @var integer  Unix timestamp */
+	protected $timestamp;
+	/** @var string Time since the note was posted, eg "1 hour ago" */
+	protected $date_rel;
+	/** @var string Message (HTML) */
+	protected $msg_html;
+	/** @var string Message (Plaintext) */
+	protected $msg_plain;
+
+	public function __construct(Notify $notify)
+	{
+		// map each notify attribute to the entity
+		foreach ($notify->toArray() as $key => $value) {
+			$this->{$key} = $value;
+		}
+
+		// add additional attributes for the API
+		try {
+			$this->timestamp = strtotime(DateTimeFormat::local($this->date));
+			$this->msg_html  = BBCode::convert($this->msg, false);
+			$this->msg_plain = explode("\n", trim(HTML::toPlaintext($this->msg_html, 0)))[0];
+		} catch (\Exception $e) {
+		}
+
+		$this->date_rel = Temporal::getRelativeDate($this->date);
+	}
+}

src/Object/Notification/Introduction.php

@@ -262,7 +262,7 @@ class Introduction implements \JsonSerializable
 	{
 		$this->label         = $data['label'] ?? '';
 		$this->type          = $data['str_type'] ?? '';
-		$this->intro_id      = $data['$intro_id'] ?? -1;
+		$this->intro_id      = $data['intro_id'] ?? -1;
 		$this->madeBy        = $data['madeBy'] ?? '';
 		$this->madeByUrl     = $data['madeByUrl'] ?? '';
 		$this->madeByZrl     = $data['madeByZrl'] ?? '';

src/Repository/Notify.php

@@ -37,8 +37,6 @@ class Notify extends BaseRepository
 	{
 		$params['order'] = $params['order'] ?? ['date' => 'DESC'];
 
-		$condition = array_merge($condition, ['uid' => local_user()]);
-
 		return parent::select($condition, $params);
 	}
 
@@ -67,7 +65,7 @@ class Notify extends BaseRepository
 	/**
 	 * @param array $fields
 	 *
-	 * @return Model\Notify
+	 * @return Model\Notify|false
 	 *
 	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
 	 * @throws Exception
@@ -75,13 +73,12 @@ class Notify extends BaseRepository
 	public function insert(array $fields)
 	{
 		$fields['date']  = DateTimeFormat::utcNow();
-		$fields['abort'] = false;
 
 		Hook::callAll('enotify_store', $fields);
 
-		if ($fields['abort']) {
+		if (empty($fields)) {
-			$this->logger->debug('Abort adding notification entry', ['fields' => $fields]);
+			$this->logger->debug('Abort adding notification entry');
-			return null;
+			return false;
 		}
 
 		$this->logger->debug('adding notification entry', ['fields' => $fields]);

tests/datasets/api.fixture.php

@@ -189,6 +189,25 @@ return [
 			'origin'      => 1,
 		],
 	],
+	'notify' => [
+		[
+			'id' => 1,
+			'type' => 8,
+			'name' => 'Reply to',
+			'url' => 'http://localhost/display/1',
+			'photo' => 'http://localhost/',
+			'date' => '2020-01-01 12:12:02',
+			'msg' => 'A test reply from an item',
+			'uid' => 42,
+			'link' => 'http://localhost/notification/1',
+			'iid' => 4,
+			'seen' => 0,
+			'verb' => '',
+			'otype' => 'item',
+			'name_cache' => 'Reply to',
+			'msg_cache' => 'A test reply from an item',
+		],
+	],
 	'thread'  => [
 		[
 			'iid'        => 1,

tests/include/ApiTest.php

@@ -14,6 +14,7 @@ use Friendica\Core\Session;
 use Friendica\Core\Session\ISession;
 use Friendica\Core\System;
 use Friendica\Database\Database;
+use Friendica\Database\DBA;
 use Friendica\DI;
 use Friendica\Model\Contact;
 use Friendica\Network\HTTPException;
@@ -3916,14 +3917,15 @@ class ApiTest extends DatabaseTest
 	}
 
 	/**
-	 * Test the api_friendica_notification() function with an argument count.
+	 * Test the api_friendica_notification() function with empty result
 	 *
 	 * @return void
 	 */
-	public function testApiFriendicaNotificationWithArgumentCount()
+	public function testApiFriendicaNotificationWithEmptyResult()
 	{
 		$this->app->argv = ['api', 'friendica', 'notification'];
 		$this->app->argc = count($this->app->argv);
+		$_SESSION['uid'] = 41;
 		$result          = api_friendica_notification('json');
 		$this->assertEquals(['note' => false], $result);
 	}
@@ -3938,7 +3940,26 @@ class ApiTest extends DatabaseTest
 		$this->app->argv = ['api', 'friendica', 'notification'];
 		$this->app->argc = count($this->app->argv);
 		$result          = api_friendica_notification('xml');
-		$this->assertXml($result, 'notes');
+		$assertXml=<<<XML
+<?xml version="1.0"?>
+<notes>
+  <note id="1" hash="" type="8" name="Reply to" url="http://localhost/display/1" photo="http://localhost/" date="2020-01-01 12:12:02" msg="A test reply from an item" uid="42" link="http://localhost/notification/1" iid="4" parent="0" seen="0" verb="" otype="item" name_cache="" msg_cache="A test reply from an item" timestamp="1577880722" date_rel="4 weeks ago" msg_html="A test reply from an item" msg_plain="A test reply from an item"/>
+</notes>
+XML;
+		$this->assertXmlStringEqualsXmlString($assertXml, $result);
+	}
+
+	/**
+	 * Test the api_friendica_notification() function with an JSON result.
+	 *
+	 * @return void
+	 */
+	public function testApiFriendicaNotificationWithJsonResult()
+	{
+		$this->app->argv = ['api', 'friendica', 'notification'];
+		$this->app->argc = count($this->app->argv);
+		$result          = json_encode(api_friendica_notification('json'));
+		$this->assertJson($result);
 	}
 
 	/**