Skip to content

Commit 951f66e

Browse files
authored
Merge pull request #265 from gadget-inc/bulk-action-inputs
Fix bulk action input processing to deal with ambiguous identifiers and nested params
2 parents 7dc7e72 + f1a93ba commit 951f66e

File tree

4 files changed

+327
-49
lines changed

4 files changed

+327
-49
lines changed

packages/react/spec/useBulkAction.spec.ts

Lines changed: 250 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe("useBulkAction", () => {
5858
expect(result.current[0].error).toBeFalsy();
5959
});
6060

61-
test("returns no data, fetching=true, and no error when the mutation is run, and then the successful data if the mutation succeeds", async () => {
61+
test("returns no data, fetching=true, and no error when the mutation is run, and then the successful data if the mutation succeeds for an ids only mutation", async () => {
6262
const { result } = renderHook(() => useBulkAction(bulkExampleApi.widget.bulkFlipDown), { wrapper: MockClientWrapper(bulkExampleApi) });
6363

6464
let mutationPromise: any;
@@ -102,6 +102,255 @@ describe("useBulkAction", () => {
102102
expect(result.current[0].error).toBeFalsy();
103103
});
104104

105+
test("can execute a bulk create with params", async () => {
106+
const mockBulkCreate = {
107+
type: "action",
108+
operationName: "bulkCreateWidgets",
109+
namespace: null,
110+
modelApiIdentifier: "widget",
111+
modelSelectionField: "widgets",
112+
isBulk: true,
113+
defaultSelection: {
114+
id: true,
115+
name: true,
116+
},
117+
selectionType: {},
118+
optionsType: {},
119+
schemaType: null,
120+
variablesType: void 0,
121+
variables: {
122+
inputs: {
123+
required: true,
124+
type: "[BulkCreateWidgetsInput!]",
125+
},
126+
},
127+
hasReturnType: false,
128+
} as any;
129+
130+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
131+
// @ts-ignore waiting for bulk params to be released gadget side
132+
const { result } = renderHook(() => useBulkAction<any, any, any, any>(mockBulkCreate), { wrapper: MockClientWrapper(bulkExampleApi) });
133+
134+
let mutationPromise: any;
135+
act(() => {
136+
mutationPromise = result.current[1]([{ name: "foo" }, { name: "bar" }]);
137+
});
138+
139+
expect(result.current[0].data).toBeFalsy();
140+
expect(result.current[0].fetching).toBe(true);
141+
expect(result.current[0].error).toBeFalsy();
142+
143+
expect(mockUrqlClient.executeMutation).toBeCalledTimes(1);
144+
expect(mockUrqlClient.executeMutation.mock.calls[0][0].variables).toMatchInlineSnapshot(`
145+
{
146+
"inputs": [
147+
{
148+
"name": "foo",
149+
},
150+
{
151+
"name": "bar",
152+
},
153+
],
154+
}
155+
`);
156+
157+
mockUrqlClient.executeMutation.pushResponse("bulkCreateWidgets", {
158+
data: {
159+
bulkCreateWidgets: {
160+
success: true,
161+
widgets: [
162+
{ id: "123", name: "foo" },
163+
{ id: "124", name: "bar" },
164+
],
165+
},
166+
},
167+
stale: false,
168+
hasNext: false,
169+
});
170+
171+
await act(async () => {
172+
const promiseResult = await mutationPromise;
173+
expect(promiseResult.data!.length).toEqual(2);
174+
expect(promiseResult.data![0].id).toEqual("123");
175+
expect(promiseResult.data![1].id).toEqual("124");
176+
});
177+
178+
expect(result.current[0].data!.length).toEqual(2);
179+
expect(result.current[0].data![0].id).toEqual("123");
180+
expect(result.current[0].data![1].id).toEqual("124");
181+
expect(result.current[0].fetching).toBe(false);
182+
expect(result.current[0].error).toBeFalsy();
183+
});
184+
185+
test("can execute a bulk create with fully qualified params", async () => {
186+
const mockBulkCreate = {
187+
type: "action",
188+
operationName: "bulkCreateWidgets",
189+
namespace: null,
190+
modelApiIdentifier: "widget",
191+
modelSelectionField: "widgets",
192+
isBulk: true,
193+
defaultSelection: {
194+
id: true,
195+
name: true,
196+
},
197+
selectionType: {},
198+
optionsType: {},
199+
schemaType: null,
200+
variablesType: void 0,
201+
variables: {
202+
inputs: {
203+
required: true,
204+
type: "[BulkCreateWidgetsInput!]",
205+
},
206+
},
207+
hasReturnType: false,
208+
} as any;
209+
210+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
211+
// @ts-ignore waiting for bulk params to be released gadget side
212+
const { result } = renderHook(() => useBulkAction<any, any, any, any>(mockBulkCreate), { wrapper: MockClientWrapper(bulkExampleApi) });
213+
214+
let mutationPromise: any;
215+
act(() => {
216+
mutationPromise = result.current[1]([{ widget: { name: "foo" } }, { widget: { name: "bar" } }]);
217+
});
218+
219+
expect(result.current[0].data).toBeFalsy();
220+
expect(result.current[0].fetching).toBe(true);
221+
expect(result.current[0].error).toBeFalsy();
222+
223+
expect(mockUrqlClient.executeMutation).toBeCalledTimes(1);
224+
expect(mockUrqlClient.executeMutation.mock.calls[0][0].variables).toMatchInlineSnapshot(`
225+
{
226+
"inputs": [
227+
{
228+
"widget": {
229+
"name": "foo",
230+
},
231+
},
232+
{
233+
"widget": {
234+
"name": "bar",
235+
},
236+
},
237+
],
238+
}
239+
`);
240+
241+
mockUrqlClient.executeMutation.pushResponse("bulkCreateWidgets", {
242+
data: {
243+
bulkCreateWidgets: {
244+
success: true,
245+
widgets: [
246+
{ id: "123", name: "foo" },
247+
{ id: "124", name: "bar" },
248+
],
249+
},
250+
},
251+
stale: false,
252+
hasNext: false,
253+
});
254+
255+
await act(async () => {
256+
const promiseResult = await mutationPromise;
257+
expect(promiseResult.data!.length).toEqual(2);
258+
expect(promiseResult.data![0].id).toEqual("123");
259+
expect(promiseResult.data![1].id).toEqual("124");
260+
});
261+
262+
expect(result.current[0].data!.length).toEqual(2);
263+
expect(result.current[0].data![0].id).toEqual("123");
264+
expect(result.current[0].data![1].id).toEqual("124");
265+
expect(result.current[0].fetching).toBe(false);
266+
expect(result.current[0].error).toBeFalsy();
267+
});
268+
269+
test("can execute a bulk update with params", async () => {
270+
const mockBulkUpdate = {
271+
type: "action",
272+
operationName: "bulkUpdateWidgets",
273+
namespace: null,
274+
modelApiIdentifier: "widget",
275+
modelSelectionField: "widgets",
276+
isBulk: true,
277+
defaultSelection: {
278+
id: true,
279+
name: true,
280+
},
281+
selectionType: {},
282+
optionsType: {},
283+
schemaType: null,
284+
variablesType: void 0,
285+
variables: {
286+
inputs: {
287+
required: true,
288+
type: "[BulkUpdateWidgetsInput!]",
289+
},
290+
},
291+
hasReturnType: false,
292+
} as any;
293+
294+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
295+
// @ts-ignore waiting for bulk params to be released gadget side
296+
const { result } = renderHook(() => useBulkAction<any, any, any, any>(mockBulkUpdate), { wrapper: MockClientWrapper(bulkExampleApi) });
297+
298+
let mutationPromise: any;
299+
act(() => {
300+
mutationPromise = result.current[1]([
301+
{ id: "123", name: "foo" },
302+
{ id: "124", name: "bar" },
303+
]);
304+
});
305+
306+
expect(result.current[0].data).toBeFalsy();
307+
expect(result.current[0].fetching).toBe(true);
308+
expect(result.current[0].error).toBeFalsy();
309+
310+
expect(mockUrqlClient.executeMutation).toBeCalledTimes(1);
311+
expect(mockUrqlClient.executeMutation.mock.calls[0][0].variables).toMatchInlineSnapshot(`
312+
{
313+
"inputs": [
314+
{
315+
"id": "123",
316+
"name": "foo",
317+
},
318+
{
319+
"id": "124",
320+
"name": "bar",
321+
},
322+
],
323+
}
324+
`);
325+
326+
mockUrqlClient.executeMutation.pushResponse("bulkUpdateWidgets", {
327+
data: {
328+
bulkUpdateWidgets: {
329+
success: true,
330+
widgets: [
331+
{ id: "123", name: "foo" },
332+
{ id: "124", name: "bar" },
333+
],
334+
},
335+
},
336+
stale: false,
337+
hasNext: false,
338+
});
339+
340+
await act(async () => {
341+
const promiseResult = await mutationPromise;
342+
expect(promiseResult.data!.length).toEqual(2);
343+
expect(promiseResult.data![0].id).toEqual("123");
344+
expect(promiseResult.data![1].id).toEqual("124");
345+
});
346+
347+
expect(result.current[0].data!.length).toEqual(2);
348+
expect(result.current[0].data![0].id).toEqual("123");
349+
expect(result.current[0].data![1].id).toEqual("124");
350+
expect(result.current[0].fetching).toBe(false);
351+
expect(result.current[0].error).toBeFalsy();
352+
});
353+
105354
test("returns an error when the mutation is run and the server responds with success: false", async () => {
106355
const { result } = renderHook(
107356
() => {

packages/react/src/useAction.ts

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,14 @@ import type { AnyVariables, OperationContext, UseMutationState } from "urql";
55
import { GadgetUrqlClientContext } from "./GadgetProvider.js";
66
import { useGadgetMutation } from "./useGadgetMutation.js";
77
import { useStructuralMemo } from "./useStructuralMemo.js";
8-
import type { ActionHookResult, ActionHookState, OptionsType } from "./utils.js";
9-
import { ErrorWrapper, noProviderErrorMessage } from "./utils.js";
8+
import {
9+
ActionHookResult,
10+
ActionHookState,
11+
ErrorWrapper,
12+
OptionsType,
13+
disambiguateActionVariables,
14+
noProviderErrorMessage,
15+
} from "./utils.js";
1016

1117
/**
1218
* React hook to run a Gadget model action. `useAction` must be passed an action function from an instance of your generated API client library, like `api.user.create` or `api.blogPost.publish`. `useAction` doesn't actually run the action when invoked, but instead returns an action function as the second result for running the action in response to an event.
@@ -81,48 +87,12 @@ export const useAction = <
8187
return [
8288
transformedResult,
8389
useCallback(
84-
async (variables: F["variablesType"], context?: Partial<OperationContext>) => {
85-
variables ??= {};
86-
if (action.hasAmbiguousIdentifier) {
87-
if (Object.keys(variables).some((key) => !action.paramOnlyVariables?.includes(key) && key !== action.modelApiIdentifier)) {
88-
throw Error(`Invalid arguments found in variables. Did you mean to use ({ ${action.modelApiIdentifier}: { ... } })?`);
89-
}
90-
}
90+
async (input: F["variablesType"], context?: Partial<OperationContext>) => {
91+
const variables = disambiguateActionVariables(action, input);
9192

92-
let newVariables: Exclude<F["variablesType"], null | undefined>;
93-
const idVariable = Object.entries(action.variables).find(([key, value]) => key === "id" && value.type === "GadgetID");
94-
95-
if (action.acceptsModelInput || action.hasCreateOrUpdateEffect) {
96-
if (
97-
action.modelApiIdentifier in variables &&
98-
typeof variables[action.modelApiIdentifier] === "object" &&
99-
variables[action.modelApiIdentifier] !== null
100-
) {
101-
newVariables = variables;
102-
} else {
103-
newVariables = {
104-
[action.modelApiIdentifier]: {},
105-
} as Exclude<F["variablesType"], null | undefined>;
106-
for (const [key, value] of Object.entries(variables)) {
107-
if (action.paramOnlyVariables?.includes(key)) {
108-
newVariables[key] = value;
109-
} else {
110-
if (idVariable && key === idVariable[0]) {
111-
newVariables.id = value;
112-
} else {
113-
newVariables[action.modelApiIdentifier][key] = value;
114-
}
115-
}
116-
}
117-
}
118-
} else {
119-
newVariables = variables;
120-
}
121-
122-
// Adding the model's additional typename ensures document cache will properly refresh, regardless of whether __typename was
123-
// selected (and sometimes we can't even select it, like delete actions!)
124-
const result = await runMutation(newVariables, {
93+
const result = await runMutation(variables, {
12594
...context,
95+
// Adding the model's additional typename ensures document cache will properly refresh, regardless of whether __typename was selected (and sometimes we can't even select it, like delete actions!)
12696
additionalTypenames: [...(context?.additionalTypenames ?? []), capitalizeIdentifier(action.modelApiIdentifier)],
12797
});
12898
return processResult({ fetching: false, ...result }, action);

packages/react/src/useBulkAction.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import { useCallback, useMemo } from "react";
44
import type { OperationContext, UseMutationState } from "urql";
55
import { useGadgetMutation } from "./useGadgetMutation.js";
66
import { useStructuralMemo } from "./useStructuralMemo.js";
7-
import type { ActionHookResult, OptionsType } from "./utils.js";
8-
import { ErrorWrapper } from "./utils.js";
7+
import { ActionHookResult, ErrorWrapper, OptionsType, disambiguateActionVariables } from "./utils.js";
98

109
/**
1110
* React hook to run a Gadget model bulk action.
@@ -78,13 +77,19 @@ export const useBulkAction = <
7877
transformedResult,
7978
useCallback(
8079
async (inputs: F["variablesType"], context?: Partial<OperationContext>) => {
80+
let variables;
8181
// accept the old style of {ids: ["1", "2", "3"]} as well as the new style of [{id: 1, foo: "bar"}, {id: 2, foo: "baz"}]
82-
const variables = inputs && "ids" in inputs ? inputs : { inputs };
82+
if (inputs && "ids" in inputs) {
83+
variables = inputs;
84+
} else {
85+
variables = {
86+
inputs: ((inputs as any[]) ?? []).map((input: any) => disambiguateActionVariables(action, input)),
87+
};
88+
}
8389

84-
// Adding the model's additional typename ensures document cache will properly refresh, regardless of whether __typename was
85-
// selected (and sometimes we can't even select it, like delete actions!)
8690
const result = await runMutation(variables, {
8791
...context,
92+
// Adding the model's additional typename ensures document cache will properly refresh, regardless of whether __typename was selected (and sometimes we can't even select it, like delete actions!)
8893
additionalTypenames: [...(context?.additionalTypenames ?? []), capitalizeIdentifier(action.modelApiIdentifier)],
8994
});
9095
return processResult({ fetching: false, ...result }, action);

0 commit comments

Comments
 (0)