Skip to content
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

Update customer baskets cache when there is a basket mutation #945

Merged
merged 13 commits into from
Jan 31, 2023
Merged

Update customer baskets cache when there is a basket mutation #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cc2ffa1
update customer baskets cache when there is a basket mutation
alexvuong Jan 27, 2023
3e7a8ac
minor fix
alexvuong Jan 27, 2023
25a085a
fixing tests
alexvuong Jan 30, 2023
f778ce9
fixing tests
alexvuong Jan 30, 2023
3322b3d
typo
alexvuong Jan 30, 2023
90777ee
adjust updater type on update
alexvuong Jan 30, 2023
6457139
adjust updater type on update
alexvuong Jan 30, 2023
6497546
rename basket id for test
alexvuong Jan 30, 2023
2389613
Merge branch 'develop' into fix/update-basket-cache-for-mutations
alexvuong Jan 30, 2023
a5d1516
uncomment tests
alexvuong Jan 30, 2023
ada964c
fix type
alexvuong Jan 31, 2023
f82b7c8
linting
alexvuong Jan 31, 2023
acd23db
remove unwanted code
alexvuong Jan 31, 2023
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
45 changes: 34 additions & 11 deletions packages/commerce-sdk-react/src/hooks/ShopperBaskets/mutation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import {
assertUpdateQuery,
DEFAULT_TEST_HOST,
mockMutationEndpoints,
NEW_DATA,
OLD_DATA,
renderHookWithProviders
} from '../../test-utils'
import {
Expand All @@ -23,7 +21,7 @@ import {
useShopperBasketsMutation
} from './mutation'
import {useBasket} from './query'
import {useCustomerBaskets} from '../ShopperCustomers/query'
import {useCustomerBaskets} from '../ShopperCustomers'
import {CacheUpdateMatrixElement} from '../utils'

const CUSTOMER_ID = 'CUSTOMER_ID'
Expand Down Expand Up @@ -113,7 +111,25 @@ const mutationPayloads: MutationPayloads = {
body: {id: '001'}
}
}
const oldCustomerBaskets = {
total: 1,
baskets: [{basketId: BASKET_ID, hello: 'world'}]
}

const newCustomerBaskets = {
total: 1,
baskets: [{basketId: BASKET_ID, hello: 'world_modified'}]
}

const oldBasket = {
basketId: BASKET_ID,
hello: 'world'
}

const newBasket = {
basketId: BASKET_ID,
hello: 'world_modified'
}
const tests = (Object.keys(mutationPayloads) as ShopperBasketsMutationType[]).map(
(mutationName) => {
const payload = mutationPayloads[mutationName]
Expand All @@ -124,7 +140,11 @@ const tests = (Object.keys(mutationPayloads) as ShopperBasketsMutationType[]).ma
{
name: 'success',
assertions: async () => {
mockMutationEndpoints('/checkout/shopper-baskets/')
mockMutationEndpoints(
'/checkout/shopper-baskets/',
{errorResponse: 200},
newBasket
)
mockRelatedQueries()

const {result, waitForValueToChange} = renderHookWithProviders(() => {
Expand All @@ -151,21 +171,24 @@ const tests = (Object.keys(mutationPayloads) as ShopperBasketsMutationType[]).ma

await waitForValueToChange(() => result.current.mutation.isSuccess)
expect(result.current.mutation.isSuccess).toBe(true)

// On successful mutation, the query cache gets updated too. Let's assert it.
const cacheUpdateMatrix = getCacheUpdateMatrix(CUSTOMER_ID)
// @ts-ignore
const matrixElement = cacheUpdateMatrix[mutationName](payload, {})
const {invalidate, update, remove}: CacheUpdateMatrixElement = matrixElement

const assertionData = {
basket: newBasket,
customerBaskets: newCustomerBaskets
}
update?.forEach(({name}) => {
// @ts-ignore
assertUpdateQuery(result.current.queries[name], NEW_DATA)
assertUpdateQuery(result.current.queries[name], assertionData[name])
})

invalidate?.forEach(({name}) => {
// @ts-ignore
assertInvalidateQuery(result.current.queries[name], OLD_DATA)
assertInvalidateQuery(result.current.queries[name], oldCustomerBaskets)
})

remove?.forEach(({name}) => {
Expand Down Expand Up @@ -221,24 +244,24 @@ const mockRelatedQueries = () => {
.get((uri) => {
return uri.includes(basketEndpoint)
})
.reply(200, OLD_DATA)
.reply(200, oldBasket)
nock(DEFAULT_TEST_HOST)
.persist()
.get((uri) => {
return uri.includes(basketEndpoint)
})
.reply(200, NEW_DATA)
.reply(200, newBasket)

// For get customer basket
nock(DEFAULT_TEST_HOST)
.get((uri) => {
return uri.includes(customerEndpoint)
})
.reply(200, OLD_DATA)
.reply(200, oldCustomerBaskets)
nock(DEFAULT_TEST_HOST)
.persist()
.get((uri) => {
return uri.includes(customerEndpoint)
})
.reply(200, NEW_DATA)
.reply(200, newCustomerBaskets)
}
83 changes: 49 additions & 34 deletions packages/commerce-sdk-react/src/hooks/ShopperBaskets/mutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {CacheUpdateMatrixElement, NotImplementedError, updateCache} from '../uti
import useCustomerId from '../useCustomerId'

type Client = ApiClients['shopperBaskets']
type CustomerClient = ApiClients['shopperCustomers']

export const ShopperBasketsMutations = {
/**
Expand Down Expand Up @@ -189,15 +190,46 @@ export type ShopperBasketsMutationType =
* @private
*/
export const getCacheUpdateMatrix = (customerId: string | null) => {
const updateBasketQuery = (basketId?: string) => {
const updateBasketQuery = (
basketId?: string,
response?: DataType<Client[ShopperBasketsMutationType]>
) => {
// TODO: we're missing headers, rawResponse -> not only {basketId}
const arg = {basketId}

return basketId
? {
update: [
{
name: 'basket',
key: ['/baskets', basketId, arg]
key: ['/baskets', basketId, arg],
updater: () => response
},
{
// Since we use baskets from customer basket query, we need to update it for any basket mutation
name: 'customerBaskets',
key: ['/customers', customerId, '/baskets', {customerId}],
updater: (
oldData: NonNullable<DataType<CustomerClient['getCustomerBaskets']>>
) => {
// do not update if responded basket does not exist inside existing customer baskets
if (
!oldData?.baskets?.some(
(basket) => basket.basketId === response?.basketId
)
) {
return undefined
}
const updatedBaskets = oldData.baskets?.map(
(basket: DataType<Client[ShopperBasketsMutationType]>) => {
return basket?.basketId === basketId ? response : basket
}
)
return {
...oldData,
baskets: updatedBaskets
}
}
}
]
}
Expand Down Expand Up @@ -241,8 +273,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
addItemToBasket: (
Expand All @@ -252,8 +283,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
removeItemFromBasket: (
Expand All @@ -263,8 +293,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params?.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
addPaymentInstrumentToBasket: (
Expand All @@ -274,18 +303,15 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
createBasket: (
params: Argument<Client['createBasket']>,
response: DataType<Client['createBasket']>
): CacheUpdateMatrixElement => {
const basketId = response.basketId

return {
...updateBasketQuery(basketId),
// we want to re-fetch basket data in this case to get the basket total and other baskets data
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the Create Basket endpoint return a basket? Could we not just use that data as we do with the other endpoints to updateBasketQuery(basketId, response) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does return a basket, but we want to fetch the basket to get the correct value of total in customer baskets

// customer basket
{
   total: 1
   baskets: []
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok... I can get on board with that. I thing there will be room for improvement/optimization here tho. But we can address that in the next feature update as originally planned. 👍

...invalidateCustomerBasketsQuery(customerId)
}
},
Expand All @@ -304,10 +330,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
params: Argument<Client['mergeBasket']>,
response: DataType<Client['mergeBasket']>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not get lint errors for unused method arguments in TS?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently do not set TS to prompt error for it.

): CacheUpdateMatrixElement => {
const basketId = response.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
}
},
Expand All @@ -318,8 +341,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params?.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
removePaymentInstrumentFromBasket: (
Expand All @@ -329,8 +351,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params?.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateBasket: (
Expand All @@ -340,8 +361,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateBillingAddressForBasket: (
Expand All @@ -351,8 +371,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateCustomerForBasket: (
Expand All @@ -362,8 +381,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateItemInBasket: (
Expand All @@ -373,8 +391,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updatePaymentInstrumentInBasket: (
Expand All @@ -384,8 +401,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateShippingAddressForShipment: (
Expand All @@ -395,8 +411,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
},
updateShippingMethodForShipment: (
Expand All @@ -406,8 +421,7 @@ export const getCacheUpdateMatrix = (customerId: string | null) => {
const basketId = params.parameters.basketId

return {
...updateBasketQuery(basketId),
...invalidateCustomerBasketsQuery(customerId)
...updateBasketQuery(basketId, response)
}
}
}
Expand Down Expand Up @@ -446,6 +460,7 @@ export function useShopperBasketsMutation<Action extends ShopperBasketsMutationT
}
const queryClient = useQueryClient()
const customerId = useCustomerId()

const cacheUpdateMatrix = getCacheUpdateMatrix(customerId)

return useMutation<Data, Error, Params>(
Expand Down
2 changes: 1 addition & 1 deletion packages/commerce-sdk-react/src/hooks/ShopperCustomers/mutation.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ const tests = (Object.keys(mutationPayloads) as ShopperCustomersMutationType[]).
]

queryKeys.forEach(({key: queryKey}: QueryKeyMap) => {
queryClient.setQueryData(queryKey, {test: true})
queryClient.setQueryData(queryKey, () => ({test: true}))
})

const button = screen.getByRole('button', {
Expand Down
Loading