Browse Source

fix(performance): Return list of team key transactions (#26316)

This replaces the endpoint returning a count with an endpoint returning the list
of key transactions and adds support for pagination. Also updates the team key
transaction button to use this new end point.
Tony Xiao 3 years ago
parent
commit
6c74045781

+ 4 - 4
src/sentry/api/urls.py

@@ -4,8 +4,8 @@ from sentry.data_export.endpoints.data_export import DataExportEndpoint
 from sentry.data_export.endpoints.data_export_details import DataExportDetailsEndpoint
 from sentry.discover.endpoints.discover_key_transactions import (
     IsKeyTransactionEndpoint,
-    KeyTransactionCountEndpoint,
     KeyTransactionEndpoint,
+    KeyTransactionListEndpoint,
 )
 from sentry.discover.endpoints.discover_query import DiscoverQueryEndpoint
 from sentry.discover.endpoints.discover_saved_queries import DiscoverSavedQueriesEndpoint
@@ -796,9 +796,9 @@ urlpatterns = [
                     name="sentry-api-0-organization-key-transactions",
                 ),
                 url(
-                    r"^(?P<organization_slug>[^\/]+)/key-transactions-count/$",
-                    KeyTransactionCountEndpoint.as_view(),
-                    name="sentry-api-0-organization-key-transactions-count",
+                    r"^(?P<organization_slug>[^\/]+)/key-transactions-list/$",
+                    KeyTransactionListEndpoint.as_view(),
+                    name="sentry-api-0-organization-key-transactions-list",
                 ),
                 url(
                     r"^(?P<organization_slug>[^\/]+)/is-key-transactions/$",

+ 38 - 15
src/sentry/discover/endpoints/discover_key_transactions.py

@@ -1,11 +1,13 @@
+from collections import defaultdict
+
 from django.db import IntegrityError, transaction
-from django.db.models import Count
 from rest_framework.exceptions import ParseError
 from rest_framework.response import Response
 
 from sentry.api.bases import KeyTransactionBase
 from sentry.api.bases.organization import OrganizationPermission
 from sentry.api.helpers.teams import get_teams
+from sentry.api.paginator import OffsetPaginator
 from sentry.api.serializers import Serializer, register, serialize
 from sentry.api.utils import InvalidParams
 from sentry.discover.endpoints import serializers
@@ -186,7 +188,7 @@ class KeyTransactionEndpoint(KeyTransactionBase):
         return Response(serializer.errors, status=400)
 
 
-class KeyTransactionCountEndpoint(KeyTransactionBase):
+class KeyTransactionListEndpoint(KeyTransactionBase):
     permission_classes = (KeyTransactionPermission,)
 
     def has_feature(self, request, organization):
@@ -203,8 +205,16 @@ class KeyTransactionCountEndpoint(KeyTransactionBase):
         except InvalidParams as err:
             return Response(str(err), status=400)
 
-        return Response(
-            serialize(list(teams), serializer=KeyTransactionTeamSerializer()), status=200
+        projects = self.get_projects(request, organization)
+
+        serializer = KeyTransactionTeamSerializer(projects)
+
+        return self.paginate(
+            request=request,
+            queryset=teams,
+            order_by="slug",
+            on_results=lambda x: serialize(x, request.user, serializer),
+            paginator_cls=OffsetPaginator,
         )
 
 
@@ -217,23 +227,36 @@ class TeamKeyTransactionSerializer(Serializer):
 
 
 class KeyTransactionTeamSerializer(Serializer):
+    def __init__(self, projects):
+        self.project_ids = {project.id for project in projects}
+
     def get_attrs(self, item_list, user, **kwargs):
-        counts = (
-            TeamKeyTransaction.objects.values("team_id")
-            .filter(team__in=[item.id for item in item_list])
-            .annotate(count=Count("team_id"))
+        team_key_transactions = TeamKeyTransaction.objects.filter(
+            team__in=[item.id for item in item_list]
+        ).order_by("transaction", "project_id")
+
+        attrs = defaultdict(
+            lambda: {
+                "count": 0,
+                "key_transactions": [],
+            }
         )
-        count_map = {}
-        for count in counts:
-            count_map[count["team_id"]] = count["count"]
 
-        attrs = {}
-        for item in item_list:
-            attrs[item] = {"count": count_map.get(item.id, 0)}
+        for kt in team_key_transactions:
+            attrs[kt.team]["count"] += 1
+            if kt.project_id in self.project_ids:
+                attrs[kt.team]["key_transactions"].append(
+                    {
+                        "project_id": str(kt.project_id),
+                        "transaction": kt.transaction,
+                    }
+                )
+
         return attrs
 
     def serialize(self, obj, attrs, user, **kwargs):
         return {
             "team": str(obj.id),
-            "count": attrs["count"],
+            "count": attrs.get("count", 0),
+            "keyed": attrs.get("key_transactions", []),
         }

+ 58 - 0
static/app/actionCreators/performance.tsx

@@ -5,6 +5,64 @@ import {
 } from 'app/actionCreators/indicator';
 import {Client} from 'app/api';
 import {t} from 'app/locale';
+import parseLinkHeader from 'app/utils/parseLinkHeader';
+
+type TeamKeyTransaction = {
+  team: string;
+  count: number;
+  keyed: {
+    project_id: string;
+    transaction: string;
+  }[];
+};
+
+export async function fetchTeamKeyTransactions(
+  api: Client,
+  orgSlug: string,
+  teams: string[],
+  projects?: string[]
+): Promise<TeamKeyTransaction[]> {
+  const url = `/organizations/${orgSlug}/key-transactions-list/`;
+
+  const datas: TeamKeyTransaction[][] = [];
+  let cursor: string | undefined = undefined;
+  let hasMore = true;
+
+  while (hasMore) {
+    try {
+      const payload = {cursor, team: teams, project: projects};
+      if (!payload.cursor) {
+        delete payload.cursor;
+      }
+      if (!payload.project?.length) {
+        delete payload.project;
+      }
+
+      const [data, , xhr] = await api.requestPromise(url, {
+        method: 'GET',
+        includeAllArgs: true,
+        query: payload,
+      });
+      datas.push(data);
+
+      const pageLinks = xhr && xhr.getResponseHeader('Link');
+      if (pageLinks) {
+        const paginationObject = parseLinkHeader(pageLinks);
+        hasMore = paginationObject?.next?.results ?? false;
+        cursor = paginationObject.next?.cursor;
+      } else {
+        hasMore = false;
+      }
+    } catch (err) {
+      addErrorMessage(
+        err.responseJSON?.detail ?? t('Error fetching team key transactions')
+      );
+      throw err;
+    }
+  }
+
+  return datas.flat();
+}
 
 export function toggleKeyTransaction(
   api: Client,

+ 7 - 2
static/app/components/performance/teamKeyTransaction.tsx

@@ -23,6 +23,7 @@ type Props = {
   teams: Team[];
   title: ComponentClass<TitleProps>;
   isLoading: boolean;
+  error: string | null;
   keyedTeams: Set<string>;
   counts: Map<string, number>;
   handleToggleKeyTransaction: (
@@ -110,9 +111,9 @@ class TeamKeyTransaction extends Component<Props> {
   }
 
   render() {
-    const {isLoading, counts, keyedTeams, teams, title} = this.props;
+    const {isLoading, error, counts, keyedTeams, teams, title} = this.props;
 
-    if (isLoading) {
+    if (isLoading || error) {
       const Title = title;
       return <Title disabled keyedTeamsCount={0} />;
     }
@@ -367,6 +368,8 @@ const DropdownWrapper = styled('div')`
   }
 
   &[data-placement*='bottom'] {
+    margin-top: 9px;
+
     &:before {
       border-bottom: 9px solid ${p => p.theme.border};
       top: -9px;
@@ -379,6 +382,8 @@ const DropdownWrapper = styled('div')`
   }
 
   &[data-placement*='top'] {
+    margin-bottom: 9px;
+
     &:before {
       border-top: 9px solid ${p => p.theme.border};
       bottom: -9px;

+ 37 - 43
static/app/views/performance/transactionSummary/teamKeyTransactionButton.tsx

@@ -1,7 +1,10 @@
 import {Component} from 'react';
 import styled from '@emotion/styled';
 
-import {toggleKeyTransaction} from 'app/actionCreators/performance';
+import {
+  fetchTeamKeyTransactions,
+  toggleKeyTransaction,
+} from 'app/actionCreators/performance';
 import {Client} from 'app/api';
 import Button from 'app/components/button';
 import TeamKeyTransaction, {
@@ -76,55 +79,45 @@ class TeamKeyTransactionButton extends Component<Props, State> {
   }
 
   async fetchData() {
+    const {api, organization, project, transactionName} = this.props;
     const keyFetchID = Symbol('keyFetchID');
     this.setState({isLoading: true, keyFetchID});
 
-    try {
-      const [keyTransactions, counts] = await Promise.all([
-        this.fetchKeyTransactionsData(),
-        this.fetchCountData(),
-      ]);
-      this.setState({
-        isLoading: false,
-        keyFetchID: undefined,
-        error: null,
-        keyedTeams: new Set(keyTransactions.map(({team}) => team)),
-        counts: new Map(counts.map(({team, count}) => [team, count])),
-      });
-    } catch (err) {
-      this.setState({
-        isLoading: false,
-        keyFetchID: undefined,
-        error: err.responseJSON?.detail ?? null,
-      });
-    }
-  }
+    let keyedTeams: Set<string> = new Set();
+    let counts: Map<string, number> = new Map();
 
-  async fetchKeyTransactionsData() {
-    const {api, organization, project, transactionName} = this.props;
+    let error: string | null = null;
 
-    const url = `/organizations/${organization.slug}/key-transactions/`;
-    const [data] = await api.requestPromise(url, {
-      method: 'GET',
-      includeAllArgs: true,
-      query: {
-        project: String(project),
-        transaction: transactionName,
-      },
-    });
-    return data;
-  }
+    try {
+      const teams = await fetchTeamKeyTransactions(
+        api,
+        organization.slug,
+        ['myteams'],
+        [String(project)]
+      );
 
-  async fetchCountData() {
-    const {api, organization, teams} = this.props;
+      keyedTeams = new Set(
+        teams
+          .filter(({keyed}) =>
+            keyed.find(
+              ({project_id, transaction}) =>
+                project_id === String(project) && transaction === transactionName
+            )
+          )
+          .map(({team}) => team)
+      );
+      counts = new Map(teams.map(({team, count}) => [team, count]));
+    } catch (err) {
+      error = err.responseJSON?.detail ?? t('Error fetching team key transactions');
+    }
 
-    const url = `/organizations/${organization.slug}/key-transactions-count/`;
-    const [data] = await api.requestPromise(url, {
-      method: 'GET',
-      includeAllArgs: true,
-      query: {team: teams.map(({id}) => id)},
+    this.setState({
+      isLoading: false,
+      keyFetchID: undefined,
+      error,
+      keyedTeams,
+      counts,
     });
-    return data;
   }
 
   handleToggleKeyTransaction = async (
@@ -155,10 +148,11 @@ class TeamKeyTransactionButton extends Component<Props, State> {
   };
 
   render() {
-    const {isLoading, counts, keyedTeams} = this.state;
+    const {isLoading, error, counts, keyedTeams} = this.state;
     return (
       <TeamKeyTransaction
         isLoading={isLoading}
+        error={error}
         counts={counts}
         keyedTeams={keyedTeams}
         handleToggleKeyTransaction={this.handleToggleKeyTransaction}

+ 411 - 372
tests/js/spec/views/performance/transactionSummary/teamKeyTransactionButton.spec.jsx

@@ -1,5 +1,3 @@
-import {Component} from 'react';
-
 import {mountWithTheme} from 'sentry-test/enzyme';
 
 import TeamStore from 'app/stores/teamStore';
@@ -7,15 +5,8 @@ import EventView from 'app/utils/discover/eventView';
 import {MAX_TEAM_KEY_TRANSACTIONS} from 'app/utils/performance/constants';
 import TeamKeyTransactionButton from 'app/views/performance/transactionSummary/teamKeyTransactionButton';
 
-class TestButton extends Component {
-  render() {
-    const {keyedTeamsCount, ...props} = this.props;
-    return <button {...props}>{`count: ${keyedTeamsCount}`}</button>;
-  }
-}
-
 async function clickTeamKeyTransactionDropdown(wrapper) {
-  wrapper.find('TestButton').simulate('click');
+  wrapper.find('TitleButton').simulate('click');
   await tick();
   wrapper.update();
 }
@@ -45,396 +36,444 @@ describe('TeamKeyTransaction', function () {
     TeamStore.loadInitialData(teams);
   });
 
-  describe('With no disabled', function () {
-    beforeEach(function () {
-      MockApiClient.addMockResponse({
+  it('fetches key transactions with project param', async function () {
+    const getTeamKeyTransactionsMock = MockApiClient.addMockResponse(
+      {
         method: 'GET',
-        url: '/organizations/org-slug/key-transactions-count/',
-        body: teams.map(({id}) => ({team: id, count: 0})),
-      });
+        url: '/organizations/org-slug/key-transactions-list/',
+        body: teams.map(({id}) => ({
+          team: id,
+          count: 1,
+          keyed: [{project_id: String(project.id), transaction: 'transaction'}],
+        })),
+      },
+      {
+        predicate: (_, options) =>
+          options.method === 'GET' &&
+          options.query.project.length === 1 &&
+          options.query.project[0] === project.id &&
+          options.query.team.length === 1 &&
+          options.query.team[0] === 'myteams',
+      }
+    );
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    expect(getTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('renders with all teams checked', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 1,
+        keyed: [{project_id: String(project.id), transaction: 'transaction'}],
+      })),
     });
 
-    it('renders with all teams checked', async function () {
-      const getTeamKeyTransactionsMock = MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions/',
-        body: teams.map(({id}) => ({team: id})),
-      });
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      // header should show the checked state
-      expect(getTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
-      expect(wrapper.find('TestButton').exists()).toBeTruthy();
-      const header = wrapper.find('DropdownMenuHeader');
-      expect(header.exists()).toBeTruthy();
-      expect(header.find('CheckboxFancy').props().isChecked).toBeTruthy();
-      expect(header.find('CheckboxFancy').props().isIndeterminate).toBeFalsy();
-
-      // all teams should be checked
-      const entries = wrapper.find('DropdownMenuItem');
-      expect(entries.length).toBe(2);
-      entries.forEach((entry, i) => {
-        expect(entry.text()).toEqual(teams[i].name);
-        expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
-      });
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    // header should show the checked state
+    expect(wrapper.find('TitleButton').exists()).toBeTruthy();
+    const header = wrapper.find('DropdownMenuHeader');
+    expect(header.exists()).toBeTruthy();
+    expect(header.find('CheckboxFancy').props().isChecked).toBeTruthy();
+    expect(header.find('CheckboxFancy').props().isIndeterminate).toBeFalsy();
+
+    // all teams should be checked
+    const entries = wrapper.find('DropdownMenuItem');
+    expect(entries.length).toBe(2);
+    entries.forEach((entry, i) => {
+      expect(entry.text()).toEqual(teams[i].name);
+      expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
     });
+  });
 
-    it('renders with some teams checked', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions/',
-        body: [{team: teams[0].id}],
-      });
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      // header should show the indeterminate state
-      const header = wrapper.find('DropdownMenuHeader');
-      expect(header.exists()).toBeTruthy();
-      expect(header.find('CheckboxFancy').props().isChecked).toBeFalsy();
-      expect(header.find('CheckboxFancy').props().isIndeterminate).toBeTruthy();
-
-      // only team 1 should be checked
-      const entries = wrapper.find('DropdownMenuItem');
-      expect(entries.length).toBe(2);
-      entries.forEach((entry, i) => {
-        expect(entry.text()).toEqual(teams[i].name);
-      });
-      expect(entries.at(0).find('CheckboxFancy').props().isChecked).toBeTruthy();
-      expect(entries.at(1).find('CheckboxFancy').props().isChecked).toBeFalsy();
+  it('renders with some teams checked', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: id === teams[0].id ? 1 : 0,
+        keyed:
+          id === teams[0].id
+            ? [{project_id: String(project.id), transaction: 'transaction'}]
+            : [],
+      })),
     });
 
-    it('renders with no teams checked', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions/',
-        body: [],
-      });
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      // header should show the unchecked state
-      const header = wrapper.find('DropdownMenuHeader');
-      expect(header.exists()).toBeTruthy();
-      expect(header.find('CheckboxFancy').props().isChecked).toBeFalsy();
-      expect(header.find('CheckboxFancy').props().isIndeterminate).toBeFalsy();
-
-      // all teams should be unchecked
-      const entries = wrapper.find('DropdownMenuItem');
-      expect(entries.length).toBe(2);
-      entries.forEach((entry, i) => {
-        expect(entry.text()).toEqual(teams[i].name);
-        expect(entry.find('CheckboxFancy').props().isChecked).toBeFalsy();
-      });
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    // header should show the indeterminate state
+    const header = wrapper.find('DropdownMenuHeader');
+    expect(header.exists()).toBeTruthy();
+    expect(header.find('CheckboxFancy').props().isChecked).toBeFalsy();
+    expect(header.find('CheckboxFancy').props().isIndeterminate).toBeTruthy();
+
+    // only team 1 should be checked
+    const entries = wrapper.find('DropdownMenuItem');
+    expect(entries.length).toBe(2);
+    entries.forEach((entry, i) => {
+      expect(entry.text()).toEqual(teams[i].name);
     });
+    expect(entries.at(0).find('CheckboxFancy').props().isChecked).toBeTruthy();
+    expect(entries.at(1).find('CheckboxFancy').props().isChecked).toBeFalsy();
+  });
 
-    it('should be able to check one team', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions/',
-        body: [],
-      });
-
-      const postTeamKeyTransactionsMock = MockApiClient.addMockResponse(
-        {
-          method: 'POST',
-          url: '/organizations/org-slug/key-transactions/',
-          body: [],
-        },
-        {
-          predicate: (_, options) =>
-            options.method === 'POST' &&
-            options.query.project.length === 1 &&
-            options.query.project[0] === project.id &&
-            options.data.team.length === 1 &&
-            options.data.team[0] === teams[0].id &&
-            options.data.transaction === 'transaction',
-        }
-      );
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      wrapper.find('DropdownMenuItem CheckboxFancy').first().simulate('click');
-      await tick();
-      wrapper.update();
-
-      const checkbox = wrapper.find('DropdownMenuItem CheckboxFancy').first();
-      expect(checkbox.props().isChecked).toBeTruthy();
-      expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+  it('renders with no teams checked', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 0,
+        keyed: [],
+      })),
     });
 
-    it('should be able to uncheck one team', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    // header should show the unchecked state
+    const header = wrapper.find('DropdownMenuHeader');
+    expect(header.exists()).toBeTruthy();
+    expect(header.find('CheckboxFancy').props().isChecked).toBeFalsy();
+    expect(header.find('CheckboxFancy').props().isIndeterminate).toBeFalsy();
+
+    // all teams should be unchecked
+    const entries = wrapper.find('DropdownMenuItem');
+    expect(entries.length).toBe(2);
+    entries.forEach((entry, i) => {
+      expect(entry.text()).toEqual(teams[i].name);
+      expect(entry.find('CheckboxFancy').props().isChecked).toBeFalsy();
+    });
+  });
+
+  it('should be able to check one team', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 0,
+        keyed: [],
+      })),
+    });
+
+    const postTeamKeyTransactionsMock = MockApiClient.addMockResponse(
+      {
+        method: 'POST',
         url: '/organizations/org-slug/key-transactions/',
-        body: teams.map(({id}) => ({team: id})),
-      });
-
-      const deleteTeamKeyTransactionsMock = MockApiClient.addMockResponse(
-        {
-          method: 'DELETE',
-          url: '/organizations/org-slug/key-transactions/',
-          body: [],
-        },
-        {
-          predicate: (_, options) =>
-            options.method === 'DELETE' &&
-            options.query.project.length === 1 &&
-            options.query.project[0] === project.id &&
-            options.data.team.length === 1 &&
-            options.data.team[0] === teams[0].id &&
-            options.data.transaction === 'transaction',
-        }
-      );
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      wrapper.find('DropdownMenuItem CheckboxFancy').first().simulate('click');
-      await tick();
-      wrapper.update();
-
-      const checkbox = wrapper.find('DropdownMenuItem CheckboxFancy').first();
-      expect(checkbox.props().isChecked).toBeFalsy();
-      expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+        body: [],
+      },
+      {
+        predicate: (_, options) =>
+          options.method === 'POST' &&
+          options.query.project.length === 1 &&
+          options.query.project[0] === project.id &&
+          options.data.team.length === 1 &&
+          options.data.team[0] === teams[0].id &&
+          options.data.transaction === 'transaction',
+      }
+    );
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    wrapper.find('DropdownMenuItem CheckboxFancy').first().simulate('click');
+    await tick();
+    wrapper.update();
+
+    const checkbox = wrapper.find('DropdownMenuItem CheckboxFancy').first();
+    expect(checkbox.props().isChecked).toBeTruthy();
+    expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('should be able to uncheck one team', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 1,
+        keyed: [{project_id: String(project.id), transaction: 'transaction'}],
+      })),
     });
 
-    it('should be able to check all with my teams', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
+    const deleteTeamKeyTransactionsMock = MockApiClient.addMockResponse(
+      {
+        method: 'DELETE',
         url: '/organizations/org-slug/key-transactions/',
         body: [],
-      });
-
-      const postTeamKeyTransactionsMock = MockApiClient.addMockResponse(
-        {
-          method: 'POST',
-          url: '/organizations/org-slug/key-transactions/',
-          body: [],
-        },
-        {
-          predicate: (_, options) =>
-            options.method === 'POST' &&
-            options.query.project.length === 1 &&
-            options.query.project[0] === project.id &&
-            options.data.team.length === 2 &&
-            options.data.team[0] === teams[0].id &&
-            options.data.team[1] === teams[1].id &&
-            options.data.transaction === 'transaction',
-        }
-      );
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      wrapper.find('DropdownMenuHeader CheckboxFancy').simulate('click');
-      await tick();
-      wrapper.update();
-
-      // header should be checked now
-      const headerCheckbox = wrapper.find('DropdownMenuHeader CheckboxFancy');
-      expect(headerCheckbox.props().isChecked).toBeTruthy();
-      expect(headerCheckbox.props().isIndeterminate).toBeFalsy();
-
-      // all teams should be checked now
-      const entries = wrapper.find('DropdownMenuItem');
-      entries.forEach(entry => {
-        expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
-      });
-      expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+      },
+      {
+        predicate: (_, options) =>
+          options.method === 'DELETE' &&
+          options.query.project.length === 1 &&
+          options.query.project[0] === project.id &&
+          options.data.team.length === 1 &&
+          options.data.team[0] === teams[0].id &&
+          options.data.transaction === 'transaction',
+      }
+    );
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    wrapper.find('DropdownMenuItem CheckboxFancy').first().simulate('click');
+    await tick();
+    wrapper.update();
+
+    const checkbox = wrapper.find('DropdownMenuItem CheckboxFancy').first();
+    expect(checkbox.props().isChecked).toBeFalsy();
+    expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('should be able to check all with my teams', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 0,
+        keyed: [],
+      })),
     });
 
-    it('should be able to uncheck all with my teams', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
+    const postTeamKeyTransactionsMock = MockApiClient.addMockResponse(
+      {
+        method: 'POST',
         url: '/organizations/org-slug/key-transactions/',
-        body: teams.map(({id}) => ({team: id})),
-      });
-
-      const deleteTeamKeyTransactionsMock = MockApiClient.addMockResponse(
-        {
-          method: 'DELETE',
-          url: '/organizations/org-slug/key-transactions/',
-          body: [],
-        },
-        {
-          predicate: (_, options) =>
-            options.method === 'DELETE' &&
-            options.query.project.length === 1 &&
-            options.query.project[0] === project.id &&
-            options.data.team.length === 2 &&
-            options.data.team[0] === teams[0].id &&
-            options.data.team[1] === teams[1].id &&
-            options.data.transaction === 'transaction',
-        }
-      );
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      wrapper.find('DropdownMenuHeader CheckboxFancy').simulate('click');
-      await tick();
-      wrapper.update();
-
-      // header should be unchecked now
-      const headerCheckbox = wrapper.find('DropdownMenuHeader CheckboxFancy');
-      expect(headerCheckbox.props().isChecked).toBeFalsy();
-      expect(headerCheckbox.props().isIndeterminate).toBeFalsy();
-
-      // all teams should be unchecked now
-      const entries = wrapper.find('DropdownMenuItem');
-      entries.forEach(entry => {
-        expect(entry.find('CheckboxFancy').props().isChecked).toBeFalsy();
-      });
-
-      expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+        body: [],
+      },
+      {
+        predicate: (_, options) =>
+          options.method === 'POST' &&
+          options.query.project.length === 1 &&
+          options.query.project[0] === project.id &&
+          options.data.team.length === 2 &&
+          options.data.team[0] === teams[0].id &&
+          options.data.team[1] === teams[1].id &&
+          options.data.transaction === 'transaction',
+      }
+    );
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    wrapper.find('DropdownMenuHeader CheckboxFancy').simulate('click');
+    await tick();
+    wrapper.update();
+
+    // header should be checked now
+    const headerCheckbox = wrapper.find('DropdownMenuHeader CheckboxFancy');
+    expect(headerCheckbox.props().isChecked).toBeTruthy();
+    expect(headerCheckbox.props().isIndeterminate).toBeFalsy();
+
+    // all teams should be checked now
+    const entries = wrapper.find('DropdownMenuItem');
+    entries.forEach(entry => {
+      expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
     });
+    expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
   });
 
-  describe('With disabled', function () {
-    it('renders unkeyed as disabled if count exceeds max', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
+  it('should be able to uncheck all with my teams', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: 1,
+        keyed: [{project_id: String(project.id), transaction: 'transaction'}],
+      })),
+    });
+
+    const deleteTeamKeyTransactionsMock = MockApiClient.addMockResponse(
+      {
+        method: 'DELETE',
         url: '/organizations/org-slug/key-transactions/',
         body: [],
-      });
+      },
+      {
+        predicate: (_, options) =>
+          options.method === 'DELETE' &&
+          options.query.project.length === 1 &&
+          options.query.project[0] === project.id &&
+          options.data.team.length === 2 &&
+          options.data.team[0] === teams[0].id &&
+          options.data.team[1] === teams[1].id &&
+          options.data.transaction === 'transaction',
+      }
+    );
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    wrapper.find('DropdownMenuHeader CheckboxFancy').simulate('click');
+    await tick();
+    wrapper.update();
+
+    // header should be unchecked now
+    const headerCheckbox = wrapper.find('DropdownMenuHeader CheckboxFancy');
+    expect(headerCheckbox.props().isChecked).toBeFalsy();
+    expect(headerCheckbox.props().isIndeterminate).toBeFalsy();
+
+    // all teams should be unchecked now
+    const entries = wrapper.find('DropdownMenuItem');
+    entries.forEach(entry => {
+      expect(entry.find('CheckboxFancy').props().isChecked).toBeFalsy();
+    });
 
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions-count/',
-        body: teams.map(({id}) => ({team: id, count: MAX_TEAM_KEY_TRANSACTIONS})),
-      });
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      const entries = wrapper.find('DropdownMenuItem');
-      expect(entries.length).toBe(2);
-      entries.forEach((entry, i) => {
-        expect(entry.props().disabled).toBeTruthy();
-        expect(entry.text()).toEqual(`${teams[i].name}Max ${MAX_TEAM_KEY_TRANSACTIONS}`);
-      });
+    expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('renders unkeyed as disabled if count exceeds max', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: MAX_TEAM_KEY_TRANSACTIONS,
+        keyed: Array.from({length: MAX_TEAM_KEY_TRANSACTIONS}, (_, i) => ({
+          project_id: String(project.id),
+          transaction: `transaction-${i}`,
+        })),
+      })),
     });
 
-    it('renders keyed as checked even if count is maxed', async function () {
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions/',
-        body: teams.map(({id}) => ({team: id})),
-      });
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    const entries = wrapper.find('DropdownMenuItem');
+    expect(entries.length).toBe(2);
+    entries.forEach((entry, i) => {
+      expect(entry.props().disabled).toBeTruthy();
+      expect(entry.text()).toEqual(`${teams[i].name}Max ${MAX_TEAM_KEY_TRANSACTIONS}`);
+    });
+  });
 
-      MockApiClient.addMockResponse({
-        method: 'GET',
-        url: '/organizations/org-slug/key-transactions-count/',
-        body: teams.map(({id}) => ({team: id, count: MAX_TEAM_KEY_TRANSACTIONS})),
-      });
-
-      const wrapper = mountWithTheme(
-        <TeamKeyTransactionButton
-          eventView={eventView}
-          organization={organization}
-          transactionName="transaction"
-          title={TestButton}
-        />
-      );
-      await tick();
-      wrapper.update();
-
-      clickTeamKeyTransactionDropdown(wrapper);
-
-      const entries = wrapper.find('DropdownMenuItem');
-      expect(entries.length).toBe(2);
-      entries.forEach((entry, i) => {
-        expect(entry.props().disabled).toBeFalsy();
-        expect(entry.text()).toEqual(teams[i].name);
-        expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
-      });
+  it('renders keyed as checked even if count is maxed', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/organizations/org-slug/key-transactions-list/',
+      body: teams.map(({id}) => ({
+        team: id,
+        count: MAX_TEAM_KEY_TRANSACTIONS,
+        keyed: [
+          {project_id: String(project.id), transaction: 'transaction'},
+          ...Array.from({length: MAX_TEAM_KEY_TRANSACTIONS - 1}, (_, i) => ({
+            project_id: String(project.id),
+            transaction: `transaction-${i}`,
+          })),
+        ],
+      })),
+    });
+
+    const wrapper = mountWithTheme(
+      <TeamKeyTransactionButton
+        eventView={eventView}
+        organization={organization}
+        transactionName="transaction"
+      />
+    );
+    await tick();
+    wrapper.update();
+
+    clickTeamKeyTransactionDropdown(wrapper);
+
+    const entries = wrapper.find('DropdownMenuItem');
+    expect(entries.length).toBe(2);
+    entries.forEach((entry, i) => {
+      expect(entry.props().disabled).toBeFalsy();
+      expect(entry.text()).toEqual(teams[i].name);
+      expect(entry.find('CheckboxFancy').props().isChecked).toBeTruthy();
     });
   });
 });

+ 156 - 13
tests/snuba/api/endpoints/test_discover_key_transactions.py

@@ -7,6 +7,7 @@ from sentry.discover.models import (
     TeamKeyTransaction,
 )
 from sentry.testutils import APITestCase, SnubaTestCase
+from sentry.testutils.helpers import parse_link_header
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.utils.samples import load_data
 
@@ -563,10 +564,10 @@ class TeamKeyTransactionTest(TeamKeyTransactionTestBase):
         assert response.status_code == 204, response.content
 
 
-class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
+class TeamKeyTransactionListTest(TeamKeyTransactionTestBase):
     def setUp(self):
         super().setUp()
-        self.url = reverse("sentry-api-0-organization-key-transactions-count", args=[self.org.slug])
+        self.url = reverse("sentry-api-0-organization-key-transactions-list", args=[self.org.slug])
 
         self.team1 = self.create_team(organization=self.org, name="Team A")
         self.team2 = self.create_team(organization=self.org, name="Team B")
@@ -600,16 +601,19 @@ class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
             ]
         )
 
-    def test_get_no_team_key_transaction_count_feature(self):
+    def test_get_no_team_key_transaction_list_feature(self):
         with self.feature(self.base_features):
             response = self.client.get(
                 self.url,
-                data={"team": ["myteam"]},
+                data={
+                    "project": [self.project.id],
+                    "team": ["myteam"],
+                },
                 format="json",
             )
         assert response.status_code == 404, response.content
 
-    def test_get_key_transaction_count_no_permissions(self):
+    def test_get_key_transaction_list_no_permissions(self):
         org = self.create_organization(
             owner=self.user,  # use other user as owner
             name="foo",
@@ -629,19 +633,25 @@ class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
 
         with self.feature(self.features):
             response = self.client.get(
-                reverse("sentry-api-0-organization-key-transactions-count", args=[org.slug]),
-                data={"team": ["myteams", other_team.id]},
+                reverse("sentry-api-0-organization-key-transactions-list", args=[org.slug]),
+                data={
+                    "project": [self.project.id],
+                    "team": ["myteams", other_team.id],
+                },
                 format="json",
             )
 
         assert response.status_code == 400, response.content
         assert response.data == f"Error: You do not have permission to access {other_team.name}"
 
-    def test_get_key_transaction_count_my_teams(self):
+    def test_get_key_transaction_list_my_teams(self):
         with self.feature(self.features):
             response = self.client.get(
                 self.url,
-                data={"team": ["myteams"]},
+                data={
+                    "project": [self.project.id],
+                    "team": ["myteams"],
+                },
                 format="json",
             )
 
@@ -650,22 +660,39 @@ class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
             {
                 "team": str(self.team1.id),
                 "count": 0,
+                "keyed": [],
             },
             {
                 "team": str(self.team2.id),
                 "count": 1,
+                "keyed": [
+                    {
+                        "project_id": str(self.project.id),
+                        "transaction": self.event_data["transaction"],
+                    },
+                ],
             },
             {
                 "team": str(self.team3.id),
                 "count": 2,
+                "keyed": [
+                    {
+                        "project_id": str(self.project.id),
+                        "transaction": self.event_data["transaction"],
+                    },
+                    {"project_id": str(self.project.id), "transaction": "other-transaction"},
+                ],
             },
         ]
 
-    def test_get_key_transaction_count_other_teams(self):
+    def test_get_key_transaction_list_other_teams(self):
         with self.feature(self.features):
             response = self.client.get(
                 self.url,
-                data={"team": [self.team4.id, self.team5.id]},
+                data={
+                    "project": [self.project.id],
+                    "team": [self.team4.id, self.team5.id],
+                },
                 format="json",
             )
 
@@ -674,18 +701,25 @@ class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
             {
                 "team": str(self.team4.id),
                 "count": 1,
+                "keyed": [
+                    {"project_id": str(self.project.id), "transaction": "other-transaction"},
+                ],
             },
             {
                 "team": str(self.team5.id),
                 "count": 0,
+                "keyed": [],
             },
         ]
 
-    def test_get_key_transaction_count_mixed_my_and_other_teams(self):
+    def test_get_key_transaction_list_mixed_my_and_other_teams(self):
         with self.feature(self.features):
             response = self.client.get(
                 self.url,
-                data={"team": ["myteams", self.team4.id, self.team5.id]},
+                data={
+                    "project": [self.project.id],
+                    "team": ["myteams", self.team4.id, self.team5.id],
+                },
                 format="json",
             )
 
@@ -694,22 +728,131 @@ class TeamKeyTransactionCountTest(TeamKeyTransactionTestBase):
             {
                 "team": str(self.team1.id),
                 "count": 0,
+                "keyed": [],
             },
             {
                 "team": str(self.team2.id),
                 "count": 1,
+                "keyed": [
+                    {
+                        "project_id": str(self.project.id),
+                        "transaction": self.event_data["transaction"],
+                    },
+                ],
             },
             {
                 "team": str(self.team3.id),
                 "count": 2,
+                "keyed": [
+                    {
+                        "project_id": str(self.project.id),
+                        "transaction": self.event_data["transaction"],
+                    },
+                    {"project_id": str(self.project.id), "transaction": "other-transaction"},
+                ],
             },
             {
                 "team": str(self.team4.id),
                 "count": 1,
+                "keyed": [
+                    {"project_id": str(self.project.id), "transaction": "other-transaction"},
+                ],
             },
             {
                 "team": str(self.team5.id),
                 "count": 0,
+                "keyed": [],
+            },
+        ]
+
+    def test_get_key_transaction_list_pagination(self):
+        user = self.create_user()
+        self.login_as(user=user)
+        org = self.create_organization(owner=user, name="foo")
+        project = self.create_project(name="baz", organization=org)
+
+        teams = []
+        for i in range(123):
+            team = self.create_team(organization=org, name=f"Team {i:02d}")
+            self.create_team_membership(team, user=user)
+            project.add_team(team)
+            teams.append(team)
+
+        # get the first page
+        with self.feature(self.features):
+            response = self.client.get(
+                reverse("sentry-api-0-organization-key-transactions-list", args=[org.slug]),
+                data={
+                    "project": [project.id],
+                    "team": ["myteams"],
+                },
+                format="json",
+            )
+
+        assert response.status_code == 200, response.content
+        assert len(response.data) == 100
+        links = {
+            link["rel"]: {"url": url, **link}
+            for url, link in parse_link_header(response["Link"]).items()
+        }
+        assert links["previous"]["results"] == "false"
+        assert links["next"]["results"] == "true"
+
+        # get the second page
+        with self.feature(self.features):
+            response = self.client.get(
+                reverse("sentry-api-0-organization-key-transactions-list", args=[org.slug]),
+                data={
+                    "project": [project.id],
+                    "team": ["myteams"],
+                    "cursor": links["next"]["cursor"],
+                },
+                format="json",
+            )
+
+        assert response.status_code == 200, response.content
+        assert len(response.data) == 23
+        links = {
+            link["rel"]: {"url": url, **link}
+            for url, link in parse_link_header(response["Link"]).items()
+        }
+        assert links["previous"]["results"] == "true"
+        assert links["next"]["results"] == "false"
+
+    def test_get_key_transaction_list_partial_project(self):
+        another_project = self.create_project(organization=self.org)
+        another_project.add_team(self.team2)
+
+        TeamKeyTransaction.objects.create(
+            team=self.team2,
+            organization=self.org,
+            transaction="another-transaction",
+            project=another_project,
+        )
+
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url,
+                data={
+                    "project": [another_project.id],
+                    "team": [self.team2.id],
+                },
+                format="json",
+            )
+
+        assert response.status_code == 200, response.content
+        assert response.data == [
+            {
+                "team": str(self.team2.id),
+                # the key transaction in self.project is counted but not in
+                # the list because self.project is not in the project param
+                "count": 2,
+                "keyed": [
+                    {
+                        "project_id": str(another_project.id),
+                        "transaction": "another-transaction",
+                    },
+                ],
             },
         ]