From e6a4303cd0bea2fc1eff88694bf21c6aaae7b5f3 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Mon, 29 Jun 2026 22:27:38 -0700 Subject: [PATCH 01/16] docs: design for station row Sync action button [skip ci] Co-Authored-By: Claude --- .../2026-06-29-station-sync-action-design.md | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 docs/claude/planning/2026-06-29-station-sync-action-design.md diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md new file mode 100644 index 000000000..d37312bce --- /dev/null +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -0,0 +1,179 @@ +# Station row "Sync" floating action button — design + +**Date:** 2026-06-29 +**Scope:** Frontend only (`ui/`). No backend change, no migration. +**Branch/worktree:** `worktree-station-sync-action` + +## Summary + +Add a "Sync" floating action button to each row in the Stations (deployments) +list table, alongside the existing Delete button. Clicking it opens a small +confirmation dialog; confirming triggers an asynchronous data-storage sync job +that scans the deployment's connected S3 storage source and imports any new +captures. The button mirrors the existing per-row action pattern (the Delete +trash icon) and reuses the sync endpoint and React Query hook that already power +the "Sync now" button in the deployment detail view. + +## What already exists (no work needed) + +- **Backend endpoint:** `POST /api/v2/deployments/{pk}/sync/` + (`DeploymentViewSet.sync`, `ami/main/api/views.py:339-361`). Creates a + `DataStorageSyncJob` and enqueues it; returns `{ "job_id": , + "project_id": }`. Raises `ValidationError` (HTTP 400) when the + deployment has no `data_source`. +- **React Query hook:** `useSyncDeploymentSourceImages` + (`ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts`). + `POST`s to the endpoint, on success invalidates the `[JOBS]` and `[CAPTURES]` + query keys, and exposes `{ syncDeploymentSourceImages, isLoading, isSuccess, + error, data }` where `data.data` is `{ job_id, project_id }`. +- **Row action toolbar:** `Toolbar` from `nova-ui-kit` + (`ui/src/nova-ui-kit/components/toolbar.tsx`) — the circular pill that holds + per-row icon buttons. +- **Reference for the confirmation-dialog action pattern:** `DeleteEntityDialog` + (`ui/src/pages/project/entities/delete-entity-dialog.tsx`) + `DeleteForm` + (`ui/src/components/form/delete-form/delete-form.tsx`). +- **Reference for the sync UX (button states + job link):** + `SyncDeploymentSourceImages` + (`ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx`). + +## Decisions (from brainstorming) + +1. **Interaction:** confirmation dialog (not one-click). Matches the Delete + button neighbor and guards against accidental row-misclicks. +2. **Gating:** `item.canUpdate` only. The deployments **list** serializer + (`DeploymentListSerializer`, `ami/main/api/serializers.py:180-211`) carries + no data-source field, so a "show only when connected" gate is not possible + without a backend change, which we are deliberately not doing here. + Consequence: the button shows on every station the user can update; clicking + it on a station with **no** data source returns HTTP 400, which the dialog + surfaces via `FormError` (see error handling below). +3. **Feedback:** inline icon state (the app has no toast system). Sync button + shows a spinner while loading and a check on success; on success the dialog + also renders an eye-icon link to the created job, mirroring the detail-view + sync component. + +## Components + +### New file: `ui/src/pages/deployments/sync-deployment-dialog.tsx` + +`SyncDeploymentDialog`, modeled on `DeleteEntityDialog`. + +Props: + +```ts +{ + id: string // deployment id + projectId: string // for the job-details link route +} +``` + +Structure: + +- `Dialog.Root` with a local `useState` `isOpen` flag (`open` / `onOpenChange`), + same as `DeleteEntityDialog`. +- `Dialog.Trigger asChild` wrapping a `nova-ui-kit` `Button`: + - `size="icon"`, `variant="ghost"`, `className="shrink-0"`, + `aria-label={translate(STRING.SYNC)}`. + - Child: lucide `RefreshCwIcon` at `className="w-4 h-4"`. +- `Dialog.Content` (`ariaCloselabel={translate(STRING.CLOSE)}`, `isCompact`) + containing the confirmation body (built inline with the shared + `FormSection` / `FormError` layout from `components/form/layout/layout`, the + same primitives `DeleteForm` uses): + - If `error`: `` + (covers the no-data-source 400). + - `FormSection` with `title` = "Sync captures from storage source?" and + `description` = `translate(STRING.MESSAGE_SYNC_CONFIRM)`. + - A right-aligned button row: + - Cancel `Button` (`size="small"`, `variant="outline"`) → + `setIsOpen(false)`. + - Sync `Button` (`size="small"`, `variant="success"`, + `disabled={isLoading || isSuccess}`) → `syncDeploymentSourceImages(id)`. + Label `translate(STRING.SYNC)`; shows `Loader2Icon` (spin) while loading, + `CheckIcon` on success. + - On success (`isSuccess && data`): render an eye-icon `Link` + (styled `buttonVariants({ size: 'icon', variant: 'ghost' })`) to + `APP_ROUTES.JOB_DETAILS({ projectId, jobId: String(data.data.job_id) })` + via `getAppRoute({ to, keepSearchParams: true })`, wrapped in a + `BasicTooltip` (`content` = "View sync job"). Same shape as the existing + detail-view sync component. + +Hook usage: `const { syncDeploymentSourceImages, isLoading, isSuccess, error, +data } = useSyncDeploymentSourceImages()`. + +### Edit: `ui/src/pages/deployments/deployment-columns.tsx` + +In the `actions` column `renderCell` (currently lines ~232-242), add the sync +button inside the existing `Toolbar`, to the **left** of the Delete button: + +```tsx + + {item.canUpdate && ( + + )} + {item.canDelete && ( + + )} + +``` + +Add the import for `SyncDeploymentDialog`. `projectId` is already in scope +(the `columns({ projectId })` factory argument). + +### Edit: `ui/src/utils/language.ts` + +Add two `STRING` enum members and their English values: + +- `SYNC` → `"Sync"` (button label + aria-label). +- `MESSAGE_SYNC_CONFIRM` → + `"This scans the connected storage source and imports any new captures as a background job."` + +Place them near the existing sync-related strings +(`MESSAGE_CAPTURE_SYNC_HIDDEN`, `FIELD_LABEL_LAST_SYNCED`) following the file's +ordering convention. + +## Data flow + +1. User hovers a station row → `Toolbar` appears → clicks the `RefreshCwIcon`. +2. `SyncDeploymentDialog` opens; user clicks **Sync**. +3. `syncDeploymentSourceImages(id)` → `POST /api/v2/deployments/{id}/sync/`. +4. **Success:** hook invalidates `[JOBS]` + `[CAPTURES]`; dialog shows the check + state and the eye-icon link to the new job. User can click through to the + job detail page or close the dialog. +5. **Failure (e.g. no data source → 400):** `error` is set; `FormError` renders + the parsed server message inside the dialog. User can cancel. + +## Error handling + +- No-data-source stations: backend returns 400 `ValidationError`; surfaced in + the dialog via `FormError` + `parseServerError`. The confirmation step plus + the in-dialog error message is the agreed mitigation for gating on + `canUpdate` only. +- Generic request failures: same `FormError` path. + +## Testing / verification + +- `cd ui && yarn lint` and `yarn format` clean. +- TypeScript compiles (`tsc --noEmit` via the build). +- Manual (Chrome DevTools MCP against the local stack): + - Sync button appears in station rows for an Update-permitted user, left of + Delete; hidden for users without Update permission. + - Click → dialog → confirm → spinner → check → job link navigates to the job + detail page; a `DataStorageSyncJob` is created. + - Confirm on a station with no configured data source → dialog shows the 400 + error message, no job created. + +## Known limitations / follow-ups + +- **`isSuccess` persistence:** the mutation state lives on the mounted hook + instance, so reopening the dialog on the same row shows the prior success + state. The detail-view sync component has the same behavior. If undesired, a + small follow-up can call the mutation's `reset()` on dialog open (requires + exposing `reset` from the hook). +- **Connected gate:** deferred. If we later want to hide the button on + unconnected stations (or add a "Last synced" column), add `data_source_uri` + (or a lightweight `data_source_connected` boolean) to + `DeploymentListSerializer.fields` and gate on it. From 26439c41e57759c445f3646044007cc1644b8a28 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Mon, 29 Jun 2026 22:27:38 -0700 Subject: [PATCH 02/16] Add a Sync action button to each station row Adds a floating action button to every row in the Stations (deployments) list table, beside the existing Delete button. Clicking it opens a confirmation dialog; confirming triggers the asynchronous data-storage sync job that scans the station's connected storage source and imports any new captures. Reuses the existing POST /deployments/{id}/sync/ endpoint and the useSyncDeploymentSourceImages hook (already powering the detail-view 'Sync now' button). On success the dialog shows a link to the created job; a station with no data source returns 400, surfaced in the dialog. The button is shown only to users with update permission on the station. Co-Authored-By: Claude --- .../pages/deployments/deployment-columns.tsx | 4 + .../deployments/sync-deployment-dialog.tsx | 87 +++++++++++++++++++ ui/src/utils/language.ts | 7 ++ 3 files changed, 98 insertions(+) create mode 100644 ui/src/pages/deployments/sync-deployment-dialog.tsx diff --git a/ui/src/pages/deployments/deployment-columns.tsx b/ui/src/pages/deployments/deployment-columns.tsx index 95fb1a9f4..3b8584b0f 100644 --- a/ui/src/pages/deployments/deployment-columns.tsx +++ b/ui/src/pages/deployments/deployment-columns.tsx @@ -12,6 +12,7 @@ import { Toolbar, } from 'nova-ui-kit' import { DeleteEntityDialog } from 'pages/project/entities/delete-entity-dialog' +import { SyncDeploymentDialog } from 'pages/deployments/sync-deployment-dialog' import { Link } from 'react-router-dom' import { APP_ROUTES } from 'utils/constants' import { getAppRoute } from 'utils/getAppRoute' @@ -231,6 +232,9 @@ export const columns = ({ sticky: true, renderCell: (item: Deployment) => ( + {item.canUpdate && ( + + )} {item.canDelete && ( { + const [isOpen, setIsOpen] = useState(false) + const { syncDeploymentSourceImages, isLoading, isSuccess, error, data } = + useSyncDeploymentSourceImages() + + const jobId = data?.data.job_id + const errorMessage = error ? parseServerError(error)?.message : undefined + + return ( + + + + + + {errorMessage && } + +
+ + + {isSuccess && jobId !== undefined && ( + + + + + + )} +
+ + + + ) +} diff --git a/ui/src/utils/language.ts b/ui/src/utils/language.ts index 6c6b3c2be..8bde7facb 100644 --- a/ui/src/utils/language.ts +++ b/ui/src/utils/language.ts @@ -46,8 +46,10 @@ export enum STRING { SUBMIT, SUGGEST_ID_SHORT, SUGGEST_ID, + SYNC, VIEW_ALL, VIEW_DOCS, + VIEW_JOB, VIEW_PUBLIC_PROJECTS, ZOOM_IN, ZOOM_OUT, @@ -210,6 +212,7 @@ export enum STRING { MESSAGE_RESET_INSTRUCTIONS_SENT, MESSAGE_RESULT_RANGE, MESSAGE_SIGNED_UP, + MESSAGE_SYNC_CONFIRM, MESSAGE_VALUE_INVALID, MESSAGE_VALUE_MISSING, @@ -406,8 +409,10 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.SUBMIT]: 'Submit', [STRING.SUGGEST_ID_SHORT]: 'Suggest', [STRING.SUGGEST_ID]: 'Suggest ID', + [STRING.SYNC]: 'Sync', [STRING.VIEW_ALL]: 'View all', [STRING.VIEW_DOCS]: 'View docs', + [STRING.VIEW_JOB]: 'View job', [STRING.VIEW_PUBLIC_PROJECTS]: 'View public projects', [STRING.ZOOM_IN]: 'Zoom in', [STRING.ZOOM_OUT]: 'Zoom out', @@ -595,6 +600,8 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.MESSAGE_RESULT_RANGE]: 'Showing {{start}}-{{end}} of {{total}} result(s)', [STRING.MESSAGE_SIGNED_UP]: 'Signed up successfully!', + [STRING.MESSAGE_SYNC_CONFIRM]: + 'This scans the connected storage source and imports any new captures as a background job.', [STRING.MESSAGE_VALUE_INVALID]: 'Please provide a valid value', [STRING.MESSAGE_VALUE_MISSING]: 'Please provide a value', From 89c257435e4ee5c983bd349e2fc7df4fdb8a92e4 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Mon, 29 Jun 2026 22:34:52 -0700 Subject: [PATCH 03/16] Address self-review on the station Sync button - Reset the sync mutation state when the dialog opens. The hook is mounted for the whole table row, so success/error/data otherwise survived a close and left the Sync button permanently disabled after one sync. The hook now exposes reset(); the dialog calls it on open for a fresh attempt each time. - Swallow the promise rejection at the call site. The no-data-source 400 is an intended path surfaced via FormError, so the bare mutateAsync call would otherwise log an unhandled rejection on every such click. - Use a clearer dialog title ('Sync captures') instead of the bare 'Sync'. Co-Authored-By: Claude --- .../2026-06-29-station-sync-action-design.md | 11 ++++--- .../useSyncDeploymentSourceImages.ts | 33 ++++++++++--------- .../deployments/sync-deployment-dialog.tsx | 32 +++++++++++++++--- ui/src/utils/language.ts | 2 ++ 4 files changed, 53 insertions(+), 25 deletions(-) diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md index d37312bce..18fff95c4 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-design.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -168,11 +168,12 @@ ordering convention. ## Known limitations / follow-ups -- **`isSuccess` persistence:** the mutation state lives on the mounted hook - instance, so reopening the dialog on the same row shows the prior success - state. The detail-view sync component has the same behavior. If undesired, a - small follow-up can call the mutation's `reset()` on dialog open (requires - exposing `reset` from the hook). +- **`isSuccess` persistence (resolved in-scope):** the mutation state lives on + the row-mounted hook instance, so without intervention reopening the dialog on + the same row would show the prior success/error state and leave the Sync + button disabled. The hook now exposes `reset`, and the dialog calls it on open + so each open offers a fresh sync. (More important here than in the detail-view + sync component, which unmounts on navigation; a list row stays mounted.) - **Connected gate:** deferred. If we later want to hide the button on unconnected stations (or add a "Last synced" column), add `data_source_uri` (or a lightweight `data_source_connected` boolean) to diff --git a/ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts b/ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts index d804cdc82..4d67250d2 100644 --- a/ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts +++ b/ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts @@ -8,25 +8,28 @@ export const useSyncDeploymentSourceImages = () => { const { user } = useUser() const queryClient = useQueryClient() - const { mutateAsync, isLoading, isSuccess, error, data } = useMutation({ - mutationFn: (id: string) => - axios.post<{ job_id: number; project_id: number }>( - `${API_URL}/${API_ROUTES.DEPLOYMENTS}/${id}/sync/`, - undefined, - { - headers: getAuthHeader(user), - } - ), - onSuccess: (resp) => { - queryClient.invalidateQueries([API_ROUTES.JOBS]) - queryClient.invalidateQueries([API_ROUTES.CAPTURES]) + const { mutateAsync, reset, isLoading, isSuccess, error, data } = useMutation( + { + mutationFn: (id: string) => + axios.post<{ job_id: number; project_id: number }>( + `${API_URL}/${API_ROUTES.DEPLOYMENTS}/${id}/sync/`, + undefined, + { + headers: getAuthHeader(user), + } + ), + onSuccess: (resp) => { + queryClient.invalidateQueries([API_ROUTES.JOBS]) + queryClient.invalidateQueries([API_ROUTES.CAPTURES]) - return resp.data - }, - }) + return resp.data + }, + } + ) return { syncDeploymentSourceImages: mutateAsync, + reset, isLoading, isSuccess, error, diff --git a/ui/src/pages/deployments/sync-deployment-dialog.tsx b/ui/src/pages/deployments/sync-deployment-dialog.tsx index 978534df4..8fc6d266b 100644 --- a/ui/src/pages/deployments/sync-deployment-dialog.tsx +++ b/ui/src/pages/deployments/sync-deployment-dialog.tsx @@ -17,14 +17,31 @@ export const SyncDeploymentDialog = ({ projectId: string }) => { const [isOpen, setIsOpen] = useState(false) - const { syncDeploymentSourceImages, isLoading, isSuccess, error, data } = - useSyncDeploymentSourceImages() + const { + syncDeploymentSourceImages, + reset, + isLoading, + isSuccess, + error, + data, + } = useSyncDeploymentSourceImages() const jobId = data?.data.job_id const errorMessage = error ? parseServerError(error)?.message : undefined return ( - + { + setIsOpen(open) + // The hook is mounted for the whole row, so success/error/data survive + // a close. Reset on open so reopening offers a fresh sync instead of + // the previous attempt's stale state. + if (open) { + reset() + } + }} + > - {errorMessage && } + {errorMessage && } Date: Wed, 1 Jul 2026 09:48:49 -0700 Subject: [PATCH 07/16] feat(deployments): add "Sync all" and hide per-row Sync on unconnected stations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - Expose data_source_connected on the stations list serializer (a read-only boolean from the data_source foreign key id, so it adds no query). - Add DeploymentViewSet.sync_all (POST /deployments/sync-all/?project_id=) that enqueues one DataStorageSyncJob per connected station in the project — separate jobs, matching the per-row sync action and the admin bulk action. Since ObjectPermission only guards detail actions via get_object(), this detail=False action checks the sync permission itself with the codebase's transient-probe pattern. Adds a permission-matrix and field/enqueue test. Frontend: - The per-row Sync button shows only when the station has a storage source, so it no longer appears where a sync would fail with a 400. - A "Sync all" button in the Stations list header syncs every connected station the user can update; the dialog reports how many jobs will run and links to the Jobs tab on success. Co-Authored-By: Claude --- ami/main/api/serializers.py | 12 +++ ami/main/api/views.py | 65 ++++++++++--- ami/main/tests.py | 88 +++++++++++++++++ .../2026-06-29-station-sync-action-design.md | 36 +++++++ .../deployments/useSyncAllDeployments.ts | 39 ++++++++ ui/src/data-services/models/deployment.ts | 4 + .../pages/deployments/deployment-columns.tsx | 2 +- ui/src/pages/deployments/deployments.tsx | 11 +++ .../sync-all-deployments-dialog.tsx | 94 +++++++++++++++++++ ui/src/utils/language.ts | 7 ++ 10 files changed, 344 insertions(+), 14 deletions(-) create mode 100644 ui/src/data-services/hooks/deployments/useSyncAllDeployments.ts create mode 100644 ui/src/pages/deployments/sync-all-deployments-dialog.tsx diff --git a/ami/main/api/serializers.py b/ami/main/api/serializers.py index 2855633e3..308947ae5 100644 --- a/ami/main/api/serializers.py +++ b/ami/main/api/serializers.py @@ -184,6 +184,7 @@ class DeploymentListSerializer(DefaultSerializer): device = DeviceNestedSerializer(read_only=True) research_site = SiteNestedSerializer(read_only=True) jobs = JobStatusSerializer(many=True, read_only=True) + data_source_connected = serializers.SerializerMethodField() class Meta: model = Deployment @@ -208,8 +209,19 @@ class Meta: "device", "research_site", "jobs", + "data_source_connected", ] + def get_data_source_connected(self, obj: Deployment) -> bool: + """ + Whether the station has a storage source configured. + + The stations list uses this to show the per-row Sync button only where a + sync can succeed, and to count how many stations "Sync all" would cover. + Reads the foreign key id already on the row, so it adds no query. + """ + return obj.data_source_id is not None + def get_events(self, obj): """ Return URL to the events endpoint filtered by this deployment. diff --git a/ami/main/api/views.py b/ami/main/api/views.py index 6ad39a2e5..c912cd575 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -336,6 +336,26 @@ def get_queryset(self) -> QuerySet: return qs + @staticmethod + def _enqueue_sync_job(deployment: Deployment): + """ + Create and enqueue a ``DataStorageSyncJob`` for a single deployment. + + Shared by the per-row ``sync`` action and the bulk ``sync_all`` action so + both build the job the same way (one job per deployment, matching the + admin bulk action). + """ + from ami.jobs.models import DataStorageSyncJob, Job + + job = Job.objects.create( + name=f"Sync captures for deployment {deployment.pk}", + deployment=deployment, + project=deployment.project, + job_type_key=DataStorageSyncJob.key, + ) + job.enqueue() + return job + @action(detail=True, methods=["post"], name="sync") def sync(self, _request, pk=None) -> Response: """ @@ -343,23 +363,42 @@ def sync(self, _request, pk=None) -> Response: """ deployment: Deployment = self.get_object() if deployment and deployment.data_source: - # queued_task = tasks.sync_source_images.delay(deployment.pk) - from ami.jobs.models import DataStorageSyncJob, Job - - job = Job.objects.create( - name=f"Sync captures for deployment {deployment.pk}", - deployment=deployment, - project=deployment.project, - job_type_key=DataStorageSyncJob.key, + job = self._enqueue_sync_job(deployment) + logger.info( + f"Syncing captures for deployment {deployment.pk} from {deployment.data_source_uri} in background." ) - job.enqueue() - msg = f"Syncing captures for deployment {deployment.pk} from {deployment.data_source_uri} in background." - logger.info(msg) - assert deployment.project - return Response({"job_id": job.pk, "project_id": deployment.project.pk}) + return Response({"job_id": job.pk, "project_id": deployment.project_id}) else: raise api_exceptions.ValidationError(detail="Deployment must have a data source to sync captures from") + @action(detail=False, methods=["post"], name="sync-all", url_path="sync-all") + def sync_all(self, request) -> Response: + """ + Queue a sync job for every station in the project that has a storage source. + + Enqueues one ``DataStorageSyncJob`` per connected station (separate jobs, + not one consolidated job), matching the per-row ``sync`` action and the + admin bulk action. Requires the ``project_id`` query parameter and the + sync permission on the project. + """ + project = self.get_active_project() + if not project: + raise api_exceptions.ValidationError(detail="A project_id is required to sync all stations.") + + # ObjectPermission.has_permission() is a no-op and has_object_permission() + # only runs for detail actions (via get_object()), so a detail=False action + # must check permissions itself. Probe the same sync permission the per-row + # action enforces, resolved against the project. + if not Deployment(project=project).check_permission(request.user, "sync"): + raise api_exceptions.PermissionDenied( + detail="You do not have permission to sync stations in this project." + ) + + deployments = self.get_queryset().filter(data_source__isnull=False) + job_ids = [self._enqueue_sync_job(deployment).pk for deployment in deployments] + logger.info(f"Queued {len(job_ids)} DataStorageSyncJob(s) for project {project.pk}: {job_ids}") + return Response({"job_ids": job_ids, "queued": len(job_ids), "project_id": project.pk}) + @action(detail=True, methods=["post"], name="regroup-sessions", url_path="regroup-sessions") def regroup_sessions(self, _request, pk=None) -> Response: """ diff --git a/ami/main/tests.py b/ami/main/tests.py index c9e3b0d8f..26923a93a 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -2900,6 +2900,94 @@ def test_sync_creates_events_and_updates_counts(self): logger.info(f"Initial events count: {initial_events_count}, Updated events count: {updated_events.count()}") +class TestDeploymentSyncAll(APITestCase): + """ + The bulk "Sync all" endpoint enqueues one sync job per connected station and + is gated by the same sync permission the per-row action enforces. + + Pins three guarantees: the ``data_source_connected`` flag the frontend reads, + the permission matrix (a ``detail=False`` action must check permissions + itself), and that only stations with a storage source get a job. + """ + + def setUp(self): + super().setUp() + from unittest import mock + + self.project = Project.objects.create(name="Sync All Project", description="Sync-all tests") + create_roles_for_project(self.project) + + self.superuser = User.objects.create_superuser(email="super-syncall@insectai.org", password="password123") + self.pm_user = User.objects.create_user(email="pm-syncall@insectai.org", password="password123") + self.basic_user = User.objects.create_user(email="basic-syncall@insectai.org", password="password123") + self.outsider = User.objects.create_user(email="outsider-syncall@insectai.org", password="password123") + ProjectManager.assign_user(self.pm_user, self.project) + BasicMember.assign_user(self.basic_user, self.project) + + source = S3StorageSource.objects.create( + name="Sync All Source", + bucket="test-bucket", + access_key="fake-access-key", + secret_key="fake-secret-key", + project=self.project, + ) + self.connected = Deployment.objects.create(name="Connected", project=self.project, data_source=source) + self.unconnected = Deployment.objects.create(name="Unconnected", project=self.project) + + # Keep the endpoint tests off the broker: assert the job rows and enqueue + # calls, not real Celery dispatch. + patcher = mock.patch("ami.jobs.models.Job.enqueue") + self.mock_enqueue = patcher.start() + self.addCleanup(patcher.stop) + + self.url = "/api/v2/deployments/sync-all/" + + def test_data_source_connected_field_in_list(self): + self.client.force_authenticate(self.superuser) + response = self.client.get(f"/api/v2/deployments/?project_id={self.project.pk}") + self.assertEqual(response.status_code, 200) + flags = {row["id"]: row["data_source_connected"] for row in response.data["results"]} + self.assertTrue(flags[self.connected.pk], "Connected station should report data_source_connected=True") + self.assertFalse(flags[self.unconnected.pk], "Unconnected station should report data_source_connected=False") + + def test_requires_project_id(self): + self.client.force_authenticate(self.superuser) + response = self.client.post(self.url) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_permission_matrix(self): + matrix = [ + ("superuser", self.superuser, status.HTTP_200_OK), + ("ProjectManager", self.pm_user, status.HTTP_200_OK), + ("BasicMember", self.basic_user, status.HTTP_403_FORBIDDEN), + ("outsider", self.outsider, status.HTTP_403_FORBIDDEN), + ] + for role_name, user, expected in matrix: + with self.subTest(role=role_name): + self.client.force_authenticate(user) + response = self.client.post(f"{self.url}?project_id={self.project.pk}") + self.assertEqual(response.status_code, expected, f"{role_name} got {response.status_code}") + + def test_anonymous_denied(self): + self.client.force_authenticate(None) + response = self.client.post(f"{self.url}?project_id={self.project.pk}") + self.assertIn(response.status_code, (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN)) + + def test_enqueues_one_job_per_connected_station(self): + from ami.jobs.models import DataStorageSyncJob + + self.client.force_authenticate(self.pm_user) + response = self.client.post(f"{self.url}?project_id={self.project.pk}") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["queued"], 1) + self.assertEqual(len(response.data["job_ids"]), 1) + self.assertEqual(self.mock_enqueue.call_count, 1, "One job should be enqueued") + + jobs = Job.objects.filter(project=self.project, job_type_key=DataStorageSyncJob.key) + self.assertEqual(jobs.count(), 1, "Exactly one sync job, for the connected station") + self.assertEqual(jobs.first().deployment_id, self.connected.pk, "Unconnected station must be skipped") + + class TestFineGrainedJobRunPermission(APITestCase): def setUp(self): super().setUp() diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md index 18fff95c4..e784cd728 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-design.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -178,3 +178,39 @@ ordering convention. unconnected stations (or add a "Last synced" column), add `data_source_uri` (or a lightweight `data_source_connected` boolean) to `DeploymentListSerializer.fields` and gate on it. + +## Addendum (2026-07-01): connected gate + "Sync all" + +After the live test, three follow-ups were pulled into this PR (per the product +owner's call): + +### 1. `data_source_connected` on the list serializer +`DeploymentListSerializer` gets a read-only `data_source_connected` boolean +(`obj.data_source_id is not None`). No query cost (the id is on the row), no +migration. The per-row Sync button now shows only when +`item.canUpdate && item.dataSourceConnected`, so it is hidden on stations with no +storage source. The endpoint's 400 stays as a safety net for races. The field +also feeds "Sync all" eligibility on the frontend. + +### 2. "Sync all" bulk endpoint +`DeploymentViewSet.sync_all` (`detail=False`, POST, `project_id` via +`ProjectMixin.get_active_project`). It enqueues one `DataStorageSyncJob` per +connected deployment in the project — **separate jobs, not one consolidated +job** — matching the admin bulk action (`DeploymentAdmin.sync_captures`) and the +single-`deployment` FK on `DataStorageSyncJob`. Returns +`{ job_ids, queued, project_id }`. + +### 3. Permissions (the parts that are invisible in the diff) +- `ObjectPermission.has_permission` returns `True` and `has_object_permission` + only fires on detail actions (via `get_object`). A `detail=False` action is + therefore unguarded unless it checks permissions itself. `sync_all` checks a + transient probe: `Deployment(project=project).check_permission(user, "sync")`, + which resolves to the `sync_deployment` guardian permission — exactly what the + per-row `sync` action requires. Superusers pass via Django's bypass; + `ProjectManager` passes via guardian; `BasicMember` is denied (403). +- The per-row button stays gated on `canUpdate`, **not** a `sync` permission. + Superusers receive `["update", "delete"]` in `user_permissions` but not + `"sync"` (guardian `get_perms` returns nothing for superusers), so gating on + `sync` would wrongly hide the button from them. `update_deployment` and + `sync_deployment` are co-granted (only to `ProjectManager`), so `canUpdate` + covers exactly the users who can sync. diff --git a/ui/src/data-services/hooks/deployments/useSyncAllDeployments.ts b/ui/src/data-services/hooks/deployments/useSyncAllDeployments.ts new file mode 100644 index 000000000..268e30288 --- /dev/null +++ b/ui/src/data-services/hooks/deployments/useSyncAllDeployments.ts @@ -0,0 +1,39 @@ +import { useMutation, useQueryClient } from '@tanstack/react-query' +import axios from 'axios' +import { API_ROUTES, API_URL } from 'data-services/constants' +import { getAuthHeader } from 'data-services/utils' +import { useUser } from 'utils/user/userContext' + +export const useSyncAllDeployments = () => { + const { user } = useUser() + const queryClient = useQueryClient() + + const { mutateAsync, reset, isLoading, isSuccess, error, data } = useMutation( + { + mutationFn: (projectId: string) => + axios.post<{ job_ids: number[]; queued: number; project_id: number }>( + `${API_URL}/${API_ROUTES.DEPLOYMENTS}/sync-all/?project_id=${projectId}`, + undefined, + { + headers: getAuthHeader(user), + } + ), + onSuccess: (resp) => { + queryClient.invalidateQueries([API_ROUTES.JOBS]) + queryClient.invalidateQueries([API_ROUTES.CAPTURES]) + queryClient.invalidateQueries([API_ROUTES.DEPLOYMENTS]) + + return resp.data + }, + } + ) + + return { + syncAllDeployments: mutateAsync, + reset, + isLoading, + isSuccess, + error, + data, + } +} diff --git a/ui/src/data-services/models/deployment.ts b/ui/src/data-services/models/deployment.ts index 069896bf8..15205299f 100644 --- a/ui/src/data-services/models/deployment.ts +++ b/ui/src/data-services/models/deployment.ts @@ -29,6 +29,10 @@ export class Deployment extends Entity { return this._deployment.user_permissions.includes(UserPermission.Update) } + get dataSourceConnected(): boolean { + return this._deployment.data_source_connected ?? false + } + get currentJob(): Job | undefined { if (!this._jobs.length) { return diff --git a/ui/src/pages/deployments/deployment-columns.tsx b/ui/src/pages/deployments/deployment-columns.tsx index 3b8584b0f..09519d0d9 100644 --- a/ui/src/pages/deployments/deployment-columns.tsx +++ b/ui/src/pages/deployments/deployment-columns.tsx @@ -232,7 +232,7 @@ export const columns = ({ sticky: true, renderCell: (item: Deployment) => ( - {item.canUpdate && ( + {item.canUpdate && item.dataSourceConnected && ( )} {item.canDelete && ( diff --git a/ui/src/pages/deployments/deployments.tsx b/ui/src/pages/deployments/deployments.tsx index 1389dff62..fdd341927 100644 --- a/ui/src/pages/deployments/deployments.tsx +++ b/ui/src/pages/deployments/deployments.tsx @@ -2,6 +2,7 @@ import { useDeployments } from 'data-services/hooks/deployments/useDeployments' import { ColumnSettings, PageHeader, SortControl, Table } from 'nova-ui-kit' import { DeploymentDetailsDialog } from 'pages/deployment-details/deployment-details-dialog' import { NewDeploymentDialog } from 'pages/deployment-details/new-deployment-dialog' +import { SyncAllDeploymentsDialog } from 'pages/deployments/sync-all-deployments-dialog' import { useParams } from 'react-router-dom' import { STRING, translate } from 'utils/language' import { useColumnSettings } from 'utils/useColumnSettings' @@ -33,6 +34,10 @@ export const Deployments = () => { }) const canCreate = userPermissions?.includes(UserPermission.Create) const tableColumns = columns({ projectId: projectId as string }) + const syncableCount = + deployments?.filter( + (deployment) => deployment.canUpdate && deployment.dataSourceConnected + ).length ?? 0 return ( <> @@ -46,6 +51,12 @@ export const Deployments = () => { tooltip={translate(STRING.TOOLTIP_DEPLOYMENT)} > {canCreate ? : null} + {syncableCount > 0 ? ( + + ) : null} { + const [isOpen, setIsOpen] = useState(false) + const { syncAllDeployments, reset, isLoading, isSuccess, error, data } = + useSyncAllDeployments() + + const queued = data?.data.queued + const errorMessage = error ? parseServerError(error)?.message : undefined + + return ( + { + setIsOpen(open) + // The hook stays mounted with the header, so reset on open to offer a + // fresh sync instead of the previous run's result. + if (open) { + reset() + } + }} + > + + + + + {errorMessage && } + +
+ + + {isSuccess && queued !== undefined && ( + + + + + + )} +
+ + + + ) +} diff --git a/ui/src/utils/language.ts b/ui/src/utils/language.ts index 5b35ef4c9..f2be6618c 100644 --- a/ui/src/utils/language.ts +++ b/ui/src/utils/language.ts @@ -47,10 +47,12 @@ export enum STRING { SUGGEST_ID_SHORT, SUGGEST_ID, SYNC, + SYNC_ALL, SYNC_CAPTURES, VIEW_ALL, VIEW_DOCS, VIEW_JOB, + VIEW_JOBS, VIEW_PUBLIC_PROJECTS, ZOOM_IN, ZOOM_OUT, @@ -213,6 +215,7 @@ export enum STRING { MESSAGE_RESET_INSTRUCTIONS_SENT, MESSAGE_RESULT_RANGE, MESSAGE_SIGNED_UP, + MESSAGE_SYNC_ALL_CONFIRM, MESSAGE_SYNC_CONFIRM, MESSAGE_VALUE_INVALID, MESSAGE_VALUE_MISSING, @@ -411,10 +414,12 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.SUGGEST_ID_SHORT]: 'Suggest', [STRING.SUGGEST_ID]: 'Suggest ID', [STRING.SYNC]: 'Sync', + [STRING.SYNC_ALL]: 'Sync all', [STRING.SYNC_CAPTURES]: 'Sync captures', [STRING.VIEW_ALL]: 'View all', [STRING.VIEW_DOCS]: 'View docs', [STRING.VIEW_JOB]: 'View job', + [STRING.VIEW_JOBS]: 'View jobs', [STRING.VIEW_PUBLIC_PROJECTS]: 'View public projects', [STRING.ZOOM_IN]: 'Zoom in', [STRING.ZOOM_OUT]: 'Zoom out', @@ -602,6 +607,8 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.MESSAGE_RESULT_RANGE]: 'Showing {{start}}-{{end}} of {{total}} result(s)', [STRING.MESSAGE_SIGNED_UP]: 'Signed up successfully!', + [STRING.MESSAGE_SYNC_ALL_CONFIRM]: + 'This starts a background sync job for each of the {{count}} station(s) with a storage source.', [STRING.MESSAGE_SYNC_CONFIRM]: 'This scans the connected storage source and imports any new captures as a background job.', [STRING.MESSAGE_VALUE_INVALID]: 'Please provide a valid value', From 25318a85c6a5408ac069401d439177ee386b47e5 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Wed, 1 Jul 2026 16:20:37 -0700 Subject: [PATCH 08/16] feat(deployments): allow ML data managers to sync stations MLDataManager already had run_data_storage_sync_job (it could run/retry a sync job) but not sync_deployment (which gates starting a sync from a station), so it could manage a sync job without being able to initiate one. Grant sync_deployment to MLDataManager so ML data managers can trigger syncs, matching ProjectManager. - roles.py: add SYNC_DEPLOYMENT to MLDataManager (ProjectManager inherits it). - Data migration backfills the permission onto existing projects' MLDataManager role groups; new projects get it via create_roles_for_project. - Frontend: gate the per-row and Sync all buttons on (canUpdate || canSync) so ML data managers see them. The canUpdate arm keeps them visible for superusers, who do not receive the sync permission in user_permissions. - Extend the permission-matrix test: MLDataManager 200; Researcher, Identifier, BasicMember, and non-members 403. Co-Authored-By: Claude --- ..._grant_sync_deployment_to_mldatamanager.py | 81 +++++++++++++++++++ ami/main/tests.py | 20 ++++- ami/users/roles.py | 3 + ui/src/data-services/models/deployment.ts | 6 ++ .../pages/deployments/deployment-columns.tsx | 2 +- ui/src/pages/deployments/deployments.tsx | 4 +- ui/src/utils/user/types.ts | 1 + 7 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py diff --git a/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py b/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py new file mode 100644 index 000000000..150b315ed --- /dev/null +++ b/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py @@ -0,0 +1,81 @@ +""" +Grant the existing ``sync_deployment`` permission to ``MLDataManager`` role +groups on projects that already exist. + +``MLDataManager`` already holds ``run_data_storage_sync_job`` (it can run/retry a +sync job) but not ``sync_deployment`` (which gates *starting* a sync from a +station via ``POST /api/v2/deployments//sync/`` and the bulk +``.../sync-all/`` action). This closes that gap so ML data managers can trigger +syncs, not only manage the resulting jobs. ``ProjectManager`` already has the +permission and is unaffected. + +New projects pick this up automatically through ``create_roles_for_project`` +(``MLDataManager`` now includes the permission); this migration backfills the +role groups of existing projects. The ``sync_deployment`` permission itself is +already defined on ``Project.Meta.permissions``, so there is no model/schema +change here — only a data backfill. +""" + +from django.db import migrations +from django.db.models import Q + + +def grant_sync_to_mldatamanager(apps, schema_editor): + Group = apps.get_model("auth", "Group") + Permission = apps.get_model("auth", "Permission") + ContentType = apps.get_model("contenttypes", "ContentType") + + try: + project_ct = ContentType.objects.get(app_label="main", model="project") + except ContentType.DoesNotExist: + return + + try: + perm = Permission.objects.get(codename="sync_deployment", content_type=project_ct) + except Permission.DoesNotExist: + return + + for group in Group.objects.filter(Q(name__endswith="_MLDataManager")): + group.permissions.add(perm) + + +def revoke_sync_from_mldatamanager(apps, schema_editor): + Group = apps.get_model("auth", "Group") + Permission = apps.get_model("auth", "Permission") + ContentType = apps.get_model("contenttypes", "ContentType") + GroupObjectPermission = apps.get_model("guardian", "GroupObjectPermission") + + try: + project_ct = ContentType.objects.get(app_label="main", model="project") + except ContentType.DoesNotExist: + return + + try: + perm = Permission.objects.get(codename="sync_deployment", content_type=project_ct) + except Permission.DoesNotExist: + return + + # Only touch MLDataManager groups; ProjectManager holds sync_deployment + # independently and must keep it. + mldm_groups = Group.objects.filter(Q(name__endswith="_MLDataManager")) + for group in mldm_groups: + group.permissions.remove(perm) + GroupObjectPermission.objects.filter( + permission=perm, + content_type=project_ct, + group__in=mldm_groups, + ).delete() + + +class Migration(migrations.Migration): + dependencies = [ + ("main", "0094_enable_async_pipeline_workers"), + ("guardian", "0002_generic_permissions_index"), + ] + + operations = [ + migrations.RunPython( + grant_sync_to_mldatamanager, + revoke_sync_from_mldatamanager, + ), + ] diff --git a/ami/main/tests.py b/ami/main/tests.py index 26923a93a..87258ca75 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -51,7 +51,14 @@ ) from ami.tests.fixtures.storage import populate_bucket from ami.users.models import User -from ami.users.roles import BasicMember, Identifier, MLDataManager, ProjectManager, create_roles_for_project +from ami.users.roles import ( + BasicMember, + Identifier, + MLDataManager, + ProjectManager, + Researcher, + create_roles_for_project, +) logger = logging.getLogger(__name__) @@ -2919,9 +2926,15 @@ def setUp(self): self.superuser = User.objects.create_superuser(email="super-syncall@insectai.org", password="password123") self.pm_user = User.objects.create_user(email="pm-syncall@insectai.org", password="password123") + self.ml_user = User.objects.create_user(email="ml-syncall@insectai.org", password="password123") + self.researcher = User.objects.create_user(email="researcher-syncall@insectai.org", password="password123") + self.identifier = User.objects.create_user(email="identifier-syncall@insectai.org", password="password123") self.basic_user = User.objects.create_user(email="basic-syncall@insectai.org", password="password123") self.outsider = User.objects.create_user(email="outsider-syncall@insectai.org", password="password123") ProjectManager.assign_user(self.pm_user, self.project) + MLDataManager.assign_user(self.ml_user, self.project) + Researcher.assign_user(self.researcher, self.project) + Identifier.assign_user(self.identifier, self.project) BasicMember.assign_user(self.basic_user, self.project) source = S3StorageSource.objects.create( @@ -2956,9 +2969,14 @@ def test_requires_project_id(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) def test_permission_matrix(self): + # Sync is allowed for MLDataManager and ProjectManager (and superusers), + # not for Researcher, Identifier, BasicMember, or non-members. matrix = [ ("superuser", self.superuser, status.HTTP_200_OK), ("ProjectManager", self.pm_user, status.HTTP_200_OK), + ("MLDataManager", self.ml_user, status.HTTP_200_OK), + ("Researcher", self.researcher, status.HTTP_403_FORBIDDEN), + ("Identifier", self.identifier, status.HTTP_403_FORBIDDEN), ("BasicMember", self.basic_user, status.HTTP_403_FORBIDDEN), ("outsider", self.outsider, status.HTTP_403_FORBIDDEN), ] diff --git a/ami/users/roles.py b/ami/users/roles.py index 5a5f9f6c8..718146e41 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -141,6 +141,9 @@ class MLDataManager(Role): Project.Permissions.RUN_ML_JOB, Project.Permissions.RUN_POPULATE_CAPTURES_COLLECTION_JOB, Project.Permissions.RUN_DATA_STORAGE_SYNC_JOB, + # Can start a storage sync from a station, not only run/retry the job it + # creates. ProjectManager also has this (inherits MLDataManager). + Project.Permissions.SYNC_DEPLOYMENT, Project.Permissions.RUN_REGROUP_EVENTS_JOB, Project.Permissions.RUN_DATA_EXPORT_JOB, Project.Permissions.DELETE_OCCURRENCES, diff --git a/ui/src/data-services/models/deployment.ts b/ui/src/data-services/models/deployment.ts index 15205299f..6986b8762 100644 --- a/ui/src/data-services/models/deployment.ts +++ b/ui/src/data-services/models/deployment.ts @@ -25,6 +25,12 @@ export class Deployment extends Entity { return this._deployment.user_permissions.includes(UserPermission.Delete) } + get canSync(): boolean { + // Superusers can sync but do not receive the `sync` permission in + // user_permissions, so callers gate on `canUpdate || canSync`. + return this._deployment.user_permissions.includes(UserPermission.Sync) + } + get canUpdate(): boolean { return this._deployment.user_permissions.includes(UserPermission.Update) } diff --git a/ui/src/pages/deployments/deployment-columns.tsx b/ui/src/pages/deployments/deployment-columns.tsx index 09519d0d9..a665480a5 100644 --- a/ui/src/pages/deployments/deployment-columns.tsx +++ b/ui/src/pages/deployments/deployment-columns.tsx @@ -232,7 +232,7 @@ export const columns = ({ sticky: true, renderCell: (item: Deployment) => ( - {item.canUpdate && item.dataSourceConnected && ( + {(item.canUpdate || item.canSync) && item.dataSourceConnected && ( )} {item.canDelete && ( diff --git a/ui/src/pages/deployments/deployments.tsx b/ui/src/pages/deployments/deployments.tsx index fdd341927..2c3f15715 100644 --- a/ui/src/pages/deployments/deployments.tsx +++ b/ui/src/pages/deployments/deployments.tsx @@ -36,7 +36,9 @@ export const Deployments = () => { const tableColumns = columns({ projectId: projectId as string }) const syncableCount = deployments?.filter( - (deployment) => deployment.canUpdate && deployment.dataSourceConnected + (deployment) => + (deployment.canUpdate || deployment.canSync) && + deployment.dataSourceConnected ).length ?? 0 return ( diff --git a/ui/src/utils/user/types.ts b/ui/src/utils/user/types.ts index 1a6d5833e..66cc5a53b 100644 --- a/ui/src/utils/user/types.ts +++ b/ui/src/utils/user/types.ts @@ -17,6 +17,7 @@ export enum UserPermission { Run = 'run', // Custom job permission RunSingleImage = 'run_single_image_ml_job', // Custom job permission Star = 'star', + Sync = 'sync', // Custom deployment permission (sync_deployment) Update = 'update', } From 99cf3911631da083936d89602833deeaa0c554e7 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 2 Jul 2026 13:37:33 -0700 Subject: [PATCH 09/16] fix(migrations): grant sync_deployment at the object level, not just globally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 0095 backfill added sync_deployment only to group.permissions (a global Django permission). Authorization here is guardian object-level: get_perms(user, project) and user.has_perm("sync_deployment", project) read the object-level GroupObjectPermission, so the global-only grant was a silent no-op — existing projects' MLDataManager groups would not actually gain the ability to sync. Mirror create_roles_for_project: keep the global add for parity and, more importantly, create the object-level GroupObjectPermission per project (reverse removes both). Add a test that strips the object-level grant, runs the backfill, and asserts the permission and endpoint access are restored — this fails against the global-only version. Reported by Copilot and CodeRabbit on the PR. Co-Authored-By: Claude --- ..._grant_sync_deployment_to_mldatamanager.py | 87 +++++++++++++------ ami/main/tests.py | 52 +++++++++++ 2 files changed, 112 insertions(+), 27 deletions(-) diff --git a/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py b/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py index 150b315ed..caad9d968 100644 --- a/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py +++ b/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py @@ -9,62 +9,95 @@ syncs, not only manage the resulting jobs. ``ProjectManager`` already has the permission and is unaffected. +Permissions here are guardian **object-level** grants on each project, which is +what ``get_perms(user, project)`` and ``user.has_perm("sync_deployment", +project)`` read. Adding the permission only to ``group.permissions`` (a global +Django permission) is not enough — it would not appear in ``get_perms`` and the +Sync buttons / endpoint would stay inaccessible. So this migration mirrors +``create_roles_for_project``: it adds the global permission for parity and, more +importantly, creates the object-level ``GroupObjectPermission`` row per project. + New projects pick this up automatically through ``create_roles_for_project`` -(``MLDataManager`` now includes the permission); this migration backfills the -role groups of existing projects. The ``sync_deployment`` permission itself is -already defined on ``Project.Meta.permissions``, so there is no model/schema -change here — only a data backfill. +(``MLDataManager`` now includes the permission). A ``post_migrate`` signal +(``ami.main.apps`` → ``create_roles``) also re-syncs every project's role +permissions on migrate, so this backfill is belt-and-suspenders; it is kept so +the grant is explicit and self-contained rather than relying on that signal. + +The ``sync_deployment`` permission itself is already defined on +``Project.Meta.permissions``, so there is no model/schema change here — only a +data backfill. """ from django.db import migrations from django.db.models import Q -def grant_sync_to_mldatamanager(apps, schema_editor): - Group = apps.get_model("auth", "Group") +def _sync_permission(apps): Permission = apps.get_model("auth", "Permission") ContentType = apps.get_model("contenttypes", "ContentType") - try: project_ct = ContentType.objects.get(app_label="main", model="project") except ContentType.DoesNotExist: - return - + return None, None try: - perm = Permission.objects.get(codename="sync_deployment", content_type=project_ct) + return Permission.objects.get(codename="sync_deployment", content_type=project_ct), project_ct except Permission.DoesNotExist: + return None, None + + +def _project_pk_from_group(group): + # Group names are "{project_pk}_{project_name}_{RoleName}"; the pk is the + # immutable leading segment (the name can contain underscores). + try: + return int(group.name.split("_", 1)[0]) + except (ValueError, IndexError): + return None + + +def grant_sync_to_mldatamanager(apps, schema_editor): + Group = apps.get_model("auth", "Group") + GroupObjectPermission = apps.get_model("guardian", "GroupObjectPermission") + + perm, project_ct = _sync_permission(apps) + if perm is None: return for group in Group.objects.filter(Q(name__endswith="_MLDataManager")): + project_pk = _project_pk_from_group(group) + if project_pk is None: + continue + # Global add for parity with create_roles_for_project; the object-level + # row below is what get_perms()/has_perm(perm, project) actually read. group.permissions.add(perm) + GroupObjectPermission.objects.get_or_create( + permission=perm, + content_type=project_ct, + object_pk=str(project_pk), + group=group, + ) def revoke_sync_from_mldatamanager(apps, schema_editor): Group = apps.get_model("auth", "Group") - Permission = apps.get_model("auth", "Permission") - ContentType = apps.get_model("contenttypes", "ContentType") GroupObjectPermission = apps.get_model("guardian", "GroupObjectPermission") - try: - project_ct = ContentType.objects.get(app_label="main", model="project") - except ContentType.DoesNotExist: - return - - try: - perm = Permission.objects.get(codename="sync_deployment", content_type=project_ct) - except Permission.DoesNotExist: + perm, project_ct = _sync_permission(apps) + if perm is None: return # Only touch MLDataManager groups; ProjectManager holds sync_deployment # independently and must keep it. - mldm_groups = Group.objects.filter(Q(name__endswith="_MLDataManager")) - for group in mldm_groups: + for group in Group.objects.filter(Q(name__endswith="_MLDataManager")): + project_pk = _project_pk_from_group(group) + if project_pk is None: + continue group.permissions.remove(perm) - GroupObjectPermission.objects.filter( - permission=perm, - content_type=project_ct, - group__in=mldm_groups, - ).delete() + GroupObjectPermission.objects.filter( + permission=perm, + content_type=project_ct, + object_pk=str(project_pk), + group=group, + ).delete() class Migration(migrations.Migration): diff --git a/ami/main/tests.py b/ami/main/tests.py index 87258ca75..62dd5c305 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3006,6 +3006,58 @@ def test_enqueues_one_job_per_connected_station(self): self.assertEqual(jobs.first().deployment_id, self.connected.pk, "Unconnected station must be skipped") +class TestSyncDeploymentBackfillMigration(APITestCase): + """The 0095 backfill grants OBJECT-LEVEL sync_deployment to existing projects' + MLDataManager groups, not just a global group permission. + + A global-only grant (``group.permissions.add``) would leave ``get_perms`` and + ``has_perm(perm, project)`` — the checks the endpoint and UI actually use — + returning False, so the backfill would silently no-op for existing projects. + This pins the object-level path. + """ + + def test_backfill_grants_object_level_sync_to_mldatamanager(self): + import importlib + from unittest import mock + + from django.apps import apps as global_apps + from django.contrib.auth.models import Group + + project = Project.objects.create(name="Sync Backfill Project") + create_roles_for_project(project) + ml_user = User.objects.create_user(email="ml-backfill@insectai.org", password="password123") + MLDataManager.assign_user(ml_user, project) + source = S3StorageSource.objects.create( + name="Backfill Source", + bucket="test-bucket", + access_key="fake-access-key", + secret_key="fake-secret-key", + project=project, + ) + Deployment.objects.create(name="Backfill Station", project=project, data_source=source) + + mldm_group = Group.objects.get(name=f"{project.pk}_{project.name}_MLDataManager") + + # Simulate a project created before MLDataManager gained the permission: + # strip the object-level grant so the sync endpoint is denied. + remove_perm("sync_deployment", mldm_group, project) + self.assertNotIn("sync_deployment", get_perms(mldm_group, project)) + + self.client.force_authenticate(ml_user) + denied = self.client.post(f"/api/v2/deployments/sync-all/?project_id={project.pk}") + self.assertEqual(denied.status_code, status.HTTP_403_FORBIDDEN) + + # Run the backfill and confirm it restores the object-level permission + # (a global-only grant would leave get_perms unchanged and the POST 403). + migration = importlib.import_module("ami.main.migrations.0095_grant_sync_deployment_to_mldatamanager") + migration.grant_sync_to_mldatamanager(global_apps, None) + + self.assertIn("sync_deployment", get_perms(mldm_group, project)) + with mock.patch("ami.jobs.models.Job.enqueue"): + granted = self.client.post(f"/api/v2/deployments/sync-all/?project_id={project.pk}") + self.assertEqual(granted.status_code, status.HTTP_200_OK) + + class TestFineGrainedJobRunPermission(APITestCase): def setUp(self): super().setUp() From 1ec372f93f034bed514c8f3915d9b2d302d7bcd3 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 2 Jul 2026 13:41:58 -0700 Subject: [PATCH 10/16] fix(ui): don't re-enable Sync while a request is still in flight Reopening a sync dialog reset the mutation state to offer a fresh sync, but reset() clears the loading state without cancelling the in-flight request. If the dialog was closed and reopened during a request, Sync became clickable again and could enqueue a second job for the same deployment (or a second bulk run). Guard the reset on `!isLoading`, so reopening during an active request keeps the button disabled until the request finishes. Applies to both the per-row and Sync all dialogs. Reported by CodeRabbit on the PR. Co-Authored-By: Claude --- ui/src/pages/deployments/sync-all-deployments-dialog.tsx | 7 +++++-- ui/src/pages/deployments/sync-deployment-dialog.tsx | 9 ++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/ui/src/pages/deployments/sync-all-deployments-dialog.tsx b/ui/src/pages/deployments/sync-all-deployments-dialog.tsx index 7a2083b4d..60e787c01 100644 --- a/ui/src/pages/deployments/sync-all-deployments-dialog.tsx +++ b/ui/src/pages/deployments/sync-all-deployments-dialog.tsx @@ -29,8 +29,11 @@ export const SyncAllDeploymentsDialog = ({ onOpenChange={(open) => { setIsOpen(open) // The hook stays mounted with the header, so reset on open to offer a - // fresh sync instead of the previous run's result. - if (open) { + // fresh sync instead of the previous run's result. Skip the reset while a + // request is in flight: reset() clears the loading state without + // cancelling the request, so resetting here would re-enable Sync all and + // allow a duplicate bulk run. + if (open && !isLoading) { reset() } }} diff --git a/ui/src/pages/deployments/sync-deployment-dialog.tsx b/ui/src/pages/deployments/sync-deployment-dialog.tsx index ffc33727f..fb975aa36 100644 --- a/ui/src/pages/deployments/sync-deployment-dialog.tsx +++ b/ui/src/pages/deployments/sync-deployment-dialog.tsx @@ -35,9 +35,12 @@ export const SyncDeploymentDialog = ({ onOpenChange={(open) => { setIsOpen(open) // The hook is mounted for the whole row, so success/error/data survive - // a close. Reset on open so reopening offers a fresh sync instead of - // the previous attempt's stale state. - if (open) { + // a close. Reset on open so reopening offers a fresh sync instead of the + // previous attempt's stale state. Skip the reset while a request is in + // flight: reset() clears the loading state without cancelling the + // request, so resetting here would re-enable Sync and allow a duplicate + // job for the same deployment. + if (open && !isLoading) { reset() } }} From 7f8e318f71ef8994df17930818fb7a1386162dfb Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 2 Jul 2026 13:41:59 -0700 Subject: [PATCH 11/16] docs: frame the station-sync design scope as initial-phase only The design and handoff notes said "frontend only, no migration", which no longer matches the shipped PR (backend serializer field, sync-all endpoint, and an ML-data-manager permission with a data migration). Mark those scope lines as the initial phase and point to the addendum / final scope. Reported by CodeRabbit on the PR. Co-Authored-By: Claude --- .../claude/planning/2026-06-29-station-sync-action-NEXT.md | 7 ++++++- .../planning/2026-06-29-station-sync-action-design.md | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md index a705e75ff..3ecd59ec4 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md @@ -12,7 +12,12 @@ A per-row "Sync" floating action button on the Stations (deployments) list, beside Delete. Confirmation dialog → fires the existing async `DataStorageSyncJob` via `POST /api/v2/deployments/{id}/sync/`; shows spinner → check + a link to the created job; no-data-source 400 surfaced via `FormError`. -Frontend-only, no backend change, no migration. + +**Note (historical):** this section describes the first phase. The shipped PR +also includes backend work — a `data_source_connected` list-serializer field, a +bulk `sync-all` endpoint, and granting the sync permission to ML data managers +(with a data migration). The "Frontend-only, no migration" framing applies to +the initial phase only. Commits on the branch: 1. `e6a4303c` docs: design doc diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md index e784cd728..40732b90d 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-design.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -1,7 +1,10 @@ # Station row "Sync" floating action button — design **Date:** 2026-06-29 -**Scope:** Frontend only (`ui/`). No backend change, no migration. +**Scope (initial phase):** Frontend only (`ui/`). No backend change, no +migration. This describes the first phase only; the PR later grew a backend +piece (list-serializer field, bulk `sync-all` endpoint, and an ML-data-manager +permission with a data migration) — see the Addendum at the end of this document. **Branch/worktree:** `worktree-station-sync-action` ## Summary From 01d1c279aceaf859c9f6a9c103374b2e59c4de0e Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 9 Jul 2026 13:13:27 -0700 Subject: [PATCH 12/16] docs: correct sync permission model and trim session handoff The design addendum and handoff doc claimed superusers do not receive the `sync` permission and that the button is gated on `canUpdate`. Both are wrong: guardian returns every project permission for superusers, so they do get `sync`, and the button is gated on `canSync`. Correct the addendum, and trim the handoff doc to a scrubbed status summary (drop local paths and the session repair note). Co-Authored-By: Claude --- .../2026-06-29-station-sync-action-NEXT.md | 166 ++++-------------- .../2026-06-29-station-sync-action-design.md | 20 ++- 2 files changed, 50 insertions(+), 136 deletions(-) diff --git a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md index 3ecd59ec4..91af141a9 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md @@ -1,128 +1,38 @@ -# Station Sync action — session handoff / next steps - -**Last updated:** 2026-07-01 -**Worktree:** `/home/michael/Projects/AMI/antenna/.claude/worktrees/station-sync-action` -**Local branch:** `worktree-station-sync-action` → tracks `origin/feat/station-sync-action` -**PR:** #1357 (draft) — https://github.com/RolnickLab/antenna/pull/1357 -**Head commit:** `89c25743` - -## What is done (shipped in PR #1357, draft) - -A per-row "Sync" floating action button on the Stations (deployments) list, -beside Delete. Confirmation dialog → fires the existing async -`DataStorageSyncJob` via `POST /api/v2/deployments/{id}/sync/`; shows spinner → -check + a link to the created job; no-data-source 400 surfaced via `FormError`. - -**Note (historical):** this section describes the first phase. The shipped PR -also includes backend work — a `data_source_connected` list-serializer field, a -bulk `sync-all` endpoint, and granting the sync permission to ML data managers -(with a data migration). The "Frontend-only, no migration" framing applies to -the initial phase only. - -Commits on the branch: -1. `e6a4303c` docs: design doc -2. `26439c41` implementation -3. `89c25743` self-review fixes (mutation `reset()` on dialog open; swallow the - intended-400 rejection; clearer "Sync captures" dialog title) - -Design doc: `docs/claude/planning/2026-06-29-station-sync-action-design.md`. - -### Verification status -- lint / prettier / tsc clean for the changed files (only the pre-existing, - unrelated `react-zoom-pan-pinch` missing-dep tsc errors remain in the tree). -- **NOT yet run against a live stack.** Local docker-compose browser test is the - first next-session task. - -### Repair note (context) -Early in the session the edits accidentally landed in the **main checkout** -(`/home/michael/Projects/AMI/antenna`, then on branch -`fix/disable-prod-admin-email-handler`). All work was moved to this worktree and -the main repo was reset back to `585b1b07` (clean, untouched). When operating in -a worktree: run git with `-C ` and edit files under the worktree path, -not the main checkout. - -## Key files (current feature) -- `ui/src/pages/deployments/sync-deployment-dialog.tsx` — the dialog component. -- `ui/src/pages/deployments/deployment-columns.tsx` — actions column `Toolbar` - (Sync gated on `item.canUpdate`, left of Delete). -- `ui/src/pages/deployments/deployments.tsx` — list page; header has - `SortControl` (line ~49) and the "Create new station" control — where a - "Sync all" button would go. -- `ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts` — - the mutation hook (now exposes `reset`). -- `ui/src/utils/language.ts` — strings `SYNC`, `SYNC_CAPTURES`, - `MESSAGE_SYNC_CONFIRM`, `VIEW_JOB`. - -## Next-session task list - -### 1. Local docker-compose test (do first) -- `docker compose up -d` (from main repo, or bind-mount worktree `ui/`). - Local UI: http://localhost:4000, API: http://localhost:8000, creds - antenna@insectai.org / localadmin. -- To test worktree FE against the main stack, see - `docs/claude/reference/worktree-testing.md` (bind-mount worktree subdirs) — - note the compose file mounts `.:/app:z`, so mount subdirectories, not `/app`. -- Verify: button visibility by permission (`canUpdate`); confirm → job link - navigates; no-data-source station → 400 shown in dialog; reopen after success - offers a fresh sync (reset works). -- Use Chrome DevTools MCP for the browser checks + a PR screenshot. - -### 2. Plan a "Sync all" button -- Location: Stations list header near "Create new station" / `SortControl` - (`deployments.tsx`). -- Scope: sync every station in the project that has a data source and that the - user can update. Confirmation dialog should state how many will be synced and - that each starts its own job. -- Backend: there is **no bulk sync endpoint** today. Options: - - (a) FE fires N `POST /deployments/{id}/sync/` calls (one per eligible - station). Simple, reuses the object-permission-checked endpoint; but N - requests + the FE must know which stations are eligible (the list serializer - has no data-source field — see the connected-gate note below). - - (b) New bulk endpoint, e.g. `POST /deployments/sync_all/?project_id=` (or a - project-scoped action) that enqueues one job per deployment server-side and - returns the job ids. Mirrors the admin bulk action (below). Fewer round - trips, server decides eligibility. - - Recommendation to discuss: (b) — it matches the admin precedent and keeps - eligibility logic server-side. - -### 3. Permissions review -- Per-row sync endpoint already enforces object perms: - `DeploymentViewSet` has `permission_classes = [ObjectPermission]` and `sync` - uses `self.get_object()` (`ami/main/api/views.py:284,303,339-361`). Good. -- FE gates the per-row button on `item.canUpdate`. -- For "Sync all": decide the required permission (project-level update? per- - deployment update filtered server-side?) and make the bulk endpoint filter the - queryset to deployments the user may update, skipping the rest (like the admin - action skips no-project / no-data-source). - -### 4. One consolidated job vs separate jobs (Sync all + Admin bulk) -**Precedent already in the codebase — separate jobs:** -- Admin bulk action `DeploymentAdmin.sync_captures` (`ami/main/admin.py:208-232`) - loops the selected queryset and creates **one `DataStorageSyncJob` per - deployment** (`job.enqueue()` each), skipping deployments with no project / no - data source, then reports the queued job ids. -- Same one-task-per-item shape as `calculate_size_async` (admin.py:791) and - `populate_collection_async` (admin.py:834). -- `DataStorageSyncJob` has a **single** `job.deployment` FK - (`ami/jobs/models.py:700+`, `run()` calls `job.deployment.sync_captures(...)`), - so a job models exactly one deployment today. - -**Decision to make:** -- Keep **separate** (one job per station): zero backend model change, per-station - progress / retry / visibility, consistent with admin + other bulk actions. - Cost: floods the jobs list when syncing many stations at once. -- **Consolidate** (one job for a Sync-all run): single jobs-list entry and one - progress bar, but requires either a new job type that iterates multiple - deployments (M2M or project-scoped) or a parent/child job structure — real - model + progress work. -- Leaning: keep separate for now (matches precedent, cheapest, most granular); - revisit consolidation only if the jobs-list flooding becomes a real complaint. - If consolidating, a "parent Sync-all job with child per-deployment jobs" is the - cleaner shape than an M2M on `DataStorageSyncJob`. - -## Open decision already flagged in PR #1357 -- Visibility gate is `canUpdate` only because `DeploymentListSerializer` - (`ami/main/api/serializers.py:180-211`) exposes no data-source field. To hide - the button on unconnected stations (and to let a FE "Sync all" know - eligibility), add `data_source_uri` (or a `data_source_connected` boolean) to - the list serializer. This ties into task #2 option (a). +# Station Sync action — status + +**Status:** shipped in PR #1357 — https://github.com/RolnickLab/antenna/pull/1357 +**Branch:** `feat/station-sync-action` +**Design doc:** `docs/claude/planning/2026-06-29-station-sync-action-design.md` + +## What shipped + +- **Per-row "Sync"** floating action button on the Stations (deployments) list. + Confirmation dialog → `POST /api/v2/deployments/{id}/sync/` → creates and + enqueues a `DataStorageSyncJob`; success shows a link to the job. +- **Header "Sync all"** button → `POST /api/v2/deployments/sync-all/?project_id=` + → one `DataStorageSyncJob` per connected station (separate jobs, matching the + admin bulk action). Shown only when at least one station is syncable. +- **`data_source_connected`** boolean on `DeploymentListSerializer` so the + frontend can hide the button on stations with no storage source. +- **Sync permission** (`sync_deployment`) granted to the ML data manager role + (in addition to project manager), with data migration + `0095_grant_sync_deployment_to_mldatamanager` backfilling existing projects. + +## Permission model + +- The sync action requires the `sync_deployment` guardian permission, held by + ML data managers, project managers, and superusers. The frontend gates the + button on `canSync` (the `sync` entry in a deployment's `user_permissions`); + superusers receive it because guardian returns every project permission for + them. +- `sync_all` is a `detail=False` action, which DRF does not route through + `has_object_permission`, so it checks the permission itself with a transient + probe: `Deployment(project=project).check_permission(user, "sync")`. + +## Concurrency + +Two syncs on the same station cannot corrupt data: event regrouping is +serialized by the per-deployment cache lock in `group_images_into_events` +(the "sync-time regroup" caller named in its docstring), and source-image +insertion upserts on `(deployment, path)`. Duplicate "Sync all" runs therefore +only cost redundant work and extra job rows, not incorrect state. diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md index 40732b90d..997e8a1ea 100644 --- a/docs/claude/planning/2026-06-29-station-sync-action-design.md +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -191,7 +191,7 @@ owner's call): `DeploymentListSerializer` gets a read-only `data_source_connected` boolean (`obj.data_source_id is not None`). No query cost (the id is on the row), no migration. The per-row Sync button now shows only when -`item.canUpdate && item.dataSourceConnected`, so it is hidden on stations with no +`item.canSync && item.dataSourceConnected`, so it is hidden on stations with no storage source. The endpoint's 400 stays as a safety net for races. The field also feeds "Sync all" eligibility on the frontend. @@ -210,10 +210,14 @@ single-`deployment` FK on `DataStorageSyncJob`. Returns transient probe: `Deployment(project=project).check_permission(user, "sync")`, which resolves to the `sync_deployment` guardian permission — exactly what the per-row `sync` action requires. Superusers pass via Django's bypass; - `ProjectManager` passes via guardian; `BasicMember` is denied (403). -- The per-row button stays gated on `canUpdate`, **not** a `sync` permission. - Superusers receive `["update", "delete"]` in `user_permissions` but not - `"sync"` (guardian `get_perms` returns nothing for superusers), so gating on - `sync` would wrongly hide the button from them. `update_deployment` and - `sync_deployment` are co-granted (only to `ProjectManager`), so `canUpdate` - covers exactly the users who can sync. + `ProjectManager` and `MLDataManager` pass via guardian; `BasicMember`, + `Researcher`, and `Identifier` are denied (403). +- The per-row button is gated on `canSync` — the `sync` entry in a deployment's + `user_permissions`, backed by the `sync_deployment` guardian permission. + Superusers receive `sync` there too: guardian's `get_perms` returns every + permission defined on the project's content type for a superuser, and + `sync_deployment` is one of them. `sync_deployment` is held by `MLDataManager` + and `ProjectManager` (and superusers); `update_deployment` is held only by + `ProjectManager`, so gating on `canUpdate` would wrongly hide the button from + ML data managers who can sync. `canSync` is the exact set of users who may + sync. From 0d60a1e6a9116f7c367f284a975331a4a040e8c0 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 9 Jul 2026 13:13:40 -0700 Subject: [PATCH 13/16] fix(deployments): gate sync button on sync permission, not update The per-row and "Sync all" buttons were gated on `canUpdate || canSync`. The `canUpdate` arm was added on the mistaken belief that superusers do not receive the `sync` permission; in fact guardian returns every project permission for superusers, so `canSync` already covers them. The OR gate is not only redundant but a latent bug: any future role granted `update_deployment` without `sync_deployment` would see a sync button that 403s on click. Gate on `canSync` alone, the exact set of users the endpoint permits (ML data managers, project managers, superusers). Add a per-row sync permission matrix test; previously only the bulk endpoint was covered, though both paths share the same `sync_deployment` check. Co-Authored-By: Claude --- ami/main/tests.py | 22 +++++++++++++++++++ ui/src/data-services/models/deployment.ts | 5 +++-- .../pages/deployments/deployment-columns.tsx | 2 +- ui/src/pages/deployments/deployments.tsx | 4 +--- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 62dd5c305..a0d0253a1 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3005,6 +3005,28 @@ def test_enqueues_one_job_per_connected_station(self): self.assertEqual(jobs.count(), 1, "Exactly one sync job, for the connected station") self.assertEqual(jobs.first().deployment_id, self.connected.pk, "Unconnected station must be skipped") + def test_per_row_sync_permission_matrix(self): + # The per-row POST /deployments/{id}/sync/ action enforces the same + # sync_deployment permission as the bulk endpoint. This pins that the + # newly-granted MLDataManager role can start a sync and that roles + # without the permission are refused (403), so the per-row and bulk + # paths cannot silently diverge on who may sync. + url = f"/api/v2/deployments/{self.connected.pk}/sync/" + matrix = [ + ("superuser", self.superuser, status.HTTP_200_OK), + ("ProjectManager", self.pm_user, status.HTTP_200_OK), + ("MLDataManager", self.ml_user, status.HTTP_200_OK), + ("Researcher", self.researcher, status.HTTP_403_FORBIDDEN), + ("Identifier", self.identifier, status.HTTP_403_FORBIDDEN), + ("BasicMember", self.basic_user, status.HTTP_403_FORBIDDEN), + ("outsider", self.outsider, status.HTTP_403_FORBIDDEN), + ] + for role_name, user, expected in matrix: + with self.subTest(role=role_name): + self.client.force_authenticate(user) + response = self.client.post(url) + self.assertEqual(response.status_code, expected, f"{role_name} got {response.status_code}") + class TestSyncDeploymentBackfillMigration(APITestCase): """The 0095 backfill grants OBJECT-LEVEL sync_deployment to existing projects' diff --git a/ui/src/data-services/models/deployment.ts b/ui/src/data-services/models/deployment.ts index 6986b8762..c99c9865e 100644 --- a/ui/src/data-services/models/deployment.ts +++ b/ui/src/data-services/models/deployment.ts @@ -26,8 +26,9 @@ export class Deployment extends Entity { } get canSync(): boolean { - // Superusers can sync but do not receive the `sync` permission in - // user_permissions, so callers gate on `canUpdate || canSync`. + // Granted to ML data managers, project managers, and superusers. Superusers + // receive `sync` here too (guardian returns every project permission for + // them), so this getter alone is the sync gate. return this._deployment.user_permissions.includes(UserPermission.Sync) } diff --git a/ui/src/pages/deployments/deployment-columns.tsx b/ui/src/pages/deployments/deployment-columns.tsx index a665480a5..0a86d0a36 100644 --- a/ui/src/pages/deployments/deployment-columns.tsx +++ b/ui/src/pages/deployments/deployment-columns.tsx @@ -232,7 +232,7 @@ export const columns = ({ sticky: true, renderCell: (item: Deployment) => ( - {(item.canUpdate || item.canSync) && item.dataSourceConnected && ( + {item.canSync && item.dataSourceConnected && ( )} {item.canDelete && ( diff --git a/ui/src/pages/deployments/deployments.tsx b/ui/src/pages/deployments/deployments.tsx index 2c3f15715..c14cb6985 100644 --- a/ui/src/pages/deployments/deployments.tsx +++ b/ui/src/pages/deployments/deployments.tsx @@ -36,9 +36,7 @@ export const Deployments = () => { const tableColumns = columns({ projectId: projectId as string }) const syncableCount = deployments?.filter( - (deployment) => - (deployment.canUpdate || deployment.canSync) && - deployment.dataSourceConnected + (deployment) => deployment.canSync && deployment.dataSourceConnected ).length ?? 0 return ( From d7ef48497f50a66443fd22e8e852823b50c248a8 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 16 Jul 2026 15:44:41 -0700 Subject: [PATCH 14/16] docs: drop station-sync planning docs from PR Keep the PR focused on the shipped code. The design notes and permission analysis remain in the branch history and are not needed in the merged tree. Co-Authored-By: Claude --- .../2026-06-29-station-sync-action-NEXT.md | 38 --- .../2026-06-29-station-sync-action-design.md | 223 ------------------ 2 files changed, 261 deletions(-) delete mode 100644 docs/claude/planning/2026-06-29-station-sync-action-NEXT.md delete mode 100644 docs/claude/planning/2026-06-29-station-sync-action-design.md diff --git a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md deleted file mode 100644 index 91af141a9..000000000 --- a/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md +++ /dev/null @@ -1,38 +0,0 @@ -# Station Sync action — status - -**Status:** shipped in PR #1357 — https://github.com/RolnickLab/antenna/pull/1357 -**Branch:** `feat/station-sync-action` -**Design doc:** `docs/claude/planning/2026-06-29-station-sync-action-design.md` - -## What shipped - -- **Per-row "Sync"** floating action button on the Stations (deployments) list. - Confirmation dialog → `POST /api/v2/deployments/{id}/sync/` → creates and - enqueues a `DataStorageSyncJob`; success shows a link to the job. -- **Header "Sync all"** button → `POST /api/v2/deployments/sync-all/?project_id=` - → one `DataStorageSyncJob` per connected station (separate jobs, matching the - admin bulk action). Shown only when at least one station is syncable. -- **`data_source_connected`** boolean on `DeploymentListSerializer` so the - frontend can hide the button on stations with no storage source. -- **Sync permission** (`sync_deployment`) granted to the ML data manager role - (in addition to project manager), with data migration - `0095_grant_sync_deployment_to_mldatamanager` backfilling existing projects. - -## Permission model - -- The sync action requires the `sync_deployment` guardian permission, held by - ML data managers, project managers, and superusers. The frontend gates the - button on `canSync` (the `sync` entry in a deployment's `user_permissions`); - superusers receive it because guardian returns every project permission for - them. -- `sync_all` is a `detail=False` action, which DRF does not route through - `has_object_permission`, so it checks the permission itself with a transient - probe: `Deployment(project=project).check_permission(user, "sync")`. - -## Concurrency - -Two syncs on the same station cannot corrupt data: event regrouping is -serialized by the per-deployment cache lock in `group_images_into_events` -(the "sync-time regroup" caller named in its docstring), and source-image -insertion upserts on `(deployment, path)`. Duplicate "Sync all" runs therefore -only cost redundant work and extra job rows, not incorrect state. diff --git a/docs/claude/planning/2026-06-29-station-sync-action-design.md b/docs/claude/planning/2026-06-29-station-sync-action-design.md deleted file mode 100644 index 997e8a1ea..000000000 --- a/docs/claude/planning/2026-06-29-station-sync-action-design.md +++ /dev/null @@ -1,223 +0,0 @@ -# Station row "Sync" floating action button — design - -**Date:** 2026-06-29 -**Scope (initial phase):** Frontend only (`ui/`). No backend change, no -migration. This describes the first phase only; the PR later grew a backend -piece (list-serializer field, bulk `sync-all` endpoint, and an ML-data-manager -permission with a data migration) — see the Addendum at the end of this document. -**Branch/worktree:** `worktree-station-sync-action` - -## Summary - -Add a "Sync" floating action button to each row in the Stations (deployments) -list table, alongside the existing Delete button. Clicking it opens a small -confirmation dialog; confirming triggers an asynchronous data-storage sync job -that scans the deployment's connected S3 storage source and imports any new -captures. The button mirrors the existing per-row action pattern (the Delete -trash icon) and reuses the sync endpoint and React Query hook that already power -the "Sync now" button in the deployment detail view. - -## What already exists (no work needed) - -- **Backend endpoint:** `POST /api/v2/deployments/{pk}/sync/` - (`DeploymentViewSet.sync`, `ami/main/api/views.py:339-361`). Creates a - `DataStorageSyncJob` and enqueues it; returns `{ "job_id": , - "project_id": }`. Raises `ValidationError` (HTTP 400) when the - deployment has no `data_source`. -- **React Query hook:** `useSyncDeploymentSourceImages` - (`ui/src/data-services/hooks/deployments/useSyncDeploymentSourceImages.ts`). - `POST`s to the endpoint, on success invalidates the `[JOBS]` and `[CAPTURES]` - query keys, and exposes `{ syncDeploymentSourceImages, isLoading, isSuccess, - error, data }` where `data.data` is `{ job_id, project_id }`. -- **Row action toolbar:** `Toolbar` from `nova-ui-kit` - (`ui/src/nova-ui-kit/components/toolbar.tsx`) — the circular pill that holds - per-row icon buttons. -- **Reference for the confirmation-dialog action pattern:** `DeleteEntityDialog` - (`ui/src/pages/project/entities/delete-entity-dialog.tsx`) + `DeleteForm` - (`ui/src/components/form/delete-form/delete-form.tsx`). -- **Reference for the sync UX (button states + job link):** - `SyncDeploymentSourceImages` - (`ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx`). - -## Decisions (from brainstorming) - -1. **Interaction:** confirmation dialog (not one-click). Matches the Delete - button neighbor and guards against accidental row-misclicks. -2. **Gating:** `item.canUpdate` only. The deployments **list** serializer - (`DeploymentListSerializer`, `ami/main/api/serializers.py:180-211`) carries - no data-source field, so a "show only when connected" gate is not possible - without a backend change, which we are deliberately not doing here. - Consequence: the button shows on every station the user can update; clicking - it on a station with **no** data source returns HTTP 400, which the dialog - surfaces via `FormError` (see error handling below). -3. **Feedback:** inline icon state (the app has no toast system). Sync button - shows a spinner while loading and a check on success; on success the dialog - also renders an eye-icon link to the created job, mirroring the detail-view - sync component. - -## Components - -### New file: `ui/src/pages/deployments/sync-deployment-dialog.tsx` - -`SyncDeploymentDialog`, modeled on `DeleteEntityDialog`. - -Props: - -```ts -{ - id: string // deployment id - projectId: string // for the job-details link route -} -``` - -Structure: - -- `Dialog.Root` with a local `useState` `isOpen` flag (`open` / `onOpenChange`), - same as `DeleteEntityDialog`. -- `Dialog.Trigger asChild` wrapping a `nova-ui-kit` `Button`: - - `size="icon"`, `variant="ghost"`, `className="shrink-0"`, - `aria-label={translate(STRING.SYNC)}`. - - Child: lucide `RefreshCwIcon` at `className="w-4 h-4"`. -- `Dialog.Content` (`ariaCloselabel={translate(STRING.CLOSE)}`, `isCompact`) - containing the confirmation body (built inline with the shared - `FormSection` / `FormError` layout from `components/form/layout/layout`, the - same primitives `DeleteForm` uses): - - If `error`: `` - (covers the no-data-source 400). - - `FormSection` with `title` = "Sync captures from storage source?" and - `description` = `translate(STRING.MESSAGE_SYNC_CONFIRM)`. - - A right-aligned button row: - - Cancel `Button` (`size="small"`, `variant="outline"`) → - `setIsOpen(false)`. - - Sync `Button` (`size="small"`, `variant="success"`, - `disabled={isLoading || isSuccess}`) → `syncDeploymentSourceImages(id)`. - Label `translate(STRING.SYNC)`; shows `Loader2Icon` (spin) while loading, - `CheckIcon` on success. - - On success (`isSuccess && data`): render an eye-icon `Link` - (styled `buttonVariants({ size: 'icon', variant: 'ghost' })`) to - `APP_ROUTES.JOB_DETAILS({ projectId, jobId: String(data.data.job_id) })` - via `getAppRoute({ to, keepSearchParams: true })`, wrapped in a - `BasicTooltip` (`content` = "View sync job"). Same shape as the existing - detail-view sync component. - -Hook usage: `const { syncDeploymentSourceImages, isLoading, isSuccess, error, -data } = useSyncDeploymentSourceImages()`. - -### Edit: `ui/src/pages/deployments/deployment-columns.tsx` - -In the `actions` column `renderCell` (currently lines ~232-242), add the sync -button inside the existing `Toolbar`, to the **left** of the Delete button: - -```tsx - - {item.canUpdate && ( - - )} - {item.canDelete && ( - - )} - -``` - -Add the import for `SyncDeploymentDialog`. `projectId` is already in scope -(the `columns({ projectId })` factory argument). - -### Edit: `ui/src/utils/language.ts` - -Add two `STRING` enum members and their English values: - -- `SYNC` → `"Sync"` (button label + aria-label). -- `MESSAGE_SYNC_CONFIRM` → - `"This scans the connected storage source and imports any new captures as a background job."` - -Place them near the existing sync-related strings -(`MESSAGE_CAPTURE_SYNC_HIDDEN`, `FIELD_LABEL_LAST_SYNCED`) following the file's -ordering convention. - -## Data flow - -1. User hovers a station row → `Toolbar` appears → clicks the `RefreshCwIcon`. -2. `SyncDeploymentDialog` opens; user clicks **Sync**. -3. `syncDeploymentSourceImages(id)` → `POST /api/v2/deployments/{id}/sync/`. -4. **Success:** hook invalidates `[JOBS]` + `[CAPTURES]`; dialog shows the check - state and the eye-icon link to the new job. User can click through to the - job detail page or close the dialog. -5. **Failure (e.g. no data source → 400):** `error` is set; `FormError` renders - the parsed server message inside the dialog. User can cancel. - -## Error handling - -- No-data-source stations: backend returns 400 `ValidationError`; surfaced in - the dialog via `FormError` + `parseServerError`. The confirmation step plus - the in-dialog error message is the agreed mitigation for gating on - `canUpdate` only. -- Generic request failures: same `FormError` path. - -## Testing / verification - -- `cd ui && yarn lint` and `yarn format` clean. -- TypeScript compiles (`tsc --noEmit` via the build). -- Manual (Chrome DevTools MCP against the local stack): - - Sync button appears in station rows for an Update-permitted user, left of - Delete; hidden for users without Update permission. - - Click → dialog → confirm → spinner → check → job link navigates to the job - detail page; a `DataStorageSyncJob` is created. - - Confirm on a station with no configured data source → dialog shows the 400 - error message, no job created. - -## Known limitations / follow-ups - -- **`isSuccess` persistence (resolved in-scope):** the mutation state lives on - the row-mounted hook instance, so without intervention reopening the dialog on - the same row would show the prior success/error state and leave the Sync - button disabled. The hook now exposes `reset`, and the dialog calls it on open - so each open offers a fresh sync. (More important here than in the detail-view - sync component, which unmounts on navigation; a list row stays mounted.) -- **Connected gate:** deferred. If we later want to hide the button on - unconnected stations (or add a "Last synced" column), add `data_source_uri` - (or a lightweight `data_source_connected` boolean) to - `DeploymentListSerializer.fields` and gate on it. - -## Addendum (2026-07-01): connected gate + "Sync all" - -After the live test, three follow-ups were pulled into this PR (per the product -owner's call): - -### 1. `data_source_connected` on the list serializer -`DeploymentListSerializer` gets a read-only `data_source_connected` boolean -(`obj.data_source_id is not None`). No query cost (the id is on the row), no -migration. The per-row Sync button now shows only when -`item.canSync && item.dataSourceConnected`, so it is hidden on stations with no -storage source. The endpoint's 400 stays as a safety net for races. The field -also feeds "Sync all" eligibility on the frontend. - -### 2. "Sync all" bulk endpoint -`DeploymentViewSet.sync_all` (`detail=False`, POST, `project_id` via -`ProjectMixin.get_active_project`). It enqueues one `DataStorageSyncJob` per -connected deployment in the project — **separate jobs, not one consolidated -job** — matching the admin bulk action (`DeploymentAdmin.sync_captures`) and the -single-`deployment` FK on `DataStorageSyncJob`. Returns -`{ job_ids, queued, project_id }`. - -### 3. Permissions (the parts that are invisible in the diff) -- `ObjectPermission.has_permission` returns `True` and `has_object_permission` - only fires on detail actions (via `get_object`). A `detail=False` action is - therefore unguarded unless it checks permissions itself. `sync_all` checks a - transient probe: `Deployment(project=project).check_permission(user, "sync")`, - which resolves to the `sync_deployment` guardian permission — exactly what the - per-row `sync` action requires. Superusers pass via Django's bypass; - `ProjectManager` and `MLDataManager` pass via guardian; `BasicMember`, - `Researcher`, and `Identifier` are denied (403). -- The per-row button is gated on `canSync` — the `sync` entry in a deployment's - `user_permissions`, backed by the `sync_deployment` guardian permission. - Superusers receive `sync` there too: guardian's `get_perms` returns every - permission defined on the project's content type for a superuser, and - `sync_deployment` is one of them. `sync_deployment` is held by `MLDataManager` - and `ProjectManager` (and superusers); `update_deployment` is held only by - `ProjectManager`, so gating on `canUpdate` would wrongly hide the button from - ML data managers who can sync. `canSync` is the exact set of users who may - sync. From cbb43ac5f13238a330098ff24fc83323a525be0e Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Thu, 16 Jul 2026 15:50:42 -0700 Subject: [PATCH 15/16] refactor(deployments): centralize sync-job creation on the model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-row sync action, the bulk sync-all action, and the Django admin bulk action each inlined the same Job.objects.create(...) + enqueue() to build a DataStorageSyncJob. Move that into a single Deployment.enqueue_sync_job() method so all three build the job identically, and drop the private DeploymentViewSet._enqueue_sync_job helper the two API actions had shared. Callers keep their own preconditions — the per-row action raises on a missing data source, sync-all pre-filters to connected stations, and the admin action reports skipped stations — the method only builds and enqueues. Co-Authored-By: Claude --- ami/main/admin.py | 11 +---------- ami/main/api/views.py | 24 ++---------------------- ami/main/models.py | 22 ++++++++++++++++++++++ 3 files changed, 25 insertions(+), 32 deletions(-) diff --git a/ami/main/admin.py b/ami/main/admin.py index 404325a93..e8fdc3cbe 100644 --- a/ami/main/admin.py +++ b/ami/main/admin.py @@ -207,8 +207,6 @@ def captures_size_display(self, obj: Deployment) -> str: # https://docs.djangoproject.com/en/3.2/ref/contrib/admin/actions/#writing-action-functions @admin.action(description="Sync captures from deployment's data source (Job)") def sync_captures(self, request: HttpRequest, queryset: QuerySet[Deployment]) -> None: - from ami.jobs.models import DataStorageSyncJob - queued_job_ids: list[int] = [] skipped: list[str] = [] for deployment in queryset: @@ -218,14 +216,7 @@ def sync_captures(self, request: HttpRequest, queryset: QuerySet[Deployment]) -> if not deployment.data_source_id: skipped.append(f"{deployment} (no data source)") continue - job = Job.objects.create( - name=f"Sync captures for deployment {deployment.pk}", - deployment=deployment, - project=deployment.project, - job_type_key=DataStorageSyncJob.key, - ) - job.enqueue() - queued_job_ids.append(job.pk) + queued_job_ids.append(deployment.enqueue_sync_job().pk) msg = f"Queued DataStorageSyncJob for {len(queued_job_ids)} deployments: {queued_job_ids}" if skipped: msg += f" — skipped: {', '.join(skipped)}" diff --git a/ami/main/api/views.py b/ami/main/api/views.py index c912cd575..b575f800b 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -336,26 +336,6 @@ def get_queryset(self) -> QuerySet: return qs - @staticmethod - def _enqueue_sync_job(deployment: Deployment): - """ - Create and enqueue a ``DataStorageSyncJob`` for a single deployment. - - Shared by the per-row ``sync`` action and the bulk ``sync_all`` action so - both build the job the same way (one job per deployment, matching the - admin bulk action). - """ - from ami.jobs.models import DataStorageSyncJob, Job - - job = Job.objects.create( - name=f"Sync captures for deployment {deployment.pk}", - deployment=deployment, - project=deployment.project, - job_type_key=DataStorageSyncJob.key, - ) - job.enqueue() - return job - @action(detail=True, methods=["post"], name="sync") def sync(self, _request, pk=None) -> Response: """ @@ -363,7 +343,7 @@ def sync(self, _request, pk=None) -> Response: """ deployment: Deployment = self.get_object() if deployment and deployment.data_source: - job = self._enqueue_sync_job(deployment) + job = deployment.enqueue_sync_job() logger.info( f"Syncing captures for deployment {deployment.pk} from {deployment.data_source_uri} in background." ) @@ -395,7 +375,7 @@ def sync_all(self, request) -> Response: ) deployments = self.get_queryset().filter(data_source__isnull=False) - job_ids = [self._enqueue_sync_job(deployment).pk for deployment in deployments] + job_ids = [deployment.enqueue_sync_job().pk for deployment in deployments] logger.info(f"Queued {len(job_ids)} DataStorageSyncJob(s) for project {project.pk}: {job_ids}") return Response({"job_ids": job_ids, "queued": len(job_ids), "project_id": project.pk}) diff --git a/ami/main/models.py b/ami/main/models.py index 3662b4107..4250af499 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -884,6 +884,28 @@ def data_source_total_size_display(self) -> str: else: return filesizeformat(self.data_source_total_size) + def enqueue_sync_job(self) -> "Job": + """ + Create and enqueue a ``DataStorageSyncJob`` to sync this station's + captures from its data source, returning the queued job. + + The single place that builds the sync job, shared by the API's per-row + and bulk sync actions and the Django admin bulk action so all three + create it identically (one job per deployment). Callers own their own + preconditions — that a data source is configured, that skipped stations + are reported — this method always builds and enqueues. + """ + from ami.jobs.models import DataStorageSyncJob, Job + + job = Job.objects.create( + name=f"Sync captures for deployment {self.pk}", + deployment=self, + project=self.project, + job_type_key=DataStorageSyncJob.key, + ) + job.enqueue() + return job + def sync_captures( self, batch_size=1000, From 2b4f68ee8d62f5566b090551064b84be4db65140 Mon Sep 17 00:00:00 2001 From: Michael Bunsen Date: Fri, 17 Jul 2026 03:25:22 -0700 Subject: [PATCH 16/16] refactor(jobs): move sync-job creation onto the job type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the previous commit: the shared sync-job factory belongs on the job type, not the Deployment model. ami.jobs.models already imports Deployment at module load, so a DataStorageSyncJob.enqueue_for(deployment) classmethod needs no deferred import, whereas a Deployment method had to defer 'from ami.jobs.models import ...' inside its body — the classic tell for orchestration living on the wrong object. It also keeps job construction next to the job type that owns run(), and survives the planned split of ami/main/models.py into a package with no second move. Deletes Deployment.enqueue_sync_job(); the per-row sync, bulk sync_all, and admin sync_captures actions now call DataStorageSyncJob.enqueue_for(deployment). Co-Authored-By: Claude --- ami/jobs/models.py | 20 ++++++++++++++++++++ ami/main/admin.py | 4 +++- ami/main/api/views.py | 8 ++++++-- ami/main/models.py | 22 ---------------------- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/ami/jobs/models.py b/ami/jobs/models.py index d0e3b0c02..bced66575 100644 --- a/ami/jobs/models.py +++ b/ami/jobs/models.py @@ -713,6 +713,26 @@ class DataStorageSyncJob(JobType): regroup_stage_key = "regroup_sessions" regroup_stage_name = "Regroup sessions" + @classmethod + def enqueue_for(cls, deployment: Deployment) -> "Job": + """ + Create and enqueue a sync job for one station, returning the queued job. + + The single place that builds a ``DataStorageSyncJob``, shared by the API's + per-row and bulk sync actions and the Django admin bulk action so all three + create it identically (one job per deployment). Callers own their own + preconditions — that a data source is configured, that skipped stations are + reported — this only builds and enqueues. + """ + job = Job.objects.create( + name=f"Sync captures for deployment {deployment.pk}", + deployment=deployment, + project=deployment.project, + job_type_key=cls.key, + ) + job.enqueue() + return job + @classmethod def run(cls, job: "Job"): """ diff --git a/ami/main/admin.py b/ami/main/admin.py index e8fdc3cbe..6101abc96 100644 --- a/ami/main/admin.py +++ b/ami/main/admin.py @@ -207,6 +207,8 @@ def captures_size_display(self, obj: Deployment) -> str: # https://docs.djangoproject.com/en/3.2/ref/contrib/admin/actions/#writing-action-functions @admin.action(description="Sync captures from deployment's data source (Job)") def sync_captures(self, request: HttpRequest, queryset: QuerySet[Deployment]) -> None: + from ami.jobs.models import DataStorageSyncJob + queued_job_ids: list[int] = [] skipped: list[str] = [] for deployment in queryset: @@ -216,7 +218,7 @@ def sync_captures(self, request: HttpRequest, queryset: QuerySet[Deployment]) -> if not deployment.data_source_id: skipped.append(f"{deployment} (no data source)") continue - queued_job_ids.append(deployment.enqueue_sync_job().pk) + queued_job_ids.append(DataStorageSyncJob.enqueue_for(deployment).pk) msg = f"Queued DataStorageSyncJob for {len(queued_job_ids)} deployments: {queued_job_ids}" if skipped: msg += f" — skipped: {', '.join(skipped)}" diff --git a/ami/main/api/views.py b/ami/main/api/views.py index b575f800b..327cd98f4 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -341,9 +341,11 @@ def sync(self, _request, pk=None) -> Response: """ Queue a task to sync data from the deployment's data source. """ + from ami.jobs.models import DataStorageSyncJob + deployment: Deployment = self.get_object() if deployment and deployment.data_source: - job = deployment.enqueue_sync_job() + job = DataStorageSyncJob.enqueue_for(deployment) logger.info( f"Syncing captures for deployment {deployment.pk} from {deployment.data_source_uri} in background." ) @@ -374,8 +376,10 @@ def sync_all(self, request) -> Response: detail="You do not have permission to sync stations in this project." ) + from ami.jobs.models import DataStorageSyncJob + deployments = self.get_queryset().filter(data_source__isnull=False) - job_ids = [deployment.enqueue_sync_job().pk for deployment in deployments] + job_ids = [DataStorageSyncJob.enqueue_for(deployment).pk for deployment in deployments] logger.info(f"Queued {len(job_ids)} DataStorageSyncJob(s) for project {project.pk}: {job_ids}") return Response({"job_ids": job_ids, "queued": len(job_ids), "project_id": project.pk}) diff --git a/ami/main/models.py b/ami/main/models.py index 4250af499..3662b4107 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -884,28 +884,6 @@ def data_source_total_size_display(self) -> str: else: return filesizeformat(self.data_source_total_size) - def enqueue_sync_job(self) -> "Job": - """ - Create and enqueue a ``DataStorageSyncJob`` to sync this station's - captures from its data source, returning the queued job. - - The single place that builds the sync job, shared by the API's per-row - and bulk sync actions and the Django admin bulk action so all three - create it identically (one job per deployment). Callers own their own - preconditions — that a data source is configured, that skipped stations - are reported — this method always builds and enqueues. - """ - from ami.jobs.models import DataStorageSyncJob, Job - - job = Job.objects.create( - name=f"Sync captures for deployment {self.pk}", - deployment=self, - project=self.project, - job_type_key=DataStorageSyncJob.key, - ) - job.enqueue() - return job - def sync_captures( self, batch_size=1000,