From 63b90f21682bfbcf668d8a90fdc8f73471d5fd97 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 26 Jul 2023 09:28:18 -0700 Subject: [PATCH 1/2] feat: change arg format of update mutations the old format encouraged sending unnecessary data and did not guard against bugs from sending extra data --- src/project/projectService.ts | 15 +++++++++++---- src/site/siteService.ts | 20 ++++++++++++++------ src/terrasoApi/utils.ts | 21 +++++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/project/projectService.ts b/src/project/projectService.ts index 4d8909fa..53cfd729 100644 --- a/src/project/projectService.ts +++ b/src/project/projectService.ts @@ -28,6 +28,8 @@ import * as terrasoApi from 'terraso-client-shared/terrasoApi/api'; import { collapseConnectionEdges, collapseFields, + UpdateArg, + updateArgToInput, } from 'terraso-client-shared/terrasoApi/utils'; const collapseProjectFields = collapseFields({ @@ -91,7 +93,9 @@ export const addProject = (project: ProjectAddMutationInput) => { .then(resp => collapseProjectFields(resp.addProject.project)); }; -export const updateProject = (project: ProjectUpdateMutationInput) => { +export const updateProject = ( + update: UpdateArg, +) => { const query = graphql(` mutation updateProject($input: ProjectUpdateMutationInput!) { updateProject(input: $input) { @@ -104,7 +108,7 @@ export const updateProject = (project: ProjectUpdateMutationInput) => { `); return terrasoApi - .requestGraphQL(query, { input: project }) + .requestGraphQL(query, updateArgToInput(update)) .then(resp => collapseProjectFields(resp.updateProject.project!)); }; @@ -112,12 +116,15 @@ export const deleteProject = (project: ProjectDeleteMutationInput) => { const query = graphql(` mutation deleteProject($input: ProjectDeleteMutationInput!) { deleteProject(input: $input) { + project { + id + } errors } } `); return terrasoApi - .requestGraphQL(query, { input: { id: project.id } }) - .then(_ => project.id); + .requestGraphQL(query, { input: project }) + .then(({ deleteProject: { project } }) => project.id); }; diff --git a/src/site/siteService.ts b/src/site/siteService.ts index 875b9ba0..7de1c16a 100644 --- a/src/site/siteService.ts +++ b/src/site/siteService.ts @@ -20,11 +20,16 @@ import { graphql } from 'terraso-client-shared/graphqlSchema'; import type { SiteAddMutationInput, SiteDataFragment, + SiteDeleteMutationInput, SiteUpdateMutationInput, } from 'terraso-client-shared/graphqlSchema/graphql'; import type { Site } from 'terraso-client-shared/site/siteSlice'; import * as terrasoApi from 'terraso-client-shared/terrasoApi/api'; -import { collapseConnectionEdges } from 'terraso-client-shared/terrasoApi/utils'; +import { + collapseConnectionEdges, + UpdateArg, + updateArgToInput, +} from 'terraso-client-shared/terrasoApi/utils'; const collapseSiteFields = (site: SiteDataFragment): Site => { const { project, owner, ...rest } = site; @@ -117,7 +122,7 @@ export const addSite = (site: SiteAddMutationInput) => { .then(resp => collapseSiteFields(resp.addSite.site)); }; -export const updateSite = (site: SiteUpdateMutationInput) => { +export const updateSite = (update: UpdateArg) => { const query = graphql(` mutation updateSite($input: SiteUpdateMutationInput!) { updateSite(input: $input) { @@ -130,20 +135,23 @@ export const updateSite = (site: SiteUpdateMutationInput) => { `); return terrasoApi - .requestGraphQL(query, { input: site }) + .requestGraphQL(query, updateArgToInput(update)) .then(resp => collapseSiteFields(resp.updateSite.site!)); }; -export const deleteSite = (site: Site) => { +export const deleteSite = (site: SiteDeleteMutationInput) => { const query = graphql(` mutation deleteSite($input: SiteDeleteMutationInput!) { deleteSite(input: $input) { + site { + id + } errors } } `); return terrasoApi - .requestGraphQL(query, { input: { id: site.id } }) - .then(_ => site.id); + .requestGraphQL(query, { input: site }) + .then(({ deleteSite: { site } }) => site.id); }; diff --git a/src/terrasoApi/utils.ts b/src/terrasoApi/utils.ts index 5170b665..89545a0a 100644 --- a/src/terrasoApi/utils.ts +++ b/src/terrasoApi/utils.ts @@ -15,6 +15,27 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ +type Resolve = T extends Function ? T : { [K in keyof T]: T[K] }; + +export type UpdateArg = Resolve<{ + id: string; + update: Resolve>; +}>; + +export const updateArgToInput = ({ + id, + update, +}: UpdateArg) => { + if ('id' in update) { + throw Error(` + update arguments should not contain IDs! + are you accidentally passing the entire model object into the + update arg instead of just the fields you need to update? + `); + } + return { input: { ...update, id } }; +}; + export const collapseConnectionEdges = (connection: { edges: { node: T }[]; }): T[] => { From 6d27db4bdfbc3c5fb32273a676833ac9402059a2 Mon Sep 17 00:00:00 2001 From: shrouxm Date: Wed, 26 Jul 2023 12:46:57 -0700 Subject: [PATCH 2/2] feat: parameterize by the object's query key --- src/project/projectService.ts | 4 ++-- src/site/siteService.ts | 6 ++++-- src/terrasoApi/utils.ts | 28 ++++++++++++++++------------ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/project/projectService.ts b/src/project/projectService.ts index 53cfd729..496a9449 100644 --- a/src/project/projectService.ts +++ b/src/project/projectService.ts @@ -94,7 +94,7 @@ export const addProject = (project: ProjectAddMutationInput) => { }; export const updateProject = ( - update: UpdateArg, + update: UpdateArg<'id', ProjectUpdateMutationInput>, ) => { const query = graphql(` mutation updateProject($input: ProjectUpdateMutationInput!) { @@ -108,7 +108,7 @@ export const updateProject = ( `); return terrasoApi - .requestGraphQL(query, updateArgToInput(update)) + .requestGraphQL(query, updateArgToInput('id', update)) .then(resp => collapseProjectFields(resp.updateProject.project!)); }; diff --git a/src/site/siteService.ts b/src/site/siteService.ts index 7de1c16a..0f8a2ba8 100644 --- a/src/site/siteService.ts +++ b/src/site/siteService.ts @@ -122,7 +122,9 @@ export const addSite = (site: SiteAddMutationInput) => { .then(resp => collapseSiteFields(resp.addSite.site)); }; -export const updateSite = (update: UpdateArg) => { +export const updateSite = ( + update: UpdateArg<'id', SiteUpdateMutationInput>, +) => { const query = graphql(` mutation updateSite($input: SiteUpdateMutationInput!) { updateSite(input: $input) { @@ -135,7 +137,7 @@ export const updateSite = (update: UpdateArg) => { `); return terrasoApi - .requestGraphQL(query, updateArgToInput(update)) + .requestGraphQL(query, updateArgToInput('id', update)) .then(resp => collapseSiteFields(resp.updateSite.site!)); }; diff --git a/src/terrasoApi/utils.ts b/src/terrasoApi/utils.ts index 89545a0a..f4b82d3a 100644 --- a/src/terrasoApi/utils.ts +++ b/src/terrasoApi/utils.ts @@ -15,25 +15,29 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -type Resolve = T extends Function ? T : { [K in keyof T]: T[K] }; - -export type UpdateArg = Resolve<{ - id: string; - update: Resolve>; -}>; +export type UpdateArg> = { + [K in ID]: T[K]; +} & { + update: Omit; +}; -export const updateArgToInput = ({ - id, - update, -}: UpdateArg) => { - if ('id' in update) { +export const updateArgToInput = >( + idField: ID, + { [idField]: id, update }: UpdateArg, +) => { + if (idField in update) { throw Error(` update arguments should not contain IDs! are you accidentally passing the entire model object into the update arg instead of just the fields you need to update? `); } - return { input: { ...update, id } }; + return { + input: { + ...({ [idField]: id } as unknown as { [K in ID]: T[K] }), + ...update, + }, + }; }; export const collapseConnectionEdges = (connection: {