Skip to content

fix: notification handling for share removal and space membership expiry - #3257

Open
maki5 wants to merge 9 commits into
mainfrom
fix/userlog_notifications_handling
Open

fix: notification handling for share removal and space membership expiry #3257
maki5 wants to merge 9 commits into
mainfrom
fix/userlog_notifications_handling

Conversation

@maki5

@maki5 maki5 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Notifications service

  • Handle the ShareRemoved event (backport of feat: Add email notification for share removed owncloud/ocis#10915):
    new email template, event dispatch and grouped-mail support, and the
    mail option is enabled by default in the notification settings
  • Rework handleSpaceMembershipExpired: resolve the space name via a
    prepare helper and send from the default email sender, dropping the
    dependency on the unresolvable space owner lookup

Userlog service

  • Add a dedicated conversion path for SpaceMembershipExpired that
    resolves only the space (the template does not reference a user),
    so the in-app notification no longer fails on the unresolvable space
    owner placeholder
  • Handle policy violation and antivirus postprocessing steps for
    granular in-app notifications (previously we didn't handle it's disabling)

Dependencies

@maki5 maki5 added the Type:Bug label Aug 6, 2026
@codacy-production

codacy-production Bot commented Aug 6, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 26 duplication

Metric Results
Complexity 0
Duplication 26

View in Codacy

🟢 Coverage 39.04% diff coverage · +0.02% coverage variation

Metric Results
Coverage variation +0.02% coverage variation (-1.00%)
Diff coverage 39.04% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (8478b5d) 84082 19349 23.01%
Head commit (1cbc833) 84206 (+124) 19397 (+48) 23.04% (+0.02%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3257) 146 57 39.04%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@maki5
maki5 requested review from aduffeck, micbar and pbleser-oc August 6, 2026 09:43
@github-project-automation github-project-automation Bot moved this to Qualification in OpenCloud Team Board Aug 6, 2026
@maki5 maki5 moved this from Qualification to Needs Tests in OpenCloud Team Board Aug 6, 2026
@micbar

micbar commented Aug 11, 2026

Copy link
Copy Markdown
Member

Please add a description.

@micbar micbar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good to me 👍

@maki5 maki5 changed the title fix userlog policy violation/antivirus event handling fix: notification handling for share removal and space membership expiry Aug 12, 2026
@micbar

micbar commented Aug 14, 2026

Copy link
Copy Markdown
Member

the last merge from main somehow messed this up a bit.

@butonic @pbleser-oc Can you help to rebase this properly?

@maki5 I tested you changes, looking good to me! 👍

We should enable this feature by default, i checked with @tbsbdr

That means we need to change the default config by setting this StructValue https://github.com/opencloud-eu/opencloud/blob/main/services/frontend/pkg/config/config.go#L62 in the defaultconfig.go to true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Needs Tests

Development

Successfully merging this pull request may close these issues.

2 participants