Skip to content

Fix deprecated InvalidArgumentException#1133

Closed
sjg2203 wants to merge 4 commits into
nextcloud:mainfrom
sjg2203:fix-notifier-exception
Closed

Fix deprecated InvalidArgumentException#1133
sjg2203 wants to merge 4 commits into
nextcloud:mainfrom
sjg2203:fix-notifier-exception

Conversation

@sjg2203

@sjg2203 sjg2203 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Fix deprecated InvalidArgumentException in Notifier::prepare() to match Nextcloud 34 requirement, removing repeated log warning such as:
Warning: OCA\UserMigration\Notification\Notifier::prepare() through \InvalidArgumentException which is deprecated. Throw \OCP\Notification\UnknownNotificationException when the notification is not known to your notifier and otherwise handle all \InvalidArgumentException yourself.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Fix deprecated InvalidArgumentException in Notifier::prepare()

Signed-off-by: Simon J Guillot <58807831+sjg2203@users.noreply.github.com>
@sjg2203 sjg2203 requested review from come-nc and samin-z as code owners June 29, 2026 18:51

@come-nc come-nc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good but this is only possible if we drop support for 29 on the main branch as the exception was added in 30.

@sjg2203

sjg2203 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

We could hang onto this for when 29 support is dropped, and in the meantime just add a wrapper that adapts the notifier based on whichever class exists

Adds a createNotificationException() helper that checks whether UnknownNotificationException exists

Signed-off-by: Simon J Guillot <58807831+sjg2203@users.noreply.github.com>
@come-nc

come-nc commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

I think it’s fine to drop 29, I can bump stable29 later if I need a release for 29.
Can you include in this PR the drop of 29 and a major version bump? (So 11.0 instead of 10.4)

@come-nc come-nc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, I did not see you added the wrapper, that works as well, thanks.

@come-nc

come-nc commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

You need to run composer run cs:fix

Signed-off-by: Simon J Guillot <58807831+sjg2203@users.noreply.github.com>
@sjg2203

sjg2203 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

My bad, forgot to format. Should be good now

@come-nc come-nc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Something is wrong, you changed formatting on the whole file.
Which command did you use?

@sjg2203

sjg2203 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

composer run cs:fix changed the whole file for no reason

Signed-off-by: Simon J Guillot <58807831+sjg2203@users.noreply.github.com>
@come-nc

come-nc commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

I cherry-picked your commits into #1134 and applied cs:fix.

@come-nc come-nc closed this Jun 30, 2026
@sjg2203 sjg2203 deleted the fix-notifier-exception branch June 30, 2026 15:09
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.

2 participants