From 5c5aeecc19600ab9e70ebb9c42cb9d24140b1761 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Fri, 31 Jul 2026 02:48:57 +0200 Subject: [PATCH] Add governed view revision resolution --- src/govoplan_views/backend/capabilities.py | 4 + src/govoplan_views/backend/router.py | 26 ++ src/govoplan_views/backend/schemas.py | 6 + src/govoplan_views/backend/service.py | 289 ++++++++++++++++++--- tests/test_views.py | 176 ++++++++++++- webui/package.json | 6 +- webui/src/api/views.ts | 24 ++ webui/src/module.ts | 8 +- 8 files changed, 496 insertions(+), 43 deletions(-) diff --git a/src/govoplan_views/backend/capabilities.py b/src/govoplan_views/backend/capabilities.py index adfdb44..26d1dcf 100644 --- a/src/govoplan_views/backend/capabilities.py +++ b/src/govoplan_views/backend/capabilities.py @@ -19,6 +19,8 @@ class ViewsResolverCapability(ViewResolver): account_id: str, group_ids: Iterable[str] = (), workflow_view_id: str | None = None, + workflow_revision_id: str | None = None, + workflow_surface_ids: Iterable[str] = (), ) -> EffectiveView: state = resolve_effective_view( session, @@ -27,6 +29,8 @@ class ViewsResolverCapability(ViewResolver): group_ids=group_ids, catalogue=self._registry.view_surfaces(), workflow_view_id=workflow_view_id, + workflow_revision_id=workflow_revision_id, + workflow_surface_ids=workflow_surface_ids, ) return state.effective diff --git a/src/govoplan_views/backend/router.py b/src/govoplan_views/backend/router.py index 0ff63f5..973df79 100644 --- a/src/govoplan_views/backend/router.py +++ b/src/govoplan_views/backend/router.py @@ -45,6 +45,7 @@ from govoplan_views.backend.schemas import ( ViewSelectionRequest, ViewSurfaceCatalogueResponse, ViewSurfaceResponse, + WorkflowViewResolutionRequest, ) from govoplan_views.backend.service import ( EffectiveViewState, @@ -493,6 +494,31 @@ def api_effective_view( ) +@router.post("/effective/workflow", response_model=EffectiveViewResponse) +def api_workflow_effective_view( + payload: WorkflowViewResolutionRequest, + session: Session = Depends(get_session), + principal: ApiPrincipal = Depends(get_api_principal), +) -> EffectiveViewResponse: + _require_any_scope( + principal, + SELECTION_READ_SCOPE, + SELECTION_WRITE_SCOPE, + ) + return _effective_response( + resolve_effective_view( + session, + tenant_id=principal.tenant_id, + account_id=principal.account_id, + group_ids=principal.group_ids, + catalogue=_catalogue(), + workflow_view_id=payload.view_id, + workflow_revision_id=payload.revision_id, + workflow_surface_ids=payload.visible_surface_ids, + ) + ) + + @router.put("/selection", response_model=EffectiveViewResponse) def api_select_view( payload: ViewSelectionRequest, diff --git a/src/govoplan_views/backend/schemas.py b/src/govoplan_views/backend/schemas.py index 652f9b1..b2d2b21 100644 --- a/src/govoplan_views/backend/schemas.py +++ b/src/govoplan_views/backend/schemas.py @@ -185,3 +185,9 @@ class EffectiveViewResponse(BaseModel): class ViewSelectionRequest(BaseModel): view_id: str | None = Field(default=None, max_length=36) + + +class WorkflowViewResolutionRequest(BaseModel): + view_id: str = Field(min_length=1, max_length=36) + revision_id: str | None = Field(default=None, max_length=36) + visible_surface_ids: list[str] = Field(default_factory=list, max_length=1000) diff --git a/src/govoplan_views/backend/service.py b/src/govoplan_views/backend/service.py index be91397..9cf3d73 100644 --- a/src/govoplan_views/backend/service.py +++ b/src/govoplan_views/backend/service.py @@ -95,6 +95,9 @@ class ViewSelection: locked: bool provenance: tuple[dict[str, object], ...] diagnostics: tuple[ViewDiagnostic, ...] = () + visible_surface_ids: frozenset[str] | None = None + surface_ceiling_ids: frozenset[str] | None = None + fallback: ViewSelection | None = None def _now() -> datetime: @@ -1049,51 +1052,82 @@ def _select_effective_assignment( preference: ViewPreference | None, account_id: str, workflow_view_id: str | None, + initial_diagnostics: Iterable[ViewDiagnostic] = (), + fallback: ViewSelection | None = None, ) -> ViewSelection: required = [ pair for pair in assignments if pair[0].mode == "required" ] - if required: - selected = max( + required_selection = ( + max( required, key=lambda pair: _assignment_order(pair[0]), ) + if required + else None + ) + diagnostics = list(initial_diagnostics) + if workflow_view_id is not None: + selected = options.get(workflow_view_id) + if selected is not None: + provenance: list[dict[str, object]] = [] + if required_selection is not None: + provenance.append( + _assignment_provenance( + "required_assignment_ceiling", + required_selection[0], + ( + "Required View assignment " + f"{required_selection[0].id} limits the workflow View" + ), + ) + ) + provenance.append( + _assignment_provenance( + "workflow_selection", + selected[0], + f"Workflow selected View {workflow_view_id}", + ) + ) + return ViewSelection( + selected=selected, + locked=required_selection is not None, + provenance=tuple(provenance), + diagnostics=tuple(diagnostics), + surface_ceiling_ids=( + frozenset(required_selection[1].visible_surface_ids) + if required_selection is not None + else None + ), + fallback=fallback, + ) + if not any( + item.code == "view.workflow_revision_unavailable" + for item in diagnostics + ): + diagnostics.append( + ViewDiagnostic( + severity="warning", + code="view.workflow_selection_unavailable", + message=( + "The workflow-selected View is not available to this " + "account. Normal View selection was used." + ), + ) + ) + + if required_selection is not None: return ViewSelection( - selected=selected, + selected=required_selection, locked=True, provenance=( _assignment_provenance( "required_assignment", - selected[0], - f"Required View assignment {selected[0].id}", + required_selection[0], + f"Required View assignment {required_selection[0].id}", ), ), - ) - - diagnostics: list[ViewDiagnostic] = [] - if workflow_view_id is not None: - selected = options.get(workflow_view_id) - if selected is not None: - return ViewSelection( - selected=selected, - locked=False, - provenance=( - _assignment_provenance( - "workflow_selection", - selected[0], - f"Workflow selected View {workflow_view_id}", - ), - ), - ) - diagnostics.append( - ViewDiagnostic( - severity="warning", - code="view.workflow_selection_unavailable", - message=( - "The workflow-selected View is not available to this " - "account. Normal View selection was used." - ), - ) + diagnostics=tuple(diagnostics), ) if preference and preference.selection_kind == "none": @@ -1162,6 +1196,7 @@ def _evaluate_selected_surfaces( selection: ViewSelection, *, catalogue: tuple[ViewSurface, ...] | None, + workflow_surface_ids: Iterable[str] = (), ) -> ViewSelection: if selection.selected is None or catalogue is None: return selection @@ -1172,6 +1207,95 @@ def _evaluate_selected_surfaces( ) diagnostics = list(selection.diagnostics) provenance = list(selection.provenance) + visible_surface_ids = set(revision.visible_surface_ids) + if selection.surface_ceiling_ids is not None: + visible_surface_ids.intersection_update(selection.surface_ceiling_ids) + requested_overlay = { + str(surface_id).strip() + for surface_id in workflow_surface_ids + if str(surface_id).strip() + } + if requested_overlay: + unknown_overlay_ids = sorted(requested_overlay - set(known_surfaces)) + bounded_overlay_ids = requested_overlay & visible_surface_ids + unavailable_overlay_ids = sorted( + requested_overlay + - set(unknown_overlay_ids) + - bounded_overlay_ids + ) + protected_surface_ids = { + surface.id + for surface in catalogue + if surface.required and surface.id in visible_surface_ids + } + if selection.locked: + ceiling_assignment = ( + selection.fallback.selected[0] + if selection.fallback is not None + and selection.fallback.selected is not None + and selection.fallback.locked + else assignment + ) + try: + protected_surface_ids.update( + lockout_required_surface_ids( + catalogue, + assignment_scope_type=ceiling_assignment.scope_type, + ) + & visible_surface_ids + ) + except ViewsValidationError: + # The lockout diagnostic below handles an incomplete contract. + pass + bounded_overlay_ids.update(protected_surface_ids) + pending = list(bounded_overlay_ids) + while pending: + surface = known_surfaces.get(pending.pop()) + if ( + surface is not None + and surface.parent_id + and surface.parent_id in visible_surface_ids + and surface.parent_id not in bounded_overlay_ids + ): + bounded_overlay_ids.add(surface.parent_id) + pending.append(surface.parent_id) + visible_surface_ids = bounded_overlay_ids + provenance.append( + { + "source": "workflow_step_overlay", + "scope_type": "workflow", + "scope_id": None, + "detail": ( + "Workflow step narrowed the active View to " + f"{len(visible_surface_ids)} surfaces" + ), + } + ) + if unknown_overlay_ids: + diagnostics.append( + ViewDiagnostic( + severity="warning", + code="view.workflow_overlay_unknown_surfaces", + message=( + "The workflow step references surfaces that are not " + "announced by the active module graph." + ), + surface_ids=tuple(unknown_overlay_ids), + ) + ) + if unavailable_overlay_ids: + diagnostics.append( + ViewDiagnostic( + severity="warning", + code="view.workflow_overlay_bounded", + message=( + "The workflow step requested surfaces outside its " + "effective View or an administrator-required ceiling. " + "Those surfaces remain hidden." + ), + surface_ids=tuple(unavailable_overlay_ids), + ) + ) if stale_ids: diagnostics.append( ViewDiagnostic( @@ -1186,9 +1310,16 @@ def _evaluate_selected_surfaces( ) if selection.locked: + ceiling_assignment, ceiling_revision = ( + selection.fallback.selected + if selection.fallback is not None + and selection.fallback.selected is not None + and selection.fallback.locked + else selection.selected + ) lockout_diagnostic = _required_lockout_diagnostic( - assignment, - revision, + ceiling_assignment, + ceiling_revision, catalogue=catalogue, ) if lockout_diagnostic is not None: @@ -1211,7 +1342,7 @@ def _evaluate_selected_surfaces( known_visible = [ known_surfaces[surface_id] - for surface_id in revision.visible_surface_ids + for surface_id in visible_surface_ids if surface_id in known_surfaces ] if ( @@ -1234,6 +1365,23 @@ def _evaluate_selected_surfaces( detail=f"Invalid View revision {revision.id}", ) ) + if selection.fallback is not None: + return ViewSelection( + selected=selection.fallback.selected, + locked=selection.fallback.locked, + provenance=( + *selection.fallback.provenance, + { + "source": "invalid_view_fallback", + "scope_type": "workflow", + "scope_id": None, + "detail": "Workflow View context was not reachable", + }, + ), + diagnostics=tuple(diagnostics), + visible_surface_ids=selection.fallback.visible_surface_ids, + surface_ceiling_ids=selection.fallback.surface_ceiling_ids, + ) return ViewSelection( selected=None, locked=False, @@ -1245,6 +1393,9 @@ def _evaluate_selected_surfaces( locked=selection.locked, provenance=tuple(provenance), diagnostics=tuple(diagnostics), + visible_surface_ids=frozenset(visible_surface_ids), + surface_ceiling_ids=selection.surface_ceiling_ids, + fallback=selection.fallback, ) @@ -1310,7 +1461,11 @@ def _effective_view_from_selection(selection: ViewSelection) -> EffectiveView: view_id=assignment.definition.id, revision_id=revision.id, name=assignment.definition.name, - visible_surface_ids=frozenset(revision.visible_surface_ids), + visible_surface_ids=( + selection.visible_surface_ids + if selection.visible_surface_ids is not None + else frozenset(revision.visible_surface_ids) + ), locked=selection.locked, provenance=selection.provenance, ) @@ -1322,6 +1477,8 @@ def _resolution_invalidation_token( preference: ViewPreference | None, catalogue: tuple[ViewSurface, ...] | None, workflow_view_id: str | None, + workflow_revision_id: str | None, + workflow_surface_ids: Iterable[str], ) -> str: payload = { "assignments": [ @@ -1361,6 +1518,10 @@ def _resolution_invalidation_token( else None ), "workflow_view_id": workflow_view_id, + "workflow_revision_id": workflow_revision_id, + "workflow_surface_ids": sorted( + str(surface_id) for surface_id in workflow_surface_ids + ), } encoded = json.dumps( payload, @@ -1378,8 +1539,17 @@ def resolve_effective_view( group_ids: Iterable[str] = (), catalogue: Iterable[ViewSurface] | None = None, workflow_view_id: str | None = None, + workflow_revision_id: str | None = None, + workflow_surface_ids: Iterable[str] = (), ) -> EffectiveViewState: groups = frozenset(str(group_id) for group_id in group_ids) + workflow_surfaces = tuple( + dict.fromkeys( + str(surface_id).strip() + for surface_id in workflow_surface_ids + if str(surface_id).strip() + ) + ) if len(groups) > MAX_EFFECTIVE_VIEW_GROUPS: raise ViewsValidationError( "Too many group memberships were supplied for View resolution" @@ -1424,21 +1594,66 @@ def resolve_effective_view( candidates, ) options = _option_assignments(resolved_assignments) + workflow_options = dict(options) + workflow_diagnostics: tuple[ViewDiagnostic, ...] = () + if workflow_view_id is not None and workflow_revision_id is not None: + option = options.get(workflow_view_id) + pinned_revision = ( + session.query(ViewRevision) + .filter( + ViewRevision.id == workflow_revision_id, + ViewRevision.definition_id == workflow_view_id, + ) + .first() + if option is not None + else None + ) + if option is not None and pinned_revision is not None: + workflow_options[workflow_view_id] = ( + option[0], + pinned_revision, + ) + elif option is not None: + workflow_options.pop(workflow_view_id, None) + workflow_diagnostics = ( + ViewDiagnostic( + severity="warning", + code="view.workflow_revision_unavailable", + message=( + "The workflow-pinned View revision is missing or does " + "not belong to the selected View. Normal View selection " + "was used." + ), + ), + ) preference = _preference( session, tenant_id=tenant_id, account_id=account_id, ) + baseline = _evaluate_selected_surfaces( + _select_effective_assignment( + resolved_assignments, + options, + preference=preference, + account_id=account_id, + workflow_view_id=None, + ), + catalogue=current_catalogue, + ) selection = _select_effective_assignment( resolved_assignments, - options, + workflow_options, preference=preference, account_id=account_id, workflow_view_id=workflow_view_id, + initial_diagnostics=workflow_diagnostics, + fallback=baseline if workflow_view_id is not None else None, ) evaluated = _evaluate_selected_surfaces( selection, catalogue=current_catalogue, + workflow_surface_ids=workflow_surfaces, ) return EffectiveViewState( effective=_effective_view_from_selection(evaluated), @@ -1449,6 +1664,8 @@ def resolve_effective_view( preference=preference, catalogue=current_catalogue, workflow_view_id=workflow_view_id, + workflow_revision_id=workflow_revision_id, + workflow_surface_ids=workflow_surfaces, ), ) diff --git a/tests/test_views.py b/tests/test_views.py index c4d51a1..540b6af 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -398,7 +398,7 @@ class ViewsServiceTests(unittest.TestCase): state.effective.view_id, ) - def test_required_then_workflow_then_user_and_default_precedence(self) -> None: + def test_required_assignment_limits_workflow_selection(self) -> None: default = self.create_published_definition(name="Default") workflow = self.create_published_definition(name="Workflow") required = self.create_published_definition( @@ -443,12 +443,182 @@ class ViewsServiceTests(unittest.TestCase): catalogue=self.catalogue, workflow_view_id=workflow.id, ) - self.assertEqual(required.id, required_state.effective.view_id) + workflow_revision = get_revision( + self.session, + definition_id=workflow.id, + ) + required_revision = get_revision( + self.session, + definition_id=required.id, + ) + self.assertEqual(workflow.id, required_state.effective.view_id) self.assertTrue(required_state.effective.locked) self.assertEqual( - "required_assignment", + "required_assignment_ceiling", required_state.effective.provenance[0]["source"], ) + self.assertEqual( + set(workflow_revision.visible_surface_ids) + & set(required_revision.visible_surface_ids), + set(required_state.effective.visible_surface_ids), + ) + + def test_workflow_can_pin_a_historical_view_revision(self) -> None: + definition = self.create_published_definition(name="Workflow") + pinned_revision = get_revision( + self.session, + definition_id=definition.id, + ) + self.assign( + definition, + scope_type="tenant", + scope_id=None, + mode="available", + ) + current_revision = create_revision( + self.session, + definition, + visible_surface_ids=["access.nav.admin", "access.route.admin"], + catalogue=self.catalogue, + actor_id="account-admin", + ) + publish_revision( + self.session, + definition, + current_revision, + catalogue=self.catalogue, + actor_id="account-admin", + ) + + state = resolve_effective_view( + self.session, + tenant_id="tenant-1", + account_id="account-user", + catalogue=self.catalogue, + workflow_view_id=definition.id, + workflow_revision_id=pinned_revision.id, + ) + + self.assertEqual(definition.id, state.effective.view_id) + self.assertEqual(pinned_revision.id, state.effective.revision_id) + self.assertIn("files.route.files", state.effective.visible_surface_ids) + self.assertNotIn("access.route.admin", state.effective.visible_surface_ids) + + def test_workflow_step_overlay_only_narrows_the_selected_view(self) -> None: + definition = self.create_published_definition( + name="Workflow", + visible_surface_ids=[ + "access.nav.admin", + "access.route.admin", + "files.nav.files", + "files.route.files", + ], + ) + self.assign( + definition, + scope_type="tenant", + scope_id=None, + mode="available", + ) + + state = resolve_effective_view( + self.session, + tenant_id="tenant-1", + account_id="account-user", + catalogue=self.catalogue, + workflow_view_id=definition.id, + workflow_surface_ids=( + "files.nav.files", + "files.route.files", + "unknown.route", + ), + ) + + self.assertIn("files.module", state.effective.visible_surface_ids) + self.assertIn("files.nav.files", state.effective.visible_surface_ids) + self.assertIn("files.route.files", state.effective.visible_surface_ids) + self.assertIn("views.selector", state.effective.visible_surface_ids) + self.assertNotIn("access.route.admin", state.effective.visible_surface_ids) + self.assertIn( + "view.workflow_overlay_unknown_surfaces", + {diagnostic.code for diagnostic in state.diagnostics}, + ) + + def test_missing_workflow_revision_falls_back_to_normal_selection(self) -> None: + default = self.create_published_definition(name="Default") + workflow = self.create_published_definition(name="Workflow") + self.assign( + default, + scope_type="tenant", + scope_id=None, + mode="default", + ) + self.assign( + workflow, + scope_type="tenant", + scope_id=None, + mode="available", + ) + + state = resolve_effective_view( + self.session, + tenant_id="tenant-1", + account_id="account-user", + catalogue=self.catalogue, + workflow_view_id=workflow.id, + workflow_revision_id="missing-revision", + ) + + self.assertEqual(default.id, state.effective.view_id) + self.assertIn( + "view.workflow_revision_unavailable", + {diagnostic.code for diagnostic in state.diagnostics}, + ) + + def test_workflow_view_falls_back_after_group_authorization_is_lost( + self, + ) -> None: + default = self.create_published_definition(name="Default") + workflow = self.create_published_definition(name="Group workflow") + self.assign( + default, + scope_type="tenant", + scope_id=None, + mode="default", + ) + self.assign( + workflow, + scope_type="group", + scope_id="group-1", + mode="available", + ) + + authorized = resolve_effective_view( + self.session, + tenant_id="tenant-1", + account_id="account-user", + group_ids=("group-1",), + catalogue=self.catalogue, + workflow_view_id=workflow.id, + ) + after_membership_removal = resolve_effective_view( + self.session, + tenant_id="tenant-1", + account_id="account-user", + group_ids=(), + catalogue=self.catalogue, + workflow_view_id=workflow.id, + ) + + self.assertEqual(workflow.id, authorized.effective.view_id) + self.assertEqual(default.id, after_membership_removal.effective.view_id) + self.assertIn( + "view.workflow_selection_unavailable", + { + diagnostic.code + for diagnostic in after_membership_removal.diagnostics + }, + ) def test_resolution_invalidation_token_tracks_inputs(self) -> None: definition = self.create_published_definition(name="Files") diff --git a/webui/package.json b/webui/package.json index 3f03c93..df74075 100644 --- a/webui/package.json +++ b/webui/package.json @@ -16,9 +16,9 @@ "peerDependencies": { "@govoplan/core-webui": "^0.1.14", "lucide-react": "^1.23.0", - "react": "^19.0.0", - "react-dom": "^19.0.0", - "react-router-dom": ">=7.18.2 <8" + "react": ">=19.2.7 <20", + "react-dom": ">=19.2.7 <20", + "react-router": ">=8.3.0 <9" }, "peerDependenciesMeta": { "@govoplan/core-webui": { diff --git a/webui/src/api/views.ts b/webui/src/api/views.ts index b0ae156..842e686 100644 --- a/webui/src/api/views.ts +++ b/webui/src/api/views.ts @@ -163,6 +163,30 @@ export async function selectEffectiveView( ); } +export async function resolveWorkflowView( + settings: ApiSettings, + context: { + viewId: string; + revisionId?: string | null; + visibleSurfaceIds?: string[]; + } +): Promise { + return projection( + await apiFetch( + settings, + "/api/v1/views/effective/workflow", + { + method: "POST", + ...jsonBody({ + view_id: context.viewId, + revision_id: context.revisionId ?? null, + visible_surface_ids: context.visibleSurfaceIds ?? [] + }) + } + ) + ); +} + export async function fetchViewDefinitions( settings: ApiSettings, scopeType: ViewScopeType, diff --git a/webui/src/module.ts b/webui/src/module.ts index b47087e..ac88239 100644 --- a/webui/src/module.ts +++ b/webui/src/module.ts @@ -7,7 +7,11 @@ import { type ViewsRuntimeUiCapability } from "@govoplan/core-webui"; import ViewSelector from "./components/ViewSelector"; -import { fetchEffectiveView, selectEffectiveView } from "./api/views"; +import { + fetchEffectiveView, + resolveWorkflowView, + selectEffectiveView +} from "./api/views"; import "./styles/views.css"; @@ -68,6 +72,8 @@ const viewsAdminSections: AdminSectionsUiCapability = { const viewsRuntime: ViewsRuntimeUiCapability = { loadEffectiveView: (settings) => fetchEffectiveView(settings), activateView: (settings, viewId) => selectEffectiveView(settings, viewId), + resolveWorkflowView: (settings, context) => + resolveWorkflowView(settings, context), Selector: ViewSelector };