From 21b4b689730b5a0f049bad34f1dfdd9f8812d73c Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 11 Nov 2026 11:27:10 -0800 Subject: [PATCH] ref(notebooks): Let the update validator archive or update a notebook NotebookUpdateValidator.save() picks archive or update from the validated fields, so the details endpoint PUT doesn't branch on the payload. --- .../organization_notebook_details.py | 53 ++++--------------- .../endpoints/validators/notebook.py | 32 ++++++++++- 2 files changed, 40 insertions(+), 45 deletions(-) diff --git a/src/sentry/notebooks/endpoints/organization_notebook_details.py b/src/sentry/notebooks/endpoints/organization_notebook_details.py index ddf74b8772d4..47f0f0f69429 100644 --- a/src/sentry/notebooks/endpoints/organization_notebook_details.py +++ b/src/sentry/notebooks/endpoints/organization_notebook_details.py @@ -30,8 +30,8 @@ NotebookDeleteValidator, NotebookUpdateValidator, ) -from sentry.notebooks.models import Notebook, NotebookStatus -from sentry.notebooks.services.producer import archive_notebook, update_notebook +from sentry.notebooks.models import Notebook +from sentry.notebooks.services.producer import archive_notebook @extend_schema(tags=["Investigations"]) @@ -108,12 +108,14 @@ def put(self, request: Request, organization: Organization, notebook: Notebook) membership. """ require_notebook_writer(request, notebook) - validator = NotebookUpdateValidator(data=request.data) + validator = NotebookUpdateValidator( + instance=notebook, + data=request.data, + context={"actor": request_creator_actor(request)}, + ) if not validator.is_valid(): return Response(validator.errors, status=status.HTTP_400_BAD_REQUEST) - values = dict(validator.validated_data) - expected_version = values.pop("notebook_version") - requested_project_ids = values.pop("project_ids", None) + requested_project_ids = validator.validated_data.get("project_ids") if requested_project_ids is not None and not set(requested_project_ids).issubset( organization_project_ids(organization) ): @@ -123,45 +125,8 @@ def put(self, request: Request, organization: Organization, notebook: Notebook) ) if requested_project_ids is not None: require_project_access(request, requested_project_ids) - restoring_only = ( - values == {"status": NotebookStatus.ACTIVE} and requested_project_ids is None - ) - if notebook.status == NotebookStatus.ARCHIVED and not restoring_only: - return Response( - {"detail": "Archived investigations are read-only."}, - status=status.HTTP_400_BAD_REQUEST, - ) - if values.get("status") == NotebookStatus.ARCHIVED: - if set(values) != {"status"} or requested_project_ids is not None: - return Response( - {"detail": "Archiving cannot be combined with other changes."}, - status=status.HTTP_400_BAD_REQUEST, - ) - try: - archived = archive_notebook( - notebook=notebook, - expected_version=expected_version, - actor=request_creator_actor(request), - ) - except Exception as error: - response = service_error(error) - if response is not None: - return response - raise - return Response( - serialize( - archived, - request.user, - NotebookDetailsSerializer(access=request.access), - ) - ) try: - updated = update_notebook( - notebook=notebook, - expected_version=expected_version, - fields=values, - project_ids=requested_project_ids, - ) + updated = validator.save() except Exception as error: response = service_error(error) if response is not None: diff --git a/src/sentry/notebooks/endpoints/validators/notebook.py b/src/sentry/notebooks/endpoints/validators/notebook.py index 5a92550386b1..07d284fb209d 100644 --- a/src/sentry/notebooks/endpoints/validators/notebook.py +++ b/src/sentry/notebooks/endpoints/validators/notebook.py @@ -18,6 +18,7 @@ create_manual_notebook, create_producer_notebook, ) +from sentry.notebooks.services.producer import archive_notebook, update_notebook from sentry.notebooks.services.producer_sources import ProducerSource, get_producer_source_type @@ -109,7 +110,12 @@ def validate_filters(self, value: Any) -> dict[str, Any]: return value -class NotebookUpdateValidator(NotebookVersionValidator): +class NotebookUpdateValidator(NotebookVersionValidator[Notebook]): + """Update a notebook, or archive it when status is archived. + + Needs actor in its context. + """ + title = serializers.CharField( max_length=255, required=False, @@ -139,6 +145,30 @@ def validate_filters(self, value: Any) -> dict[str, Any]: raise serializers.ValidationError("Must be an object.") return value + def update(self, instance: Notebook, validated_data: dict[str, Any]) -> Notebook: + fields = dict(validated_data) + expected_version = fields.pop("notebook_version") + project_ids = fields.pop("project_ids", None) + restoring_only = fields == {"status": NotebookStatus.ACTIVE} and project_ids is None + if instance.status == NotebookStatus.ARCHIVED and not restoring_only: + raise NotebookValidationError({"detail": "Archived investigations are read-only."}) + if fields.get("status") == NotebookStatus.ARCHIVED: + if set(fields) != {"status"} or project_ids is not None: + raise NotebookValidationError( + {"detail": "Archiving cannot be combined with other changes."} + ) + return archive_notebook( + notebook=instance, + expected_version=expected_version, + actor=self.context["actor"], + ) + return update_notebook( + notebook=instance, + expected_version=expected_version, + fields=fields, + project_ids=project_ids, + ) + class NotebookDeleteValidator(NotebookVersionValidator): pass