Skip to content

feat: add notifications when sharing articles with users#3542

Merged
Grotax merged 7 commits intonextcloud:masterfrom
eureka928:fix/share-notifications-2102
Feb 8, 2026
Merged

feat: add notifications when sharing articles with users#3542
Grotax merged 7 commits intonextcloud:masterfrom
eureka928:fix/share-notifications-2102

Conversation

@eureka928
Copy link
Contributor

Summary

This PR adds in-app notifications when a user shares an article with another user, addressing the second issue reported in #2102.

  • Register a Notifier service in the Nextcloud notification framework to prepare and display share notifications
  • Send a notification to the recipient in ShareService::shareItemWithUser() with the sharer's name and article title
  • Use UnknownNotificationException per the Nextcloud notification API docs
  • Use Application::NAME constant instead of hardcoded 'news' string
  • Notification includes rich subject with user and article parameters, a link to the News app, and the app icon
  • Notification failures are caught and logged so they don't break the share operation

New files

  • lib/Notification/Notifier.php - INotifier implementation for the News app
  • tests/Unit/Notification/NotifierTest.php - Unit tests for the Notifier class

Modified files

  • lib/AppInfo/Application.php - Registers the Notifier service
  • lib/Service/ShareService.php - Sends notification after sharing
  • tests/Unit/Service/ShareServiceTest.php - Updated with notification expectations

Checklist

@eureka928 eureka928 force-pushed the fix/share-notifications-2102 branch from 892bdfa to 61db68c Compare February 4, 2026 22:25
@eureka928
Copy link
Contributor Author

Hi @Grotax how are you?
Would you review this PR?
Thank you

@Grotax
Copy link
Member

Grotax commented Feb 6, 2026

No

@eureka928
Copy link
Contributor Author

No

What's wrong? I'd like to contribute so I put the PR for feed-io as well as here...

@Grotax
Copy link
Member

Grotax commented Feb 6, 2026

I review when I have time and energy and your pinging is not helpful.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Nextcloud in-app notifications for the News app when an article is shared with another user, by registering a notification Notifier and sending a notification from ShareService::shareItemWithUser().

Changes:

  • Register a News Notifier with Nextcloud’s notification framework.
  • Send a “shared article” notification after successfully sharing an item.
  • Add unit tests for the Notifier and update ShareService unit tests to assert notification behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
lib/AppInfo/Application.php Registers the News notification notifier service.
lib/Service/ShareService.php Injects OCP\Notification\IManager and sends a notification after sharing.
lib/Notification/Notifier.php Implements notification preparation (parsed/rich subject, link, icon).
tests/Unit/Service/ShareServiceTest.php Adds notification manager + notification expectations for sharing tests.
tests/Unit/Notification/NotifierTest.php New unit tests covering notifier behavior and edge cases.

@Grotax Grotax force-pushed the fix/share-notifications-2102 branch from e89c95f to 70027d3 Compare February 8, 2026 09:21
eureka928 and others added 7 commits February 8, 2026 10:24
Users now receive a notification when someone shares an article with
them. The notification includes:
- Sharer's display name
- Article title (truncated if > 100 chars)
- Clickable link to open the News app
- News app icon

Implementation details:
- Added Notifier class implementing INotifier interface
- Registered notifier in Application.php
- ShareService sends notification after successful share
- Notification failures are logged but don't break the share flow

Fixes nextcloud#2102 (notification part - focus fix was already implemented)

Signed-off-by: eureka928 <meobius123@gmail.com>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
… add Notifier tests

Replace hardcoded 'news' string with Application::NAME in ShareService
and use the proper UnknownNotificationException from the Nextcloud
notification API instead of \InvalidArgumentException in the Notifier.
Add unit tests for the Notifier class.

Signed-off-by: eureka928 <meobius123@gmail.com>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax force-pushed the fix/share-notifications-2102 branch from 70027d3 to 4279e10 Compare February 8, 2026 09:24
@Grotax Grotax merged commit 60958ee into nextcloud:master Feb 8, 2026
27 checks passed
throw new UnknownNotificationException();
}

$l = $this->l10nFactory->get(Application::NAME, $languageCode);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please give variables a descriptive name. L is not enough to know it's an l10n factory


$l = $this->l10nFactory->get(Application::NAME, $languageCode);

switch ($notification->getSubject()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks nicer as a match

private function prepareSharedArticle(INotification $notification, \OCP\IL10N $l): INotification
{
$params = $notification->getSubjectParameters();
$sharerUserId = $params['sharedBy'] ?? '';
Copy link
Contributor

Choose a reason for hiding this comment

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

When would this be null?

Grotax added a commit that referenced this pull request Feb 8, 2026
Changed
- Notifications for shared articles - recipients now receive a notification when someone shares an article with them (#3542)

Fixed
- global starred count not updated when deleting a feed with starred items (#3507)
- feed fetcher requests may get stuck (#3528)
- feed logo download and `fulltext` scraper don't use configured proxy (#3533)
- Dependency scoping incompatible with auto-downloaded composer (#3418)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Feb 8, 2026
Grotax added a commit that referenced this pull request Feb 8, 2026
Changed
- Notifications for shared articles - recipients now receive a notification when someone shares an article with them (#3542)

Fixed
- global starred count not updated when deleting a feed with starred items (#3507)
- feed fetcher requests may get stuck (#3528)
- feed logo download and `fulltext` scraper don't use configured proxy (#3533)
- Dependency scoping incompatible with auto-downloaded composer (#3418)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problematic behavior of the "Share" button

3 participants