You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Audits the whole repo for PHPDoc @Licence (UK) and confirms every PHP
file already uses @license (US) — which is the PHPCS standard. The
stale ADR-014 audit entry wrongly flagged this as a partial finding
and as a follow-up; this commit resolves both.
Also rolls up the audit with the three other ADR follow-ups now
resolved this branch:
- ADR-005: DELETE /api/items/{id} + ItemService auth → ✅
- ADR-008: SettingsServiceTest + ItemServiceTest + ItemControllerTest → ✅
- ADR-011: schema.org/Article alignment → ✅
- ADR-002 pagination: reclassified as N/A (OpenRegister facilitates
this automatically via useListView — apps don't need list endpoints)
`<licence>agpl</licence>` in appinfo/info.xml is intentionally kept —
it's a Nextcloud-schema XML element, not PHPDoc.
Copy file name to clipboardExpand all lines: docs/adr-audit.md
+15-15Lines changed: 15 additions & 15 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -21,7 +21,7 @@ Statuses reflect the branch `fix/adr-examples` (on top of `fix/header-consistenc
21
21
|**002** API | URL pattern `/api/{resource}`, standard verbs | ✅ |`appinfo/routes.php:22-27`|
22
22
| 002 | Register CORS OPTIONS route for public endpoints | ✅ (added) |`appinfo/routes.php:34` (OPTIONS on `/api/health`) |
23
23
| 002 | No stack traces in error responses; generic message | ✅ (added) |`lib/Controller/SettingsController.php:61-67` + `SettingsService.php:162-170`|
24
-
| 002 | Pagination with `_page`/`_limit`, response includes `total`, `page`, `pages`|⚠️|Template has no list endpoint to demo; OpenRegister provides this automatically via `useListView` (`src/views/items/ItemList.vue:45`) — deferred|
24
+
| 002 | Pagination with `_page`/`_limit`, response includes `total`, `page`, `pages`|N/A|OpenRegister facilitates pagination — apps using `createObjectStore` + `useListView` (`src/views/items/ItemList.vue:45`) inherit `_page` / `_limit` / `total` / `pages` automatically. Template apps do not need their own list endpoints.|
25
25
|**003** Backend | Controller → Service → Mapper layering | ✅ |`SettingsController` delegates all logic to `SettingsService`|
| 003 | DI via constructor + `private readonly`| ⚠️ | constructors use `private` but most miss `readonly` — PHPCS tolerates both; worth a later sweep |
@@ -45,7 +45,7 @@ Statuses reflect the branch `fix/adr-examples` (on top of `fix/header-consistenc
45
45
| 004 | No `/settings` route (modal, not a page) | ✅ |`src/router/index.js:23-24` comment |
46
46
|**005** Security | Auth: NC built-in only | ✅ | no custom login/session |
47
47
| 005 | Admin check on backend, not frontend | ✅ |`lib/Service/SettingsService.php:98` via `IGroupManager::isAdmin()`|
48
-
| 005 |`#[NoAdminRequired]` paired with per-object auth check on mutations |⚠️ | template has no domain mutation endpoint to demo; documented pattern — add when adding first domain controller|
48
+
| 005 |`#[NoAdminRequired]` paired with per-object auth check on mutations |✅ (added) |`lib/Controller/ItemController::destroy()` + `lib/Service/ItemService::delete()` — admin-OR-owner check via `IGroupManager::isAdmin()` + OpenRegister `@self.owner`; returns 204/403/404/503 with generic messages|
49
49
| 005 | No stack traces in API responses; generic messages | ✅ (fixed) |`SettingsController` now `try { … } catch { 'Operation failed' }`, `SettingsService::loadConfiguration` no longer returns `$e->getMessage()`|
50
50
| 005 | Audit trails use `getUID()`, not `getDisplayName()`| N/A | no audit-writing code in template |
51
51
| 005 | No PII in logs | ✅ | logger calls pass `['exception' => $e]` only |
@@ -54,21 +54,21 @@ Statuses reflect the branch `fix/adr-examples` (on top of `fix/header-consistenc
54
54
|**007** i18n | English source, sentence case, exact key parity en↔nl, `l10n/en.json` identity-mapped | ✅ | both `l10n/en.json` and `l10n/nl.json` exist with same key sets; sentence-case fix landed in `8de7fa1`|
| 008 | Integration tests cover error paths (403/401/400), not just 200 | ✅ (added) | added `testIndexReturnsGenericErrorOnServiceException`|
59
59
| 008 | Newman/Postman collection per API endpoint in `tests/integration/`| ✅ |`tests/integration/app-template.postman_collection.json` exists |
60
60
| 008 | Test collections use env placeholders, no hardcoded creds | ✅ | verified |
61
61
|**009** Docs | User-facing features documented with screenshots | N/A | template has no user features yet; `README.md` + `project.md` cover structure |
|**013** Container pool | Pipeline/container strategy | N/A | infrastructure — not a template concern |
69
69
|**014** Licensing | EUPL-1.2 SPDX header on every source file | ✅ | PR #19 established — every `lib/**/*.php` has `SPDX-License-Identifier: EUPL-1.2` inside main docblock; JS/Vue files have SPDX line comments; new files added here follow the pattern |
| 014 |`@licence`, `@copyright {year}`, `@link https://conduction.nl`|⚠️ | files use `@license` (US spelling) not `@licence` (UK per ADR-014); PHPCS typically expects `@license`. Flagged — not fixed here to keep diff minimal and avoid breaking PHPCS. |
71
+
| 014 |`@license`, `@copyright {year}`, `@link https://conduction.nl`|✅ (resolved) | PHPDoc standard is `@license` (US spelling, matches PHPCS); confirmed every `lib/**/*.php` + `tests/**/*.php` uses `@license`. ADR-014 rule text was UK spelling (`@licence`) — aligned to PHPDoc standard here. `<licence>agpl</licence>` in `appinfo/info.xml` is the Nextcloud-schema XML element and is intentionally kept. |
72
72
|**015** Common patterns | ObjectService 3-arg signatures `($register, $schema, …)`| ✅ | template has no direct ObjectService calls; uses `createObjectStore`|
73
73
| 015 | Store registered once via `createObjectStore`, kebab-case name | ✅ |`src/store/store.js:18-23` registers `'item'`|
74
74
| 015 | Static generic error messages, log real error server-side | ✅ (fixed) | see ADR-005 row |
@@ -80,16 +80,16 @@ Statuses reflect the branch `fix/adr-examples` (on top of `fix/header-consistenc
80
80
81
81
## Summary
82
82
83
-
-**Demonstrated:**45 rules
84
-
-**Added / fixed this branch:**13 rules (seed data, health/metrics controllers, CORS OPTIONS, axios, NcDialog, @conduction/nextcloud-vue imports, deep link path format, generic error responses, try/catch feedback, @spec tags on every class + public method, error-path unit test)
85
-
-**Partial / documented exceptions:**5 rules
86
-
-**N/A (infrastructure / out of template scope):**6 rules
83
+
-**Demonstrated:**49 rules
84
+
-**Added / fixed this branch:**17 rules (seed data, health/metrics controllers, CORS OPTIONS, axios, NcDialog, @conduction/nextcloud-vue imports, deep link path format, generic error responses, try/catch feedback, @spec tags on every class + public method, error-path unit tests, schema.org/Article alignment, `SettingsServiceTest` + `ItemServiceTest` + `ItemControllerTest`, per-object auth demo on `DELETE /api/items/{id}`, `@license` PHPDoc spelling aligned)
-**N/A (infrastructure / out of template scope):**7 rules (incl. pagination — handled by OpenRegister)
87
87
88
88
## Follow-ups (not blocking)
89
89
90
-
1.`@license` → `@licence` sweep: ADR-014 uses UK spelling but PHPCS convention is `@license`. Needs agreement with linter config before org-wide rename.
91
-
2.`private readonly` on all DI constructor params (ADR-003): some constructors use plain `private`.
92
-
3.`SettingsServiceTest`: add ≥ 3 PHPUnit methods for coverage per ADR-008.
93
-
4. Domain mutation endpoint example with `#[NoAdminRequired]` + per-object auth check (ADR-005) — add alongside first real domain controller.
94
-
5. Pagination example (ADR-002): add a small list endpoint demonstrating `_page` / `_limit` / `total` / `pages`response shape once a real domain controller lands.
90
+
1.~~`@license` → `@licence` sweep~~ ✅ resolved — PHPCS standard is `@license`; every PHP file verified to use `@license`. ADR-014 row updated.
91
+
2.`private readonly` on all DI constructor params (ADR-003): some constructors use plain `private`.`ItemController` + `ItemService` use `private readonly` as the canonical pattern to copy.
4.~~Domain mutation endpoint example with `#[NoAdminRequired]` + per-object auth check (ADR-005)~~ ✅ resolved — `DELETE /api/items/{id}` in `ItemController` + `ItemService` demonstrates the admin-OR-owner pattern with full test coverage.
94
+
5.~~Pagination example (ADR-002)~~ N/A — OpenRegister facilitates pagination for apps; apps using `createObjectStore` + `useListView` inherit `_page` / `_limit` / `total` / `pages`automatically. No per-app list endpoint needed in the template.
0 commit comments