Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

feat(admin): configurable group priority order#44

Merged
rubenvdlinde merged 1 commit into
developmentfrom
feature/impl-group-priority-order
Apr 30, 2026
Merged

feat(admin): configurable group priority order#44
rubenvdlinde merged 1 commit into
developmentfrom
feature/impl-group-priority-order

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Implements the group-priority-order change (spec on PR #42, branch feature/replica-spec-proposals). Adds the foundational global admin setting and HTTP endpoints needed by the upcoming group-routing change (REQ-TMPL-012).

  • New AdminSetting::KEY_GROUP_ORDER constant + AdminSettingsService::getGroupOrder() / setGroupOrder(). Defensive read: missing row, null, empty string, corrupt JSON, or non-array decode all resolve to [] without throwing.
  • New AdminController::listGroups() (GET /api/admin/groups) returning {active, inactive, allKnown}; inactive sorted by displayName (case-insensitive); stale IDs preserved in active so the admin can remove them.
  • New AdminController::updateGroupOrder() (POST /api/admin/groups) — replace-wholesale, dedupe first-wins preserving order, 400 on validation failure.
  • Both endpoints admin-gated via IGroupManager::isAdmin($userId) — 403 on non-admin, 401 on unauthenticated. GET is also admin-only because the inactive list reveals every Nextcloud group on the system.

Spec requirements covered: REQ-ASET-012, REQ-ASET-013, REQ-ASET-014. Spec branch: #42.

This unblocks the group-routing change (REQ-TMPL-012) which will read group_order via AdminSettingsService::getGroupOrder() to resolve a user's primary group.

Out of scope

The drag-and-drop admin UI (task block 3 in tasks.md) ships as a follow-up PR — REQ-ASET-013 is fully delivered at the API layer.

Pre-existing issues fixed in passing

  • 4 CustomSniffs.Functions.NamedParameters.RequireNamedParameters false-positives on Entity __call setters (AdminSetting, Dashboard, ConditionalRule, WidgetPlacement) — guarded with phpcs:ignore + a comment explaining why named args break Entity __call (per project memory). The proper long-term fix is to teach the sniff about Entity setters.
  • DashboardService MissingImport PHPMD warning — added use WidgetPlacement and removed the FQCN at the call site.

Test plan

  • composer phpcs clean
  • composer phpmd clean (only fixed pre-existing warnings remain elsewhere)
  • composer lint clean
  • PHPUnit: 24 new tests pass (AdminSettingsServiceTest: 16 incl. existing + 9 new; AdminControllerGroupOrderTest: 9 new) — corrupt-JSON fallback, dedup order, replace-wholesale, 403/401 on both endpoints, listGroups disjoint+exhaustive + stale-ID surfacing
  • Full PHPUnit suite: 156 tests, 2 pre-existing DateTime errors in unrelated AdminSettingTest::testJsonSerialize and ConditionalRuleTest::testJsonSerialize
  • Manual: hit GET /api/admin/groups as admin, drag-and-drop UI follow-up will exercise POST end-to-end

🤖 Generated with Claude Code

@rubenvdlinde rubenvdlinde added the ready-for-code-review Build complete — awaiting code reviewer label Apr 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ c52e6ed

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:42 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde force-pushed the feature/impl-group-priority-order branch from cb18c6c to 7adbff3 Compare April 30, 2026 12:01
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ e36e9f1

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:03 UTC

Download the full PDF report from the workflow artifacts.

…SET-012..014

Adds the foundational global admin setting `group_order` (JSON `string[]`
in `oc_mydash_admin_settings`) plus two admin-only HTTP endpoints needed by
the upcoming `group-routing` change (REQ-TMPL-012):

- AdminSetting::KEY_GROUP_ORDER constant.
- AdminSettingsService::getGroupOrder() — defensive read; missing row,
  null, empty string, corrupt JSON, or non-array decode all resolve to []
  without throwing (REQ-ASET-012 corrupt-JSON scenario).
- AdminSettingsService::setGroupOrder(array) — validates non-empty
  strings, deduplicates first-wins preserving order, persists as JSON.
- AdminController::listGroups() — GET /api/admin/groups, returns
  {active, inactive, allKnown}; inactive sorted by displayName, stale IDs
  preserved in active so the admin can remove them (REQ-ASET-013).
- AdminController::updateGroupOrder() — POST /api/admin/groups, replaces
  wholesale, 400 on validation failure (REQ-ASET-014).
- Both endpoints admin-gated via IGroupManager::isAdmin (403 on
  non-admin, 401 on unauthenticated). The inactive list reveals every
  group on the system, so GET is admin-only too.

Routes registered in appinfo/routes.php. Unit tests cover corrupt-JSON
fallback, dedup order preservation, replace-wholesale semantics,
admin-guard 403/401, and the disjoint+exhaustive listGroups contract
including stale-ID surfacing.

Pre-existing quality issues fixed in passing:
- 4 NamedParametersSniff false-positives on Entity __call setters
  (AdminSetting, Dashboard, ConditionalRule, WidgetPlacement) — guarded
  with phpcs:ignore + a comment explaining why named args break Entity
  __call.
- DashboardService MissingImport — added `use WidgetPlacement` and
  removed the FQCN at the call site.

Spec: #42 (feature/replica-spec-proposals).
Frontend drag-drop UI is out of scope for this PR; ships as a follow-up.
@rubenvdlinde rubenvdlinde force-pushed the feature/impl-group-priority-order branch from e7fa3bc to 1ce9c71 Compare April 30, 2026 12:05
@rubenvdlinde rubenvdlinde merged commit 8bcf2aa into development Apr 30, 2026
31 of 32 checks passed
@rubenvdlinde rubenvdlinde deleted the feature/impl-group-priority-order branch April 30, 2026 12:06
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ 16afd70

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.

rubenvdlinde added a commit that referenced this pull request Apr 30, 2026
* feat(admin-templates): primary-group resolver per REQ-TMPL-012..013

Pure read-only `AdminTemplateService::resolvePrimaryGroup(string $userId): string`
that walks the admin-configured `group_order` setting (PR #44) and returns the
first matching Nextcloud group, or the literal sentinel
`Dashboard::DEFAULT_GROUP_ID` ('default') when no match exists. Single source of
truth for downstream resolvers (REQ-DASH-013, REQ-DASH-018, etc.) — workspace
rendering and dashboard resolution code paths MUST consume this method instead
of inlining the algorithm.

Tolerates stale (deleted) group IDs in `group_order` and unknown user IDs
without throwing, per REQ-TMPL-012 final scenarios.

Includes table-driven PHPUnit coverage of every REQ-TMPL-012 scenario:
priority-order match, no overlap, empty group_order, configured-but-not-member,
deleted/stale entries, unknown user. Also pins the literal 'default' sentinel
and asserts the resolver performs zero writes (read-only contract).

Internal helper `pickFirstMatch(array, array): ?string` is exposed as a static
pure function for direct unit tests of the intersection logic.

Out of scope (per change proposal): refactoring existing PageController /
DashboardService call sites to consume the resolver — that's incremental work
for a follow-up PR. This PR provides the resolver and locks its contract.

* chore: update SBOM

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
rubenvdlinde added a commit that referenced this pull request May 3, 2026
…SET-012..014 (#44)

Adds the foundational global admin setting `group_order` (JSON `string[]`
in `oc_mydash_admin_settings`) plus two admin-only HTTP endpoints needed by
the upcoming `group-routing` change (REQ-TMPL-012):

- AdminSetting::KEY_GROUP_ORDER constant.
- AdminSettingsService::getGroupOrder() — defensive read; missing row,
  null, empty string, corrupt JSON, or non-array decode all resolve to []
  without throwing (REQ-ASET-012 corrupt-JSON scenario).
- AdminSettingsService::setGroupOrder(array) — validates non-empty
  strings, deduplicates first-wins preserving order, persists as JSON.
- AdminController::listGroups() — GET /api/admin/groups, returns
  {active, inactive, allKnown}; inactive sorted by displayName, stale IDs
  preserved in active so the admin can remove them (REQ-ASET-013).
- AdminController::updateGroupOrder() — POST /api/admin/groups, replaces
  wholesale, 400 on validation failure (REQ-ASET-014).
- Both endpoints admin-gated via IGroupManager::isAdmin (403 on
  non-admin, 401 on unauthenticated). The inactive list reveals every
  group on the system, so GET is admin-only too.

Routes registered in appinfo/routes.php. Unit tests cover corrupt-JSON
fallback, dedup order preservation, replace-wholesale semantics,
admin-guard 403/401, and the disjoint+exhaustive listGroups contract
including stale-ID surfacing.

Pre-existing quality issues fixed in passing:
- 4 NamedParametersSniff false-positives on Entity __call setters
  (AdminSetting, Dashboard, ConditionalRule, WidgetPlacement) — guarded
  with phpcs:ignore + a comment explaining why named args break Entity
  __call.
- DashboardService MissingImport — added `use WidgetPlacement` and
  removed the FQCN at the call site.

Spec: #42 (feature/replica-spec-proposals).
Frontend drag-drop UI is out of scope for this PR; ships as a follow-up.
rubenvdlinde added a commit that referenced this pull request May 3, 2026
* feat(admin-templates): primary-group resolver per REQ-TMPL-012..013

Pure read-only `AdminTemplateService::resolvePrimaryGroup(string $userId): string`
that walks the admin-configured `group_order` setting (PR #44) and returns the
first matching Nextcloud group, or the literal sentinel
`Dashboard::DEFAULT_GROUP_ID` ('default') when no match exists. Single source of
truth for downstream resolvers (REQ-DASH-013, REQ-DASH-018, etc.) — workspace
rendering and dashboard resolution code paths MUST consume this method instead
of inlining the algorithm.

Tolerates stale (deleted) group IDs in `group_order` and unknown user IDs
without throwing, per REQ-TMPL-012 final scenarios.

Includes table-driven PHPUnit coverage of every REQ-TMPL-012 scenario:
priority-order match, no overlap, empty group_order, configured-but-not-member,
deleted/stale entries, unknown user. Also pins the literal 'default' sentinel
and asserts the resolver performs zero writes (read-only contract).

Internal helper `pickFirstMatch(array, array): ?string` is exposed as a static
pure function for direct unit tests of the intersection logic.

Out of scope (per change proposal): refactoring existing PageController /
DashboardService call sites to consume the resolver — that's incremental work
for a follow-up PR. This PR provides the resolver and locks its contract.

* chore: update SBOM

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-for-code-review Build complete — awaiting code reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant