Skip to content

Commit

Permalink
Merge pull request #582 from open-formulieren/issue/3536-appointments…
Browse files Browse the repository at this point in the history
…-crash

Fix appointments crash
  • Loading branch information
sergei-maertens authored Nov 2, 2023
2 parents e439a35 + ba7f83f commit fc50278
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,25 @@ describe('Preselecting a product via querystring', () => {
expect(await screen.findByText('Paspoort aanvraag')).toBeVisible();
});
});

describe('Changing the product amounts', () => {
// regression test for https://github.com/open-formulieren/open-forms/issues/3536
it('does not crash when clearing the amount field to enter a value', async () => {
mswServer.use(mockSubmissionPost(buildSubmission({steps: []})), mockAppointmentProductsGet);
const user = userEvent.setup({delay: null});

renderApp();

const productDropdown = await screen.findByRole('combobox');
expect(productDropdown).toBeVisible();

const amountInput = screen.getByLabelText('Amount');
expect(amountInput).toBeVisible();
expect(amountInput).toHaveDisplayValue('1');

// clear the field value to enter a different value
await user.clear(amountInput);
await user.type(amountInput, '3');
expect(amountInput).toHaveDisplayValue('3');
});
});
7 changes: 3 additions & 4 deletions src/components/appointments/fields/Product.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@ import {defineMessage, useIntl} from 'react-intl';
import {NumberField} from 'components/forms';

import {AppointmentConfigContext} from '../Context';
import {ProductsType} from '../types';
import ProductSelect from './ProductSelect';

export const amountLabel = defineMessage({
description: 'Appointments: product amount field label',
defaultMessage: 'Amount',
});

const Product = ({namePrefix, index, selectedProducts}) => {
const Product = ({namePrefix, index, selectedProductIds}) => {
const intl = useIntl();
const {supportsMultipleProducts} = useContext(AppointmentConfigContext);
return (
<div className="openforms-form-field-container">
<ProductSelect
name={`${namePrefix}[${index}].productId`}
selectedProducts={selectedProducts}
selectedProductIds={selectedProductIds}
/>
<NumberField
name={`${namePrefix}[${index}].amount`}
Expand All @@ -38,7 +37,7 @@ const Product = ({namePrefix, index, selectedProducts}) => {
Product.propTypes = {
namePrefix: PropTypes.string.isRequired,
index: PropTypes.number.isRequired,
selectedProducts: ProductsType.isRequired,
selectedProductIds: PropTypes.arrayOf(PropTypes.string).isRequired,
};

export default Product;
8 changes: 4 additions & 4 deletions src/components/appointments/fields/Product.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,17 @@ export default {
index: {table: {disable: true}},
},
args: {
selectedProducts: [],
selectedProductIds: [],
supportsMultipleProducts: true,
},
};

const render = ({productId, selectedProducts, supportsMultipleProducts = true}) => {
const render = ({productId, selectedProductIds, supportsMultipleProducts = true}) => {
const data_entry = PRODUCTS_DATA.find(prod => prod.productId === productId);
const index = supportsMultipleProducts ? PRODUCTS_DATA.indexOf(data_entry) : 0;
return (
<AppointmentConfigContext.Provider value={{supportsMultipleProducts}}>
<Product namePrefix="products" index={index} selectedProducts={selectedProducts} />
<Product namePrefix="products" index={index} selectedProductIds={selectedProductIds} />
</AppointmentConfigContext.Provider>
);
};
Expand Down Expand Up @@ -97,7 +97,7 @@ export const NoMultipleProductsSupport = {
},
argTypes: {
productId: {table: {disable: true}},
selectedProducts: {table: {disable: true}},
selectedProductIds: {table: {disable: true}},
},
play: async ({canvasElement}) => {
const canvas = within(canvasElement);
Expand Down
18 changes: 8 additions & 10 deletions src/components/appointments/fields/ProductSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import {get} from 'api';
import {getCached, setCached} from 'cache';
import {AsyncSelectField} from 'components/forms';

import {ProductsType} from '../types';

const CACHED_PRODUCTS_KEY = 'appointment|all-products';
const CACHED_PRODUCTS_MAX_AGE_MS = 15 * 60 * 1000; // 15 minutes

Expand All @@ -27,10 +25,10 @@ export const getAllProducts = async baseUrl => {
return products;
};

const getProducts = async (baseUrl, selectedProducts, currentProductId) => {
const otherProductIds = selectedProducts
.map(p => p.productId)
.filter(productId => productId && productId !== currentProductId);
const getProducts = async (baseUrl, selectedProductIds, currentProductId) => {
const otherProductIds = selectedProductIds.filter(
productId => productId && productId !== currentProductId
);
if (!otherProductIds.length) {
return await getAllProducts(baseUrl);
}
Expand All @@ -49,15 +47,15 @@ const getProducts = async (baseUrl, selectedProducts, currentProductId) => {
return products;
};

const ProductSelect = ({name, selectedProducts}) => {
const ProductSelect = ({name, selectedProductIds}) => {
const {getFieldProps} = useFormikContext();
const intl = useIntl();
const {baseUrl} = useContext(ConfigContext);
const {value} = getFieldProps(name);
const getOptions = useCallback(
async () => await getProducts(baseUrl, selectedProducts, value),
async () => await getProducts(baseUrl, selectedProductIds, value),
// eslint-disable-next-line react-hooks/exhaustive-deps
[baseUrl, JSON.stringify(selectedProducts), value]
[baseUrl, JSON.stringify(selectedProductIds), value]
);
return (
<AsyncSelectField
Expand All @@ -72,6 +70,6 @@ const ProductSelect = ({name, selectedProducts}) => {
};
ProductSelect.propTypes = {
name: PropTypes.string.isRequired,
selectedProducts: ProductsType.isRequired,
selectedProductIds: PropTypes.arrayOf(PropTypes.string).isRequired,
};
export default ProductSelect;
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default {
},
args: {
name: 'productId',
selectedProducts: [],
selectedProductIds: [],
},
argTypes: {
name: {table: {disable: true}},
Expand Down
8 changes: 6 additions & 2 deletions src/components/appointments/steps/ChooseProductStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const chooseMultiProductSchema = z.object({products: productSchema});
const ChooseProductStepFields = ({values: {products = []}, validateForm}) => {
const intl = useIntl();
const {supportsMultipleProducts} = useContext(AppointmentConfigContext);
products = productSchema.parse(products);
const selectedProductIds = products.map(p => p.productId).filter(Boolean);

/**
* Decorate an arrayHelper callback to invoke the validate function.
Expand Down Expand Up @@ -84,7 +84,11 @@ const ChooseProductStepFields = ({values: {products = []}, validateForm}) => {
numProducts={numProducts}
onRemove={withValidate(() => arrayHelpers.remove(index))}
>
<Product namePrefix="products" index={index} selectedProducts={products} />
<Product
namePrefix="products"
index={index}
selectedProductIds={selectedProductIds}
/>
</ProductWrapper>
))}
</EditGrid>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,6 @@ describe('The product selection step', () => {
await user.click(secondDropdown);
await user.keyboard('[ArrowDown]');
expect(await screen.findByText('Paspoort aanvraag')).toBeVisible();
expect(await screen.queryByText('Not available with drivers license')).not.toBeInTheDocument();
expect(screen.queryByText('Not available with drivers license')).not.toBeInTheDocument();
});
});

0 comments on commit fc50278

Please sign in to comment.