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/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..caad9d968 --- /dev/null +++ b/ami/main/migrations/0095_grant_sync_deployment_to_mldatamanager.py @@ -0,0 +1,114 @@ +""" +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. + +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). 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 _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 None, None + try: + 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") + GroupObjectPermission = apps.get_model("guardian", "GroupObjectPermission") + + perm, project_ct = _sync_permission(apps) + if perm is None: + return + + # Only touch MLDataManager groups; ProjectManager holds sync_deployment + # independently and must keep it. + 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, + object_pk=str(project_pk), + group=group, + ).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 c9e3b0d8f..62dd5c305 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__) @@ -2900,6 +2907,157 @@ 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.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( + 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): + # 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), + ] + 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 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() 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/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md new file mode 100644 index 000000000..3ecd59ec4 --- /dev/null +++ b/docs/claude/planning/2026-06-29-station-sync-action-NEXT.md @@ -0,0 +1,128 @@ +# 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). 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..40732b90d --- /dev/null +++ b/docs/claude/planning/2026-06-29-station-sync-action-design.md @@ -0,0 +1,219 @@ +# 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.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/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/data-services/models/deployment.ts b/ui/src/data-services/models/deployment.ts index 069896bf8..6986b8762 100644 --- a/ui/src/data-services/models/deployment.ts +++ b/ui/src/data-services/models/deployment.ts @@ -25,10 +25,20 @@ 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) } + 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 95fb1a9f4..a665480a5 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.canSync) && item.dataSourceConnected && ( + + )} {item.canDelete && ( { }) const canCreate = userPermissions?.includes(UserPermission.Create) const tableColumns = columns({ projectId: projectId as string }) + const syncableCount = + deployments?.filter( + (deployment) => + (deployment.canUpdate || deployment.canSync) && + deployment.dataSourceConnected + ).length ?? 0 return ( <> @@ -46,6 +53,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. 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() + } + }} + > + + + + + {errorMessage && } + +
+ + + {isSuccess && queued !== undefined && ( + + + + + + )} +
+ + + + ) +} diff --git a/ui/src/pages/deployments/sync-deployment-dialog.tsx b/ui/src/pages/deployments/sync-deployment-dialog.tsx new file mode 100644 index 000000000..fb975aa36 --- /dev/null +++ b/ui/src/pages/deployments/sync-deployment-dialog.tsx @@ -0,0 +1,112 @@ +import { FormError, FormSection } from 'components/form/layout/layout' +import { useSyncDeploymentSourceImages } from 'data-services/hooks/deployments/useSyncDeploymentSourceImages' +import { CheckIcon, EyeIcon, Loader2Icon, RefreshCwIcon } from 'lucide-react' +import { BasicTooltip, Button, buttonVariants, Dialog } from 'nova-ui-kit' +import { useState } from 'react' +import { Link } from 'react-router-dom' +import { APP_ROUTES } from 'utils/constants' +import { getAppRoute } from 'utils/getAppRoute' +import { STRING, translate } from 'utils/language' +import { parseServerError } from 'utils/parseServerError/parseServerError' + +export const SyncDeploymentDialog = ({ + id, + projectId, +}: { + id: string + projectId: string +}) => { + const [isOpen, setIsOpen] = useState(false) + 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. 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() + } + }} + > + + + + + {errorMessage && } + +
+ + + {isSuccess && jobId !== undefined && ( + + + + + + )} +
+ + + + ) +} diff --git a/ui/src/utils/language.ts b/ui/src/utils/language.ts index 6c6b3c2be..f2be6618c 100644 --- a/ui/src/utils/language.ts +++ b/ui/src/utils/language.ts @@ -46,8 +46,13 @@ export enum STRING { SUBMIT, 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, @@ -210,6 +215,8 @@ 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, @@ -406,8 +413,13 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.SUBMIT]: 'Submit', [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', @@ -595,6 +607,10 @@ 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', [STRING.MESSAGE_VALUE_MISSING]: 'Please provide a value', diff --git a/ui/src/utils/parseServerError/parseServerError.test.ts b/ui/src/utils/parseServerError/parseServerError.test.ts index a91c21b9f..8741764d6 100644 --- a/ui/src/utils/parseServerError/parseServerError.test.ts +++ b/ui/src/utils/parseServerError/parseServerError.test.ts @@ -73,6 +73,20 @@ describe('parseServerError', () => { }) }) + test('reads a top-level list of strings (DRF plain-string ValidationError) as the message', () => { + const EXAMPLE_ERROR = { + message: 'Request failed with status code 400', + response: { + data: ['Deployment must have a data source to sync captures from'], + }, + } + + expect(parseServerError(EXAMPLE_ERROR)).toEqual({ + message: 'Deployment must have a data source to sync captures from', + fieldErrors: [], + }) + }) + test('returns a default error if no information is present', () => { const { message, fieldErrors } = parseServerError({}) diff --git a/ui/src/utils/parseServerError/parseServerError.ts b/ui/src/utils/parseServerError/parseServerError.ts index 5c11efe2e..a149038de 100644 --- a/ui/src/utils/parseServerError/parseServerError.ts +++ b/ui/src/utils/parseServerError/parseServerError.ts @@ -2,8 +2,18 @@ export const parseServerError = (error: any) => { let message = '' const fieldErrors: { key: string; message: string }[] = [] - if (error.response?.data && typeof error.response.data === 'object') { - Object.entries(error.response.data).forEach(([key, details]) => { + const data = error.response?.data + + if (Array.isArray(data)) { + // DRF serializes `raise ValidationError("some message")` as a top-level + // JSON list of strings, e.g. ["Deployment must have a data source ..."]. + // Join them into a single message so the reason reaches the user instead + // of the generic axios "Request failed with status code 400". + message = data + .filter((entry) => typeof entry === 'string' && entry.length) + .join(' ') + } else if (data && typeof data === 'object') { + Object.entries(data).forEach(([key, details]) => { if (key && details) { if (key === 'non_field_errors' || key === 'detail') { message = details as string 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', }