MAN-183 fix sonar code issues wip

ministryofjustice · Dec 18, 2024 · f1af067 · f1af067
1 parent 57db3d0
commit f1af067
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 58 deletions.
diff --git 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
@@ -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
@@ -108,50 +108,48 @@ const appointments: Route<void> = (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<void> = (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 })