From 48438dbba7050e96e4bf96d5f0c06523e78d33fb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 21:43:32 +0000 Subject: [PATCH] fix scope migration and keep link-shared views reachable Existing property definitions become public on the root location, existing task presets become private and formerly link-shared saved views keep a link-only SHARED visibility instead of turning private. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UpemKsko6UowS3SmrN4TdQ --- backend/api/inputs.py | 1 + backend/api/resolvers/saved_view.py | 6 +- backend/api/services/scope.py | 19 +++- .../versions/add_scope_visibility.py | 79 +++++++++++--- backend/schema.graphql | 1 + .../integration/test_authorization_scoping.py | 102 ++++++++++++++++++ docs/VIEWS_ARCHITECTURE.md | 7 +- web/api/gql/types.ts | 3 +- web/components/locations/ScopeChip.tsx | 10 +- .../locations/ScopeVisibilityField.tsx | 70 ++++++++---- web/components/views/SaveViewDialog.tsx | 2 +- web/i18n/translations.ts | 13 +++ web/locales/de-DE.arb | 2 + web/locales/en-US.arb | 2 + web/locales/es-ES.arb | 2 + web/locales/fr-FR.arb | 2 + web/locales/nl-NL.arb | 2 + web/locales/pt-BR.arb | 2 + web/pages/settings/views.tsx | 2 +- web/schema.graphql | 1 + 20 files changed, 277 insertions(+), 51 deletions(-) diff --git a/backend/api/inputs.py b/backend/api/inputs.py index 45f1d4cd..bfafa60d 100644 --- a/backend/api/inputs.py +++ b/backend/api/inputs.py @@ -154,6 +154,7 @@ class UpdateLocationNodeInput: @strawberry.enum class ScopeVisibility(Enum): PRIVATE = "private" + SHARED = "shared" PUBLIC = "public" diff --git a/backend/api/resolvers/saved_view.py b/backend/api/resolvers/saved_view.py index 62e52289..e7579f47 100644 --- a/backend/api/resolvers/saved_view.py +++ b/backend/api/resolvers/saved_view.py @@ -86,7 +86,7 @@ async def create_saved_view( ) -> SavedViewType: user = _require_user(info) visibility, location_id = await resolve_scope_input( - info, user, data.visibility, data.location_id + info, user, data.visibility, data.location_id, allow_shared=True ) for blob, label in ( (data.filter_definition, "filter_definition"), @@ -158,7 +158,9 @@ async def update_saved_view( row.related_parameters = _validated_json( data.related_parameters, "related_parameters" ) - await apply_scope_update(info, user, row, data.visibility, data.location_id) + await apply_scope_update( + info, user, row, data.visibility, data.location_id, allow_shared=True + ) await db.commit() await db.refresh(row) diff --git a/backend/api/services/scope.py b/backend/api/services/scope.py index 48de4fe9..98cbd163 100644 --- a/backend/api/services/scope.py +++ b/backend/api/services/scope.py @@ -13,7 +13,9 @@ from database import models PRIVATE = ScopeVisibility.PRIVATE.value +SHARED = ScopeVisibility.SHARED.value PUBLIC = ScopeVisibility.PUBLIC.value +LINK_READABLE = (PUBLIC, SHARED) def normalize_root_location_ids( @@ -47,7 +49,7 @@ async def can_read_scoped(info: Info, user: models.User | None, row: Any) -> boo owner_user_id = getattr(row, "owner_user_id", None) if owner_user_id is not None and owner_user_id == user.id: return True - if row.visibility != PUBLIC: + if row.visibility not in LINK_READABLE: return False if row.location_id is None: return True @@ -61,9 +63,16 @@ async def resolve_scope_input( user: models.User, visibility: ScopeVisibility, location_id: strawberry.ID | str | None, + *, + allow_shared: bool = False, ) -> tuple[str, str | None]: if visibility == ScopeVisibility.PRIVATE: return PRIVATE, None + if visibility == ScopeVisibility.SHARED and not allow_shared: + raise GraphQLError( + "Link sharing is only available for saved views.", + extensions={"code": "BAD_REQUEST"}, + ) auth_service = AuthorizationService(info.context.db) if location_id is None: default_location_id = await auth_service.default_scope_location_id( @@ -74,12 +83,12 @@ async def resolve_scope_input( "A location is required to share this entry.", extensions={"code": "BAD_REQUEST"}, ) - return PUBLIC, default_location_id + return visibility.value, default_location_id if not await auth_service.can_access_location( user, str(location_id), info.context ): raise_forbidden() - return PUBLIC, str(location_id) + return visibility.value, str(location_id) async def apply_scope_update( @@ -88,6 +97,8 @@ async def apply_scope_update( row: Any, visibility: ScopeVisibility | None, location_id: strawberry.ID | None, + *, + allow_shared: bool = False, ) -> None: if visibility is None and location_id is None: return @@ -98,7 +109,7 @@ async def apply_scope_update( str(location_id) if location_id is not None else row.location_id ) row.visibility, row.location_id = await resolve_scope_input( - info, user, target_visibility, target_location_id + info, user, target_visibility, target_location_id, allow_shared=allow_shared ) diff --git a/backend/database/migrations/versions/add_scope_visibility.py b/backend/database/migrations/versions/add_scope_visibility.py index 60d0e5d9..b3236cc9 100644 --- a/backend/database/migrations/versions/add_scope_visibility.py +++ b/backend/database/migrations/versions/add_scope_visibility.py @@ -1,5 +1,10 @@ """Scope presets, saved views and property definitions to a location node. +Existing property definitions become public on the root location so everyone +keeps seeing them. Existing link-shared saved views stay reachable by link +(``shared``) without being listed publicly, everything else becomes private. +Existing task presets become private. + Revision ID: add_scope_visibility Revises: add_scope_location_prop_view Create Date: 2026-09-02 12:00:00.000000 @@ -15,11 +20,30 @@ branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None +PRIVATE = "private" +SHARED = "shared" +PUBLIC = "public" +LEGACY_LINK_SHARED = "link_shared" + def _root_location_id(conn) -> str | None: rows = conn.execute( sa.text( - "SELECT id FROM location_nodes WHERE parent_id IS NULL ORDER BY title, id" + """ + WITH RECURSIVE subtree(root_id, id) AS ( + SELECT id, id FROM location_nodes WHERE parent_id IS NULL + UNION ALL + SELECT subtree.root_id, child.id + FROM location_nodes AS child + JOIN subtree ON child.parent_id = subtree.id + ) + SELECT root.id, COUNT(subtree.id) AS size + FROM location_nodes AS root + JOIN subtree ON subtree.root_id = root.id + WHERE root.parent_id IS NULL + GROUP BY root.id, root.title + ORDER BY size DESC, root.title, root.id + """ ) ).fetchall() return rows[0][0] if rows else None @@ -35,7 +59,7 @@ def upgrade() -> None: "visibility", sa.String(length=16), nullable=False, - server_default="private", + server_default=PRIVATE, ) ) batch_op.add_column(sa.Column("owner_user_id", sa.String(), nullable=True)) @@ -45,7 +69,10 @@ def upgrade() -> None: ["owner_user_id"], ["id"], ) - conn.execute(sa.text("UPDATE property_definitions SET visibility = 'public'")) + conn.execute( + sa.text("UPDATE property_definitions SET visibility = :visibility"), + {"visibility": PUBLIC}, + ) if root_id: conn.execute( sa.text( @@ -56,8 +83,27 @@ def upgrade() -> None: ) conn.execute( - sa.text("UPDATE saved_views SET visibility = 'private', location_id = NULL") + sa.text( + "UPDATE saved_views SET visibility = :shared " + "WHERE visibility = :link_shared" + ), + {"shared": SHARED, "link_shared": LEGACY_LINK_SHARED}, ) + conn.execute( + sa.text( + "UPDATE saved_views SET visibility = :private, location_id = NULL " + "WHERE visibility <> :shared" + ), + {"private": PRIVATE, "shared": SHARED}, + ) + if root_id: + conn.execute( + sa.text( + "UPDATE saved_views SET location_id = :root_id " + "WHERE visibility = :shared AND location_id IS NULL" + ), + {"root_id": root_id, "shared": SHARED}, + ) with op.batch_alter_table("task_presets") as batch_op: batch_op.add_column( @@ -65,7 +111,7 @@ def upgrade() -> None: "visibility", sa.String(length=16), nullable=False, - server_default="private", + server_default=PRIVATE, ) ) batch_op.add_column(sa.Column("location_id", sa.String(), nullable=True)) @@ -75,14 +121,10 @@ def upgrade() -> None: ["location_id"], ["id"], ) - if root_id: - conn.execute( - sa.text( - "UPDATE task_presets SET visibility = 'public', location_id = :root_id " - "WHERE owner_user_id IS NULL" - ), - {"root_id": root_id}, - ) + conn.execute( + sa.text("UPDATE task_presets SET visibility = :private, location_id = NULL"), + {"private": PRIVATE}, + ) with op.batch_alter_table("task_presets") as batch_op: batch_op.drop_column("scope") @@ -100,13 +142,22 @@ def downgrade() -> None: ) ) conn.execute( - sa.text("UPDATE task_presets SET scope = 'GLOBAL' WHERE visibility = 'public'") + sa.text("UPDATE task_presets SET scope = 'GLOBAL' WHERE visibility = :public"), + {"public": PUBLIC}, ) with op.batch_alter_table("task_presets") as batch_op: batch_op.drop_constraint("fk_task_presets_location_id", type_="foreignkey") batch_op.drop_column("location_id") batch_op.drop_column("visibility") + conn.execute( + sa.text( + "UPDATE saved_views SET visibility = :link_shared " + "WHERE visibility IN (:shared, :public)" + ), + {"link_shared": LEGACY_LINK_SHARED, "shared": SHARED, "public": PUBLIC}, + ) + with op.batch_alter_table("property_definitions") as batch_op: batch_op.drop_constraint( "fk_property_definitions_owner_user_id", type_="foreignkey" diff --git a/backend/schema.graphql b/backend/schema.graphql index 9ebd9d03..eca41c46 100644 --- a/backend/schema.graphql +++ b/backend/schema.graphql @@ -403,6 +403,7 @@ enum SavedViewEntityType { enum ScopeVisibility { PRIVATE + SHARED PUBLIC } diff --git a/backend/tests/integration/test_authorization_scoping.py b/backend/tests/integration/test_authorization_scoping.py index 3f9a4a83..069c3197 100644 --- a/backend/tests/integration/test_authorization_scoping.py +++ b/backend/tests/integration/test_authorization_scoping.py @@ -423,6 +423,108 @@ async def test_private_view_is_owner_only_and_needs_no_location( ) +@pytest.mark.asyncio +async def test_shared_view_is_reachable_by_link_but_not_listed( + two_tenants, db_session +): + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + info3 = MockInfo(db_session, two_tenants["user3"]) + + view = await SavedViewMutation().create_saved_view( + info1, + CreateSavedViewInput( + name="Shared by link", + base_entity_type=SavedViewEntityType.PATIENT, + filter_definition="{}", + sort_definition="{}", + parameters="{}", + visibility=ScopeVisibility.SHARED, + location_id="loc-a", + ), + ) + assert view.visibility == ScopeVisibility.SHARED + assert view.location_id == "loc-a" + assert view.id in [v.id for v in await SavedViewQuery().my_saved_views(info1)] + assert view.id not in [v.id for v in await SavedViewQuery().my_saved_views(info3)] + assert (await SavedViewQuery().saved_view(info3, view.id)) is not None + with pytest.raises(GraphQLError): + await SavedViewQuery().saved_view(info2, view.id) + + copy = await SavedViewMutation().duplicate_saved_view(info3, view.id, "Copy") + assert copy.visibility == ScopeVisibility.PRIVATE + assert copy.owner_user_id == "user-3" + + +@pytest.mark.asyncio +async def test_shared_view_defaults_into_scope_and_can_be_updated( + two_tenants, db_session +): + info1 = MockInfo(db_session, two_tenants["user1"]) + info3 = MockInfo(db_session, two_tenants["user3"]) + + view = await SavedViewMutation().create_saved_view( + info1, + CreateSavedViewInput( + name="Shared", + base_entity_type=SavedViewEntityType.TASK, + filter_definition="{}", + sort_definition="{}", + parameters="{}", + visibility=ScopeVisibility.SHARED, + ), + ) + assert view.location_id == "loc-a" + + listed = await SavedViewMutation().update_saved_view( + info1, view.id, UpdateSavedViewInput(visibility=ScopeVisibility.PUBLIC) + ) + assert listed.visibility == ScopeVisibility.PUBLIC + assert listed.location_id == "loc-a" + assert view.id in [v.id for v in await SavedViewQuery().my_saved_views(info3)] + + unlisted = await SavedViewMutation().update_saved_view( + info1, view.id, UpdateSavedViewInput(visibility=ScopeVisibility.SHARED) + ) + assert unlisted.visibility == ScopeVisibility.SHARED + assert unlisted.location_id == "loc-a" + assert view.id not in [v.id for v in await SavedViewQuery().my_saved_views(info3)] + assert (await SavedViewQuery().saved_view(info3, view.id)) is not None + + +@pytest.mark.asyncio +async def test_shared_visibility_is_rejected_for_presets_and_definitions( + two_tenants, db_session +): + info1 = MockInfo(db_session, two_tenants["user1"]) + + with pytest.raises(GraphQLError): + await TaskPresetMutation().create_task_preset( + info1, + _preset_input("Shared", visibility=ScopeVisibility.SHARED, location_id="loc-a"), + ) + with pytest.raises(GraphQLError): + await PropertyDefinitionMutation().create_property_definition( + info1, + CreatePropertyDefinitionInput( + name="Shared", + field_type=FieldType.FIELD_TYPE_TEXT, + allowed_entities=[PropertyEntity.PATIENT], + visibility=ScopeVisibility.SHARED, + location_id="loc-a", + ), + ) + + preset = await TaskPresetMutation().create_task_preset( + info1, _preset_input("Private") + ) + with pytest.raises(GraphQLError): + await TaskPresetMutation().update_task_preset( + info1, preset.id, UpdateTaskPresetInput(visibility=ScopeVisibility.SHARED) + ) + assert preset.visibility == ScopeVisibility.PRIVATE + + def _preset_input(name: str, **kwargs) -> CreateTaskPresetInput: return CreateTaskPresetInput( name=name, diff --git a/docs/VIEWS_ARCHITECTURE.md b/docs/VIEWS_ARCHITECTURE.md index 4ac1de06..24c3a723 100644 --- a/docs/VIEWS_ARCHITECTURE.md +++ b/docs/VIEWS_ARCHITECTURE.md @@ -10,8 +10,8 @@ A **SavedView** stores a named configuration for list screens: | `sortDefinition` | JSON string: TanStack `SortingState` array. | | `parameters` | JSON string: **scope** and cross-entity context — `rootLocationIds`, `locationId`, `searchQuery` (patient), `assigneeId` (task / my tasks). | | `baseEntityType` | `PATIENT` or `TASK` — primary tab when opening `/view/:uid`. | -| `visibility` | `PRIVATE` (owner only, no location needed) or `PUBLIC` (stored at a scaffold node via `locationId`). | -| `locationId` | Scaffold node a `PUBLIC` view is stored at. Everyone who can reach that node sees the view: users whose selected root location lies on the node's path (ancestor or descendant). | +| `visibility` | `PRIVATE` (owner only, no location needed), `SHARED` (reachable by link at a scaffold node via `locationId`, listed only for the owner) or `PUBLIC` (stored at a scaffold node via `locationId`, listed for everyone in scope). | +| `locationId` | Scaffold node a `SHARED` or `PUBLIC` view is stored at. Everyone who can reach that node sees the view: users whose selected root location lies on the node's path (ancestor or descendant). | Location is **not** a separate route anymore for saved views: it is encoded in `parameters` (`rootLocationIds`, `locationId`). @@ -21,10 +21,11 @@ Saved views, task presets and property definitions share the same scoping model - **Private** (default): only the owner sees the entry. No scaffold node is required. - **Public**: the entry is stored at a scaffold node (`locationId`). It is visible to everyone who can access that node and whose selected root location lies on the node's path, i.e. the node itself, its subtree and its ancestors. Storing an entry at the root makes it visible to everyone. +- **Shared** (saved views only): the view is stored at a scaffold node like a public one, but it is only listed for its owner. Everyone in the node's scope can open it via its link (`savedView(id)`), which mirrors the former `LINK_SHARED` mode. List queries (`mySavedViews`, `taskPresets`, `propertyDefinitions`) accept `rootLocationIds`; the web client passes the currently selected root locations so lists follow the app node selection. Editing stays with the owner (views, presets) or with users who can access the node (property definitions). -Migration `add_scope_visibility` makes existing property definitions public on the root node and turns existing saved views and presets private. +Migration `add_scope_visibility` makes existing property definitions public on the root node, keeps formerly link-shared saved views reachable by link (`SHARED`, stored at their node or the root) while other views become private, and turns existing presets private. ## Cross-entity model diff --git a/web/api/gql/types.ts b/web/api/gql/types.ts index 0ea63b2d..ce9c9d60 100644 --- a/web/api/gql/types.ts +++ b/web/api/gql/types.ts @@ -769,7 +769,8 @@ export enum SavedViewEntityType { export enum ScopeVisibility { Private = 'PRIVATE', - Public = 'PUBLIC' + Public = 'PUBLIC', + Shared = 'SHARED' } export type ScopedPatientCountsType = { diff --git a/web/components/locations/ScopeChip.tsx b/web/components/locations/ScopeChip.tsx index ff60d248..ba1ef750 100644 --- a/web/components/locations/ScopeChip.tsx +++ b/web/components/locations/ScopeChip.tsx @@ -2,7 +2,7 @@ import { useMemo } from 'react' import { Chip } from '@helpwave/hightide' -import { Lock } from 'lucide-react' +import { Link2, Lock } from 'lucide-react' import clsx from 'clsx' import { ScopeVisibility, type LocationType } from '@/api/gql/generated' import { LocationChips } from '@/components/locations/LocationChips' @@ -78,6 +78,7 @@ export function ScopeChip({ visibility, location, small = false, className }: Sc return } const isPublic = visibility === ScopeVisibility.Public + const isShared = visibility === ScopeVisibility.Shared return ( - {!isPublic && } - {isPublic ? translation('scopePublic') : translation('scopePrivate')} + {isShared && } + {!isPublic && !isShared && } + + {isPublic ? translation('scopePublic') : isShared ? translation('scopeShared') : translation('scopePrivate')} + ) } diff --git a/web/components/locations/ScopeVisibilityField.tsx b/web/components/locations/ScopeVisibilityField.tsx index bdd6eaec..42600834 100644 --- a/web/components/locations/ScopeVisibilityField.tsx +++ b/web/components/locations/ScopeVisibilityField.tsx @@ -1,7 +1,7 @@ 'use client' import { useState } from 'react' -import { Button, Checkbox } from '@helpwave/hightide' +import { Button, Checkbox, Select, SelectOption } from '@helpwave/hightide' import { MapPin } from 'lucide-react' import clsx from 'clsx' import { ScopeVisibility } from '@/api/gql/generated' @@ -24,7 +24,7 @@ export const scopeFromEntity = (entity: { location?: ScopeLocation | null, }): ScopeValue => ({ visibility: entity.visibility, - location: entity.visibility === ScopeVisibility.Public ? entity.location ?? null : null, + location: entity.visibility === ScopeVisibility.Private ? null : entity.location ?? null, }) export const isScopeComplete = (value: ScopeValue): boolean => @@ -32,7 +32,7 @@ export const isScopeComplete = (value: ScopeValue): boolean => export const scopeToInput = (value: ScopeValue): { visibility: ScopeVisibility, locationId: string | null } => ({ visibility: value.visibility, - locationId: value.visibility === ScopeVisibility.Public ? value.location?.id ?? null : null, + locationId: value.visibility === ScopeVisibility.Private ? null : value.location?.id ?? null, }) export const scopeEquals = (a: ScopeValue, b: ScopeValue): boolean => @@ -42,48 +42,74 @@ type ScopeVisibilityFieldProps = { value: ScopeValue, onChange: (value: ScopeValue) => void, disabled?: boolean, + allowShared?: boolean, className?: string, } +const visibilityDescriptionKey = { + [ScopeVisibility.Private]: 'scopePrivateDescription', + [ScopeVisibility.Shared]: 'scopeSharedDescription', + [ScopeVisibility.Public]: 'scopePublicDescription', +} as const + export function ScopeVisibilityField({ value, onChange, disabled = false, + allowShared = false, className, }: ScopeVisibilityFieldProps) { const translation = useTasksTranslation() const [dialogOpen, setDialogOpen] = useState(false) const isPublic = value.visibility === ScopeVisibility.Public + const needsLocation = value.visibility !== ScopeVisibility.Private - const setPublic = (checked: boolean) => { + const setVisibility = (visibility: ScopeVisibility) => { if (disabled) return onChange({ - visibility: checked ? ScopeVisibility.Public : ScopeVisibility.Private, - location: checked ? value.location : null, + visibility, + location: visibility === ScopeVisibility.Private ? null : value.location, }) } return (
{translation('scopeVisibility')} -
- -
setPublic(!isPublic)} - > - {translation('scopePublic')} + {allowShared ? ( +
+ + value={value.visibility} + disabled={disabled} + onValueChange={setVisibility} + > + + + + - {isPublic ? translation('scopePublicDescription') : translation('scopePrivateDescription')} + {translation(visibilityDescriptionKey[value.visibility])}
-
- {isPublic && ( + ) : ( +
+ setVisibility(checked ? ScopeVisibility.Public : ScopeVisibility.Private)} + className="mt-0.5 shrink-0" + /> +
setVisibility(isPublic ? ScopeVisibility.Private : ScopeVisibility.Public)} + > + {translation('scopePublic')} + + {isPublic ? translation('scopePublicDescription') : translation('scopePrivateDescription')} + +
+
+ )} + {needsLocation && (
{translation('scopeStoredAt')} @@ -111,7 +137,7 @@ export function ScopeVisibilityField({ onSelect={(locations) => { const node = locations[0] if (!node) return - onChange({ visibility: ScopeVisibility.Public, location: node }) + onChange({ visibility: value.visibility, location: node }) }} initialSelectedIds={value.location ? [value.location.id] : []} multiSelect={false} diff --git a/web/components/views/SaveViewDialog.tsx b/web/components/views/SaveViewDialog.tsx index ca55a577..c468f148 100644 --- a/web/components/views/SaveViewDialog.tsx +++ b/web/components/views/SaveViewDialog.tsx @@ -85,7 +85,7 @@ export function SaveViewDialog({ onChange={(e) => setName(e.target.value)} />
- +
- +