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

Repair conditional rules + admin templates + permission defaults#39

Merged
rubenvdlinde merged 1 commit into
developmentfrom
chore/polish-and-walkthrough-2026-04-29
Apr 30, 2026
Merged

Repair conditional rules + admin templates + permission defaults#39
rubenvdlinde merged 1 commit into
developmentfrom
chore/polish-and-walkthrough-2026-04-29

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

End-to-end walkthrough of all 9 archived OpenSpec features (using fresh test users mydash-marketing-{a,b}, mydash-sales-a, mydash-viewer, mydash-admin) uncovered three runtime bugs in features the spec set claims are implemented. This PR repairs them and clears the supporting quality stack.

Critical bug fixes

1. Conditional visibility — broken at INSERT

  • ConditionalRule::createdAt was typed ?DateTime while sibling entities (Dashboard, Tile, WidgetPlacement) use ?string with 'c' format.
  • DBAL cannot bind a DateTime object without addType('createdAt', 'datetime'); every POST /api/widgets/{id}/rules returned 500 Object of class DateTime could not be converted to string.
  • Same pattern issue in AdminSetting::updatedAt — fixed identically.
  • ConditionalService::addRule updated to call setCreatedAt((new DateTime())->format('c')).

2. Admin templates — Ramsey UUID class missing

  • AdminTemplateService.php:108 and TemplateService.php:143 called Ramsey\Uuid\Uuid::uuid4()->toString().
  • vendor/autoload.php is never required by Application.php, so Ramsey\\Uuid was unreachable at runtime even though composer.json declared it.
  • Both services now use the same random_bytes(16) UUID generator pattern as DashboardFactory::generateUuid().
  • ramsey/uuid removed from composer.json (it had no other consumers and was not actually loaded).
  • POST /api/admin/templates and template-based dashboard provisioning now succeed.

3. Default permission level ignored on auto-provisioned dashboards

  • DashboardService::tryCreateFromTemplate hardcoded Dashboard::PERMISSION_FULL for the user-fallback dashboard.
  • DashboardFactory::create likewise hardcoded PERMISSION_FULL and gridColumns: 12.
  • Both now read KEY_DEFAULT_PERMISSION_LEVEL and KEY_DEFAULT_GRID_COLUMNS from AdminSettingMapper and pass them through. Verified: setting admin default to add_only makes new dashboards add_only.

PHP quality

  • PHPCS: custom NamedParametersSniff now skips Entity-magic accessors (set* / get* / is*) on classes extending OCP\AppFramework\Db\Entity. Entity::__call uses \$args[0], so named-arg calls break those entities. Eliminates 4 false positives without weakening the rule for ordinary methods.
  • PHPMD: add the missing WidgetPlacement import in DashboardService (was inline FQN).
  • Psalm: replace \OC::\$server->get(IManager::class) in PageController with proper DI; replace IUser::getLanguage() (does not exist) in UserAttributeResolver with IConfig::getUserValue(\$userId, 'core', 'lang', 'en').

Frontend warnings

Walkthrough reduced console warnings from ~15 to 1 transient init warning:

  • vue-select option.Icon mismatch: TileEditor and WidgetStyleEditor were passing :label="t('mydash', 'Icon')" to NcSelect, which made vue-select look up a non-existent option.Icon key. Switched to :input-label (visible label) + label="label" (option key).
  • NcSelect missing input-label in AdminSettings: replaced HTML <label> siblings with the proper :input-label prop.
  • NcButton missing aria-label on icon-only buttons in WidgetPicker, WidgetWrapper, DashboardSwitcher, TileEditor, WidgetStyleEditor — added :aria-label.
  • NcTextField missing label in the WidgetPicker search field — added :label.

The 1 remaining warning (NcInputField missing label from the leading-icon variant of the search field) appears to be a Vue reactivity timing artifact: the :label prop is set but the warning fires before props resolve. Out of scope to chase further.

Out of scope (follow-ups, noted)

  • Native prompt() and confirm() for dashboard create / rename / delete. Replacing requires a real dialog component / @nextcloud/dialogs builder; logged as UX follow-up rather than mixed into this PR.
  • PHPStan reports 719 unknown OCP\* errors — that's an environment configuration issue (phpstan.neon doesn't load Nextcloud server source), separate from any real defects.

Test plan

  • composer phpcs — clean
  • composer phpmd — clean
  • composer psalm — 0 errors (21 info-level types, suppressed)
  • phpunit (run inside Docker against installed Nextcloud) — 106 / 106 passing, 293 assertions (added 2 new tests for DashboardFactory permissionLevel + gridColumns parameters)
  • npm run build — succeeds
  • End-to-end API verified for conditional rules, admin templates, prometheus metrics, dashboards, widgets, tiles
  • Permission boundary: non-admin → 403 on /api/admin/templates
  • Default permission level: admin sets add_only → newly auto-provisioned dashboards return permissionLevel: "add_only"

@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ ac53ee0

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 342/342
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-04-30 05:22 UTC

Download the full PDF report from the workflow artifacts.

Walkthrough of all 9 archived OpenSpec features uncovered three critical
runtime bugs in features marked "implemented". This commit restores them
and clears the supporting quality stack.

Critical fixes
--------------
- ConditionalRule.createdAt: switch ?DateTime to ?string ('c' format) to
  match sibling entities. Doctrine DBAL cannot bind a DateTime without
  an explicit addType registration; INSERT now succeeds. Same change in
  AdminSetting.updatedAt.
- AdminTemplateService + TemplateService: drop the ramsey/uuid require
  and inline the random_bytes(16) UUID generator already used by
  DashboardFactory. The ramsey class was never autoloaded at runtime
  (Application.php does not require vendor/autoload.php), so admin
  template creation and template-based dashboard provisioning both
  threw 500.
- DashboardService: stop hardcoding PERMISSION_FULL when auto-creating
  user dashboards. Read defaultPermissionLevel and defaultGridColumns
  from AdminSettingMapper and pass them to DashboardFactory.

Quality
-------
- NamedParametersSniff: skip Entity-magic accessors (set*/get*/is*) on
  classes extending OCP\AppFramework\Db\Entity. Entity::__call uses
  args[0], so named arguments break those calls. Eliminates 4 false
  positives.
- DashboardService: add the missing WidgetPlacement import (PHPMD).
- PageController: replace \OC::server->get(IManager::class) with proper
  DI (Psalm).
- UserAttributeResolver: drop the non-existent IUser::getLanguage() call
  and read 'core/lang' from IConfig instead (Psalm).

Frontend warnings
-----------------
- TileEditor + WidgetStyleEditor: NcSelect was passing 'Icon' as the
  vue-select label key (option.Icon does not exist). Use input-label
  for the visible label and label="label" for the option key.
- AdminSettings: replace HTML <label> + bare NcSelect with input-label
  prop, satisfying NcSelect's accessibility requirement.
- DashboardSwitcher / WidgetPicker / WidgetWrapper / TileEditor /
  WidgetStyleEditor: add aria-label or input-label to icon-only NcButton
  / unlabeled NcTextField / inline color-pick NcButton instances.

Tests: 106 / 106 passing (293 assertions). composer phpcs / phpmd /
psalm clean. End-to-end API verified for conditional rules, admin
templates, prometheus metrics.
@rubenvdlinde rubenvdlinde force-pushed the chore/polish-and-walkthrough-2026-04-29 branch from d9ffbb5 to 7eba48d Compare April 30, 2026 21:07
@rubenvdlinde rubenvdlinde merged commit 0782915 into development Apr 30, 2026
4 of 6 checks passed
@rubenvdlinde rubenvdlinde deleted the chore/polish-and-walkthrough-2026-04-29 branch April 30, 2026 21:07
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/mydash @ 7cb953b

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 342/342
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-04-30 21:09 UTC

Download the full PDF report from the workflow artifacts.

rubenvdlinde added a commit that referenced this pull request May 3, 2026
…#39)

Walkthrough of all 9 archived OpenSpec features uncovered three critical
runtime bugs in features marked "implemented". This commit restores them
and clears the supporting quality stack.

Critical fixes
--------------
- ConditionalRule.createdAt: switch ?DateTime to ?string ('c' format) to
  match sibling entities. Doctrine DBAL cannot bind a DateTime without
  an explicit addType registration; INSERT now succeeds. Same change in
  AdminSetting.updatedAt.
- AdminTemplateService + TemplateService: drop the ramsey/uuid require
  and inline the random_bytes(16) UUID generator already used by
  DashboardFactory. The ramsey class was never autoloaded at runtime
  (Application.php does not require vendor/autoload.php), so admin
  template creation and template-based dashboard provisioning both
  threw 500.
- DashboardService: stop hardcoding PERMISSION_FULL when auto-creating
  user dashboards. Read defaultPermissionLevel and defaultGridColumns
  from AdminSettingMapper and pass them to DashboardFactory.

Quality
-------
- NamedParametersSniff: skip Entity-magic accessors (set*/get*/is*) on
  classes extending OCP\AppFramework\Db\Entity. Entity::__call uses
  args[0], so named arguments break those calls. Eliminates 4 false
  positives.
- DashboardService: add the missing WidgetPlacement import (PHPMD).
- PageController: replace \OC::server->get(IManager::class) with proper
  DI (Psalm).
- UserAttributeResolver: drop the non-existent IUser::getLanguage() call
  and read 'core/lang' from IConfig instead (Psalm).

Frontend warnings
-----------------
- TileEditor + WidgetStyleEditor: NcSelect was passing 'Icon' as the
  vue-select label key (option.Icon does not exist). Use input-label
  for the visible label and label="label" for the option key.
- AdminSettings: replace HTML <label> + bare NcSelect with input-label
  prop, satisfying NcSelect's accessibility requirement.
- DashboardSwitcher / WidgetPicker / WidgetWrapper / TileEditor /
  WidgetStyleEditor: add aria-label or input-label to icon-only NcButton
  / unlabeled NcTextField / inline color-pick NcButton instances.

Tests: 106 / 106 passing (293 assertions). composer phpcs / phpmd /
psalm clean. End-to-end API verified for conditional rules, admin
templates, prometheus metrics.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant