Skip to content

fix(api-conventions): Remove callback API requests from group action creators #126268

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.

Sign up for GitHub

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

Open
sentry wants to merge 6 commits into
base: master
Choose a base branch
from
+222 −316
Open

fix(api-conventions): Remove callback API requests from group action creators #126268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87c8fc7
fix(api-conventions): Remove callback API requests from group action …
sentry[bot] Oct 1, 2026
b20ee86
fix(api-callbacks): Migrate group action creators to promises
sentry[bot] Oct 1, 2026
92e46c1
ref(group): Migrate bulkUpdate and mergeGroups to async/await
sentry[bot] Oct 1, 2026
e652159
ref(group): Await bulkUpdate and mergeGroups at every callsite
ryan953 Oct 2, 2026
5f61171
ref(group): Pass includeAllArgs to requestPromise in bulkUpdate and m…
ryan953 Oct 2, 2026
43a5973
ref(group): Keep the response name in bulkUpdate and mergeGroups
ryan953 Oct 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 65 additions & 101 deletions static/app/actionCreators/group.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,19 @@ describe('group', () => {
jest.spyOn(GroupStore, 'onUpdate'); // stub GroupStore.onUpdate call from update
});

it('should use itemIds as query if provided', () => {
it('should use itemIds as query if provided', async () => {
const request = MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1', '2', '3'],
data: {status: 'unresolved'},
query: 'is:resolved',
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1', '2', '3'],
data: {status: 'unresolved'},
query: 'is:resolved',
});

expect(request).toHaveBeenCalledTimes(1);
expect(request).toHaveBeenCalledWith(
Expand All @@ -103,23 +99,19 @@ describe('group', () => {
);
});

it('should use query as query if itemIds are absent', () => {
it('should use query as query if itemIds are absent', async () => {
const request = MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: undefined,
data: {status: 'unresolved'},
query: 'is:resolved',
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: undefined,
data: {status: 'unresolved'},
query: 'is:resolved',
});

expect(request).toHaveBeenCalledTimes(1);
expect(request).toHaveBeenCalledWith(
Expand All @@ -128,22 +120,18 @@ describe('group', () => {
);
});

it('should apply project option', () => {
it('should apply project option', async () => {
const request = MockApiClient.addMockResponse({
url: '/organizations/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
project: [99],
itemIds: ['1', '2', '3'],
data: {status: 'unresolved'},
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
project: [99],
itemIds: ['1', '2', '3'],
data: {status: 'unresolved'},
});

expect(request).toHaveBeenCalledTimes(1);
expect(request).toHaveBeenCalledWith(
Expand All @@ -152,88 +140,72 @@ describe('group', () => {
);
});

it('should normalize string assignedTo to Actor object for optimistic update', () => {
it('should normalize string assignedTo to Actor object for optimistic update', async () => {
MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'user:123'},
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'user:123'},
});

expect(GroupStore.onUpdate).toHaveBeenCalledWith(expect.any(String), ['1'], {
assignedTo: {type: 'user', id: '123', name: ''},
});
});

it('should normalize empty assignedTo string to null for optimistic update', () => {
it('should normalize empty assignedTo string to null for optimistic update', async () => {
MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: ''},
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: ''},
});

expect(GroupStore.onUpdate).toHaveBeenCalledWith(expect.any(String), ['1'], {
assignedTo: null,
});
});

it('should normalize team assignedTo string for optimistic update', () => {
it('should normalize team assignedTo string for optimistic update', async () => {
MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'team:456'},
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'team:456'},
});

expect(GroupStore.onUpdate).toHaveBeenCalledWith(expect.any(String), ['1'], {
assignedTo: {type: 'team', id: '456', name: ''},
});
});

it('should send raw string assignedTo to the API', () => {
it('should send raw string assignedTo to the API', async () => {
const request = MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

bulkUpdate(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'user:123'},
},
{}
);
await bulkUpdate(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1'],
data: {assignedTo: 'user:123'},
});

expect(request).toHaveBeenCalledWith(
'/projects/1337/1337/issues/',
Expand All @@ -249,22 +221,18 @@ describe('group', () => {
jest.spyOn(GroupStore, 'onMerge'); // stub GroupStore.onMerge call from mergeGroups
});

it('should use itemIds as query if provided', () => {
it('should use itemIds as query if provided', async () => {
const request = MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

mergeGroups(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: ['1', '2', '3'],
query: 'is:resolved',
},
{}
);
await mergeGroups(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: ['1', '2', '3'],
query: 'is:resolved',
});

expect(request).toHaveBeenCalledTimes(1);
expect(request).toHaveBeenCalledWith(
Expand All @@ -273,22 +241,18 @@ describe('group', () => {
);
});

it('should use query as query if itemIds are absent', () => {
it('should use query as query if itemIds are absent', async () => {
const request = MockApiClient.addMockResponse({
url: '/projects/1337/1337/issues/',
method: 'PUT',
});

mergeGroups(
new MockApiClient(),
{
orgId: '1337',
projectId: '1337',
itemIds: undefined,
query: 'is:resolved',
},
{}
);
await mergeGroups(new MockApiClient(), {
orgId: '1337',
projectId: '1337',
itemIds: undefined,
query: 'is:resolved',
});

expect(request).toHaveBeenCalledTimes(1);
expect(request).toHaveBeenCalledWith(
Expand Down
37 changes: 7 additions & 30 deletions static/app/actionCreators/group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,7 @@ type BulkUpdateParams = UpdateParams & {
failSilently?: boolean;
};

export async function bulkUpdate(
api: Client,
params: BulkUpdateParams,
options: RequestCallbacks = {}
) {
export async function bulkUpdate(api: Client, params: BulkUpdateParams) {
const {itemIds, failSilently, data} = params;
const path = getUpdateUrl(params);

Expand All @@ -138,35 +134,23 @@ export async function bulkUpdate(
: data;
GroupStore.onUpdate(id, itemIds, optimisticData);

let responseMeta: any;
let statusText: string | undefined;

try {
const [response, status, meta] = await api.requestPromise(path, {
const [response] = await api.requestPromise(path, {
query,
method: 'PUT',
data,
includeAllArgs: true,
});
statusText = status;
responseMeta = meta;
GroupStore.onUpdateSuccess(id, itemIds, response);
options?.success?.(response, statusText, responseMeta);
} catch (error) {
GroupStore.onUpdateError(id, itemIds, !!failSilently);
options?.error?.(error);
} finally {
options?.complete?.(responseMeta, statusText ?? '');
throw error;
}
}

type MergeGroupsParams = UpdateParams;

export async function mergeGroups(
api: Client,
params: MergeGroupsParams,
options: RequestCallbacks = {}
) {
export async function mergeGroups(api: Client, params: MergeGroupsParams) {
const {itemIds} = params;
const path = getUpdateUrl(params);

Expand All @@ -175,25 +159,18 @@ export async function mergeGroups(

GroupStore.onMerge(id, itemIds);

let responseMeta: any;
let statusText: string | undefined;

try {
const [response, status, meta] = await api.requestPromise(path, {
const [response] = await api.requestPromise(path, {
query,
method: 'PUT',
data: {merge: 1},
includeAllArgs: true,
});
statusText = status;
responseMeta = meta;
GroupStore.onMergeSuccess(id, itemIds, response);
options?.success?.(response, statusText, responseMeta);
return response;
} catch (error) {
GroupStore.onMergeError(id, itemIds, error);
options?.error?.(error);
} finally {
options?.complete?.(responseMeta, statusText ?? '');
throw error;
}
}

Expand Down
Loading
Loading