From f1af067383fa300c60366fa11bedf111924a36b4 Mon Sep 17 00:00:00 2001 From: Neil Mills Date: Wed, 18 Dec 2024 14:12:02 +0000 Subject: [PATCH] MAN-183 fix sonar code issues wip --- assets/scss/application.scss | 20 ++--- .../e2e/appointments/sentence.cy.ts | 2 + server/middleware/validation/appointments.ts | 90 +++++++++---------- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/assets/scss/application.scss b/assets/scss/application.scss index 604b18f6..56fe0073 100755 --- a/assets/scss/application.scss +++ b/assets/scss/application.scss @@ -62,6 +62,11 @@ dd.govuk-summary-list__value span.app-notes-author { outline: none; z-index: 10052; margin: 0; + @include govuk-font($size: 19); + border: 2px solid govuk-colour('black'); + border-top: none; + max-width: 10.8ex; + box-sizing: border-box; } .ui-timepicker-wrapper .ui-timepicker-list li { @@ -84,12 +89,13 @@ dd.govuk-summary-list__value span.app-notes-author { } .ui-timepicker-list li { - padding: 3px 0 3px 5px; cursor: pointer; white-space: nowrap; color: #000; list-style: none; margin: 0; + padding: 8px; + border-bottom: 1px solid govuk-colour('mid-grey'); } .ui-timepicker-list:hover .ui-timepicker-selected { @@ -120,15 +126,3 @@ li.ui-timepicker-selected .ui-timepicker-duration, .ui-timepicker-list li.ui-timepicker-selected.ui-timepicker-disabled { background: #f2f2f2; } - -.ui-timepicker-wrapper { - @include govuk-font($size: 19); - border: 2px solid govuk-colour('black'); - border-top: none; - max-width: 10.8ex; - box-sizing: border-box; -} -.ui-timepicker-list li { - padding: 8px; - border-bottom: 1px solid govuk-colour('mid-grey'); -} diff --git a/integration_tests/e2e/appointments/sentence.cy.ts b/integration_tests/e2e/appointments/sentence.cy.ts index 825dfd0b..73083dd7 100644 --- a/integration_tests/e2e/appointments/sentence.cy.ts +++ b/integration_tests/e2e/appointments/sentence.cy.ts @@ -27,6 +27,8 @@ const checkRequirementSentence = (type = 1) => { }) describe('Continue is clicked without selecting a requirement', () => { beforeEach(() => { + loadPage(type) + sentencePage.getElement(`#appointments-${crn}-${uuid}-sentence-2`).click() sentencePage.getSubmitBtn().click() }) it('should display the error summary box', () => { diff --git a/server/middleware/validation/appointments.ts b/server/middleware/validation/appointments.ts index acd86da0..6e2f7e5b 100644 --- a/server/middleware/validation/appointments.ts +++ b/server/middleware/validation/appointments.ts @@ -108,50 +108,48 @@ const appointments: Route = (req, res, next) => { } const validateRepeating = () => { - if (req.url.includes('/repeating')) { - const repeatingValue = req.body?.appointments?.[crn]?.[id]?.repeating - const { data } = req.session - const repeatingCountValue = getDataValue(data, ['appointments', crn, id, 'repeating-count']) - const validRepeatingCount = !Number.isNaN(parseInt(repeatingCountValue, 10)) - const appointmentDate = getDataValue(data, ['appointments', crn, id, 'date']) - const appointmentRepeatingDates = getDataValue(data, ['appointments', crn, id, 'repeating-dates']) - const oneYearFromDate = new Date(appointmentDate) - oneYearFromDate.setFullYear(oneYearFromDate.getFullYear() + 1) - let finalAppointmentDate = null - let isMoreThanAYear = false - if (appointmentRepeatingDates) { - finalAppointmentDate = appointmentRepeatingDates[appointmentRepeatingDates.length - 1] - isMoreThanAYear = new Date(finalAppointmentDate) > oneYearFromDate - } - if (!repeatingValue) { - logger.info(properties.errorMessages.appointments.repeating.log) - const text = properties.errorMessages.appointments.repeating.errors.isEmpty - const anchor = `appointments-${crn}-${id}-repeating` - errors = utils.addError(errors, { text, anchor }) - } - if (repeatingValue === 'Yes' && !req.body?.appointments?.[crn]?.[id]?.['repeating-frequency']) { - logger.info(properties.errorMessages.appointments['repeating-frequency'].log) - const text = properties.errorMessages.appointments['repeating-frequency'].errors.isEmpty - const anchor = `appointments-${crn}-${id}-repeating-frequency` - errors = utils.addError(errors, { text, anchor }) - } - if (repeatingValue === 'Yes' && !req.body?.appointments?.[crn]?.[id]?.['repeating-count']) { - logger.info(properties.errorMessages.appointments['repeating-count'].log) - const text = properties.errorMessages.appointments['repeating-count'].errors.isEmpty - const anchor = `appointments-${crn}-${id}-repeating-count` - errors = utils.addError(errors, { text, anchor }) - } - if (repeatingCountValue && !validRepeatingCount) { - logger.info(properties.errorMessages.appointments['repeating-count'].log) - const text = properties.errorMessages.appointments['repeating-count'].errors.isInvalid - const anchor = `appointments-${crn}-${id}-repeating-count` - errors = utils.addError(errors, { text, anchor }) - } - if (isMoreThanAYear) { - const textFrequency = properties.errorMessages.appointments['repeating-frequency'].errors.isMoreThanAYear - const anchor = `appointments-${crn}-${id}-repeating-frequency` - errors = utils.addError(errors, { text: textFrequency, anchor }) - } + const repeatingValue = req.body?.appointments?.[crn]?.[id]?.repeating + const { data } = req.session + const repeatingCountValue = getDataValue(data, ['appointments', crn, id, 'repeating-count']) + const validRepeatingCount = !Number.isNaN(parseInt(repeatingCountValue, 10)) + const appointmentDate = getDataValue(data, ['appointments', crn, id, 'date']) + const appointmentRepeatingDates = getDataValue(data, ['appointments', crn, id, 'repeating-dates']) + const oneYearFromDate = new Date(appointmentDate) + oneYearFromDate.setFullYear(oneYearFromDate.getFullYear() + 1) + let finalAppointmentDate = null + let isMoreThanAYear = false + if (appointmentRepeatingDates) { + finalAppointmentDate = appointmentRepeatingDates[appointmentRepeatingDates.length - 1] + isMoreThanAYear = new Date(finalAppointmentDate) > oneYearFromDate + } + if (!repeatingValue) { + logger.info(properties.errorMessages.appointments.repeating.log) + const text = properties.errorMessages.appointments.repeating.errors.isEmpty + const anchor = `appointments-${crn}-${id}-repeating` + errors = utils.addError(errors, { text, anchor }) + } + if (repeatingValue === 'Yes' && !req.body?.appointments?.[crn]?.[id]?.['repeating-frequency']) { + logger.info(properties.errorMessages.appointments['repeating-frequency'].log) + const text = properties.errorMessages.appointments['repeating-frequency'].errors.isEmpty + const anchor = `appointments-${crn}-${id}-repeating-frequency` + errors = utils.addError(errors, { text, anchor }) + } + if (repeatingValue === 'Yes' && !req.body?.appointments?.[crn]?.[id]?.['repeating-count']) { + logger.info(properties.errorMessages.appointments['repeating-count'].log) + const text = properties.errorMessages.appointments['repeating-count'].errors.isEmpty + const anchor = `appointments-${crn}-${id}-repeating-count` + errors = utils.addError(errors, { text, anchor }) + } + if (repeatingCountValue && !validRepeatingCount) { + logger.info(properties.errorMessages.appointments['repeating-count'].log) + const text = properties.errorMessages.appointments['repeating-count'].errors.isInvalid + const anchor = `appointments-${crn}-${id}-repeating-count` + errors = utils.addError(errors, { text, anchor }) + } + if (isMoreThanAYear) { + const textFrequency = properties.errorMessages.appointments['repeating-frequency'].errors.isMoreThanAYear + const anchor = `appointments-${crn}-${id}-repeating-frequency` + errors = utils.addError(errors, { text: textFrequency, anchor }) } } @@ -160,7 +158,9 @@ const appointments: Route = (req, res, next) => { validateSentence() validateLocation() validateDateTime() - validateRepeating() + if (req.url.includes('/repeating')) { + validateRepeating() + } if (errors) { res.locals.errors = errors return res.render(render, { errors, ...localParams })