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.

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

Modify dialogue error messages when editing a catalogue item that has child elements #1136 #1165

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 18 additions & 1 deletion cypress/e2e/with_mock_data/catalogueItems.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -581,15 +581,32 @@ describe('Catalogue Items', () => {

cy.findByLabelText('Name *').clear();
cy.findByLabelText('Name *').type('test_has_children_elements');

cy.findByLabelText('Manufacturer *').type('Man{downArrow}{enter}');
cy.findByRole('button', { name: 'Next' }).click();

cy.findByRole('button', { name: 'Finish'}).click();
cy.findByRole('dialog')
.should('be.visible')
.within(() => {
cy.contains('Catalogue item has child elements, so you cannot update the properties, '
+ 'and the manufacturer cannot be changed')
});
Comment on lines +588 to +594
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

cy.findByRole('button', { name: 'Finish' }).should('be.disabled');

cy.findByRole('button', { name: 'Back' }).click();
cy.findByLabelText('Manufacturer *').type('Man{upArrow}{enter}');
cy.findByRole('button', { name: 'Next' }).click();
cy.findByRole('button', { name: 'Finish' }).should('be.enabled');

cy.findByLabelText('Measurement Range (Joules) *').type('0');

cy.findByRole('button', { name: 'Finish' }).click();
cy.findByRole('dialog')
.should('be.visible')
.within(() => {
cy.contains('Catalogue item has child elements and cannot be edited');
cy.contains('Catalogue item has child elements, so you cannot update the properties, '
+ 'and the manufacturer cannot be changed')
});
cy.findByRole('button', { name: 'Finish' }).should('be.disabled');
});
Expand Down
50 changes: 48 additions & 2 deletions src/catalogue/items/catalogueItemsDialog.component.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,39 @@ describe('Catalogue Items Dialog', () => {
});
});

it('displays error message if catalogue item has children elements', async () => {
it('displays error message when editing manufacturer_id if catalogue item has child elements', async () => {
props = {
...props,
parentInfo: getCatalogueCategoryById('4'),
selectedCatalogueItem: getCatalogueItemById('1'),
};

createView();

await modifyValues({
name: 'test_has_children_elements',
manufacturer: 'Man{arrowdown}{arrowdown}{enter}',
});

await user.click(screen.getByRole('button', { name: 'Next' }));
await user.click(screen.getByRole('button', { name: 'Finish' }));

expect(axiosPatchSpy).toHaveBeenCalledWith('/v1/catalogue-items/1', {
name: 'test_has_children_elements',
manufacturer_id: '3',
});

await waitFor(() => {
expect(
screen.getByText(
'Catalogue item has child elements, so you cannot update the properties, '
+ 'and the manufacturer cannot be changed'
)
).toBeInTheDocument();
Comment on lines +1111 to +1115
Copy link
Collaborator

Choose a reason for hiding this comment

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

The error message is missing the name of the manufacturer

});
});

it('displays error message when editing properties if catalogue item has child elements', async () => {
props = {
...props,
parentInfo: getCatalogueCategoryById('4'),
Expand All @@ -1098,16 +1130,30 @@ describe('Catalogue Items Dialog', () => {
});

await user.click(screen.getByRole('button', { name: 'Next' }));

await modifyValues({
resolution: '24',
});

await user.click(screen.getByRole('button', { name: 'Finish' }));

expect(axiosPatchSpy).toHaveBeenCalledWith('/v1/catalogue-items/1', {
name: 'test_has_children_elements',
properties: [
{ id: '1', value: 24 },
{ id: '2', value: 30 },
{ id: '3', value: 'CMOS' },
{ id: '4', value: null },
{ id: '5', value: true },
{ id: '6', value: false },
],
});

await waitFor(() => {
expect(
screen.getByText(
'Catalogue item has child elements and cannot be edited'
'Catalogue item has child elements, so you cannot update the properties, '
+ 'and the manufacturer cannot be changed'
)
).toBeInTheDocument();
});
Expand Down
13 changes: 12 additions & 1 deletion src/catalogue/items/catalogueItemsDialog.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,18 @@ function CatalogueItemsDialog(props: CatalogueItemsDialogProps) {

if (response && error.response?.status === 409) {
if (response.detail.includes('child elements')) {
// find the name of the manufacturer, so it can be used in the error message
const manufacturerName = manufacturerList?.find(
(manufacturer) => manufacturer.id === selectedCatalogueItem.manufacturer_id
) || null
// add the manufacturer name into the error message
const message = response.detail.replace(
"so the following fields cannot be updated: manufacturer_id, properties",
("so you cannot update the properties, and the manufacturer cannot be changed from "
+ manufacturerName?.name)
)
Comment on lines +394 to +398
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of replacing the text directly in the backend response, consider moving this string transformation entirely to the frontend. This approach will ensure consistency in the response formatting, prevent future inconsistencies, and make it easier to manage localization or text changes.

setErrorPropertiesStep('root.formError', {
message: response.detail,
message: message,
});
}
return;
Expand All @@ -407,6 +417,7 @@ function CatalogueItemsDialog(props: CatalogueItemsDialogProps) {
patchCatalogueItem,
handleClose,
setErrorPropertiesStep,
manufacturerList,
]
);

Expand Down
3 changes: 2 additions & 1 deletion src/mocks/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ export const handlers = [
if (body.name === 'test_has_children_elements') {
return HttpResponse.json(
{
detail: 'Catalogue item has child elements and cannot be edited',
detail: 'Catalogue item has child elements, so you cannot update the properties, '
+ 'and the manufacturer cannot be changed'
},
{ status: 409 }
);
Expand Down
Loading