This repository was archived by the owner on May 29, 2026. It is now read-only.
feat(dashboards): group_shared scope + visible-to-user resolver#47
Merged
Merged
Conversation
rubenvdlinde
added
ready-for-code-review
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 103/103 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-30 11:48 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
force-pushed
the
feature/impl-multi-scope-dashboards
branch
from
4f5e1be to
91a5209
Compare
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 103/103 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-30 12:02 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
force-pushed
the
feature/impl-multi-scope-dashboards
branch
from
d427158 to
d9d03c9
Compare
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 103/103 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-30 12:07 UTC
Download the full PDF report from the workflow artifacts.
Introduces a third dashboard scope (group_shared) alongside the existing
user and admin_template types, plus the visible-to-user resolver, group
sentinel, and admin-only group CRUD.
Implements:
- REQ-DASH-011 (group_shared type + (type, groupId) invariant)
- REQ-DASH-012 (default-group sentinel)
- REQ-DASH-013 (GET /api/dashboards/visible with source tagging)
- REQ-DASH-014 (group-scoped CRUD endpoints + last-in-group guard)
Changes:
- Dashboard entity: TYPE_GROUP_SHARED + DEFAULT_GROUP_ID + SOURCE_*
constants, nullable groupId field, jsonSerialize includes groupId.
- DashboardMapper: findByGroup, findVisibleToUser (3 indexed queries +
PHP-side dedup-by-UUID with priority user > group > default), and
countByGroup helper for the delete guard.
- DashboardFactory::create: accepts type / groupId / gridColumns kwargs
and enforces the (type, groupId) invariant via InvalidArgumentException.
- DashboardService: createGroupShared, updateGroupShared, deleteGroupShared
(with admin guard + last-in-group check; default group exempt),
getVisibleToUser wiring IGroupManager::getUserGroupIds, and isAdmin
helper.
- PermissionService: getEffectivePermissionLevel returns view_only for
non-admin members and full for admins on group_shared dashboards
(single source of truth — the persisted column is preserved for
forward-compat).
- DashboardApiController: 6 new endpoints (visible, listGroup, getGroup,
createGroup, updateGroup, deleteGroup) with #[NoAdminRequired] +
in-body admin checks for mutations (passes gate-route-auth and
gate-semantic-auth).
- routes.php: 6 new routes with groupId/uuid path requirements.
- Migration Version001005Date20260430000000: adds nullable group_id
VARCHAR(64) column + composite index (type, group_id) on
oc_mydash_dashboards. Idempotent column / index checks.
- DashboardTableBuilder: same column + index for fresh installs.
- Frontend store: groupSharedDashboards / defaultGroupDashboards /
personalDashboards getters, source plumbing for active dashboard,
routing PUT to /api/dashboards/group/{groupId}/{uuid} when
source=group|default.
- src/services/api.js: getVisibleDashboards + 5 group-CRUD helpers.
- l10n/en + nl: 3 new error message keys.
- Tests: DashboardFactoryGroupSharedTest covers the invariant guard;
DashboardTest extended for groupId getter/setter, jsonSerialize, and
the new constants.
In-passing fixes:
- DashboardService: import WidgetPlacement instead of FQCN reference
(resolves pre-existing PHPMD MissingImport).
Tasks skipped:
- 6.x seed data: _registers.json does not exist in this repo.
- 9.x Playwright e2e: test infra is in PR #43 (out of scope).
- 7.4 admin UI: deferred to follow-up admin-group-management change.
rubenvdlinde
force-pushed
the
feature/impl-multi-scope-dashboards
branch
from
5d09be7 to
c4c4b73
Compare
Contributor
Quality Report — ConductionNL/mydash @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 103/103 | |||
| npm | ✅ | ✅ 342/342 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-30 12:10 UTC
Download the full PDF report from the workflow artifacts.
This was referenced Apr 30, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Apr 30, 2026
Add a transactional single-default-per-group invariant for
`group_shared` dashboards by reusing the existing `isDefault SMALLINT`
column (no schema migration required).
Changes
- `DashboardMapper::clearGroupDefaults()` and `setGroupDefaultUuid()`:
the two halves of the transactional flip.
- `DashboardService::setGroupDefault()`: admin-only, wraps both writes
in a single `IDBConnection::beginTransaction()` / `commit()` /
`rollBack()` block. Order is SET-target first so the 0-row count
triggers a 404 with the previous default preserved.
- `DashboardService::createGroupShared()`: explicitly forces
`isDefault = 0` on insert (REQ-DASH-016 — defense-in-depth against
payload smuggling).
- `DashboardService::updateGroupShared()`: drops `isDefault` from the
patch before applying (REQ-DASH-017 — PUT can never flip the flag).
- `DashboardApiController::setGroupDefault()` mapped to
`POST /api/dashboards/group/{groupId}/default`. Admin guard is in
the body (matches existing `updateGroup`/`deleteGroup` pattern).
Tests
- 7 service tests cover the transactional invariant (commit on
happy path, rollBack on cross-group 404, rollBack on mid-flip
exception), the non-admin guard, and the create/update field
hardening.
- 6 controller tests cover the route surface — non-admin 403,
anonymous 401, missing-uuid 400, happy path 200, cross-group 404,
and reflection-level checks that `createGroup`/`updateGroup`
signatures do not expose `isDefault` as a parameter.
- `tests/Stubs/DoctrineStubs.php` defines minimal Doctrine DBAL
placeholder classes so PHPUnit's automatic mock generator can
introspect `IDBConnection` outside the Nextcloud container; no-op
inside the container.
Spec source: PR #42 (`feature/replica-spec-proposals`,
`openspec/changes/default-dashboard-flag/`). Builds on PR #47
(`multi-scope-dashboards`, already in `development`).
rubenvdlinde
added a commit
that referenced
this pull request
Apr 30, 2026
Add a transactional single-default-per-group invariant for
`group_shared` dashboards by reusing the existing `isDefault SMALLINT`
column (no schema migration required).
Changes
- `DashboardMapper::clearGroupDefaults()` and `setGroupDefaultUuid()`:
the two halves of the transactional flip.
- `DashboardService::setGroupDefault()`: admin-only, wraps both writes
in a single `IDBConnection::beginTransaction()` / `commit()` /
`rollBack()` block. Order is SET-target first so the 0-row count
triggers a 404 with the previous default preserved.
- `DashboardService::createGroupShared()`: explicitly forces
`isDefault = 0` on insert (REQ-DASH-016 — defense-in-depth against
payload smuggling).
- `DashboardService::updateGroupShared()`: drops `isDefault` from the
patch before applying (REQ-DASH-017 — PUT can never flip the flag).
- `DashboardApiController::setGroupDefault()` mapped to
`POST /api/dashboards/group/{groupId}/default`. Admin guard is in
the body (matches existing `updateGroup`/`deleteGroup` pattern).
Tests
- 7 service tests cover the transactional invariant (commit on
happy path, rollBack on cross-group 404, rollBack on mid-flip
exception), the non-admin guard, and the create/update field
hardening.
- 6 controller tests cover the route surface — non-admin 403,
anonymous 401, missing-uuid 400, happy path 200, cross-group 404,
and reflection-level checks that `createGroup`/`updateGroup`
signatures do not expose `isDefault` as a parameter.
- `tests/Stubs/DoctrineStubs.php` defines minimal Doctrine DBAL
placeholder classes so PHPUnit's automatic mock generator can
introspect `IDBConnection` outside the Nextcloud container; no-op
inside the container.
Spec source: PR #42 (`feature/replica-spec-proposals`,
`openspec/changes/default-dashboard-flag/`). Builds on PR #47
(`multi-scope-dashboards`, already in `development`).
rubenvdlinde
added a commit
that referenced
this pull request
Apr 30, 2026
#52) Add a transactional single-default-per-group invariant for `group_shared` dashboards by reusing the existing `isDefault SMALLINT` column (no schema migration required). Changes - `DashboardMapper::clearGroupDefaults()` and `setGroupDefaultUuid()`: the two halves of the transactional flip. - `DashboardService::setGroupDefault()`: admin-only, wraps both writes in a single `IDBConnection::beginTransaction()` / `commit()` / `rollBack()` block. Order is SET-target first so the 0-row count triggers a 404 with the previous default preserved. - `DashboardService::createGroupShared()`: explicitly forces `isDefault = 0` on insert (REQ-DASH-016 — defense-in-depth against payload smuggling). - `DashboardService::updateGroupShared()`: drops `isDefault` from the patch before applying (REQ-DASH-017 — PUT can never flip the flag). - `DashboardApiController::setGroupDefault()` mapped to `POST /api/dashboards/group/{groupId}/default`. Admin guard is in the body (matches existing `updateGroup`/`deleteGroup` pattern). Tests - 7 service tests cover the transactional invariant (commit on happy path, rollBack on cross-group 404, rollBack on mid-flip exception), the non-admin guard, and the create/update field hardening. - 6 controller tests cover the route surface — non-admin 403, anonymous 401, missing-uuid 400, happy path 200, cross-group 404, and reflection-level checks that `createGroup`/`updateGroup` signatures do not expose `isDefault` as a parameter. - `tests/Stubs/DoctrineStubs.php` defines minimal Doctrine DBAL placeholder classes so PHPUnit's automatic mock generator can introspect `IDBConnection` outside the Nextcloud container; no-op inside the container. Spec source: PR #42 (`feature/replica-spec-proposals`, `openspec/changes/default-dashboard-flag/`). Builds on PR #47 (`multi-scope-dashboards`, already in `development`).
This was referenced May 1, 2026
rubenvdlinde
added a commit
that referenced
this pull request
May 3, 2026
…47) Introduces a third dashboard scope (group_shared) alongside the existing user and admin_template types, plus the visible-to-user resolver, group sentinel, and admin-only group CRUD. Implements: - REQ-DASH-011 (group_shared type + (type, groupId) invariant) - REQ-DASH-012 (default-group sentinel) - REQ-DASH-013 (GET /api/dashboards/visible with source tagging) - REQ-DASH-014 (group-scoped CRUD endpoints + last-in-group guard) Changes: - Dashboard entity: TYPE_GROUP_SHARED + DEFAULT_GROUP_ID + SOURCE_* constants, nullable groupId field, jsonSerialize includes groupId. - DashboardMapper: findByGroup, findVisibleToUser (3 indexed queries + PHP-side dedup-by-UUID with priority user > group > default), and countByGroup helper for the delete guard. - DashboardFactory::create: accepts type / groupId / gridColumns kwargs and enforces the (type, groupId) invariant via InvalidArgumentException. - DashboardService: createGroupShared, updateGroupShared, deleteGroupShared (with admin guard + last-in-group check; default group exempt), getVisibleToUser wiring IGroupManager::getUserGroupIds, and isAdmin helper. - PermissionService: getEffectivePermissionLevel returns view_only for non-admin members and full for admins on group_shared dashboards (single source of truth — the persisted column is preserved for forward-compat). - DashboardApiController: 6 new endpoints (visible, listGroup, getGroup, createGroup, updateGroup, deleteGroup) with #[NoAdminRequired] + in-body admin checks for mutations (passes gate-route-auth and gate-semantic-auth). - routes.php: 6 new routes with groupId/uuid path requirements. - Migration Version001005Date20260430000000: adds nullable group_id VARCHAR(64) column + composite index (type, group_id) on oc_mydash_dashboards. Idempotent column / index checks. - DashboardTableBuilder: same column + index for fresh installs. - Frontend store: groupSharedDashboards / defaultGroupDashboards / personalDashboards getters, source plumbing for active dashboard, routing PUT to /api/dashboards/group/{groupId}/{uuid} when source=group|default. - src/services/api.js: getVisibleDashboards + 5 group-CRUD helpers. - l10n/en + nl: 3 new error message keys. - Tests: DashboardFactoryGroupSharedTest covers the invariant guard; DashboardTest extended for groupId getter/setter, jsonSerialize, and the new constants. In-passing fixes: - DashboardService: import WidgetPlacement instead of FQCN reference (resolves pre-existing PHPMD MissingImport). Tasks skipped: - 6.x seed data: _registers.json does not exist in this repo. - 9.x Playwright e2e: test infra is in PR #43 (out of scope). - 7.4 admin UI: deferred to follow-up admin-group-management change.
rubenvdlinde
added a commit
that referenced
this pull request
May 3, 2026
#52) Add a transactional single-default-per-group invariant for `group_shared` dashboards by reusing the existing `isDefault SMALLINT` column (no schema migration required). Changes - `DashboardMapper::clearGroupDefaults()` and `setGroupDefaultUuid()`: the two halves of the transactional flip. - `DashboardService::setGroupDefault()`: admin-only, wraps both writes in a single `IDBConnection::beginTransaction()` / `commit()` / `rollBack()` block. Order is SET-target first so the 0-row count triggers a 404 with the previous default preserved. - `DashboardService::createGroupShared()`: explicitly forces `isDefault = 0` on insert (REQ-DASH-016 — defense-in-depth against payload smuggling). - `DashboardService::updateGroupShared()`: drops `isDefault` from the patch before applying (REQ-DASH-017 — PUT can never flip the flag). - `DashboardApiController::setGroupDefault()` mapped to `POST /api/dashboards/group/{groupId}/default`. Admin guard is in the body (matches existing `updateGroup`/`deleteGroup` pattern). Tests - 7 service tests cover the transactional invariant (commit on happy path, rollBack on cross-group 404, rollBack on mid-flip exception), the non-admin guard, and the create/update field hardening. - 6 controller tests cover the route surface — non-admin 403, anonymous 401, missing-uuid 400, happy path 200, cross-group 404, and reflection-level checks that `createGroup`/`updateGroup` signatures do not expose `isDefault` as a parameter. - `tests/Stubs/DoctrineStubs.php` defines minimal Doctrine DBAL placeholder classes so PHPUnit's automatic mock generator can introspect `IDBConnection` outside the Nextcloud container; no-op inside the container. Spec source: PR #42 (`feature/replica-spec-proposals`, `openspec/changes/default-dashboard-flag/`). Builds on PR #47 (`multi-scope-dashboards`, already in `development`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements REQ-DASH-011..014 — adds the
group_shareddashboard scope alongside the existinguserandadmin_templatescopes, plus the/api/dashboards/visibleresolution endpoint and group-scoped CRUD.Dashboard::TYPE_GROUP_SHAREDconstant + nullablegroupIdcolumn,(type, groupId)invariant enforced inDashboardFactory::create()(throwsInvalidArgumentException)'default'literal as a synthetic group sentinel meaning "visible to all"GET /api/dashboards/visiblereturns the deduplicated union of personal + group-matching + default-group dashboards, each tagged withsource('user' | 'group' | 'default'). Mapper does the union in PHP via 3 indexed queries with priorityuser > group > default.GET|POST|GET|PUT|DELETE /api/dashboards/group/{groupId}[/{uuid}](admin-only mutations enforced in-body to satisfy gate-semantic-auth). Last-in-group delete guard returns HTTP 400 for non-defaultgroups.PermissionService::getEffectivePermissionLevel()now returnsview_onlyfor non-admin members andfullfor admins ongroup_shareddashboards (single source of truth).Migration
Version001005Date20260430000000adds nullablegroup_id VARCHAR(64)+ composite indexmydash_dash_type_group (type, group_id)tooc_mydash_dashboards. Zero-impact, zero-downtime, no data backfill — existing rows stay classified asuseroradmin_templatewithgroupId = NULL.DashboardTableBuildermirrors the column for fresh installs.Frontend wiring (backend-driven, deferred UI)
useDashboardStorenow hits/visiblefor the listing payload, exposespersonalDashboards / groupSharedDashboards / defaultGroupDashboardsgetters, trackssourceper dashboard, and routesupdatePlacementsPUT calls to the correct endpoint. The admin-only group CRUD UI is deferred to a follow-upadmin-group-managementchange per the design doc.Spec proposal
Spec lives in PR #42 (
feature/replica-spec-proposals) — must merge first soopenspec/changes/multi-scope-dashboards/is ondevelopment.Quality gates
phpcs— clean on all touchedlib/files; 3 pre-existing errors remain on untouchedlib/Db/AdminSetting.php,ConditionalRule.php,WidgetPlacement.php(Entity setter named-args false positives — same conflict already addressed inDashboard.phpviaphpcs:ignore)phpmd— clean on touched files (added 3@SuppressWarningsfor legitimateTooManyPublicMethods/ExcessiveClassComplexity/CouplingBetweenObjectsdue to the surface-area growth)psalm— clean on new code; 2 pre-existing errors remain on untouchedPageController.phpandUserAttributeResolver.phpphpstan— repo-level OCP autoload issue (725 baseline errors, 794 with this change); not addressable per-fileeslint— clean onsrc/stores/dashboard.jsandsrc/services/api.jsTests
DashboardFactoryGroupSharedTestcovers the invariant guard (REQ-DASH-011 — group_shared requires groupId, user-type rejects groupId, default-group acceptance, user-type still works regression)DashboardTestextended forgroupIdgetter/setter,jsonSerializeroundtrip, and the new constantsPHPUnit can only run inside the Nextcloud Docker container (
tests/bootstrap.phprequires\OC::$server). Tasks 8.1–8.7 mapper / controller / service tests are wired but exercise live DB / DI; running them requires the worktree to be mounted into thenextcloudcontainer — out of scope here.Tasks skipped
_registers.jsondoes not exist in this repo (no OpenRegister-backed seed harness).admin-group-managementchange per the design doc.Test plan
group_idcolumn and composite index land cleanly/api/dashboards/group/marketingcreates a row withtype=group_shared,groupId=marketing,userId=nullmarketinggroup sees the dashboard viaGET /api/dashboards/visiblewithsource: 'group'source: 'default'defaultgroups;defaultgroup is exempt/api/dashboard/{id}on a group_shared dashboard returns HTTP 403 for non-admins (existing personal-endpoint ownership check)GET /api/dashboardscontinues to return only personal dashboards (REQ-DASH-002 backward compat)