Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class BaseDetectorTypeValidator(CamelSnakeSerializer):
type = serializers.CharField()
config = serializers.JSONField(default=dict)
owner = ActorField(required=False, allow_null=True)
description = serializers.CharField(required=False, allow_null=True, allow_blank=True)
enabled = serializers.BooleanField(required=False)
condition_group = BaseDataConditionGroupValidator(required=False)

Expand Down Expand Up @@ -97,6 +98,10 @@ def update(self, instance: Detector, validated_data: dict[str, Any]):
if "name" in validated_data:
instance.name = validated_data.get("name", instance.name)

# Handle description field update
if "description" in validated_data:
instance.description = validated_data.get("description", instance.description)

# Handle enable/disable detector
if "enabled" in validated_data:
enabled = validated_data.get("enabled")
Expand Down Expand Up @@ -180,6 +185,7 @@ def create(self, validated_data):
detector = Detector(
project_id=self.context["project"].id,
name=validated_data["name"],
description=validated_data.get("description"),
workflow_condition_group=condition_group,
type=validated_data["type"].slug,
config=validated_data.get("config", {}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,24 @@ def test_update(self, mock_schedule_update_project_config: mock.MagicMock) -> No
self.assert_snuba_query_updated(snuba_query)
mock_schedule_update_project_config.assert_called_once_with(detector)

def test_update_description(self) -> None:
assert self.detector.description is None

data = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since i'm not quite sure of the convention for validators, is it worth testing that not providing a description doesn't make it go away, and that setting it to empty works? Probably not, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did write these tests initially, but it felt a bit excessive just for the description. These cases do work tho, I tested them locally 🙃

"description": "New description for the detector",
}

with self.tasks():
self.get_success_response(
self.organization.slug,
self.detector.id,
**data,
status_code=200,
)

self.detector.refresh_from_db()
assert self.detector.description == "New description for the detector"

def test_update_add_data_condition(self) -> None:
"""
Test that we can add an additional data condition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ def test_valid_creation(
assert detector.name == "Test Detector"
assert detector.type == MetricIssue.slug
assert detector.project_id == self.project.id
assert detector.description is None

# Verify data source
data_source = DataSource.objects.get(detector=detector)
Expand Down Expand Up @@ -890,6 +891,18 @@ def test_valid_creation(
)
mock_schedule_update_project_config.assert_called_once_with(detector)

def test_creation_with_description(self) -> None:
data = {**self.valid_data, "description": "This is a test metric detector"}
with self.tasks():
response = self.get_success_response(
self.organization.slug,
**data,
status_code=201,
)

detector = Detector.objects.get(id=response.data["id"])
assert detector.description == "This is a test metric detector"

def test_invalid_workflow_ids(self) -> None:
# Workflow doesn't exist at all
data = {**self.valid_data, "workflowIds": [999999]}
Expand Down
Loading