Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix/noid/improve the notifications #50

Merged
merged 4 commits into from Sep 11, 2019
Merged

Bugfix/noid/improve the notifications #50

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d9256c
Allow to directly disable the nextcloud announcements from the notifi…
nickvergessen Sep 11, 2019
8d2fab6
Add a note that the announcements are only shown to administrators by…
nickvergessen Sep 11, 2019
34693d3
Add a primary action to read more
nickvergessen Sep 11, 2019
75c42c1
Only show the uninstall button on default settings
nickvergessen Sep 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 38 additions & 10 deletions lib/Notification/Notifier.php
View file
Open in desktop
Expand Up @@ -24,8 +24,11 @@
namespace OCA\NextcloudAnnouncements\Notification;


use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IURLGenerator;
use OCP\L10N\IFactory;
use OCP\Notification\IAction;
use OCP\Notification\INotification;
use OCP\Notification\INotifier;

Expand All @@ -35,22 +38,25 @@ class Notifier implements INotifier {

/** @var string */
protected $appName;

/** @var IFactory */
protected $l10nFactory;

/** @var IURLGenerator */
protected $url;

/**
* @param string $appName
* @param IFactory $l10nFactory
* @param IURLGenerator $url
*/
public function __construct($appName, IFactory $l10nFactory, IURLGenerator $url) {
/** @var IConfig */
protected $config;
/** @var IGroupManager */
protected $groupManager;

public function __construct(string $appName,
IFactory $l10nFactory,
IURLGenerator $url,
IConfig $config,
IGroupManager $groupManager) {
$this->appName = $appName;
$this->l10nFactory = $l10nFactory;
$this->url = $url;
$this->config = $config;
$this->groupManager = $groupManager;
}

/**
Expand Down Expand Up @@ -91,10 +97,32 @@ public function prepare(INotification $notification, string $languageCode): INot
switch ($notification->getSubject()) {
case self::SUBJECT:
$parameters = $notification->getSubjectParameters();
$message = $parameters[0];
$notification->setParsedSubject($l->t('Nextcloud announcement'))
->setParsedMessage($parameters[0])
->setIcon($this->url->getAbsoluteURL($this->url->imagePath($this->appName, 'app-dark.svg')));

$action = $notification->createAction();
$action->setParsedLabel($l->t('Read more'))
->setLink($notification->getLink(), IAction::TYPE_WEB)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature is only 17+ so it needs to be removed below, but not sure if the disable button is too confusing then?

->setPrimary(true);
$notification->addParsedAction($action);

$isAdmin = $this->groupManager->isAdmin($notification->getUser());
if ($isAdmin) {
$groups = $this->config->getAppValue($this->appName, 'notification_groups', '');
if ($groups === '') {
$action = $notification->createAction();
$action->setParsedLabel($l->t('Disable announcements'))
->setLink($this->url->linkToOCSRouteAbsolute('provisioning_api.AppsController.disable', ['app' => 'nextcloud_announcements']), IAction::TYPE_DELETE)
->setPrimary(false);
$notification->addParsedAction($action);

$message .= "\n\n" . $l->t('(These announcements are only shown to administrators)');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$message .= "\n\n" . $l->t('(These announcements are only shown to administrators)');
$message .= "\n\n" . $l->t('(These announcements are only shown to administrators.)');

Very small nitpick. ;D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it in place before, found it looked weird and removed it, damn :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does look a bit strange and unfinished cause the sentences of the announcement will all have punctuation. As said, just a tiny detail. :)

}
}

$notification->setParsedMessage($message);

return $notification;

default:
Expand Down