Skip to content

3.9 identity/access maintainability follow-ups (Dashboard state machine, landing redirect, async void handlers) #1113

Description

@sfmskywalker

Summary

Maintainability follow-ups from the retro review of the 3.9 identity and access work. None of these is a functional blocker; each one makes the code harder to read or more fragile than it needs to be.

Checklist (release/3.9.0 @ bd44366)

  • The Dashboard page state machine is overly clever. src/modules/Elsa.Studio.Dashboard/Pages/Index.razor.cs (294 lines) combines:

    • a DataScope enum;
    • a permission-equivalence check (_loadedFor, :189-195);
    • an IFeatureService.IsInitialized default member plus a 3-second TimeProvider timer (:15, :69-77, :140-146) to guess whether companion widgets are "still registering".

    Scenario: the timer exists because some hosts never initialize features (see Features never initialize when DisableAuthorization is true (MainLayout skips InitializeFeaturesAsync) #1065), and a slow backend can still show the welcome panel before the widgets arrive. Fix: have the feature service signal "registration complete" deterministically, fix Features never initialize when DisableAuthorization is true (MainLayout skips InitializeFeaturesAsync) #1065, and remove the timer. Consider extracting the scope and load logic into a small testable service.

  • The Dashboard shows raw exception text. src/modules/Elsa.Studio.Dashboard/Pages/Index.razor.cs:257 sets _message = e.Message, which goes against feat(studio): explain refused API calls instead of showing the raw ApiException #1094 (no raw exception messages in the UI). Fix: use e.ToUserMessage(Localizer) or a fixed message, and log the exception.

  • The PermissionPageGuard landing redirect is now only a fallback. src/framework/Elsa.Studio.Shared/Components/PermissionPageGuard.razor:73-120 relies on a resolution counter, an IsStale predicate and reference equality on UserPermissions. Since feat(dashboard): open the dashboard to every user and gate widgets by permission #1103 the stock Dashboard never triggers it. Fix: simplify it (resolve once per route and permission set), or remove it and document how hosts with a gated / should handle this.

  • async void auth-change handlers.

    • src/framework/Elsa.Studio.Shared/Components/PermissionTrackingComponentBase.cs:61-66
    • src/framework/Elsa.Studio.Shared/Components/NavMenu.razor:59-64

    Scenario: if GetPermissionsAsync or GetNavigationAsync throws during an auth change (for example, JS interop is gone mid sign-out on Blazor Server), the exception is unobserved and can tear down the circuit. Fix: wrap each body in try/catch, log, keep the previous state, and add a test that throws from the permission service on an auth change.

Verification

Code-read for all items. The existing Dashboard (108), Core (158) and Workflows (490) test suites pass at bd44366.

Originating PRs

#1103 and #1091 (Dashboard), #1093 (landing redirect), #1076 (PermissionTrackingComponentBase), #1075 (NavMenu re-resolution).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions