From fa123bd765ee3f7731ce041da7d4ede7c673b9c0 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 2 Nov 2022 20:28:53 -0400 Subject: [PATCH 1/5] Rename Repository\Notify->NotifyOnDesktop to shouldShowOnDesktop - New name better reflect the behavior --- src/Model/Subscription.php | 2 +- src/Module/Notifications/Ping.php | 2 +- src/Navigation/Notifications/Repository/Notify.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Model/Subscription.php b/src/Model/Subscription.php index 2796b37a0f..88aa820674 100644 --- a/src/Model/Subscription.php +++ b/src/Model/Subscription.php @@ -141,7 +141,7 @@ class Subscription { $type = NotificationFactory::getType($notification); - if (DI::notify()->NotifyOnDesktop($notification, $type)) { + if (DI::notify()->shouldShowOnDesktop($notification, $type)) { DI::notify()->createFromNotification($notification); } diff --git a/src/Module/Notifications/Ping.php b/src/Module/Notifications/Ping.php index a0fe8e9aee..3838c0e15e 100644 --- a/src/Module/Notifications/Ping.php +++ b/src/Module/Notifications/Ping.php @@ -190,7 +190,7 @@ class Ping extends BaseModule $owner = User::getOwnerDataById(DI::userSession()->getLocalUserId()); $navNotifications = array_map(function (Entity\Notification $notification) use ($owner) { - if (!DI::notify()->NotifyOnDesktop($notification)) { + if (!DI::notify()->shouldShowOnDesktop($notification)) { return null; } if (($notification->type == Post\UserNotification::TYPE_NONE) && in_array($owner['page-flags'], [User::PAGE_FLAGS_NORMAL, User::PAGE_FLAGS_PRVGROUP])) { diff --git a/src/Navigation/Notifications/Repository/Notify.php b/src/Navigation/Notifications/Repository/Notify.php index 5ed62c8c3b..52c51fcb8e 100644 --- a/src/Navigation/Notifications/Repository/Notify.php +++ b/src/Navigation/Notifications/Repository/Notify.php @@ -664,7 +664,7 @@ class Notify extends BaseRepository return false; } - public function NotifyOnDesktop(Entity\Notification $Notification, string $type = null): bool + public function shouldShowOnDesktop(Entity\Notification $Notification, string $type = null): bool { if (is_null($type)) { $type = NotificationFactory::getType($Notification); From da3041a4d43de46d70dd407caf1e9ff531fc55cf Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 2 Nov 2022 20:37:14 -0400 Subject: [PATCH 2/5] Add new Conversation\Network::getTimelineOrderBySession method --- src/Module/Conversation/Network.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Module/Conversation/Network.php b/src/Module/Conversation/Network.php index ce2afbfc75..928f2e2838 100644 --- a/src/Module/Conversation/Network.php +++ b/src/Module/Conversation/Network.php @@ -29,7 +29,9 @@ use Friendica\Content\Widget; use Friendica\Content\Text\HTML; use Friendica\Core\ACL; use Friendica\Core\Hook; +use Friendica\Core\PConfig\Capability\IManagePersonalConfigValues; use Friendica\Core\Renderer; +use Friendica\Core\Session\Capability\IHandleUserSessions; use Friendica\Database\DBA; use Friendica\DI; use Friendica\Model\Contact; @@ -306,7 +308,7 @@ class Network extends BaseModule self::$forumContactId = $this->parameters['contact_id'] ?? 0; - self::$selectedTab = DI::session()->get('network-tab', DI::pConfig()->get(DI::userSession()->getLocalUserId(), 'network.view', 'selected_tab', '')); + self::$selectedTab = self::getTimelineOrderBySession(DI::userSession(), DI::pConfig()); if (!empty($get['star'])) { self::$selectedTab = 'star'; @@ -486,4 +488,18 @@ class Network extends BaseModule return $items; } + + /** + * Returns the selected network tab of the currently logged-in user + * + * @param IHandleUserSessions $session + * @param IManagePersonalConfigValues $pconfig + * @return string + */ + public static function getTimelineOrderBySession(IHandleUserSessions $session, IManagePersonalConfigValues $pconfig): string + { + return $session->get('network-tab') + ?? $pconfig->get($session->getLocalUserId(), 'network.view', 'selected_tab') + ?? ''; + } } From 23dda5d5105830a643271b2e9ff4f13c0c83abbe Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 2 Nov 2022 20:38:23 -0400 Subject: [PATCH 3/5] Deprecate default value parameters in Session and Config interfaces - It is more efficient to use the null coalescing operator ?? that prevents the computation of the default value if the key has been found --- src/Core/Config/Capability/IManageConfigValues.php | 2 +- src/Core/PConfig/Capability/IManagePersonalConfigValues.php | 2 +- src/Core/Session/Capability/IHandleSessions.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Config/Capability/IManageConfigValues.php b/src/Core/Config/Capability/IManageConfigValues.php index f18e8e1e3d..bf9144cf50 100644 --- a/src/Core/Config/Capability/IManageConfigValues.php +++ b/src/Core/Config/Capability/IManageConfigValues.php @@ -51,7 +51,7 @@ interface IManageConfigValues * * @param string $cat The category of the configuration value * @param string $key The configuration key to query - * @param mixed $default_value optional, The value to return if key is not set (default: null) + * @param mixed $default_value Deprecated, use `Config->get($cat, $key, null, $refresh) ?? $default_value` instead * @param boolean $refresh optional, If true the config is loaded from the db and not from the cache (default: false) * * @return mixed Stored value or null if it does not exist diff --git a/src/Core/PConfig/Capability/IManagePersonalConfigValues.php b/src/Core/PConfig/Capability/IManagePersonalConfigValues.php index ead3c26147..191869daa8 100644 --- a/src/Core/PConfig/Capability/IManagePersonalConfigValues.php +++ b/src/Core/PConfig/Capability/IManagePersonalConfigValues.php @@ -51,7 +51,7 @@ interface IManagePersonalConfigValues * @param int $uid The user_id * @param string $cat The category of the configuration value * @param string $key The configuration key to query - * @param mixed $default_value optional, The value to return if key is not set (default: null) + * @param mixed $default_value Deprecated, use `PConfig->get($uid, $cat, $key, null, $refresh) ?? $default_value` instead * @param boolean $refresh optional, If true the config is loaded from the db and not from the cache (default: false) * * @return mixed Stored value or null if it does not exist diff --git a/src/Core/Session/Capability/IHandleSessions.php b/src/Core/Session/Capability/IHandleSessions.php index d0b649845b..72905fc576 100644 --- a/src/Core/Session/Capability/IHandleSessions.php +++ b/src/Core/Session/Capability/IHandleSessions.php @@ -48,7 +48,7 @@ interface IHandleSessions * Handle the case where session_start() hasn't been called and the super global isn't available. * * @param string $name - * @param mixed $defaults + * @param mixed $defaults Deprecated, use `Session->get($name) ?? $defaults` instead * * @return mixed */ @@ -58,7 +58,7 @@ interface IHandleSessions * Retrieves a value from the provided key if it exists and removes it from session * * @param string $name - * @param mixed $defaults + * @param mixed $defaults Deprecated, use `Session->pop($name) ?? $defaults` instead * * @return mixed */ From 8092bfe27722ca310803a671af606a4b24bb5aad Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 2 Nov 2022 20:44:35 -0400 Subject: [PATCH 4/5] Remove dependency on DI in Notifications\Ping module --- src/Module/Notifications/Ping.php | 188 ++++++++++++++++-------------- 1 file changed, 100 insertions(+), 88 deletions(-) diff --git a/src/Module/Notifications/Ping.php b/src/Module/Notifications/Ping.php index 3838c0e15e..2d91105700 100644 --- a/src/Module/Notifications/Ping.php +++ b/src/Module/Notifications/Ping.php @@ -25,16 +25,21 @@ use Friendica\App; use Friendica\BaseModule; use Friendica\Contact\Introduction\Repository\Introduction; use Friendica\Content\ForumManager; +use Friendica\Core\Cache\Capability\ICanCache; use Friendica\Core\Cache\Enum\Duration; +use Friendica\Core\Config\Capability\IManageConfigValues; use Friendica\Core\Hook; use Friendica\Core\L10n; +use Friendica\Core\PConfig\Capability\IManagePersonalConfigValues; +use Friendica\Core\Session\Capability\IHandleUserSessions; use Friendica\Core\System; +use Friendica\Database\Database; use Friendica\Database\DBA; -use Friendica\DI; use Friendica\Model\Group; use Friendica\Model\Post; use Friendica\Model\User; use Friendica\Model\Verb; +use Friendica\Module\Conversation\Network; use Friendica\Module\Register; use Friendica\Module\Response; use Friendica\Navigation\Notifications\Entity; @@ -59,8 +64,22 @@ class Ping extends BaseModule private $introductionRepo; /** @var Factory\FormattedNavNotification */ private $formattedNavNotification; + /** @var IHandleUserSessions */ + private $session; + /** @var IManageConfigValues */ + private $config; + /** @var IManagePersonalConfigValues */ + private $pconfig; + /** @var Database */ + private $database; + /** @var ICanCache */ + private $cache; + /** @var Repository\Notify */ + private $notify; + /** @var App */ + private $app; - public function __construct(SystemMessages $systemMessages, Repository\Notification $notificationRepo, Introduction $introductionRepo, Factory\FormattedNavNotification $formattedNavNotification, L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, array $server, array $parameters = []) + public function __construct(App $app, Repository\Notify $notify, ICanCache $cache, Database $database, IManagePersonalConfigValues $pconfig, IManageConfigValues $config, IHandleUserSessions $session, SystemMessages $systemMessages, Repository\Notification $notificationRepo, Introduction $introductionRepo, Factory\FormattedNavNotification $formattedNavNotification, L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, array $server, array $parameters = []) { parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters); @@ -68,11 +87,18 @@ class Ping extends BaseModule $this->notificationRepo = $notificationRepo; $this->introductionRepo = $introductionRepo; $this->formattedNavNotification = $formattedNavNotification; + $this->session = $session; + $this->config = $config; + $this->pconfig = $pconfig; + $this->database = $database; + $this->cache = $cache; + $this->notify = $notify; + $this->app = $app; } protected function rawContent(array $request = []) { - $regs = []; + $registrations = []; $navNotifications = []; $intro_count = 0; @@ -90,107 +116,95 @@ class Ping extends BaseModule $today_birthday_count = 0; - if (DI::userSession()->getLocalUserId()) { - if (DI::pConfig()->get(DI::userSession()->getLocalUserId(), 'system', 'detailed_notif')) { - $notifications = $this->notificationRepo->selectDetailedForUser(DI::userSession()->getLocalUserId()); + if ($this->session->getLocalUserId()) { + if ($this->pconfig->get($this->session->getLocalUserId(), 'system', 'detailed_notif')) { + $notifications = $this->notificationRepo->selectDetailedForUser($this->session->getLocalUserId()); } else { - $notifications = $this->notificationRepo->selectDigestForUser(DI::userSession()->getLocalUserId()); + $notifications = $this->notificationRepo->selectDigestForUser($this->session->getLocalUserId()); } $condition = [ "`unseen` AND `uid` = ? AND NOT `origin` AND (`vid` != ? OR `vid` IS NULL)", - DI::userSession()->getLocalUserId(), Verb::getID(Activity::FOLLOW) + $this->session->getLocalUserId(), Verb::getID(Activity::FOLLOW) ]; - $items = Post::selectForUser(DI::userSession()->getLocalUserId(), ['wall', 'uid', 'uri-id'], $condition, ['limit' => 1000]); - if (DBA::isResult($items)) { - $items_unseen = Post::toArray($items, false); - $arr = ['items' => $items_unseen]; - Hook::callAll('network_ping', $arr); - foreach ($items_unseen as $item) { - if ($item['wall']) { - $home_count++; - } else { - $network_count++; - } + $items_unseen = $this->database->toArray(Post::selectForUser( + $this->session->getLocalUserId(), + ['wall', 'uid', 'uri-id'], + $condition, + ['limit' => 1000], + )); + $arr = ['items' => $items_unseen]; + Hook::callAll('network_ping', $arr); + + foreach ($items_unseen as $item) { + if ($item['wall']) { + $home_count++; + } else { + $network_count++; } } - DBA::close($items); - $compute_group_counts = DI::config()->get('system','compute_group_counts'); + $compute_group_counts = $this->config->get('system','compute_group_counts'); if ($network_count && $compute_group_counts) { // Find out how unseen network posts are spread across groups - $group_counts = Group::countUnseen(); - if (DBA::isResult($group_counts)) { - foreach ($group_counts as $group_count) { - if ($group_count['count'] > 0) { - $groups_unseen[] = $group_count; - } + foreach (Group::countUnseen() as $group_count) { + if ($group_count['count'] > 0) { + $groups_unseen[] = $group_count; } } - $forum_counts = ForumManager::countUnseenItems(); - if (DBA::isResult($forum_counts)) { - foreach ($forum_counts as $forum_count) { - if ($forum_count['count'] > 0) { - $forums_unseen[] = $forum_count; - } + foreach (ForumManager::countUnseenItems() as $forum_count) { + if ($forum_count['count'] > 0) { + $forums_unseen[] = $forum_count; } } } - $intros = $this->introductionRepo->selectForUser(DI::userSession()->getLocalUserId()); + $intros = $this->introductionRepo->selectForUser($this->session->getLocalUserId()); $intro_count = $intros->count(); - $myurl = DI::baseUrl() . '/profile/' . DI::app()->getLoggedInUserNickname(); - $mail_count = DBA::count('mail', ["`uid` = ? AND NOT `seen` AND `from-url` != ?", DI::userSession()->getLocalUserId(), $myurl]); + $myurl = $this->session->getMyUrl(); + $mail_count = $this->database->count('mail', ["`uid` = ? AND NOT `seen` AND `from-url` != ?", $this->session->getLocalUserId(), $myurl]); - if (intval(DI::config()->get('config', 'register_policy')) === Register::APPROVE && DI::app()->isSiteAdmin()) { - $regs = \Friendica\Model\Register::getPending(); - - if (DBA::isResult($regs)) { - $register_count = count($regs); - } + if (intval($this->config->get('config', 'register_policy')) === Register::APPROVE && $this->app->isSiteAdmin()) { + $registrations = \Friendica\Model\Register::getPending(); + $register_count = count($registrations); } - $cachekey = 'ping:events:' . DI::userSession()->getLocalUserId(); - $ev = DI::cache()->get($cachekey); - if (is_null($ev)) { - $ev = DBA::selectToArray('event', ['type', 'start'], + $cachekey = 'ping:events:' . $this->session->getLocalUserId(); + $events = $this->cache->get($cachekey); + if (is_null($events)) { + $events = $this->database->selectToArray('event', ['type', 'start'], ["`uid` = ? AND `start` < ? AND `finish` > ? AND NOT `ignore`", - DI::userSession()->getLocalUserId(), DateTimeFormat::utc('now + 7 days'), DateTimeFormat::utcNow()]); - DI::cache()->set($cachekey, $ev, Duration::HOUR); + $this->session->getLocalUserId(), DateTimeFormat::utc('now + 7 days'), DateTimeFormat::utcNow()]); + $this->cache->set($cachekey, $events, Duration::HOUR); } - if (DBA::isResult($ev)) { - $all_events = count($ev); + $now_date = DateTimeFormat::localNow('Y-m-d'); + foreach ($events as $event) { + $is_birthday = false; + if ($event['type'] === 'birthday') { + $birthday_count++; + $is_birthday = true; + } else { + $event_count++; + } - if ($all_events) { - $str_now = DateTimeFormat::localNow('Y-m-d'); - foreach ($ev as $x) { - $bd = false; - if ($x['type'] === 'birthday') { - $birthday_count++; - $bd = true; - } else { - $event_count++; - } - if (DateTimeFormat::local($x['start'], 'Y-m-d') === $str_now) { - if ($bd) { - $today_birthday_count++; - } else { - $today_event_count++; - } - } + if (DateTimeFormat::local($event['start'], 'Y-m-d') === $now_date) { + if ($is_birthday) { + $today_birthday_count++; + } else { + $today_event_count++; } } } - $owner = User::getOwnerDataById(DI::userSession()->getLocalUserId()); + $owner = User::getOwnerDataById($this->session->getLocalUserId()); $navNotifications = array_map(function (Entity\Notification $notification) use ($owner) { - if (!DI::notify()->shouldShowOnDesktop($notification)) { + if (!$this->notify->shouldShowOnDesktop($notification)) { return null; } if (($notification->type == Post\UserNotification::TYPE_NONE) && in_array($owner['page-flags'], [User::PAGE_FLAGS_NORMAL, User::PAGE_FLAGS_PRVGROUP])) { @@ -213,30 +227,28 @@ class Ping extends BaseModule $navNotifications[] = $this->formattedNavNotification->createFromIntro($intro); } - if (DBA::isResult($regs)) { - if (count($regs) <= 1 || DI::pConfig()->get(DI::userSession()->getLocalUserId(), 'system', 'detailed_notif')) { - foreach ($regs as $reg) { - $navNotifications[] = $this->formattedNavNotification->createFromParams( - [ - 'name' => $reg['name'], - 'url' => $reg['url'], - ], - DI::l10n()->t('{0} requested registration'), - new \DateTime($reg['created'], new \DateTimeZone('UTC')), - new Uri(DI::baseUrl()->get(true) . '/admin/users/pending') - ); - } - } else { + if (count($registrations) <= 1 || $this->pconfig->get($this->session->getLocalUserId(), 'system', 'detailed_notif')) { + foreach ($registrations as $reg) { $navNotifications[] = $this->formattedNavNotification->createFromParams( [ - 'name' => $regs[0]['name'], - 'url' => $regs[0]['url'], + 'name' => $reg['name'], + 'url' => $reg['url'], ], - DI::l10n()->t('{0} and %d others requested registration', count($regs) - 1), - new \DateTime($regs[0]['created'], new \DateTimeZone('UTC')), - new Uri(DI::baseUrl()->get(true) . '/admin/users/pending') + $this->l10n->t('{0} requested registration'), + new \DateTime($reg['created'], new \DateTimeZone('UTC')), + new Uri($this->baseUrl->get(true) . '/admin/users/pending') ); } + } elseif (count($registrations) > 1) { + $navNotifications[] = $this->formattedNavNotification->createFromParams( + [ + 'name' => $registrations[0]['name'], + 'url' => $registrations[0]['url'], + ], + $this->l10n->t('{0} and %d others requested registration', count($registrations) - 1), + new \DateTime($registrations[0]['created'], new \DateTimeZone('UTC')), + new Uri($this->baseUrl->get(true) . '/admin/users/pending') + ); } // sort notifications by $[]['date'] From 6c745c85226988bf4be4bb490c14c5134423168d Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Wed, 2 Nov 2022 20:45:42 -0400 Subject: [PATCH 5/5] Remove non-top-level posts from notification labels when network sort order is "received" - These posts don't alter the network view in this sort order and so are distracting with no actionable benefits --- src/Module/Notifications/Ping.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Module/Notifications/Ping.php b/src/Module/Notifications/Ping.php index 2d91105700..e80d3b8ee0 100644 --- a/src/Module/Notifications/Ping.php +++ b/src/Module/Notifications/Ping.php @@ -128,6 +128,11 @@ class Ping extends BaseModule $this->session->getLocalUserId(), Verb::getID(Activity::FOLLOW) ]; + // No point showing counts for non-top-level posts when the network page is ordered by received field + if (Network::getTimelineOrderBySession($this->session, $this->pconfig) == 'received') { + $condition = DBA::mergeConditions($condition, ["`parent` = `id`"]); + } + $items_unseen = $this->database->toArray(Post::selectForUser( $this->session->getLocalUserId(), ['wall', 'uid', 'uri-id'],