-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(aci): Add ability to filter workflows by connected detectors #105695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| detector_ids = [int(id) for id in raw_detectorlist] | ||
| except ValueError: | ||
| raise ValidationError({"detector": ["Invalid detector ID format"]}) | ||
| queryset = queryset.filter(detectorworkflow__detector_id__in=detector_ids).distinct() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's also add a filter here for the users organization -- that way we can prevent any possible security issues where a user asks for a detector not part of their org.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are already adding the org filter at the top of filter_workflows
| queryset: QuerySet[Workflow] = Workflow.objects.filter(organization_id=organization.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, i didn't notice it was the same queryset 👍
| assert len(response3.data) == 2 | ||
| assert {self.workflow.name, self.workflow_two.name} == {w["name"] for w in response3.data} | ||
|
|
||
| def test_filter_by_detector(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thoughts on adding a test case for selecting a detector not part of your org and ensure it returns the correct status codes? (my guess is we'd either want to 401 or 404)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add a test case for it certainly. I think the current behavior is correct (which is that it will just return an empty list)
Adds the ability to do
/organizations/org-slug/workflows/?detector=1&detector=2which will return workflows connected to either of those two detectors.