-
Notifications
You must be signed in to change notification settings - Fork 149
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.
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
Fix ecommerce-app-base bug in drag-n-drop refetch logic #7944
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
import React from 'react'; | ||
import { render, cleanup, waitFor } from '@testing-library/react'; | ||
import { SortableComponent } from './SortableComponent'; | ||
import { makeSdkMock, productsList } from '../__mocks__'; | ||
import { FieldAppSDK } from '@contentful/app-sdk'; | ||
import { ProductPreviewsFn } from '../types'; | ||
|
||
const mockSdk = makeSdkMock(); | ||
const skus = ['M0E20130820E90Z', 'A0E2300FX102203', 'M0E21300900DZN7']; | ||
const mockConfig = {}; | ||
const mockSkuType = 'mock-sku-type'; | ||
|
||
describe('SortableComponent', () => { | ||
let mockFetchProductPreviews: ProductPreviewsFn; | ||
|
||
beforeEach(() => { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore - typescript is upset because jest.fn() returns a type different than ProductPreviewsFn | ||
mockFetchProductPreviews = jest.fn().mockImplementation(() => { | ||
return Promise.resolve(productsList); | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
cleanup(); | ||
jest.resetAllMocks(); | ||
}); | ||
|
||
it('calls `fetchProductPreviews()` to retrieve list of products for associated skus', async () => { | ||
waitFor(() => { | ||
render( | ||
<SortableComponent | ||
sdk={mockSdk as unknown as FieldAppSDK} | ||
disabled={false} | ||
config={mockConfig} | ||
skus={skus} | ||
onChange={jest.fn()} | ||
fetchProductPreviews={mockFetchProductPreviews} | ||
skuType={mockSkuType} | ||
/> | ||
); | ||
}); | ||
|
||
expect(mockFetchProductPreviews).toHaveBeenCalledTimes(1); | ||
expect(mockFetchProductPreviews).toHaveBeenLastCalledWith(skus, mockConfig, mockSkuType); | ||
}); | ||
|
||
it('refetches list of products for associated skus, when skus are added/removed', async () => { | ||
const newSkus = [...skus, 'new-sku-item']; | ||
|
||
await waitFor(() => { | ||
// initial mount with original skus | ||
const { rerender } = render( | ||
<SortableComponent | ||
sdk={mockSdk as unknown as FieldAppSDK} | ||
disabled={false} | ||
config={mockConfig} | ||
skus={skus} | ||
onChange={jest.fn()} | ||
fetchProductPreviews={mockFetchProductPreviews} | ||
skuType={mockSkuType} | ||
/> | ||
); | ||
|
||
// rerender with additional sku | ||
rerender( | ||
<SortableComponent | ||
skus={newSkus} | ||
sdk={mockSdk as unknown as FieldAppSDK} | ||
disabled={false} | ||
config={mockConfig} | ||
onChange={jest.fn()} | ||
fetchProductPreviews={mockFetchProductPreviews} | ||
skuType={mockSkuType} | ||
/> | ||
); | ||
}); | ||
|
||
expect(mockFetchProductPreviews).toHaveBeenCalledTimes(2); | ||
expect(mockFetchProductPreviews).toHaveBeenLastCalledWith(newSkus, mockConfig, mockSkuType); | ||
}); | ||
|
||
it('does NOT refetch list of products when skus have simply been reordered', async () => { | ||
const reorderedSkus = [skus[1], skus[0], skus[2]]; | ||
|
||
await waitFor(() => { | ||
// initial mount with original skus | ||
const { rerender } = render( | ||
<SortableComponent | ||
sdk={mockSdk as unknown as FieldAppSDK} | ||
disabled={false} | ||
config={mockConfig} | ||
skus={skus} | ||
onChange={jest.fn()} | ||
fetchProductPreviews={mockFetchProductPreviews} | ||
skuType={mockSkuType} | ||
/> | ||
); | ||
|
||
// rerender with new skus, just reordered | ||
rerender( | ||
<SortableComponent | ||
skus={reorderedSkus} | ||
sdk={mockSdk as unknown as FieldAppSDK} | ||
disabled={false} | ||
config={mockConfig} | ||
onChange={jest.fn()} | ||
fetchProductPreviews={mockFetchProductPreviews} | ||
skuType={mockSkuType} | ||
/> | ||
); | ||
|
||
expect(mockFetchProductPreviews).toHaveBeenCalledTimes(1); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,15 @@ interface Props { | |
skuType?: string; | ||
} | ||
|
||
/** | ||
* @description - helper function to compare two lists of skus for order agnostic equality | ||
*/ | ||
const skusAreEqual = (prevSkus: string[], currentSkus: string[]): boolean => { | ||
const sortedPreviousSkus = [...prevSkus].sort(); | ||
const sortedCurrentSkus = [...currentSkus].sort(); | ||
return !isEqual(sortedPreviousSkus, sortedCurrentSkus); | ||
}; | ||
|
||
/** | ||
* @description - hook to track previous version of skus prop | ||
* @param skus - list of most current skus | ||
|
@@ -57,28 +66,31 @@ export const SortableComponent: FC<Props> = ({ | |
const [productPreviews, setProductPreviews] = useState<Product[]>([]); | ||
const previousSkus = usePreviousSkus(skus); | ||
|
||
const getProductPreviews = useCallback( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed This has the added benefit of removing the |
||
async (shouldRefetch: boolean = true) => { | ||
try { | ||
const productPreviewsUnsorted = shouldRefetch | ||
? await fetchProductPreviews(skus, config, skuType) | ||
: productPreviews; | ||
const getProductPreviews = useCallback(async () => { | ||
try { | ||
const shouldRefetch = skusAreEqual([...previousSkus], [...skus]); | ||
|
||
if (shouldRefetch) { | ||
const productPreviewsUnsorted = await fetchProductPreviews(skus, config, skuType); | ||
const sortedProductPreviews = mapSort(productPreviewsUnsorted, skus, 'sku'); | ||
setProductPreviews(sortedProductPreviews); | ||
} catch (error) { | ||
sdk.notifier.error('There was an error fetching the data for the selected products.'); | ||
} | ||
}, | ||
[skus, skuType, config, fetchProductPreviews, setProductPreviews, sdk.notifier, productPreviews] | ||
); | ||
} catch (error) { | ||
sdk.notifier.error('There was an error fetching the data for the selected products.'); | ||
} | ||
}, [skus, skuType, config, fetchProductPreviews, setProductPreviews, sdk.notifier, previousSkus]); | ||
|
||
/** | ||
* @description - Compare previous list of skus (see `usePreviousSkus`) to the list of skus | ||
* passed as a prop. If the previous & current skus differ (a sku was added/removed, order agnostic), | ||
* then fetch/refetch associated productPreviews given list of skus. | ||
*/ | ||
useEffect(() => { | ||
// only refetch product previews if new skus do not match previous skus | ||
const sortedPreviousSkus = previousSkus.sort(); | ||
const sortedCurrentSkus = skus.sort(); | ||
const shouldRefetch = !isEqual(sortedPreviousSkus, sortedCurrentSkus); | ||
const shouldRefetch = skusAreEqual([...previousSkus], [...skus]); | ||
|
||
getProductPreviews(shouldRefetch); | ||
if (shouldRefetch) { | ||
getProductPreviews(); | ||
} | ||
}, [skus, skuType, config, getProductPreviews, previousSkus]); | ||
|
||
const deleteItem = useCallback( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
export { products, productsList } from './products'; | ||
export { makeSdkMock } from './mockSdk'; | ||
export { mockContentTypes } from './mockContentTypes'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
export const mockContentTypes = [ | ||
{ | ||
sys: { id: 'ct1' }, | ||
name: 'CT1', | ||
fields: [ | ||
{ id: 'product_x', name: 'Product X', type: 'Symbol' }, | ||
{ id: 'y', name: 'Y', type: 'Object' }, | ||
], | ||
}, | ||
{ | ||
sys: { id: 'ct2' }, | ||
name: 'CT2', | ||
fields: [ | ||
{ id: 'foo', name: 'FOO', type: 'Text' }, | ||
{ id: 'z', name: 'Z', type: 'Array', items: { type: 'Symbol' } }, | ||
], | ||
}, | ||
{ | ||
sys: { id: 'ct3' }, | ||
name: 'CT3', | ||
fields: [ | ||
{ id: 'bar', name: 'BAR', type: 'Object' }, | ||
{ id: 'baz', name: 'BAZ', type: 'Object' }, | ||
{ id: 'product_d', name: 'Product D', type: 'Array', items: { type: 'Symbol' } }, | ||
{ id: 'product_a', name: 'Product A', type: 'Symbol' }, | ||
], | ||
}, | ||
]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { mockContentTypes } from './mockContentTypes'; | ||
|
||
export const makeSdkMock = () => ({ | ||
ids: { | ||
app: 'some-app', | ||
}, | ||
hostnames: { | ||
webapp: 'app.contentful.com', | ||
}, | ||
space: { | ||
getContentTypes: jest.fn().mockResolvedValue({ items: mockContentTypes }), | ||
getEditorInterfaces: jest.fn().mockResolvedValue({ items: [] }), | ||
}, | ||
app: { | ||
setReady: jest.fn(), | ||
getParameters: jest.fn().mockResolvedValue(null), | ||
onConfigure: jest.fn().mockReturnValue(undefined), | ||
}, | ||
notifier: { | ||
error: (msg: string) => console.log(`[mockSdk] error: ${msg}`), | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied/pasted this code out of
AppConfig
into__mocks__
dir so that other test files can re-use this functionality.