-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(orgstats): Update the query to be able to filter by user's project #102762
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
fix(orgstats): Update the query to be able to filter by user's project #102762
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #102762 +/- ##
========================================
Coverage 80.92% 80.92%
========================================
Files 8931 8931
Lines 391099 391096 -3
Branches 24863 24862 -1
========================================
- Hits 316485 316483 -2
+ Misses 74246 74245 -1
Partials 368 368 |
…rameter-filter-logic
| {"by": {}, "series": {"sum(quantity)": [0, 8]}, "totals": {"sum(quantity)": 8}} | ||
| ], | ||
| "start": isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), | ||
| "end": isoformat_z(floor_to_utc_day(self._now) + timedelta(days=1)), | ||
| } | ||
|
|
||
| response = self.do_request( | ||
| { | ||
| "project": [-1], | ||
| "statsPeriod": "1d", | ||
| "interval": "6h", | ||
| "field": ["sum(quantity)"], | ||
| "category": ["error"], | ||
| }, | ||
| status_code=200, | ||
| ) | ||
|
|
||
| assert result_sorted(response.data) == { | ||
| "intervals": [ | ||
| isoformat_z((self._now - timedelta(days=1)).replace(hour=12, minute=0, second=0)), | ||
| isoformat_z((self._now - timedelta(days=1)).replace(hour=18, minute=0, second=0)), | ||
| isoformat_z(self._now.replace(hour=0, minute=0, second=0)), | ||
| isoformat_z(self._now.replace(hour=6, minute=0, second=0)), | ||
| isoformat_z(self._now.replace(hour=12, minute=0, second=0)), | ||
| ], | ||
| "groups": [ | ||
| { | ||
| "by": {}, | ||
| "series": {"sum(quantity)": [0, 0, 0, 6, 0]}, | ||
| "totals": {"sum(quantity)": 6}, | ||
| "series": {"sum(quantity)": [0, 0, 0, 8, 0]}, | ||
| "totals": {"sum(quantity)": 8}, | ||
| } | ||
| ], | ||
| "start": isoformat_z( | ||
| self._now.replace(hour=12, minute=0, second=0) - timedelta(days=1) | ||
| ), | ||
| "end": isoformat_z(self._now.replace(hour=18, minute=0, second=0)), | ||
| } | ||
|
|
||
| @freeze_time(_now) | ||
| def test_user_org_total_all_accessible(self) -> None: | ||
| response = self.do_request( | ||
| { | ||
| "project": [-1], | ||
| "statsPeriod": "1d", | ||
| "interval": "1d", | ||
| "field": ["sum(quantity)"], | ||
| "category": ["error", "transaction"], | ||
| }, | ||
| user=self.user2, | ||
| status_code=200, | ||
| ) | ||
|
|
||
| assert result_sorted(response.data) == { | ||
| "start": isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), | ||
| "end": isoformat_z(floor_to_utc_day(self._now) + timedelta(days=1)), | ||
| "intervals": [ | ||
| isoformat_z(floor_to_utc_day(self._now) - timedelta(days=1)), | ||
| isoformat_z(floor_to_utc_day(self._now)), | ||
| ], | ||
| "groups": [ | ||
| {"by": {}, "series": {"sum(quantity)": [0, 7]}, "totals": {"sum(quantity)": 7}} | ||
| {"by": {}, "series": {"sum(quantity)": [0, 9]}, "totals": {"sum(quantity)": 9}} |
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.
this is because of the new added outcome
| self.store_outcomes( |
| { | ||
| "by": {"project": self.project5.id}, | ||
| "totals": {"sum(quantity)": 2}, | ||
| }, |
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.
same here
| self.store_outcomes( |
| [ | ||
| not project_ids or project_ids == ALL_ACCESS_PROJECTS, | ||
| # ALL_ACCESS_PROJECTS ({-1}) signals that stats should aggregate across | ||
| # all projects rather than filtering to specific project IDs |
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 would really replace this with an if-statement with the two conditions anded together.
| return all( | ||
| [ | ||
| not project_ids or project_ids == ALL_ACCESS_PROJECTS, | ||
| # ALL_ACCESS_PROJECTS ({-1}) signals that stats should aggregate across | ||
| # all projects rather than filtering to specific project IDs | ||
| project_ids == ALL_ACCESS_PROJECTS, | ||
| "project" not in request.GET.get("groupBy", []), | ||
| ] | ||
| ) |
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.
| return all( | |
| [ | |
| not project_ids or project_ids == ALL_ACCESS_PROJECTS, | |
| # ALL_ACCESS_PROJECTS ({-1}) signals that stats should aggregate across | |
| # all projects rather than filtering to specific project IDs | |
| project_ids == ALL_ACCESS_PROJECTS, | |
| "project" not in request.GET.get("groupBy", []), | |
| ] | |
| ) | |
| return ( | |
| # ALL_ACCESS_PROJECTS ({-1}) signals that stats should aggregate across | |
| # all projects rather than filtering to specific project IDs | |
| project_ids == ALL_ACCESS_PROJECTS | |
| and "project" not in request.GET.get("groupBy", []) | |
| ) |
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
|
PR reverted: 574cda2 |
closes https://linear.app/getsentry/issue/TET-1370/check-filters-on-stats-and-usage-page-for-project-dropdown