fix: answer 404 from the notification endpoint when the notifications app is not enabled

[oleksandr-nc]

POST /api/v1/notification returned 200 with the notifications app not enabled, while the core notification manager delivered to nobody: no row, no log line. That is the state of every instance built from the server git checkout, since the app ships only in release tarballs.

Now it logs a warning and answers 404 naming the missing app. Unit test added, OpenAPI regenerated.

Verified on NC master: 404 with the app disabled, 200 and the row with it enabled.

[coderabbitai]

📝 Walkthrough

Walkthrough

The notification controller now checks whether the Notifications app is enabled for anyone before sending a notification. If the app is disabled, it logs a warning and throws OCSNotFoundException, producing a 404 response. OpenAPI documents and the changelog describe this response. PHPUnit tests cover disabled-app rejection and enabled-app delivery.

Priority: ⬇️ Low — Impact reflects low issue severity.

Severity of issue fixed: Low

Merge Risk: 🔵 Low · up to f31f9

When Notifications is disabled, callers receive a 404 rather than a successful undeliverable request. The error text identifying the disabled app is not regression-tested, so it could change without detection.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 3…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: the notification endpoint returns 404 when the notifications app is not enabled.
Description check	✅ Passed	The description directly explains the existing behavior, the 404 and warning changes, the test coverage, and the OpenAPI update.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 090104fa-9dbf-4daa-9312-d9b48c1abd41

📥 Commits

Reviewing files that changed from the base of the PR and between c85ab14 and f31f9b2.

📒 Files selected for processing (5)

• CHANGELOG.md
• lib/Controller/NotificationsController.php
• openapi-full.json
• openapi.json
• tests/php/Controller/NotificationsControllerTest.php

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.