diff --git a/src/sentry/api/endpoints/organization_stats_v2.py b/src/sentry/api/endpoints/organization_stats_v2.py index bc5b6771e455d7..9a355f4567e7ad 100644 --- a/src/sentry/api/endpoints/organization_stats_v2.py +++ b/src/sentry/api/endpoints/organization_stats_v2.py @@ -216,7 +216,7 @@ def build_outcomes_query(self, request: Request, organization): return QueryDefinition.from_query_dict(request.GET, params) def _get_projects_for_orgstats_query(self, request: Request, organization): - # look at the raw project_id filter passed in, if its empty + # look at the raw project_id filter passed in, if its -1 # and project_id is not in groupBy filter, treat it as an # org wide query and don't pass project_id in to QueryDefinition req_proj_ids = self.get_requested_project_ids_unchecked(request) @@ -229,11 +229,10 @@ def _get_projects_for_orgstats_query(self, request: Request, organization): return [p.id for p in projects] def _is_org_total_query(self, request: Request, project_ids): - return all( - [ - not project_ids or project_ids == ALL_ACCESS_PROJECTS, - "project" not in request.GET.get("groupBy", []), - ] + # ALL_ACCESS_PROJECTS ({-1}) signals that stats should aggregate across + # all projects rather than filtering to specific project IDs + return project_ids == ALL_ACCESS_PROJECTS and "project" not in request.GET.get( + "groupBy", [] ) @contextmanager diff --git a/static/app/views/organizationStats/index.spec.tsx b/static/app/views/organizationStats/index.spec.tsx index 552076c9bfc6ba..e15ef611368bea 100644 --- a/static/app/views/organizationStats/index.spec.tsx +++ b/static/app/views/organizationStats/index.spec.tsx @@ -75,7 +75,15 @@ describe('OrganizationStats', () => { * Base + Error Handling */ it('renders the base view', async () => { - render(, {organization}); + render(, { + organization, + initialRouterConfig: { + location: { + pathname: '/organizations/org-slug/stats/', + query: {project: [ALL_ACCESS_PROJECTS.toString()]}, + }, + }, + }); expect(await screen.findByTestId('usage-stats-chart')).toBeInTheDocument(); @@ -245,6 +253,12 @@ describe('OrganizationStats', () => { OrganizationStore.onUpdate(newOrg, {replace: true}); render(, { organization: newOrg, + initialRouterConfig: { + location: { + pathname: '/organizations/org-slug/stats/', + query: {project: [ALL_ACCESS_PROJECTS.toString()]}, + }, + }, }); expect(await screen.findByText('All Projects')).toBeInTheDocument(); diff --git a/static/app/views/organizationStats/index.tsx b/static/app/views/organizationStats/index.tsx index 8fc576789808cf..5fec9fca907849 100644 --- a/static/app/views/organizationStats/index.tsx +++ b/static/app/views/organizationStats/index.tsx @@ -19,7 +19,6 @@ import {normalizeDateTimeParams} from 'sentry/components/organizations/pageFilte import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilter'; import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle'; import {DATA_CATEGORY_INFO, DEFAULT_STATS_PERIOD} from 'sentry/constants'; -import {ALL_ACCESS_PROJECTS} from 'sentry/constants/pageFilters'; import {t, tct} from 'sentry/locale'; import ConfigStore from 'sentry/stores/configStore'; import {space} from 'sentry/styles/space'; @@ -167,10 +166,7 @@ export class OrganizationStatsInner extends Component { // Project selection from GlobalSelectionHeader get projectIds(): number[] { - const selection_projects = this.props.selection.projects.length - ? this.props.selection.projects - : [ALL_ACCESS_PROJECTS]; - return selection_projects; + return this.props.selection.projects; } get isSingleProject(): boolean { diff --git a/tests/snuba/api/endpoints/test_organization_stats_v2.py b/tests/snuba/api/endpoints/test_organization_stats_v2.py index 9bf670cd9bf938..7f0da07fa095ad 100644 --- a/tests/snuba/api/endpoints/test_organization_stats_v2.py +++ b/tests/snuba/api/endpoints/test_organization_stats_v2.py @@ -33,12 +33,17 @@ def setUp(self) -> None: self.project3 = self.create_project(organization=self.org2) self.user2 = self.create_user(is_superuser=False) + self.user3 = self.create_user(is_superuser=False) self.create_member(user=self.user2, organization=self.organization, role="member", teams=[]) self.create_member(user=self.user2, organization=self.org3, role="member", teams=[]) self.project4 = self.create_project( name="users2sproj", teams=[self.create_team(organization=self.org, members=[self.user2])], ) + self.project5 = self.create_project( + name="users3sproj", + teams=[self.create_team(organization=self.org, members=[self.user3])], + ) self.store_outcomes( { @@ -85,6 +90,18 @@ def setUp(self) -> None: "quantity": 1, } ) + self.store_outcomes( + { + "org_id": self.org.id, + "timestamp": self._now - timedelta(hours=1), + "project_id": self.project5.id, + "outcome": Outcome.ACCEPTED, + "reason": "none", + "category": DataCategory.ERROR, + "quantity": 1, + }, + 2, + ) # Add profile_duration outcome data self.store_outcomes( @@ -284,7 +301,7 @@ def test_timeseries_interval(self) -> None: isoformat_z(floor_to_utc_day(self._now)), ], "groups": [ - {"by": {}, "series": {"sum(quantity)": [0, 6]}, "totals": {"sum(quantity)": 6}} + {"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)), @@ -312,8 +329,8 @@ def test_timeseries_interval(self) -> None: "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( @@ -344,7 +361,7 @@ def test_user_org_total_all_accessible(self) -> None: 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}} ], } @@ -450,6 +467,10 @@ def test_open_membership_semantics(self) -> None: "by": {"project": self.project2.id}, "totals": {"sum(quantity)": 1}, }, + { + "by": {"project": self.project5.id}, + "totals": {"sum(quantity)": 2}, + }, ], } @@ -973,6 +994,118 @@ def test_profile_duration_groupby(self) -> None: ], } + @freeze_time(_now) + def test_project_filtering_with_all_projects(self) -> None: + """Test that project=-1 aggregates data across all projects in the org""" + response = self.do_request( + { + "project": [-1], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + status_code=200, + ) + + assert response.data["groups"] == [ + { + "by": {}, + "totals": {"sum(quantity)": 9}, + "series": {"sum(quantity)": [0, 9]}, + } + ] + + @freeze_time(_now) + def test_project_filtering_without_project_param(self) -> None: + """Test that when no project parameter is provided, it filters by user's projects (my projects)""" + response = self.do_request( + { + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + status_code=200, + ) + + assert response.data["groups"] == [ + { + "by": {}, + "totals": {"sum(quantity)": 7}, + "series": {"sum(quantity)": [0, 7]}, + } + ] + + @freeze_time(_now) + def test_project_filtering_with_specific_project(self) -> None: + """Test that when a specific project id is provided, it filters by that project only""" + response = self.do_request( + { + "project": [self.project.id], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + status_code=200, + ) + + assert response.data["groups"] == [ + { + "by": {}, + "totals": {"sum(quantity)": 6}, + "series": {"sum(quantity)": [0, 6]}, + } + ] + + @freeze_time(_now) + def test_project_filtering_with_multiple_specific_projects(self) -> None: + """Test filtering with multiple specific project IDs""" + response = self.do_request( + { + "project": [self.project.id, self.project2.id], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + }, + status_code=200, + ) + + assert response.data["groups"] == [ + { + "by": {}, + "totals": {"sum(quantity)": 7}, + "series": {"sum(quantity)": [0, 7]}, + } + ] + + @freeze_time(_now) + def test_with_groupby_project(self) -> None: + """Test that groupBy=project shows individual project stats""" + response = self.do_request( + { + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + "category": ["error", "transaction"], + "groupBy": ["project"], + }, + status_code=200, + ) + + assert response.data["groups"] == [ + { + "by": {"project": self.project.id}, + "totals": {"sum(quantity)": 6}, + }, + { + "by": {"project": self.project2.id}, + "totals": {"sum(quantity)": 1}, + }, + ] + def result_sorted(result): """sort the groups of the results array by the `by` object, ensuring a stable order"""