Skip to content
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

ref(releases): remove extra filter from environment filter util #87257

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aliu39
Copy link
Member

@aliu39 aliu39 commented Mar 18, 2025

Aside from the location below, every callsite for this util is after an explicit project filter. It's only used in this file and organization_release_details. The naming is confusing so let's remove this filter

@aliu39 aliu39 requested a review from a team March 18, 2025 00:45
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 18, 2025
@aliu39 aliu39 requested a review from a team March 18, 2025 00:46
@JoshFerge
Copy link
Member

what harm does having the project_ids in the fiter do? it can be a potential security issue if the upstream querys do not filter on project

@@ -63,7 +63,6 @@ def add_environment_to_queryset(queryset, filter_params):
if "environment" in filter_params:
return queryset.filter(
releaseprojectenvironment__environment__name__in=filter_params["environment"],
releaseprojectenvironment__project_id__in=filter_params["project_id"],
Copy link
Member

Choose a reason for hiding this comment

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

This is changing the query you're making. Right now it's filtering on the releaseprojectenvironment__project_id, and you're moving it to releaseproject instead. I don't know if this will break anything, but it seems risky for not much gain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants