From 664c59c15669c6170ab8985f2cb06f65454b9e91 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Thu, 2 Nov 2023 15:27:13 +0000 Subject: [PATCH 01/16] made address field changes and updated tests accordingly #117 --- cypress/e2e/manufacturer/manufacturer.cy.ts | 50 +++--- src/app.types.tsx | 7 +- .../DeleteManufacturerDialog.test.tsx | 4 +- src/manufacturer/manufacturer.component.tsx | 8 +- .../manufacturerDialog.component.test.tsx | 148 ++++++++---------- .../manufacturerDialog.component.tsx | 144 ++++++++--------- src/mocks/handlers.ts | 8 +- src/mocks/manufacturer.json | 18 +-- 8 files changed, 184 insertions(+), 203 deletions(-) diff --git a/cypress/e2e/manufacturer/manufacturer.cy.ts b/cypress/e2e/manufacturer/manufacturer.cy.ts index 704b454e6..b715cc63f 100644 --- a/cypress/e2e/manufacturer/manufacturer.cy.ts +++ b/cypress/e2e/manufacturer/manufacturer.cy.ts @@ -21,15 +21,15 @@ describe('Manufacturer', () => { cy.findByText('http://example.com').should('be.visible'); cy.findByText('http://test.com').should('be.visible'); cy.findByText('http://test.co.uk').should('be.visible'); - cy.findByText('1 Example Street Oxford Oxfordshire OX1 2AB').should( - 'be.visible' - ); - cy.findByText('2 Example Street Oxford Oxfordshire OX1 2AB').should( - 'be.visible' - ); - cy.findByText('3 Example Street Oxford Oxfordshire OX1 2AB').should( - 'be.visible' - ); + cy.findByText( + 'United Kingdom 1 Example Street Oxford Oxfordshire OX1 2AB' + ).should('be.visible'); + cy.findByText( + 'United Kingdom 2 Example Street Oxford Oxfordshire OX1 2AB' + ).should('be.visible'); + cy.findByText( + 'United Kingdom 3 Example Street Oxford Oxfordshire OX1 2AB' + ).should('be.visible'); }); it('manufacturer url is correct and opens new webpage', () => { @@ -48,8 +48,8 @@ describe('Manufacturer', () => { cy.findByRole('button', { name: 'Add Manufacturer' }).click(); cy.findByLabelText('Name *').type('Manufacturer D'); cy.findByLabelText('URL').type('http://test.co.uk'); - cy.findByLabelText('Building number *').type('1'); - cy.findByLabelText('Street name *').type('Example Street'); + cy.findByLabelText('Country *').type('United Kingdom'); + cy.findByLabelText('Address Line *').type('4 Example Street'); cy.findByLabelText('Town').type('Oxford'); cy.findByLabelText('County').type('Oxfordshire'); cy.findByLabelText('Post/Zip code *').type('OX1 2AB'); @@ -66,7 +66,7 @@ describe('Manufacturer', () => { expect(patchRequests.length).equal(1); const request = patchRequests[0]; expect(JSON.stringify(request.body)).equal( - '{"name":"Manufacturer D","url":"http://test.co.uk","address":{"building_number":"1","street_name":"Example Street","town":"Oxford","county":"Oxfordshire","postcode":"OX1 2AB"},"telephone":"07349612203"}' + '{"name":"Manufacturer D","url":"http://test.co.uk","address":{"address_line":"4 Example Street","town":"Oxford","county":"Oxfordshire","postcode":"OX1 2AB","country":"United Kingdom"},"telephone":"07349612203"}' ); }); }); @@ -82,12 +82,12 @@ describe('Manufacturer', () => { cy.findByRole('dialog') .should('be.visible') .within(() => { - cy.contains('Please enter a building number.'); + cy.contains('Please enter a country.'); }); cy.findByRole('dialog') .should('be.visible') .within(() => { - cy.contains('Please enter a street name.'); + cy.contains('Please enter an address.'); }); cy.findByRole('dialog') .should('be.visible') @@ -99,8 +99,8 @@ describe('Manufacturer', () => { it('displays error message when duplicate name entered', () => { cy.findByRole('button', { name: 'Add Manufacturer' }).click(); cy.findByLabelText('Name *').type('Manufacturer A'); - cy.findByLabelText('Building number *').type('1'); - cy.findByLabelText('Street name *').type('Example Street'); + cy.findByLabelText('Country *').type('United Kingdom'); + cy.findByLabelText('Address Line *').type('4 Example Street'); cy.findByLabelText('Post/Zip code *').type('OX1 2AB'); cy.findByRole('button', { name: 'Save' }).click(); @@ -167,11 +167,11 @@ describe('Manufacturer', () => { cy.findByLabelText('Name').clear(); cy.findByLabelText('Name').type('test'); - cy.findByLabelText('Building number').clear(); - cy.findByLabelText('Building number').type('100'); + cy.findByLabelText('Country').clear(); + cy.findByLabelText('Country').type('test'); - cy.findByLabelText('Street name').clear(); - cy.findByLabelText('Street name').type('test'); + cy.findByLabelText('Address Line').clear(); + cy.findByLabelText('Address Line').type('test'); cy.findByLabelText('Town').clear(); cy.findByLabelText('Town').type('test'); @@ -196,7 +196,7 @@ describe('Manufacturer', () => { expect(patchRequests.length).equal(1); const request = patchRequests[0]; expect(JSON.stringify(request.body)).equal( - '{"name":"test","address":{"building_number":"100","street_name":"test","town":"test","county":"test","postcode":"test"},"telephone":"0000000000"}' + '{"name":"test","address":{"address_line":"test","town":"test","county":"test","postcode":"test","country":"test"},"telephone":"0000000000"}' ); }); }); @@ -259,8 +259,8 @@ describe('Manufacturer', () => { }).click(); cy.findByLabelText('Name').clear(); - cy.findByLabelText('Building number').clear(); - cy.findByLabelText('Street name').clear(); + cy.findByLabelText('Country').clear(); + cy.findByLabelText('Address Line').clear(); cy.findByLabelText('Post/Zip code').clear(); cy.findByRole('button', { name: 'Save' }).click(); @@ -272,12 +272,12 @@ describe('Manufacturer', () => { cy.findByRole('dialog') .should('be.visible') .within(() => { - cy.contains('Please enter a building number.'); + cy.contains('Please enter a country.'); }); cy.findByRole('dialog') .should('be.visible') .within(() => { - cy.contains('Please enter a street name.'); + cy.contains('Please enter an address.'); }); cy.findByRole('dialog') .should('be.visible') diff --git a/src/app.types.tsx b/src/app.types.tsx index e45915f38..bf8aa93ba 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -132,19 +132,20 @@ export interface ErrorParsing { } interface Address { - building_number: string; - street_name: string; + address_line: string; town?: string; county?: string; postcode: string; + country: string; } interface EditAddress { - building_number?: string; + address_line?: string; street_name?: string; town?: string; county?: string; postcode?: string; + country?: string; } export enum SystemImportanceType { diff --git a/src/manufacturer/DeleteManufacturerDialog.test.tsx b/src/manufacturer/DeleteManufacturerDialog.test.tsx index e0a23d5fa..e8c5efca4 100644 --- a/src/manufacturer/DeleteManufacturerDialog.test.tsx +++ b/src/manufacturer/DeleteManufacturerDialog.test.tsx @@ -21,11 +21,11 @@ describe('Delete Manufacturer Dialog', () => { name: 'test', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '056896598', id: '1', diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index bf6e55697..98de3c6e6 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -25,11 +25,11 @@ function Manufacturer() { name: '', url: undefined, address: { - building_number: '', - street_name: '', + address_line: '', town: '', county: '', postcode: '', + country: '', }, telephone: '', }); @@ -208,9 +208,9 @@ function Manufacturer() { borderRight: '1px solid #e0e0e0', }} > - {item.address.building_number + + {item.address.country + ' \n' + - item.address.street_name + + item.address.address_line + ' \n' + item.address.town + ' \n' + diff --git a/src/manufacturer/manufacturerDialog.component.test.tsx b/src/manufacturer/manufacturerDialog.component.test.tsx index 8e06b4ca2..4bd89609d 100644 --- a/src/manufacturer/manufacturerDialog.component.test.tsx +++ b/src/manufacturer/manufacturerDialog.component.test.tsx @@ -25,11 +25,11 @@ describe('Add manufacturer dialog', () => { name: '', url: undefined, address: { - building_number: '', - street_name: '', + address_line: '', town: '', county: '', postcode: '', + country: '', }, telephone: '', }, @@ -46,8 +46,8 @@ describe('Add manufacturer dialog', () => { createView(); expect(screen.getByLabelText('Name *')).toBeInTheDocument(); expect(screen.getByLabelText('URL')).toBeInTheDocument(); - expect(screen.getByLabelText('Building number *')).toBeInTheDocument(); - expect(screen.getByLabelText('Street name *')).toBeInTheDocument(); + expect(screen.getByLabelText('Country *')).toBeInTheDocument(); + expect(screen.getByLabelText('Address Line *')).toBeInTheDocument(); expect(screen.getByLabelText('Town')).toBeInTheDocument(); expect(screen.getByLabelText('County')).toBeInTheDocument(); expect(screen.getByLabelText('Post/Zip code *')).toBeInTheDocument(); @@ -57,14 +57,13 @@ describe('Add manufacturer dialog', () => { it('adds manufacturer correctly', async () => { props.manufacturer = { name: 'Manufacturer D', - url: 'http://test.co.uk', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '4 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07349612203', }; @@ -74,16 +73,16 @@ describe('Add manufacturer dialog', () => { await user.click(saveButton); expect(axiosPostSpy).toHaveBeenCalledWith('/v1/manufacturers', { + name: 'Manufacturer D', + url: 'http://test.co.uk', address: { - building_number: '1', + address_line: '4 Example Street', + town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', - street_name: 'Example Street', - town: 'Oxford', + country: 'United Kingdom', }, - name: 'Manufacturer D', telephone: '07349612203', - url: 'http://test.co.uk', }); expect(onClose).toHaveBeenCalled(); @@ -102,14 +101,13 @@ describe('Add manufacturer dialog', () => { it('duplicate manufacturer name displays warning message', async () => { props.manufacturer = { name: 'Manufacturer A', - url: 'http://test.co.uk', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '4 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07349612203', }; @@ -134,10 +132,8 @@ describe('Add manufacturer dialog', () => { await user.click(saveButton); expect(screen.getByText('Please enter a name.')).toBeInTheDocument(); - expect( - screen.getByText('Please enter a building number.') - ).toBeInTheDocument(); - expect(screen.getByText('Please enter a street name.')).toBeInTheDocument(); + expect(screen.getByText('Please enter a country.')).toBeInTheDocument(); + expect(screen.getByText('Please enter an address.')).toBeInTheDocument(); expect( screen.getByText('Please enter a post code or zip code.') ).toBeInTheDocument(); @@ -146,15 +142,14 @@ describe('Add manufacturer dialog', () => { it('invalid url displays error', async () => { props.manufacturer = { - name: 'Manufacturer A', - + name: 'Manufacturer D', url: 'invalid', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '4 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07349612203', }; @@ -202,43 +197,43 @@ describe('Add manufacturer dialog', () => { }); }); - it('handles manufacturer building number input correctly', async () => { - const newManufacturerBuildingNumber = 'Test'; + it('handles manufacturer country input correctly', async () => { + const newManufacturerCountry = 'Test'; createView(); - const manufacturerBuildingNumberInput = - screen.getByLabelText('Building number *'); + const manufacturerCountryInput = screen.getByLabelText('Country *'); - fireEvent.change(manufacturerBuildingNumberInput, { - target: { value: newManufacturerBuildingNumber }, + fireEvent.change(manufacturerCountryInput, { + target: { value: newManufacturerCountry }, }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ ...props.manufacturer, address: { ...props.manufacturer.address, - building_number: newManufacturerBuildingNumber, + country: newManufacturerCountry, }, }); }); - it('handles manufacturer street name input correctly', async () => { - const newManufacturerStreetName = 'Test'; + it('handles manufacturer address line input correctly', async () => { + const newManufacturerAddressLine = 'Test'; createView(); - const manufacturerStreetNameInput = screen.getByLabelText('Street name *'); + const manufacturerAddressLineInput = + screen.getByLabelText('Address Line *'); - fireEvent.change(manufacturerStreetNameInput, { - target: { value: newManufacturerStreetName }, + fireEvent.change(manufacturerAddressLineInput, { + target: { value: newManufacturerAddressLine }, }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ ...props.manufacturer, address: { ...props.manufacturer.address, - street_name: newManufacturerStreetName, + address_line: newManufacturerAddressLine, }, }); }); @@ -337,11 +332,11 @@ describe('Add manufacturer dialog', () => { name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -350,11 +345,11 @@ describe('Add manufacturer dialog', () => { props.manufacturer = { name: 'test', address: { - building_number: '100', - street_name: 'test', + address_line: 'test', town: 'test', county: 'test', postcode: 'test', + country: 'test', }, telephone: '0000000000', }; @@ -368,11 +363,11 @@ describe('Add manufacturer dialog', () => { expect(axiosPatchSpy).toHaveBeenCalledWith('/v1/manufacturers/1', { name: 'test', address: { - building_number: '100', - street_name: 'test', + address_line: 'test', town: 'test', county: 'test', postcode: 'test', + country: 'test', }, telephone: '0000000000', }); @@ -388,11 +383,11 @@ describe('Add manufacturer dialog', () => { name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -400,11 +395,11 @@ describe('Add manufacturer dialog', () => { name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -430,11 +425,11 @@ describe('Add manufacturer dialog', () => { name: 'test', url: 'invalid', address: { - building_number: '100', - street_name: 'test', + address_line: 'test', town: 'test', county: 'test', postcode: 'test', + country: 'test', }, telephone: '0000000000', }, @@ -443,11 +438,11 @@ describe('Add manufacturer dialog', () => { name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -468,11 +463,11 @@ describe('Add manufacturer dialog', () => { manufacturer: { name: 'test_dup', address: { - building_number: '100', - street_name: 'test', + address_line: 'test', town: 'test', county: 'test', postcode: 'test', + country: 'test', }, telephone: '0000000000', }, @@ -481,11 +476,11 @@ describe('Add manufacturer dialog', () => { name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -510,24 +505,24 @@ describe('Add manufacturer dialog', () => { manufacturer: { name: '', address: { - building_number: '', - street_name: '', - town: 'test', - county: 'test', + address_line: '', + town: '', + county: '', postcode: '', + country: '', }, - telephone: '0000000000', + telephone: '', }, selectedManufacturer: { id: '1', name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -540,12 +535,8 @@ describe('Add manufacturer dialog', () => { await user.click(saveButton); expect(screen.getByText('Please enter a name.')).toBeInTheDocument(); - expect( - screen.getByText('Please enter a building number.') - ).toBeInTheDocument(); - expect( - screen.getByText('Please enter a street name.') - ).toBeInTheDocument(); + expect(screen.getByText('Please enter a country.')).toBeInTheDocument(); + expect(screen.getByText('Please enter an address.')).toBeInTheDocument(); expect( screen.getByText('Please enter a post code or zip code.') ).toBeInTheDocument(); @@ -596,43 +587,42 @@ describe('Add manufacturer dialog', () => { }); }); - it('handles manufacturer building number input correctly', async () => { - const newManufacturerBuildingNumber = 'Test'; + it('handles manufacturer country input correctly', async () => { + const newManufacturerCountry = 'Test'; createView(); - const manufacturerBuildingNumberInput = - screen.getByLabelText('Building number'); + const manufacturerCountryInput = screen.getByLabelText('Country'); - fireEvent.change(manufacturerBuildingNumberInput, { - target: { value: newManufacturerBuildingNumber }, + fireEvent.change(manufacturerCountryInput, { + target: { value: newManufacturerCountry }, }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ ...props.manufacturer, address: { ...props.manufacturer.address, - building_number: newManufacturerBuildingNumber, + country: newManufacturerCountry, }, }); }); - it('handles manufacturer street name input correctly', async () => { - const newManufacturerStreetName = 'Test'; + it('handles manufacturer address line input correctly', async () => { + const newManufacturerAddressLine = 'Test'; createView(); - const manufacturerStreetNameInput = screen.getByLabelText('Street name'); + const manufacturerStreetNameInput = screen.getByLabelText('Address Line'); fireEvent.change(manufacturerStreetNameInput, { - target: { value: newManufacturerStreetName }, + target: { value: newManufacturerAddressLine }, }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ ...props.manufacturer, address: { ...props.manufacturer.address, - street_name: newManufacturerStreetName, + address_line: newManufacturerAddressLine, }, }); }); diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index aea9ce2a2..12b78158c 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -64,19 +64,17 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const [URLErrorMessage, setURLErrorMessage] = React.useState< string | undefined >(undefined); - const [addressBuildingNumberError, setAddressBuildingNumberError] = - React.useState(false); - const [ - addressBuildingNumberErrorMessage, - setAddressBuildingNumberErrorMessage, - ] = React.useState(undefined); - const [addressStreetNameError, setAddressStreetNameError] = - React.useState(false); - const [addressStreetNameErrorMessage, setaddressStreetNameErrorMessage] = - React.useState(undefined); + const [addressLineError, setAddressLineError] = React.useState(false); + const [addressLineErrorMessage, setAddressLineErrorMessage] = React.useState< + string | undefined + >(undefined); const [addresspostcodeError, setAddresspostcodeError] = React.useState(false); const [AddresspostcodeErrorMessage, setAddresspostcodeErrorMessage] = React.useState(undefined); + const [countryError, setCountryError] = React.useState(false); + const [countryErrorMessage, setCountryErrorMessage] = React.useState< + string | undefined + >(undefined); const [formError, setFormError] = React.useState(false); const [formErrorMessage, setFormErrorMessage] = React.useState< @@ -94,11 +92,11 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { name: '', url: undefined, address: { - building_number: '', - street_name: '', + address_line: '', town: '', county: '', postcode: '', + country: '', }, telephone: '', }); @@ -106,10 +104,10 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { setNameErrorMessage(undefined); setURLError(false); setURLErrorMessage(undefined); - setAddressBuildingNumberError(false); - setAddressBuildingNumberErrorMessage(undefined); - setAddressStreetNameError(false); - setaddressStreetNameErrorMessage(undefined); + setAddressLineError(false); + setAddressLineErrorMessage(undefined); + setCountryError(false); + setCountryErrorMessage(undefined); setAddresspostcodeError(false); setAddresspostcodeErrorMessage(undefined); setFormError(false); @@ -135,24 +133,16 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { setNameError(true); setNameErrorMessage('Please enter a name.'); } - //check building number + //check address line if ( - !manufacturer.address?.building_number || - manufacturer.address.building_number.trim().length === 0 + !manufacturer.address?.address_line || + manufacturer.address.address_line.trim().length === 0 ) { hasErrors = true; - setAddressBuildingNumberError(true); - setAddressBuildingNumberErrorMessage('Please enter a building number.'); - } - //check street name - if ( - !manufacturer.address?.street_name || - manufacturer.address.street_name?.trim().length === 0 - ) { - hasErrors = true; - setAddressStreetNameError(true); - setaddressStreetNameErrorMessage('Please enter a street name.'); + setAddressLineError(true); + setAddressLineErrorMessage('Please enter an address.'); } + //check post code if ( !manufacturer.address?.postcode || @@ -162,15 +152,18 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { setAddresspostcodeError(true); setAddresspostcodeErrorMessage('Please enter a post code or zip code.'); } + //check country + if ( + !manufacturer.address?.country || + manufacturer.address.country?.trim().length === 0 + ) { + hasErrors = true; + setCountryError(true); + setCountryErrorMessage('Please enter a country.'); + } return hasErrors; - }, [ - manufacturer.address.building_number, - manufacturer.address.postcode, - manufacturer.address.street_name, - manufacturer.name, - manufacturer.url, - ]); + }, [manufacturer]); const handleAddManufacturer = React.useCallback(() => { const hasErrors = handleErrors(); @@ -183,11 +176,11 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { name: manufacturer.name, url: manufacturer.url, address: { - building_number: manufacturer.address.building_number, - street_name: manufacturer.address.street_name, + address_line: manufacturer.address.address_line, town: manufacturer.address.town, county: manufacturer.address.county, postcode: manufacturer.address.postcode, + country: manufacturer.address.country, }, telephone: manufacturer.telephone, }; @@ -218,19 +211,18 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const isURLUpdated = manufacturer.url !== selectedManufacturer.url && manufacturer.url !== undefined; - const isBuildingNumberUpdated = - manufacturer.address?.building_number !== - selectedManufacturer.address.building_number; - const isStreetNameUpdated = - manufacturer.address?.street_name !== - selectedManufacturer.address.street_name; + const isAddressLineUpdated = + manufacturer.address?.address_line !== + selectedManufacturer.address.address_line; const isTownUpdated = manufacturer.address?.town !== selectedManufacturer.address.town; const isCountyUpdated = manufacturer.address?.county !== selectedManufacturer.address.county; - const ispostcodeUpdated = + const isPostcodeUpdated = manufacturer.address?.postcode !== selectedManufacturer.address.postcode; + const isCountryUpdated = + manufacturer.address?.country !== selectedManufacturer.address.country; const isTelephoneUpdated = manufacturer.telephone !== selectedManufacturer.telephone; @@ -250,21 +242,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { url: manufacturer.url, }; } - if (isBuildingNumberUpdated) { + if (isAddressLineUpdated) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { ...manufacturer.address, - building_number: manufacturer.address?.building_number, - }, - }; - } - if (isStreetNameUpdated) { - ManufacturerToEdit = { - ...ManufacturerToEdit, - address: { - ...ManufacturerToEdit.address, - street_name: manufacturer.address?.street_name, + address_line: manufacturer.address?.address_line, }, }; } @@ -286,7 +269,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, }; } - if (ispostcodeUpdated) { + if (isPostcodeUpdated) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { @@ -295,6 +278,15 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, }; } + if (isCountryUpdated) { + ManufacturerToEdit = { + ...ManufacturerToEdit, + address: { + ...ManufacturerToEdit.address, + country: manufacturer.address?.country, + }, + }; + } if (isTelephoneUpdated) { ManufacturerToEdit = { ...ManufacturerToEdit, @@ -304,11 +296,11 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { if ( (selectedManufacturer.id && isNameUpdated) || - isBuildingNumberUpdated || - isStreetNameUpdated || + isAddressLineUpdated || isTownUpdated || isCountyUpdated || - ispostcodeUpdated || + isPostcodeUpdated || + isCountryUpdated || isTelephoneUpdated ) { editManufacturer(ManufacturerToEdit) @@ -386,49 +378,47 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { /> Address { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - building_number: event.target.value, + country: event.target.value, }, }); - setAddressBuildingNumberError(false); - setAddressBuildingNumberErrorMessage(undefined); + setCountryError(false); + setCountryErrorMessage(undefined); setFormError(false); setFormErrorMessage(undefined); }} - error={addressBuildingNumberError} - helperText={ - addressBuildingNumberError && addressBuildingNumberErrorMessage - } + error={countryError} + helperText={countryError && countryErrorMessage} fullWidth /> { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - street_name: event.target.value, + address_line: event.target.value, }, }); - setAddressStreetNameError(false); - setaddressStreetNameErrorMessage(undefined); + setAddressLineError(false); + setAddressLineErrorMessage(undefined); setFormError(false); setFormErrorMessage(undefined); }} - error={addressStreetNameError} - helperText={addressStreetNameError && addressStreetNameErrorMessage} + error={addressLineError} + helperText={addressLineError && addressLineErrorMessage} fullWidth /> Date: Thu, 2 Nov 2023 15:46:36 +0000 Subject: [PATCH 02/16] fixed unit tests #117 --- src/api/manufacturer.test.tsx | 26 +++++++++---------- .../manufacturer.component.test.tsx | 12 ++++++--- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/api/manufacturer.test.tsx b/src/api/manufacturer.test.tsx index faaa48594..4878906c8 100644 --- a/src/api/manufacturer.test.tsx +++ b/src/api/manufacturer.test.tsx @@ -19,11 +19,11 @@ describe('manufacturer api functions', () => { name: 'Manufacturer D', url: 'http://test.co.uk', address: { - building_number: '1', - street_name: 'Example', + address_line: '4 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07349612203', }; @@ -43,11 +43,11 @@ describe('manufacturer api functions', () => { code: 'manufacturer-d', url: 'http://test.co.uk', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '4 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07349612203', id: '4', @@ -63,17 +63,17 @@ describe('manufacturer api functions', () => { let mockDataView: ViewManufacturerResponse; beforeEach(() => { mockDataView = { + id: '1', name: 'Manufacturer A', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', - id: '1', }; }); it('posts a request to delete a manufacturer and return a successful response', async () => { @@ -110,11 +110,11 @@ describe('manufacturer api functions', () => { code: 'manufacturer-a', url: 'http://example.com', address: { - building_number: '1', - street_name: 'Example Street', + address_line: '1 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07334893348', }, @@ -124,11 +124,11 @@ describe('manufacturer api functions', () => { code: 'manufacturer-b', url: 'http://test.com', address: { - building_number: '2', - street_name: 'Example Street', + address_line: '2 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07294958549', }, @@ -138,11 +138,11 @@ describe('manufacturer api functions', () => { code: 'manufacturer-c', url: 'http://test.co.uk', address: { - building_number: '3', - street_name: 'Example Street', + address_line: '3 Example Street', town: 'Oxford', county: 'Oxfordshire', postcode: 'OX1 2AB', + country: 'United Kingdom', }, telephone: '07934303412', }, diff --git a/src/manufacturer/manufacturer.component.test.tsx b/src/manufacturer/manufacturer.component.test.tsx index b0d2c0184..eff15607c 100644 --- a/src/manufacturer/manufacturer.component.test.tsx +++ b/src/manufacturer/manufacturer.component.test.tsx @@ -37,13 +37,19 @@ describe('Manufacturer', () => { expect(screen.getByText('http://test.com')).toBeInTheDocument(); expect(screen.getByText('http://test.co.uk')).toBeInTheDocument(); expect( - screen.getByText('1 Example Street Oxford Oxfordshire OX1 2AB') + screen.getByText( + 'United Kingdom 1 Example Street Oxford Oxfordshire OX1 2AB' + ) ).toBeInTheDocument(); expect( - screen.getByText('2 Example Street Oxford Oxfordshire OX1 2AB') + screen.getByText( + 'United Kingdom 2 Example Street Oxford Oxfordshire OX1 2AB' + ) ).toBeInTheDocument(); expect( - screen.getByText('3 Example Street Oxford Oxfordshire OX1 2AB') + screen.getByText( + 'United Kingdom 3 Example Street Oxford Oxfordshire OX1 2AB' + ) ).toBeInTheDocument(); expect(screen.getByText('07334893348')).toBeInTheDocument(); expect(screen.getByText('07294958549')).toBeInTheDocument(); From ba29a8532518feca1e5f8130135ffe5a06226419 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 8 Nov 2023 09:26:28 +0000 Subject: [PATCH 03/16] change errors to use less states #123 --- .../manufacturerDialog.component.tsx | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index 12b78158c..d2f46c18e 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -56,30 +56,34 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { selectedManufacturer, } = props; - const [nameError, setNameError] = React.useState(false); const [nameErrorMessage, setNameErrorMessage] = React.useState< string | undefined >(undefined); - const [URlerror, setURLError] = React.useState(false); - const [URLErrorMessage, setURLErrorMessage] = React.useState< + const nameError = nameErrorMessage !== undefined; + + const [urlErrorMessage, seturlErrorMessage] = React.useState< string | undefined >(undefined); - const [addressLineError, setAddressLineError] = React.useState(false); + const urlError = urlErrorMessage !== undefined; + const [addressLineErrorMessage, setAddressLineErrorMessage] = React.useState< string | undefined >(undefined); - const [addresspostcodeError, setAddresspostcodeError] = React.useState(false); + const addressLineError = addressLineErrorMessage !== undefined; + const [AddresspostcodeErrorMessage, setAddresspostcodeErrorMessage] = React.useState(undefined); - const [countryError, setCountryError] = React.useState(false); + const addresspostcodeError = AddresspostcodeErrorMessage !== undefined; + const [countryErrorMessage, setCountryErrorMessage] = React.useState< string | undefined >(undefined); + const countryError = countryErrorMessage !== undefined; - const [formError, setFormError] = React.useState(false); const [formErrorMessage, setFormErrorMessage] = React.useState< string | undefined >(undefined); + const formError = formErrorMessage !== undefined; const { mutateAsync: addManufacturer } = useAddManufacturer(); const { mutateAsync: editManufacturer } = useEditManufacturer(); @@ -100,17 +104,17 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, telephone: '', }); - setNameError(false); + setNameErrorMessage(undefined); - setURLError(false); - setURLErrorMessage(undefined); - setAddressLineError(false); + + seturlErrorMessage(undefined); + setAddressLineErrorMessage(undefined); - setCountryError(false); + setCountryErrorMessage(undefined); - setAddresspostcodeError(false); + setAddresspostcodeErrorMessage(undefined); - setFormError(false); + setFormErrorMessage(undefined); onClose(); }, [onClose, onChangeManufacturerDetails]); @@ -122,15 +126,15 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { if (manufacturer.url) { if (!isValidUrl(manufacturer.url)) { hasErrors = true; - setURLError(true); - setURLErrorMessage('Please enter a valid URL'); + + seturlErrorMessage('Please enter a valid URL'); } } //check name if (!manufacturer.name || manufacturer.name?.trim().length === 0) { hasErrors = true; - setNameError(true); + // setNameError(true); setNameErrorMessage('Please enter a name.'); } //check address line @@ -139,7 +143,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { manufacturer.address.address_line.trim().length === 0 ) { hasErrors = true; - setAddressLineError(true); + setAddressLineErrorMessage('Please enter an address.'); } @@ -149,7 +153,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { manufacturer.address.postcode?.trim().length === 0 ) { hasErrors = true; - setAddresspostcodeError(true); + setAddresspostcodeErrorMessage('Please enter a post code or zip code.'); } //check country @@ -158,7 +162,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { manufacturer.address.country?.trim().length === 0 ) { hasErrors = true; - setCountryError(true); + setCountryErrorMessage('Please enter a country.'); } @@ -191,7 +195,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { console.log(error.response?.status, manufacturer.name); if (error.response?.status === 409) { - setNameError(true); + // setNameError(true); setNameErrorMessage( 'A manufacturer with the same name already exists.' ); @@ -309,7 +313,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const response = error.response?.data as ErrorParsing; console.log(error); if (response && error.response?.status === 409) { - setNameError(true); + // setNameError(true); setNameErrorMessage( 'A manufacturer with the same name has been found. Please enter a different name' ); @@ -317,7 +321,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } }); } else { - setFormError(true); setFormErrorMessage( "There have been no changes made. Please change a field's value or press Cancel to exit" ); @@ -348,9 +351,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, name: event.target.value, }); - setNameError(false); + // setNameError(false); setNameErrorMessage(undefined); - setFormError(false); + setFormErrorMessage(undefined); }} error={nameError} @@ -367,13 +370,13 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, url: event.target.value, }); - setURLError(false); - setURLErrorMessage(undefined); - setFormError(false); + + seturlErrorMessage(undefined); + setFormErrorMessage(undefined); }} - error={URlerror} - helperText={URlerror && URLErrorMessage} + error={urlError} + helperText={urlError && urlErrorMessage} fullWidth /> Address @@ -390,9 +393,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { country: event.target.value, }, }); - setCountryError(false); + setCountryErrorMessage(undefined); - setFormError(false); + setFormErrorMessage(undefined); }} error={countryError} @@ -412,9 +415,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { address_line: event.target.value, }, }); - setAddressLineError(false); + setAddressLineErrorMessage(undefined); - setFormError(false); + setFormErrorMessage(undefined); }} error={addressLineError} @@ -434,7 +437,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { town: event.target.value, }, }); - setFormError(false); + setFormErrorMessage(undefined); }} fullWidth @@ -452,7 +455,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { county: event.target.value, }, }); - setFormError(false); + setFormErrorMessage(undefined); }} fullWidth @@ -470,9 +473,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { postcode: event.target.value, }, }); - setAddresspostcodeError(false); + setAddresspostcodeErrorMessage(undefined); - setFormError(false); + setFormErrorMessage(undefined); }} error={addresspostcodeError} @@ -489,7 +492,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, telephone: event.target.value, }); - setFormError(false); + setFormErrorMessage(undefined); }} fullWidth From b647844951453dd1b4334850dea3edb9d2a2dc6f Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 8 Nov 2023 10:49:50 +0000 Subject: [PATCH 04/16] changed types to be | null #68 --- src/api/manufacturer.test.tsx | 4 +- src/api/manufacturer.tsx | 33 ++++++------- src/app.types.tsx | 49 ++++++++----------- .../DeleteManufacturerDialog.test.tsx | 4 +- .../deleteManufacturerDialog.component.tsx | 4 +- src/manufacturer/manufacturer.component.tsx | 14 +++--- .../manufacturerDialog.component.tsx | 23 +++------ 7 files changed, 55 insertions(+), 76 deletions(-) diff --git a/src/api/manufacturer.test.tsx b/src/api/manufacturer.test.tsx index 4878906c8..f219eeaec 100644 --- a/src/api/manufacturer.test.tsx +++ b/src/api/manufacturer.test.tsx @@ -1,5 +1,5 @@ import { renderHook, waitFor } from '@testing-library/react'; -import { AddManufacturer, ViewManufacturerResponse } from '../app.types'; +import { AddManufacturer, Manufacturer } from '../app.types'; import { hooksWrapperWithProviders } from '../setupTests'; import { useAddManufacturer, @@ -60,7 +60,7 @@ describe('manufacturer api functions', () => { }); describe('useDeleteManufacturer', () => { - let mockDataView: ViewManufacturerResponse; + let mockDataView: Manufacturer; beforeEach(() => { mockDataView = { id: '1', diff --git a/src/api/manufacturer.tsx b/src/api/manufacturer.tsx index d3a90b097..99f6eab57 100644 --- a/src/api/manufacturer.tsx +++ b/src/api/manufacturer.tsx @@ -13,10 +13,10 @@ import { AddManufacturerResponse, EditManufacturer, ManufacturerDetail, - ViewManufacturerResponse, + Manufacturer, } from '../app.types'; -const getAllManufacturers = async (): Promise => { +const getAllManufacturers = async (): Promise => { let apiUrl: string; apiUrl = ''; const settingsResult = await settings; @@ -30,10 +30,10 @@ const getAllManufacturers = async (): Promise => { }; export const useManufacturers = (): UseQueryResult< - ViewManufacturerResponse[], + Manufacturer[], AxiosError > => { - return useQuery( + return useQuery( ['Manufacturers'], (params) => { return getAllManufacturers(); @@ -79,9 +79,7 @@ export const useAddManufacturer = (): UseMutationResult< ); }; -const deleteManufacturer = async ( - session: ViewManufacturerResponse -): Promise => { +const deleteManufacturer = async (session: Manufacturer): Promise => { let apiUrl: string; apiUrl = ''; const settingsResult = await settings; @@ -96,20 +94,17 @@ const deleteManufacturer = async ( export const useDeleteManufacturer = (): UseMutationResult< void, AxiosError, - ViewManufacturerResponse + Manufacturer > => { const queryClient = useQueryClient(); - return useMutation( - (session: ViewManufacturerResponse) => deleteManufacturer(session), - { - onError: (error) => { - console.log('Got error ' + error.message); - }, - onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['Manufacturers'] }); - }, - } - ); + return useMutation((session: Manufacturer) => deleteManufacturer(session), { + onError: (error) => { + console.log('Got error ' + error.message); + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ['Manufacturers'] }); + }, + }); }; const fetchManufacturerById = async ( diff --git a/src/app.types.tsx b/src/app.types.tsx index bf8aa93ba..9242876c5 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -28,43 +28,35 @@ export interface CatalogueCategory { catalogue_item_properties?: CatalogueCategoryFormData[]; } -export interface ViewManufacturerResponse { - name: string; - url: string; - address: Address; - telephone: string; - id: string; -} - export interface AddManufacturer { name: string; - url?: string; + url: string | undefined; address: Address | undefined; - telephone?: string; + telephone: string | null; } export interface AddManufacturerResponse { + id: string; name: string; - code: string; - url: string; + url: string | undefined; address: Address; - telephone: string; - id: string; + telephone: string | null; + code: string; } export interface EditManufacturer { - name?: string; - url?: string; - address?: EditAddress; - telephone?: string; - id?: string; + name?: string | null; + url?: string | undefined; + address?: EditAddress | null; + telephone?: string | null; + id?: string | null; } export interface ManufacturerDetail { name: string; - url?: string; + url: string | undefined; address: Address; - telephone: string; + telephone: string | null; } export interface Manufacturer { @@ -133,19 +125,18 @@ export interface ErrorParsing { interface Address { address_line: string; - town?: string; - county?: string; + town: string | null; + county: string | null; postcode: string; country: string; } interface EditAddress { - address_line?: string; - street_name?: string; - town?: string; - county?: string; - postcode?: string; - country?: string; + address_line: string | null; + town: string | null; + county: string | null; + postcode: string | null; + country: string | null; } export enum SystemImportanceType { diff --git a/src/manufacturer/DeleteManufacturerDialog.test.tsx b/src/manufacturer/DeleteManufacturerDialog.test.tsx index e8c5efca4..268996708 100644 --- a/src/manufacturer/DeleteManufacturerDialog.test.tsx +++ b/src/manufacturer/DeleteManufacturerDialog.test.tsx @@ -4,12 +4,12 @@ import userEvent from '@testing-library/user-event'; import { renderComponentWithBrowserRouter } from '../setupTests'; import { DeleteManufacturerProps } from './deleteManufacturerDialog.component'; import DeleteManufacturerDialog from './deleteManufacturerDialog.component'; -import { ViewManufacturerResponse } from '../app.types'; +import { Manufacturer } from '../app.types'; describe('Delete Manufacturer Dialog', () => { const onClose = jest.fn(); let props: DeleteManufacturerProps; - let manufacturer: ViewManufacturerResponse; + let manufacturer: Manufacturer; let user; const createView = (): RenderResult => { return renderComponentWithBrowserRouter( diff --git a/src/manufacturer/deleteManufacturerDialog.component.tsx b/src/manufacturer/deleteManufacturerDialog.component.tsx index 131c6cabd..ab1cc6e30 100644 --- a/src/manufacturer/deleteManufacturerDialog.component.tsx +++ b/src/manufacturer/deleteManufacturerDialog.component.tsx @@ -8,14 +8,14 @@ import { FormHelperText, } from '@mui/material'; import React from 'react'; -import { ErrorParsing, ViewManufacturerResponse } from '../app.types'; +import { ErrorParsing, Manufacturer } from '../app.types'; import { AxiosError } from 'axios'; import { useDeleteManufacturer } from '../api/manufacturer'; export interface DeleteManufacturerProps { open: boolean; onClose: () => void; - manufacturer: ViewManufacturerResponse | undefined; + manufacturer: Manufacturer | undefined; } const DeleteManufacturerDialog = (props: DeleteManufacturerProps) => { diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 98de3c6e6..b064a0d43 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -17,21 +17,21 @@ import EditIcon from '@mui/icons-material/Edit'; import React from 'react'; import { useManufacturers } from '../api/manufacturer'; import DeleteManufacturerDialog from './deleteManufacturerDialog.component'; -import { ManufacturerDetail, ViewManufacturerResponse } from '../app.types'; +import { ManufacturerDetail, Manufacturer } from '../app.types'; import ManufacturerDialog from './manufacturerDialog.component'; -function Manufacturer() { +function ManufacturerComponent() { const [Manufacturer, setManufacturer] = React.useState({ name: '', url: undefined, address: { address_line: '', - town: '', - county: '', + town: null, + county: null, postcode: '', country: '', }, - telephone: '', + telephone: null, }); const [editManufacturerDialogOpen, setEditManufacturerDialogOpen] = @@ -46,7 +46,7 @@ function Manufacturer() { React.useState(false); const [selectedManufacturer, setSelectedManufacturer] = React.useState< - ViewManufacturerResponse | undefined + Manufacturer | undefined >(undefined); const [hoveredRow, setHoveredRow] = React.useState(null); @@ -242,4 +242,4 @@ function Manufacturer() { ); } -export default Manufacturer; +export default ManufacturerComponent; diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index d2f46c18e..e3fedecb9 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -97,24 +97,18 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { url: undefined, address: { address_line: '', - town: '', - county: '', + town: null, + county: null, postcode: '', country: '', }, - telephone: '', + telephone: null, }); - setNameErrorMessage(undefined); - seturlErrorMessage(undefined); - setAddressLineErrorMessage(undefined); - setCountryErrorMessage(undefined); - setAddresspostcodeErrorMessage(undefined); - setFormErrorMessage(undefined); onClose(); }, [onClose, onChangeManufacturerDetails]); @@ -178,15 +172,15 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const manufacturerToAdd: AddManufacturer = { name: manufacturer.name, - url: manufacturer.url, + url: manufacturer.url ?? undefined, address: { address_line: manufacturer.address.address_line, - town: manufacturer.address.town, - county: manufacturer.address.county, + town: manufacturer.address.town ?? null, + county: manufacturer.address.county ?? null, postcode: manufacturer.address.postcode, country: manufacturer.address.country, }, - telephone: manufacturer.telephone, + telephone: manufacturer.telephone ?? null, }; addManufacturer(manufacturerToAdd) @@ -286,7 +280,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...ManufacturerToEdit.address, + ...manufacturer.address, country: manufacturer.address?.country, }, }; @@ -353,7 +347,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }); // setNameError(false); setNameErrorMessage(undefined); - setFormErrorMessage(undefined); }} error={nameError} From f252a490f51c84aa5611aedafe283faca57c7a4c Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Thu, 9 Nov 2023 13:32:53 +0000 Subject: [PATCH 05/16] simplified edit #73 --- .../manufacturerDialog.component.tsx | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index e3fedecb9..50a16c027 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -228,18 +228,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { id: selectedManufacturer?.id, }; - if (isNameUpdated) { - ManufacturerToEdit = { - ...ManufacturerToEdit, - name: manufacturer.name, - }; - } - if (isURLUpdated) { - ManufacturerToEdit = { - ...ManufacturerToEdit, - url: manufacturer.url, - }; - } + isNameUpdated && (ManufacturerToEdit.name = manufacturer.name); + isURLUpdated && (ManufacturerToEdit.url = manufacturer.url); + if (isAddressLineUpdated) { ManufacturerToEdit = { ...ManufacturerToEdit, @@ -285,12 +276,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, }; } - if (isTelephoneUpdated) { - ManufacturerToEdit = { - ...ManufacturerToEdit, - telephone: manufacturer.telephone, - }; - } + + isTelephoneUpdated && + (ManufacturerToEdit.telephone = manufacturer.telephone); if ( (selectedManufacturer.id && isNameUpdated) || From cf5ccce1dbc93ae59dc0108285ed34f9d3346c15 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Mon, 13 Nov 2023 10:09:04 +0000 Subject: [PATCH 06/16] requested changes #117 --- src/app.types.tsx | 36 +++++++++---------- src/manufacturer/manufacturer.component.tsx | 20 ++++++----- .../manufacturerDialog.component.tsx | 17 +++++---- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/app.types.tsx b/src/app.types.tsx index 9d5221888..35c69c3a2 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -35,24 +35,24 @@ export interface CatalogueCategory { export interface AddManufacturer { name: string; - url: string | undefined; - address: Address | undefined; - telephone: string | null; + url?: string; + address?: Address; + telephone?: string; } export interface AddManufacturerResponse { id: string; name: string; - url: string | undefined; - address: Address; - telephone: string | null; + url?: string; + address?: Address; + telephone?: string; code: string; } export interface EditManufacturer { - name?: string | null; - url?: string | undefined; - address?: EditAddress | null; + name?: string; + url?: string; + address?: EditAddress; telephone?: string | null; id?: string | null; } @@ -60,9 +60,9 @@ export interface EditManufacturer { export interface Manufacturer { id?: string; name: string; - url: string | undefined; + url?: string; address: Address; - telephone: string | null; + telephone?: string; } export interface CatalogueCategoryFormData { @@ -123,18 +123,18 @@ export interface ErrorParsing { interface Address { address_line: string; - town: string | null; - county: string | null; + town?: string; + county?: string; postcode: string; country: string; } interface EditAddress { - address_line: string | null; - town: string | null; - county: string | null; - postcode: string | null; - country: string | null; + address_line?: string; + town?: string | null; + county?: string | null; + postcode?: string; + country?: string; } export interface CatalogueCategoryTransferState { name: string; diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 4459830e5..628b59317 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -26,12 +26,12 @@ function ManufacturerComponent() { url: undefined, address: { address_line: '', - town: null, - county: null, + town: undefined, + county: undefined, postcode: '', country: '', }, - telephone: null, + telephone: undefined, }); const [editManufacturerDialogOpen, setEditManufacturerDialogOpen] = @@ -208,15 +208,19 @@ function ManufacturerComponent() { borderRight: '1px solid #e0e0e0', }} > + {/* {item.address.country} + {item.address.address_line} + {item.address.town ?? ''} + {item.address.county ?? ''} + {item.address.postcode} */} + {item.address.country + ' \n' + item.address.address_line + ' \n' + - item.address.town + - ' \n' + - item.address.county + - ' \n' + - item.address.postcode} + item.address.town ?? + '' + ' \n' + item.address.county ?? + '' + ' \n' + item.address.postcode} - + Date: Mon, 13 Nov 2023 10:27:40 +0000 Subject: [PATCH 07/16] changed address view #117 --- src/manufacturer/manufacturer.component.tsx | 28 +++++++++++---------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 628b59317..2545557d4 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -208,19 +208,21 @@ function ManufacturerComponent() { borderRight: '1px solid #e0e0e0', }} > - {/* {item.address.country} - {item.address.address_line} - {item.address.town ?? ''} - {item.address.county ?? ''} - {item.address.postcode} */} - - {item.address.country + - ' \n' + - item.address.address_line + - ' \n' + - item.address.town ?? - '' + ' \n' + item.address.county ?? - '' + ' \n' + item.address.postcode} + + {item.address.country} + + + {item.address.address_line} + + + {item.address.town ?? ''} + + + {item.address.county ?? ''} + + + {item.address.postcode} + Date: Mon, 13 Nov 2023 11:11:06 +0000 Subject: [PATCH 08/16] fix tests #117 --- cypress/e2e/manufacturer/manufacturer.cy.ts | 14 ++++---------- .../manufacturer.component.test.tsx | 18 ++---------------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/cypress/e2e/manufacturer/manufacturer.cy.ts b/cypress/e2e/manufacturer/manufacturer.cy.ts index 2473e78a6..37d5ab010 100644 --- a/cypress/e2e/manufacturer/manufacturer.cy.ts +++ b/cypress/e2e/manufacturer/manufacturer.cy.ts @@ -24,15 +24,9 @@ describe('Manufacturer', () => { cy.findByText('http://example.com').should('be.visible'); cy.findByText('http://test.com').should('be.visible'); cy.findByText('http://test.co.uk').should('be.visible'); - cy.findByText( - 'United Kingdom 1 Example Street Oxford Oxfordshire OX1 2AB' - ).should('be.visible'); - cy.findByText( - 'United Kingdom 2 Example Street Oxford Oxfordshire OX1 2AB' - ).should('be.visible'); - cy.findByText( - 'United Kingdom 3 Example Street Oxford Oxfordshire OX1 2AB' - ).should('be.visible'); + cy.findByText('07334893348').should('be.visible'); + cy.findByText('07294958549').should('be.visible'); + cy.findByText('07934303412').should('be.visible'); }); it('manufacturer url is correct and opens new webpage', () => { @@ -93,7 +87,7 @@ describe('Manufacturer', () => { expect(patchRequests.length).equal(1); const request = patchRequests[0]; expect(JSON.stringify(request.body)).equal( - '{"name":"Manufacturer D","address":{"address_line":"4 Example Street","town":null,"county":null,"postcode":"OX1 2AB","country":"United Kingdom"},"telephone":null}' + '{"name":"Manufacturer D","address":{"address_line":"4 Example Street","postcode":"OX1 2AB","country":"United Kingdom"}}' ); }); diff --git a/src/manufacturer/manufacturer.component.test.tsx b/src/manufacturer/manufacturer.component.test.tsx index 75bf06c6e..780d1bbea 100644 --- a/src/manufacturer/manufacturer.component.test.tsx +++ b/src/manufacturer/manufacturer.component.test.tsx @@ -3,6 +3,7 @@ import { renderComponentWithBrowserRouter } from '../setupTests'; import { screen, waitFor } from '@testing-library/react'; import Manufacturer from './manufacturer.component'; import userEvent from '@testing-library/user-event'; +import exp from 'constants'; describe('Manufacturer', () => { let user; @@ -25,7 +26,7 @@ describe('Manufacturer', () => { expect(screen.getByText('Telephone')).toBeInTheDocument(); }); - it('renders table data correctly', async () => { + it.only('renders table data correctly', async () => { createView(); await waitFor(() => { expect(screen.getByText('Manufacturer A')).toBeInTheDocument(); @@ -36,21 +37,6 @@ describe('Manufacturer', () => { expect(screen.getByText('http://example.com')).toBeInTheDocument(); expect(screen.getByText('http://test.com')).toBeInTheDocument(); expect(screen.getByText('http://test.co.uk')).toBeInTheDocument(); - expect( - screen.getByText( - 'United Kingdom 1 Example Street Oxford Oxfordshire OX1 2AB' - ) - ).toBeInTheDocument(); - expect( - screen.getByText( - 'United Kingdom 2 Example Street Oxford Oxfordshire OX1 2AB' - ) - ).toBeInTheDocument(); - expect( - screen.getByText( - 'United Kingdom 3 Example Street Oxford Oxfordshire OX1 2AB' - ) - ).toBeInTheDocument(); expect(screen.getByText('07334893348')).toBeInTheDocument(); expect(screen.getByText('07294958549')).toBeInTheDocument(); expect(screen.getByText('07934303412')).toBeInTheDocument(); From ee5c48305b8d9374f5a1296cef9961622bac70e1 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Mon, 13 Nov 2023 16:23:04 +0000 Subject: [PATCH 09/16] requested changes --- cypress/e2e/manufacturer/manufacturer.cy.ts | 30 ++++----- src/api/manufacturer.test.tsx | 4 +- src/api/manufacturer.tsx | 9 ++- src/app.types.tsx | 20 +++--- .../manufacturer.component.test.tsx | 2 +- src/manufacturer/manufacturer.component.tsx | 12 ++-- .../manufacturerDialog.component.test.tsx | 16 +++-- .../manufacturerDialog.component.tsx | 67 +++++++++---------- src/mocks/handlers.ts | 4 +- 9 files changed, 82 insertions(+), 82 deletions(-) diff --git a/cypress/e2e/manufacturer/manufacturer.cy.ts b/cypress/e2e/manufacturer/manufacturer.cy.ts index 37d5ab010..def80a659 100644 --- a/cypress/e2e/manufacturer/manufacturer.cy.ts +++ b/cypress/e2e/manufacturer/manufacturer.cy.ts @@ -87,7 +87,7 @@ describe('Manufacturer', () => { expect(patchRequests.length).equal(1); const request = patchRequests[0]; expect(JSON.stringify(request.body)).equal( - '{"name":"Manufacturer D","address":{"address_line":"4 Example Street","postcode":"OX1 2AB","country":"United Kingdom"}}' + '{"name":"Manufacturer D","address":{"address_line":"4 Example Street","town":null,"county":null,"postcode":"OX1 2AB","country":"United Kingdom"},"telephone":null}' ); }); @@ -177,14 +177,14 @@ describe('Manufacturer', () => { cy.findByRole('button', { name: 'Edit Manufacturer A manufacturer', }).click(); - cy.findByLabelText('Name').clear(); - cy.findByLabelText('Name').type('test'); + cy.findByLabelText('Name *').clear(); + cy.findByLabelText('Name *').type('test'); - cy.findByLabelText('Country').clear(); - cy.findByLabelText('Country').type('test'); + cy.findByLabelText('Country *').clear(); + cy.findByLabelText('Country *').type('test'); - cy.findByLabelText('Address Line').clear(); - cy.findByLabelText('Address Line').type('test'); + cy.findByLabelText('Address Line *').clear(); + cy.findByLabelText('Address Line *').type('test'); cy.findByLabelText('Town').clear(); cy.findByLabelText('Town').type('test'); @@ -192,8 +192,8 @@ describe('Manufacturer', () => { cy.findByLabelText('County').clear(); cy.findByLabelText('County').type('test'); - cy.findByLabelText('Post/Zip code').clear(); - cy.findByLabelText('Post/Zip code').type('test'); + cy.findByLabelText('Post/Zip code *').clear(); + cy.findByLabelText('Post/Zip code *').type('test'); cy.findByLabelText('Telephone number').clear(); cy.findByLabelText('Telephone number').type('0000000000'); @@ -219,8 +219,8 @@ describe('Manufacturer', () => { name: 'Edit Manufacturer A manufacturer', }).click(); - cy.findByLabelText('Name').clear(); - cy.findByLabelText('Name').type('test_dup'); + cy.findByLabelText('Name *').clear(); + cy.findByLabelText('Name *').type('test_dup'); cy.findByRole('button', { name: 'Save' }).click(); @@ -271,10 +271,10 @@ describe('Manufacturer', () => { name: 'Edit Manufacturer A manufacturer', }).click(); - cy.findByLabelText('Name').clear(); - cy.findByLabelText('Country').clear(); - cy.findByLabelText('Address Line').clear(); - cy.findByLabelText('Post/Zip code').clear(); + cy.findByLabelText('Name *').clear(); + cy.findByLabelText('Country *').clear(); + cy.findByLabelText('Address Line *').clear(); + cy.findByLabelText('Post/Zip code *').clear(); cy.findByRole('button', { name: 'Save' }).click(); cy.findByRole('dialog') diff --git a/src/api/manufacturer.test.tsx b/src/api/manufacturer.test.tsx index f219eeaec..dea108d38 100644 --- a/src/api/manufacturer.test.tsx +++ b/src/api/manufacturer.test.tsx @@ -1,5 +1,5 @@ import { renderHook, waitFor } from '@testing-library/react'; -import { AddManufacturer, Manufacturer } from '../app.types'; +import { Manufacturer } from '../app.types'; import { hooksWrapperWithProviders } from '../setupTests'; import { useAddManufacturer, @@ -13,7 +13,7 @@ describe('manufacturer api functions', () => { }); describe('useAddManufacturer', () => { - let mockDataAdd: AddManufacturer; + let mockDataAdd: Manufacturer; beforeEach(() => { mockDataAdd = { name: 'Manufacturer D', diff --git a/src/api/manufacturer.tsx b/src/api/manufacturer.tsx index 925c08e5a..63b79fe5a 100644 --- a/src/api/manufacturer.tsx +++ b/src/api/manufacturer.tsx @@ -9,10 +9,9 @@ import { import { settings } from '../settings'; import { - AddManufacturer, + Manufacturer, AddManufacturerResponse, EditManufacturer, - Manufacturer, } from '../app.types'; const getAllManufacturers = async (): Promise => { @@ -46,7 +45,7 @@ export const useManufacturers = (): UseQueryResult< }; const addManufacturer = async ( - manufacturer: AddManufacturer + manufacturer: Manufacturer ): Promise => { let apiUrl: string; apiUrl = ''; @@ -62,11 +61,11 @@ const addManufacturer = async ( export const useAddManufacturer = (): UseMutationResult< AddManufacturerResponse, AxiosError, - AddManufacturer + Manufacturer > => { const queryClient = useQueryClient(); return useMutation( - (manufacturer: AddManufacturer) => addManufacturer(manufacturer), + (manufacturer: Manufacturer) => addManufacturer(manufacturer), { onError: (error) => { console.log('Got error ' + error.message); diff --git a/src/app.types.tsx b/src/app.types.tsx index 0a1433d08..36b67627e 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -33,18 +33,11 @@ export interface CatalogueCategory { catalogue_item_properties?: CatalogueCategoryFormData[]; } -export interface AddManufacturer { - name: string; - url?: string; - address?: Address; - telephone?: string; -} - export interface AddManufacturerResponse { id: string; name: string; url?: string; - address?: Address; + address: Address; telephone?: string; code: string; } @@ -61,8 +54,8 @@ export interface Manufacturer { id?: string; name: string; url?: string; - address: Address; - telephone?: string; + address: AddAddress; + telephone: string | null; } export interface CatalogueCategoryFormData { @@ -129,6 +122,13 @@ interface Address { country: string; } +interface AddAddress { + address_line: string; + town: string | null; + county: string | null; + postcode: string; + country: string; +} interface EditAddress { address_line?: string; town?: string | null; diff --git a/src/manufacturer/manufacturer.component.test.tsx b/src/manufacturer/manufacturer.component.test.tsx index 956eb2cbb..dce02ab20 100644 --- a/src/manufacturer/manufacturer.component.test.tsx +++ b/src/manufacturer/manufacturer.component.test.tsx @@ -25,7 +25,7 @@ describe('Manufacturer', () => { expect(screen.getByText('Telephone')).toBeInTheDocument(); }); - it.only('renders table data correctly', async () => { + it('renders table data correctly', async () => { createView(); await waitFor(() => { expect(screen.getByText('Manufacturer A')).toBeInTheDocument(); diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 2545557d4..983c5c978 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -26,12 +26,12 @@ function ManufacturerComponent() { url: undefined, address: { address_line: '', - town: undefined, - county: undefined, + town: null, + county: null, postcode: '', country: '', }, - telephone: undefined, + telephone: null, }); const [editManufacturerDialogOpen, setEditManufacturerDialogOpen] = @@ -208,9 +208,6 @@ function ManufacturerComponent() { borderRight: '1px solid #e0e0e0', }} > - - {item.address.country} - {item.address.address_line} @@ -220,6 +217,9 @@ function ManufacturerComponent() { {item.address.county ?? ''} + + {item.address.country} + {item.address.postcode} diff --git a/src/manufacturer/manufacturerDialog.component.test.tsx b/src/manufacturer/manufacturerDialog.component.test.tsx index b37bc9ed6..13a48674e 100644 --- a/src/manufacturer/manufacturerDialog.component.test.tsx +++ b/src/manufacturer/manufacturerDialog.component.test.tsx @@ -26,12 +26,12 @@ describe('Add manufacturer dialog', () => { url: undefined, address: { address_line: '', - town: '', - county: '', + town: null, + county: null, postcode: '', country: '', }, - telephone: '', + telephone: null, }, type: 'create', }; @@ -602,7 +602,7 @@ describe('Add manufacturer dialog', () => { createView(); - const manufacturerNameInput = screen.getByLabelText('Name'); + const manufacturerNameInput = screen.getByLabelText('Name *'); fireEvent.change(manufacturerNameInput, { target: { value: newManufacturerName }, @@ -636,7 +636,7 @@ describe('Add manufacturer dialog', () => { createView(); - const manufacturerCountryInput = screen.getByLabelText('Country'); + const manufacturerCountryInput = screen.getByLabelText('Country *'); fireEvent.change(manufacturerCountryInput, { target: { value: newManufacturerCountry }, @@ -656,7 +656,8 @@ describe('Add manufacturer dialog', () => { createView(); - const manufacturerStreetNameInput = screen.getByLabelText('Address Line'); + const manufacturerStreetNameInput = + screen.getByLabelText('Address Line *'); fireEvent.change(manufacturerStreetNameInput, { target: { value: newManufacturerAddressLine }, @@ -713,7 +714,8 @@ describe('Add manufacturer dialog', () => { createView(); - const manufacturerpostcodeInput = screen.getByLabelText('Post/Zip code'); + const manufacturerpostcodeInput = + screen.getByLabelText('Post/Zip code *'); fireEvent.change(manufacturerpostcodeInput, { target: { value: newManufacturerpostcode }, diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index ae3dcf3de..ba0a26467 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -13,12 +13,7 @@ import { import React from 'react'; -import { - AddManufacturer, - EditManufacturer, - ErrorParsing, - Manufacturer, -} from '../app.types'; +import { Manufacturer, EditManufacturer, ErrorParsing } from '../app.types'; import { useAddManufacturer, useEditManufacturer, @@ -61,7 +56,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { >(undefined); const nameError = nameErrorMessage !== undefined; - const [urlErrorMessage, seturlErrorMessage] = React.useState< + const [urlErrorMessage, setUrlErrorMessage] = React.useState< string | undefined >(undefined); const urlError = urlErrorMessage !== undefined; @@ -71,9 +66,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { >(undefined); const addressLineError = addressLineErrorMessage !== undefined; - const [AddresspostcodeErrorMessage, setAddresspostcodeErrorMessage] = + const [addressPostcodeErrorMessage, setAddressPostcodeErrorMessage] = React.useState(undefined); - const addresspostcodeError = AddresspostcodeErrorMessage !== undefined; + const addressPostcodeError = addressPostcodeErrorMessage !== undefined; const [countryErrorMessage, setCountryErrorMessage] = React.useState< string | undefined @@ -99,18 +94,18 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { url: undefined, address: { address_line: '', - town: undefined, - county: undefined, + town: null, + county: null, postcode: '', country: '', }, - telephone: undefined, + telephone: null, }); setNameErrorMessage(undefined); - seturlErrorMessage(undefined); + setUrlErrorMessage(undefined); setAddressLineErrorMessage(undefined); setCountryErrorMessage(undefined); - setAddresspostcodeErrorMessage(undefined); + setAddressPostcodeErrorMessage(undefined); setFormErrorMessage(undefined); onClose(); }, [onClose, onChangeManufacturerDetails]); @@ -122,14 +117,13 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { if (manufacturer.url || manufacturer.url?.trim().length === 0) { if (!isValidUrl(manufacturer.url)) { hasErrors = true; - seturlErrorMessage('Please enter a valid URL'); + setUrlErrorMessage('Please enter a valid URL'); } } //check name if (!manufacturer.name || manufacturer.name?.trim().length === 0) { hasErrors = true; - // setNameError(true); setNameErrorMessage('Please enter a name.'); } //check address line @@ -149,7 +143,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ) { hasErrors = true; - setAddresspostcodeErrorMessage('Please enter a post code or zip code.'); + setAddressPostcodeErrorMessage('Please enter a post code or zip code.'); } //check country if ( @@ -171,17 +165,17 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { return; } - const manufacturerToAdd: AddManufacturer = { + const manufacturerToAdd: Manufacturer = { name: manufacturer.name, url: manufacturer.url ?? undefined, address: { address_line: manufacturer.address.address_line, - town: manufacturer.address.town ?? undefined, - county: manufacturer.address.county ?? undefined, + town: manufacturer.address.town ?? null, + county: manufacturer.address.county ?? null, postcode: manufacturer.address.postcode, country: manufacturer.address.country, }, - telephone: manufacturer.telephone ?? undefined, + telephone: manufacturer.telephone ?? null, }; addManufacturer(manufacturerToAdd) @@ -208,21 +202,28 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } const isNameUpdated = manufacturer.name !== selectedManufacturer.name; + const isURLUpdated = manufacturer.url !== selectedManufacturer.url && manufacturer.url !== undefined; + const isAddressLineUpdated = manufacturer.address?.address_line !== selectedManufacturer.address.address_line; + const isTownUpdated = manufacturer.address?.town !== selectedManufacturer.address.town; + const isCountyUpdated = manufacturer.address?.county !== selectedManufacturer.address.county; + const isPostcodeUpdated = manufacturer.address?.postcode !== selectedManufacturer.address.postcode; + const isCountryUpdated = manufacturer.address?.country !== selectedManufacturer.address.country; + const isTelephoneUpdated = manufacturer.telephone !== selectedManufacturer.telephone; @@ -297,7 +298,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const response = error.response?.data as ErrorParsing; console.log(error); if (response && error.response?.status === 409) { - // setNameError(true); setNameErrorMessage( 'A manufacturer with the same name has been found. Please enter a different name' ); @@ -330,7 +330,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { { @@ -338,7 +338,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, name: event.target.value, }); - // setNameError(false); setNameErrorMessage(undefined); setFormErrorMessage(undefined); }} @@ -359,7 +358,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { url: event.target.value, }); - seturlErrorMessage(undefined); + setUrlErrorMessage(undefined); setFormErrorMessage(undefined); }} @@ -374,7 +373,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { { @@ -398,7 +397,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { { @@ -430,7 +429,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, address: { ...manufacturer.address, - town: event.target.value, + town: event.target.value || null, }, }); @@ -450,7 +449,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ...manufacturer, address: { ...manufacturer.address, - county: event.target.value, + county: event.target.value || null, }, }); @@ -462,7 +461,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { { @@ -474,12 +473,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, }); - setAddresspostcodeErrorMessage(undefined); + setAddressPostcodeErrorMessage(undefined); setFormErrorMessage(undefined); }} - error={addresspostcodeError} - helperText={addresspostcodeError && AddresspostcodeErrorMessage} + error={addressPostcodeError} + helperText={addressPostcodeError && addressPostcodeErrorMessage} fullWidth /> @@ -492,7 +491,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { onChange={(event) => { onChangeManufacturerDetails({ ...manufacturer, - telephone: event.target.value, + telephone: event.target.value || null, }); setFormErrorMessage(undefined); diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index 8735e2de0..43f826a54 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -1,7 +1,7 @@ import { rest } from 'msw'; import { AddCatalogueCategory, - AddManufacturer, + Manufacturer, AddSystem, CatalogueItem, EditCatalogueCategory, @@ -281,7 +281,7 @@ export const handlers = [ }), rest.post('/v1/manufacturers', async (req, res, ctx) => { - const body = (await req.json()) as AddManufacturer; + const body = (await req.json()) as Manufacturer; if (body.name === 'Manufacturer A') { return res(ctx.status(409), ctx.json('')); From 865ef4d7d9329e1a0b9d8aedaa55c882f6f1651b Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 08:59:28 +0000 Subject: [PATCH 10/16] country field moved in dialog #117 --- .../manufacturerDialog.component.tsx | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index ba0a26467..e574edc32 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -370,66 +370,63 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { Address + { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - country: event.target.value, + address_line: event.target.value, }, }); - setCountryErrorMessage(undefined); + setAddressLineErrorMessage(undefined); setFormErrorMessage(undefined); }} - error={countryError} - helperText={countryError && countryErrorMessage} + error={addressLineError} + helperText={addressLineError && addressLineErrorMessage} fullWidth /> { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - address_line: event.target.value, + town: event.target.value || null, }, }); - setAddressLineErrorMessage(undefined); - setFormErrorMessage(undefined); }} - error={addressLineError} - helperText={addressLineError && addressLineErrorMessage} fullWidth /> { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - town: event.target.value || null, + county: event.target.value || null, }, }); @@ -440,21 +437,25 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { { onChangeManufacturerDetails({ ...manufacturer, address: { ...manufacturer.address, - county: event.target.value || null, + country: event.target.value, }, }); + setCountryErrorMessage(undefined); + setFormErrorMessage(undefined); }} + error={countryError} + helperText={countryError && countryErrorMessage} fullWidth /> From fe3506a89aef8cb7b29398a14d165ddd6e293958 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 09:53:22 +0000 Subject: [PATCH 11/16] requested interface changes #117 --- src/api/manufacturer.tsx | 18 +++++++---------- src/app.types.tsx | 20 +++++-------------- .../manufacturerDialog.component.tsx | 15 +++++++++----- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/api/manufacturer.tsx b/src/api/manufacturer.tsx index 63b79fe5a..29c6c2136 100644 --- a/src/api/manufacturer.tsx +++ b/src/api/manufacturer.tsx @@ -8,11 +8,7 @@ import { } from '@tanstack/react-query'; import { settings } from '../settings'; -import { - Manufacturer, - AddManufacturerResponse, - EditManufacturer, -} from '../app.types'; +import { AddManufacturer, Manufacturer, EditManufacturer } from '../app.types'; const getAllManufacturers = async (): Promise => { let apiUrl: string; @@ -45,8 +41,8 @@ export const useManufacturers = (): UseQueryResult< }; const addManufacturer = async ( - manufacturer: Manufacturer -): Promise => { + manufacturer: AddManufacturer +): Promise => { let apiUrl: string; apiUrl = ''; const settingsResult = await settings; @@ -54,18 +50,18 @@ const addManufacturer = async ( apiUrl = settingsResult['apiUrl']; } return axios - .post(`${apiUrl}/v1/manufacturers`, manufacturer) + .post(`${apiUrl}/v1/manufacturers`, manufacturer) .then((response) => response.data); }; export const useAddManufacturer = (): UseMutationResult< - AddManufacturerResponse, + Manufacturer, AxiosError, - Manufacturer + AddManufacturer > => { const queryClient = useQueryClient(); return useMutation( - (manufacturer: Manufacturer) => addManufacturer(manufacturer), + (manufacturer: AddManufacturer) => addManufacturer(manufacturer), { onError: (error) => { console.log('Got error ' + error.message); diff --git a/src/app.types.tsx b/src/app.types.tsx index 36b67627e..26163bc81 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -33,13 +33,11 @@ export interface CatalogueCategory { catalogue_item_properties?: CatalogueCategoryFormData[]; } -export interface AddManufacturerResponse { - id: string; +export interface AddManufacturer { name: string; url?: string; - address: Address; - telephone?: string; - code: string; + address: AddAddress; + telephone?: string | null; } export interface EditManufacturer { @@ -114,18 +112,10 @@ export interface ErrorParsing { detail: string; } -interface Address { - address_line: string; - town?: string; - county?: string; - postcode: string; - country: string; -} - interface AddAddress { address_line: string; - town: string | null; - county: string | null; + town?: string | null; + county?: string | null; postcode: string; country: string; } diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index e574edc32..e96233143 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -13,7 +13,12 @@ import { import React from 'react'; -import { Manufacturer, EditManufacturer, ErrorParsing } from '../app.types'; +import { + AddManufacturer, + Manufacturer, + EditManufacturer, + ErrorParsing, +} from '../app.types'; import { useAddManufacturer, useEditManufacturer, @@ -82,7 +87,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const [catchAllError, setCatchAllError] = React.useState(false); - const { mutateAsync: addManufacturer } = useAddManufacturer(); + const { mutateAsync: AddManufacturer } = useAddManufacturer(); const { mutateAsync: editManufacturer } = useEditManufacturer(); const { data: selectedManufacturerData } = useManufacturer( selectedManufacturer?.id @@ -165,7 +170,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { return; } - const manufacturerToAdd: Manufacturer = { + const manufacturerToAdd: AddManufacturer = { name: manufacturer.name, url: manufacturer.url ?? undefined, address: { @@ -178,7 +183,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { telephone: manufacturer.telephone ?? null, }; - addManufacturer(manufacturerToAdd) + AddManufacturer(manufacturerToAdd) .then((response) => handleClose()) .catch((error: AxiosError) => { console.log(error.response?.status, manufacturer.name); @@ -191,7 +196,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } setCatchAllError(true); }); - }, [handleErrors, manufacturer, addManufacturer, handleClose]); + }, [handleErrors, manufacturer, AddManufacturer, handleClose]); const handleEditManufacturer = React.useCallback(() => { if (selectedManufacturer && selectedManufacturerData) { From 6ab3c460546fb3bcadc3d4135149b3afcae9bd6b Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 09:57:27 +0000 Subject: [PATCH 12/16] changed to camelcase function name --- src/manufacturer/manufacturerDialog.component.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index e96233143..4b9096e1b 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -87,7 +87,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const [catchAllError, setCatchAllError] = React.useState(false); - const { mutateAsync: AddManufacturer } = useAddManufacturer(); + const { mutateAsync: addManufacturer } = useAddManufacturer(); const { mutateAsync: editManufacturer } = useEditManufacturer(); const { data: selectedManufacturerData } = useManufacturer( selectedManufacturer?.id @@ -183,7 +183,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { telephone: manufacturer.telephone ?? null, }; - AddManufacturer(manufacturerToAdd) + addManufacturer(manufacturerToAdd) .then((response) => handleClose()) .catch((error: AxiosError) => { console.log(error.response?.status, manufacturer.name); @@ -196,7 +196,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } setCatchAllError(true); }); - }, [handleErrors, manufacturer, AddManufacturer, handleClose]); + }, [handleErrors, manufacturer, addManufacturer, handleClose]); const handleEditManufacturer = React.useCallback(() => { if (selectedManufacturer && selectedManufacturerData) { From 3188b701a0c9e2015983fe23bc6658fb44a08240 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 15:51:58 +0000 Subject: [PATCH 13/16] removed unneeded useState --- src/manufacturer/manufacturer.component.tsx | 12 ++---- .../manufacturerDialog.component.tsx | 41 ++++++++----------- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 983c5c978..d4825e818 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -45,10 +45,6 @@ function ManufacturerComponent() { const [deleteManufacturerDialog, setDeleteManufacturerDialog] = React.useState(false); - const [selectedManufacturer, setSelectedManufacturer] = React.useState< - Manufacturer | undefined - >(undefined); - const [hoveredRow, setHoveredRow] = React.useState(null); const tableHeight = `calc(100vh)-(64px + 36px +50px)`; const theme = useTheme(); @@ -82,7 +78,6 @@ function ManufacturerComponent() { manufacturer={manufacturer} onChangeManufacturerDetails={setManufacturer} type="edit" - selectedManufacturer={selectedManufacturer} /> @@ -163,7 +158,7 @@ function ManufacturerComponent() { aria-label={`Edit ${item.name} manufacturer`} onClick={() => { setEditManufacturerDialogOpen(true); - setSelectedManufacturer(item); + // setSelectedManufacturer(item); setManufacturer(item); }} > @@ -174,7 +169,8 @@ function ManufacturerComponent() { aria-label={`Delete ${item.name} manufacturer`} onClick={() => { setDeleteManufacturerDialog(true); - setSelectedManufacturer(item); + // setSelectedManufacturer(item); + setManufacturer(item); }} > @@ -242,7 +238,7 @@ function ManufacturerComponent() { setDeleteManufacturerDialog(false)} - manufacturer={selectedManufacturer} + manufacturer={manufacturer} /> ); diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index 4b9096e1b..a49cbfdda 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -31,7 +31,7 @@ export interface ManufacturerDialogProps { onClose: () => void; onChangeManufacturerDetails: (manufacturer: Manufacturer) => void; manufacturer: Manufacturer; - selectedManufacturer?: Manufacturer; + type: 'edit' | 'create'; } function isValidUrl(url: string) { @@ -47,14 +47,8 @@ function isValidUrl(url: string) { } function ManufacturerDialog(props: ManufacturerDialogProps) { - const { - open, - onClose, - manufacturer, - onChangeManufacturerDetails, - type, - selectedManufacturer, - } = props; + const { open, onClose, manufacturer, onChangeManufacturerDetails, type } = + props; const [nameErrorMessage, setNameErrorMessage] = React.useState< string | undefined @@ -89,9 +83,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const { mutateAsync: addManufacturer } = useAddManufacturer(); const { mutateAsync: editManufacturer } = useEditManufacturer(); - const { data: selectedManufacturerData } = useManufacturer( - selectedManufacturer?.id - ); + const { data: selectedManufacturerData } = useManufacturer(manufacturer?.id); const handleClose = React.useCallback(() => { onChangeManufacturerDetails({ @@ -199,41 +191,43 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, [handleErrors, manufacturer, addManufacturer, handleClose]); const handleEditManufacturer = React.useCallback(() => { - if (selectedManufacturer && selectedManufacturerData) { + if (manufacturer && selectedManufacturerData) { const hasErrors = handleErrors(); if (hasErrors) { return; } - const isNameUpdated = manufacturer.name !== selectedManufacturer.name; + const isNameUpdated = manufacturer.name !== selectedManufacturerData.name; const isURLUpdated = - manufacturer.url !== selectedManufacturer.url && + manufacturer.url !== selectedManufacturerData.url && manufacturer.url !== undefined; const isAddressLineUpdated = manufacturer.address?.address_line !== - selectedManufacturer.address.address_line; + selectedManufacturerData.address.address_line; const isTownUpdated = - manufacturer.address?.town !== selectedManufacturer.address.town; + manufacturer.address?.town !== selectedManufacturerData.address.town; const isCountyUpdated = - manufacturer.address?.county !== selectedManufacturer.address.county; + manufacturer.address?.county !== + selectedManufacturerData.address.county; const isPostcodeUpdated = manufacturer.address?.postcode !== - selectedManufacturer.address.postcode; + selectedManufacturerData.address.postcode; const isCountryUpdated = - manufacturer.address?.country !== selectedManufacturer.address.country; + manufacturer.address?.country !== + selectedManufacturerData.address.country; const isTelephoneUpdated = - manufacturer.telephone !== selectedManufacturer.telephone; + manufacturer.telephone !== selectedManufacturerData.telephone; let ManufacturerToEdit: EditManufacturer = { - id: selectedManufacturer?.id, + id: selectedManufacturerData?.id, }; isNameUpdated && (ManufacturerToEdit.name = manufacturer.name); @@ -289,7 +283,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { (ManufacturerToEdit.telephone = manufacturer.telephone); if ( - (selectedManufacturer.id && isNameUpdated) || + (selectedManufacturerData.id && isNameUpdated) || isAddressLineUpdated || isTownUpdated || isCountyUpdated || @@ -321,7 +315,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { handleClose, handleErrors, manufacturer, - selectedManufacturer, selectedManufacturerData, ]); From 166237b38595c0afe40370a8fac581c33eadaf1d Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 16:50:51 +0000 Subject: [PATCH 14/16] reverted changes that didn't work --- src/app.types.tsx | 7 +- src/manufacturer/manufacturer.component.tsx | 49 +++--- .../manufacturerDialog.component.tsx | 165 ++++++++++-------- 3 files changed, 126 insertions(+), 95 deletions(-) diff --git a/src/app.types.tsx b/src/app.types.tsx index b22a2f398..1782e4753 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -54,14 +54,17 @@ export interface EditManufacturer { id?: string | null; } -export interface Manufacturer { - id?: string; +export interface ManufacturerDetails { name: string; url?: string; address: AddAddress; telephone: string | null; } +export interface Manufacturer extends ManufacturerDetails { + id: string; +} + export interface CatalogueCategoryFormData { name: string; type: string; diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index d4825e818..899256310 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -17,22 +17,23 @@ import EditIcon from '@mui/icons-material/Edit'; import React from 'react'; import { useManufacturers } from '../api/manufacturer'; import DeleteManufacturerDialog from './deleteManufacturerDialog.component'; -import { Manufacturer } from '../app.types'; +import { Manufacturer, ManufacturerDetails } from '../app.types'; import ManufacturerDialog from './manufacturerDialog.component'; function ManufacturerComponent() { - const [manufacturer, setManufacturer] = React.useState({ - name: '', - url: undefined, - address: { - address_line: '', - town: null, - county: null, - postcode: '', - country: '', - }, - telephone: null, - }); + const [manufacturerDetails, setManufacturerDetails] = + React.useState({ + name: '', + url: undefined, + address: { + address_line: '', + town: null, + county: null, + postcode: '', + country: '', + }, + telephone: null, + }); const [editManufacturerDialogOpen, setEditManufacturerDialogOpen] = React.useState(false); @@ -45,6 +46,10 @@ function ManufacturerComponent() { const [deleteManufacturerDialog, setDeleteManufacturerDialog] = React.useState(false); + const [selectedManufacturer, setSelectedManufacturer] = React.useState< + Manufacturer | undefined + >(undefined); + const [hoveredRow, setHoveredRow] = React.useState(null); const tableHeight = `calc(100vh)-(64px + 36px +50px)`; const theme = useTheme(); @@ -68,16 +73,17 @@ function ManufacturerComponent() { setAddManufacturerDialogOpen(false)} - manufacturer={manufacturer} - onChangeManufacturerDetails={setManufacturer} + manufacturerDetails={manufacturerDetails} + onChangeManufacturerDetails={setManufacturerDetails} type="create" /> setEditManufacturerDialogOpen(false)} - manufacturer={manufacturer} - onChangeManufacturerDetails={setManufacturer} + manufacturerDetails={manufacturerDetails} + onChangeManufacturerDetails={setManufacturerDetails} type="edit" + selectedManufacturer={selectedManufacturer} /> @@ -158,8 +164,8 @@ function ManufacturerComponent() { aria-label={`Edit ${item.name} manufacturer`} onClick={() => { setEditManufacturerDialogOpen(true); - // setSelectedManufacturer(item); - setManufacturer(item); + setSelectedManufacturer(item); + setManufacturerDetails(item); }} > @@ -169,8 +175,7 @@ function ManufacturerComponent() { aria-label={`Delete ${item.name} manufacturer`} onClick={() => { setDeleteManufacturerDialog(true); - // setSelectedManufacturer(item); - setManufacturer(item); + setSelectedManufacturer(item); }} > @@ -238,7 +243,7 @@ function ManufacturerComponent() { setDeleteManufacturerDialog(false)} - manufacturer={manufacturer} + manufacturer={selectedManufacturer} /> ); diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index a49cbfdda..84046342c 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -18,6 +18,7 @@ import { Manufacturer, EditManufacturer, ErrorParsing, + ManufacturerDetails, } from '../app.types'; import { useAddManufacturer, @@ -29,9 +30,11 @@ import { AxiosError } from 'axios'; export interface ManufacturerDialogProps { open: boolean; onClose: () => void; - onChangeManufacturerDetails: (manufacturer: Manufacturer) => void; - manufacturer: Manufacturer; - + onChangeManufacturerDetails: ( + manufacturerDetails: ManufacturerDetails + ) => void; + manufacturerDetails: ManufacturerDetails; + selectedManufacturer?: Manufacturer; type: 'edit' | 'create'; } function isValidUrl(url: string) { @@ -47,8 +50,14 @@ function isValidUrl(url: string) { } function ManufacturerDialog(props: ManufacturerDialogProps) { - const { open, onClose, manufacturer, onChangeManufacturerDetails, type } = - props; + const { + open, + onClose, + manufacturerDetails, + onChangeManufacturerDetails, + selectedManufacturer, + type, + } = props; const [nameErrorMessage, setNameErrorMessage] = React.useState< string | undefined @@ -83,7 +92,9 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { const { mutateAsync: addManufacturer } = useAddManufacturer(); const { mutateAsync: editManufacturer } = useEditManufacturer(); - const { data: selectedManufacturerData } = useManufacturer(manufacturer?.id); + const { data: selectedManufacturerData } = useManufacturer( + selectedManufacturer?.id + ); const handleClose = React.useCallback(() => { onChangeManufacturerDetails({ @@ -111,22 +122,28 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { let hasErrors = false; //check url is valid - if (manufacturer.url || manufacturer.url?.trim().length === 0) { - if (!isValidUrl(manufacturer.url)) { + if ( + manufacturerDetails.url || + manufacturerDetails.url?.trim().length === 0 + ) { + if (!isValidUrl(manufacturerDetails.url)) { hasErrors = true; setUrlErrorMessage('Please enter a valid URL'); } } //check name - if (!manufacturer.name || manufacturer.name?.trim().length === 0) { + if ( + !manufacturerDetails.name || + manufacturerDetails.name?.trim().length === 0 + ) { hasErrors = true; setNameErrorMessage('Please enter a name.'); } //check address line if ( - !manufacturer.address?.address_line || - manufacturer.address.address_line.trim().length === 0 + !manufacturerDetails.address?.address_line || + manufacturerDetails.address.address_line.trim().length === 0 ) { hasErrors = true; @@ -135,8 +152,8 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { //check post code if ( - !manufacturer.address?.postcode || - manufacturer.address.postcode?.trim().length === 0 + !manufacturerDetails.address?.postcode || + manufacturerDetails.address.postcode?.trim().length === 0 ) { hasErrors = true; @@ -144,8 +161,8 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } //check country if ( - !manufacturer.address?.country || - manufacturer.address.country?.trim().length === 0 + !manufacturerDetails.address?.country || + manufacturerDetails.address.country?.trim().length === 0 ) { hasErrors = true; @@ -153,7 +170,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } return hasErrors; - }, [manufacturer]); + }, [manufacturerDetails]); const handleAddManufacturer = React.useCallback(() => { const hasErrors = handleErrors(); @@ -163,22 +180,22 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } const manufacturerToAdd: AddManufacturer = { - name: manufacturer.name, - url: manufacturer.url ?? undefined, + name: manufacturerDetails.name, + url: manufacturerDetails.url ?? undefined, address: { - address_line: manufacturer.address.address_line, - town: manufacturer.address.town ?? null, - county: manufacturer.address.county ?? null, - postcode: manufacturer.address.postcode, - country: manufacturer.address.country, + address_line: manufacturerDetails.address.address_line, + town: manufacturerDetails.address.town ?? null, + county: manufacturerDetails.address.county ?? null, + postcode: manufacturerDetails.address.postcode, + country: manufacturerDetails.address.country, }, - telephone: manufacturer.telephone ?? null, + telephone: manufacturerDetails.telephone ?? null, }; addManufacturer(manufacturerToAdd) .then((response) => handleClose()) .catch((error: AxiosError) => { - console.log(error.response?.status, manufacturer.name); + console.log(error.response?.status, manufacturerDetails.name); if (error.response?.status === 409) { setNameErrorMessage( @@ -188,57 +205,63 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { } setCatchAllError(true); }); - }, [handleErrors, manufacturer, addManufacturer, handleClose]); + }, [handleErrors, manufacturerDetails, addManufacturer, handleClose]); const handleEditManufacturer = React.useCallback(() => { - if (manufacturer && selectedManufacturerData) { + console.log(manufacturerDetails); + console.log(selectedManufacturerData); + if (manufacturerDetails && selectedManufacturerData) { const hasErrors = handleErrors(); if (hasErrors) { return; } - const isNameUpdated = manufacturer.name !== selectedManufacturerData.name; + const isNameUpdated = + manufacturerDetails.name !== selectedManufacturerData.name; const isURLUpdated = - manufacturer.url !== selectedManufacturerData.url && - manufacturer.url !== undefined; + manufacturerDetails.url !== selectedManufacturerData.url && + manufacturerDetails.url !== undefined; const isAddressLineUpdated = - manufacturer.address?.address_line !== + manufacturerDetails.address?.address_line !== selectedManufacturerData.address.address_line; const isTownUpdated = - manufacturer.address?.town !== selectedManufacturerData.address.town; + manufacturerDetails.address?.town !== + selectedManufacturerData.address.town; const isCountyUpdated = - manufacturer.address?.county !== + manufacturerDetails.address?.county !== selectedManufacturerData.address.county; const isPostcodeUpdated = - manufacturer.address?.postcode !== + manufacturerDetails.address?.postcode !== selectedManufacturerData.address.postcode; const isCountryUpdated = - manufacturer.address?.country !== + manufacturerDetails.address?.country !== selectedManufacturerData.address.country; const isTelephoneUpdated = - manufacturer.telephone !== selectedManufacturerData.telephone; + manufacturerDetails.telephone !== selectedManufacturerData.telephone; let ManufacturerToEdit: EditManufacturer = { id: selectedManufacturerData?.id, }; - isNameUpdated && (ManufacturerToEdit.name = manufacturer.name); - isURLUpdated && (ManufacturerToEdit.url = manufacturer.url); + console.log(selectedManufacturerData); + + isNameUpdated && (ManufacturerToEdit.name = manufacturerDetails.name); + isURLUpdated && (ManufacturerToEdit.url = manufacturerDetails.url); if (isAddressLineUpdated) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...manufacturer.address, - address_line: manufacturer.address?.address_line, + ...manufacturerDetails.address, + address_line: manufacturerDetails.address?.address_line, }, }; } @@ -246,8 +269,8 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...manufacturer.address, - town: manufacturer.address?.town, + ...manufacturerDetails.address, + town: manufacturerDetails.address?.town, }, }; } @@ -255,8 +278,8 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...manufacturer.address, - county: manufacturer.address?.county, + ...manufacturerDetails.address, + county: manufacturerDetails.address?.county, }, }; } @@ -264,8 +287,8 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...manufacturer.address, - postcode: manufacturer.address?.postcode, + ...manufacturerDetails.address, + postcode: manufacturerDetails.address?.postcode, }, }; } @@ -273,14 +296,14 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { ManufacturerToEdit = { ...ManufacturerToEdit, address: { - ...manufacturer.address, - country: manufacturer.address?.country, + ...manufacturerDetails.address, + country: manufacturerDetails.address?.country, }, }; } isTelephoneUpdated && - (ManufacturerToEdit.telephone = manufacturer.telephone); + (ManufacturerToEdit.telephone = manufacturerDetails.telephone); if ( (selectedManufacturerData.id && isNameUpdated) || @@ -314,7 +337,7 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { editManufacturer, handleClose, handleErrors, - manufacturer, + manufacturerDetails, selectedManufacturerData, ]); @@ -330,10 +353,10 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Name" required={true} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.name} + value={manufacturerDetails.name} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, name: event.target.value, }); setNameErrorMessage(undefined); @@ -349,10 +372,10 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="URL" required={false} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.url} + value={manufacturerDetails.url} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, url: event.target.value, }); @@ -374,12 +397,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Address Line" required={true} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.address.address_line} + value={manufacturerDetails.address.address_line} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, address: { - ...manufacturer.address, + ...manufacturerDetails.address, address_line: event.target.value, }, }); @@ -398,12 +421,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Town" required={false} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.address.town} + value={manufacturerDetails.address.town} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, address: { - ...manufacturer.address, + ...manufacturerDetails.address, town: event.target.value || null, }, }); @@ -418,12 +441,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="County" required={false} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.address.county} + value={manufacturerDetails.address.county} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, address: { - ...manufacturer.address, + ...manufacturerDetails.address, county: event.target.value || null, }, }); @@ -438,12 +461,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Country" required={true} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.address.country} + value={manufacturerDetails.address.country} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, address: { - ...manufacturer.address, + ...manufacturerDetails.address, country: event.target.value, }, }); @@ -462,12 +485,12 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Post/Zip code" required={true} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.address.postcode} + value={manufacturerDetails.address.postcode} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, address: { - ...manufacturer.address, + ...manufacturerDetails.address, postcode: event.target.value, }, }); @@ -486,10 +509,10 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { label="Telephone number" required={false} sx={{ marginLeft: '4px', my: '8px' }} // Adjusted the width and margin - value={manufacturer.telephone} + value={manufacturerDetails.telephone} onChange={(event) => { onChangeManufacturerDetails({ - ...manufacturer, + ...manufacturerDetails, telephone: event.target.value || null, }); From 9d885f663eff1fb0df7e9da85e1285fc6352e824 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 15 Nov 2023 17:00:04 +0000 Subject: [PATCH 15/16] fixed unit tests --- .../manufacturerDialog.component.test.tsx | 78 ++++++++++--------- .../manufacturerDialog.component.tsx | 4 - 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/manufacturer/manufacturerDialog.component.test.tsx b/src/manufacturer/manufacturerDialog.component.test.tsx index 13a48674e..1812a7a45 100644 --- a/src/manufacturer/manufacturerDialog.component.test.tsx +++ b/src/manufacturer/manufacturerDialog.component.test.tsx @@ -21,7 +21,7 @@ describe('Add manufacturer dialog', () => { open: true, onClose: onClose, onChangeManufacturerDetails: onChangeManufacturerDetails, - manufacturer: { + manufacturerDetails: { name: '', url: undefined, address: { @@ -55,7 +55,7 @@ describe('Add manufacturer dialog', () => { }); it('adds manufacturer correctly', async () => { - props.manufacturer = { + props.manufacturerDetails = { name: 'Manufacturer D', url: 'http://test.co.uk', address: { @@ -99,7 +99,7 @@ describe('Add manufacturer dialog', () => { }); it('duplicate manufacturer name displays warning message', async () => { - props.manufacturer = { + props.manufacturerDetails = { name: 'Manufacturer A', url: 'http://test.co.uk', address: { @@ -141,7 +141,7 @@ describe('Add manufacturer dialog', () => { }); it('invalid url displays error', async () => { - props.manufacturer = { + props.manufacturerDetails = { name: 'Manufacturer D', url: 'invalid', address: { @@ -175,7 +175,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, name: newManufacturerName, }); }); @@ -192,7 +192,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, url: newManufacturerURL, }); }); @@ -209,9 +209,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, country: newManufacturerCountry, }, }); @@ -230,9 +230,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, address_line: newManufacturerAddressLine, }, }); @@ -250,8 +250,11 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, - address: { ...props.manufacturer.address, town: newManufacturerTown }, + ...props.manufacturerDetails, + address: { + ...props.manufacturerDetails.address, + town: newManufacturerTown, + }, }); }); @@ -267,9 +270,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, county: newManufacturerCounty, }, }); @@ -287,9 +290,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, postcode: newManufacturerpostcode, }, }); @@ -308,7 +311,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, telephone: newManufacturerTelephone, }); }); @@ -342,7 +345,7 @@ describe('Add manufacturer dialog', () => { }, }; - props.manufacturer = { + props.manufacturerDetails = { name: 'test', url: 'https://test.co.uk', address: { @@ -393,7 +396,7 @@ describe('Add manufacturer dialog', () => { }, telephone: '07334893348', }, - manufacturer: { + manufacturerDetails: { name: 'Manufacturer A', url: 'http://example.com', address: { @@ -423,7 +426,7 @@ describe('Add manufacturer dialog', () => { it('Invalid url displays correct error message', async () => { props = { ...props, - manufacturer: { + manufacturerDetails: { name: 'test', url: 'invalid', address: { @@ -462,7 +465,7 @@ describe('Add manufacturer dialog', () => { it('Duplicate name displays error message', async () => { props = { ...props, - manufacturer: { + manufacturerDetails: { name: 'test_dup', url: 'https://test.co.uk', address: { @@ -505,7 +508,7 @@ describe('Add manufacturer dialog', () => { it('Required fields show error if they are whitespace or current value just removed', async () => { props = { ...props, - manufacturer: { + manufacturerDetails: { name: '', url: 'https://test.co.uk', address: { @@ -550,7 +553,7 @@ describe('Add manufacturer dialog', () => { it('CatchAllError request works correctly and displays refresh page message', async () => { props = { ...props, - manufacturer: { + manufacturerDetails: { name: 'Error 500', url: 'https://test.co.uk', address: { @@ -609,7 +612,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, name: newManufacturerName, }); }); @@ -626,7 +629,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, url: newManufacturerURL, }); }); @@ -643,9 +646,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, country: newManufacturerCountry, }, }); @@ -664,9 +667,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, address_line: newManufacturerAddressLine, }, }); @@ -684,8 +687,11 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, - address: { ...props.manufacturer.address, town: newManufacturerTown }, + ...props.manufacturerDetails, + address: { + ...props.manufacturerDetails.address, + town: newManufacturerTown, + }, }); }); @@ -701,9 +707,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, county: newManufacturerCounty, }, }); @@ -722,9 +728,9 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, address: { - ...props.manufacturer.address, + ...props.manufacturerDetails.address, postcode: newManufacturerpostcode, }, }); @@ -743,7 +749,7 @@ describe('Add manufacturer dialog', () => { }); expect(onChangeManufacturerDetails).toHaveBeenCalledWith({ - ...props.manufacturer, + ...props.manufacturerDetails, telephone: newManufacturerTelephone, }); }); diff --git a/src/manufacturer/manufacturerDialog.component.tsx b/src/manufacturer/manufacturerDialog.component.tsx index 84046342c..ee02dbf73 100644 --- a/src/manufacturer/manufacturerDialog.component.tsx +++ b/src/manufacturer/manufacturerDialog.component.tsx @@ -208,8 +208,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { }, [handleErrors, manufacturerDetails, addManufacturer, handleClose]); const handleEditManufacturer = React.useCallback(() => { - console.log(manufacturerDetails); - console.log(selectedManufacturerData); if (manufacturerDetails && selectedManufacturerData) { const hasErrors = handleErrors(); @@ -251,8 +249,6 @@ function ManufacturerDialog(props: ManufacturerDialogProps) { id: selectedManufacturerData?.id, }; - console.log(selectedManufacturerData); - isNameUpdated && (ManufacturerToEdit.name = manufacturerDetails.name); isURLUpdated && (ManufacturerToEdit.url = manufacturerDetails.url); From c53efea451e02feb466c41611bfbea4db157b2c5 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Thu, 16 Nov 2023 15:03:31 +0000 Subject: [PATCH 16/16] requested changes --- src/app.types.tsx | 6 +++--- src/manufacturer/manufacturer.component.tsx | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/app.types.tsx b/src/app.types.tsx index 1782e4753..281c6e868 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -41,14 +41,14 @@ export interface CatalogueCategory { export interface AddManufacturer { name: string; - url?: string; + url?: string | null; address: AddAddress; telephone?: string | null; } export interface EditManufacturer { name?: string; - url?: string; + url?: string | null; address?: EditAddress; telephone?: string | null; id?: string | null; @@ -56,7 +56,7 @@ export interface EditManufacturer { export interface ManufacturerDetails { name: string; - url?: string; + url?: string | null; address: AddAddress; telephone: string | null; } diff --git a/src/manufacturer/manufacturer.component.tsx b/src/manufacturer/manufacturer.component.tsx index 899256310..116bdb461 100644 --- a/src/manufacturer/manufacturer.component.tsx +++ b/src/manufacturer/manufacturer.component.tsx @@ -197,9 +197,11 @@ function ManufacturerComponent() { borderRight: '1px solid #e0e0e0', }} > - - {item.url} - + {item.url && ( + + {item.url} + + )}