Skip to content

fix(NotificationGenerator): catch InvalidArgumentException from notification setters in prepare()#2604

Merged
miaulalala merged 1 commit into
masterfrom
fix/nc39/notification-generator-invalid-argument-exception
May 20, 2026
Merged

fix(NotificationGenerator): catch InvalidArgumentException from notification setters in prepare()#2604
miaulalala merged 1 commit into
masterfrom
fix/nc39/notification-generator-invalid-argument-exception

Conversation

@miaulalala
Copy link
Copy Markdown
Collaborator

@miaulalala miaulalala commented May 19, 2026

Summary

  • INotification setters (setRichSubject(), setParsedSubject()) throw \InvalidArgumentException for empty/invalid values
  • These previously escaped prepare(), triggering the NC39 deprecation: "threw \InvalidArgumentException which is deprecated"
  • Fix catches the exception, logs it as an error, and throws AlreadyProcessedException to discard the unrenderable notification

Test plan

  • Run unit tests: composer test:unit
  • Verify no \InvalidArgumentException escapes prepare() in the activity notifier logs

🤖 Generated with Claude Code

@miaulalala
Copy link
Copy Markdown
Collaborator Author

/backport to stable34

@cypress
Copy link
Copy Markdown

cypress Bot commented May 20, 2026

Activity    Run #3813

Run Properties:  status check failed Failed #3813  •  git commit 435670382d: fix(NotificationGenerator): catch InvalidArgumentException from notification set...
Project Activity
Branch Review fix/nc39/notification-generator-invalid-argument-exception
Run status status check failed Failed #3813
Run duration 02m 17s
Commit git commit 435670382d: fix(NotificationGenerator): catch InvalidArgumentException from notification set...
Committer Anna
View all properties for this run ↗︎

Test results
Tests that failed  Failures 1
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 1
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 8
View all changes introduced in this branch ↗︎

Tests for review

Failed  cypress/e2e/settings.cy.ts • 1 failed test • Run E2E

View Output

Test Artifacts
Check that user's settings survive a reload > Form survive a reload Test Replay Screenshots

Copy link
Copy Markdown
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Reviewed the second commit. First one seems to be another PR

@miaulalala miaulalala force-pushed the fix/nc39/notification-generator-invalid-argument-exception branch from 17a425e to dbe268a Compare May 20, 2026 09:39
…ication setters in prepare()

INotification setters like setRichSubject() and setParsedSubject() throw
\InvalidArgumentException for empty/invalid values, which previously escaped
prepare() — triggering the NC39 deprecation. Catch and discard the notification
as already-processed since it cannot be rendered.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala force-pushed the fix/nc39/notification-generator-invalid-argument-exception branch from dbe268a to 2c05121 Compare May 20, 2026 10:45
@miaulalala miaulalala merged commit 73c58a0 into master May 20, 2026
54 of 58 checks passed
@miaulalala miaulalala deleted the fix/nc39/notification-generator-invalid-argument-exception branch May 20, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants