feat: improve validation and UI/UX

* fix: centered layout in home page * fix: don't mention E.164 in phone number validation E.164 is unknown to most users. Besides, the validation never checked for it anyway and the majority of past onboardings were performed with no international prefix. * feat: accept spaces in phone numbers to lower the chance for error and improve the UX. * feat: better PA picker (Fixes #158) The PA picker in home page does not flicker when typing, provides loading feedback, it's properly aligned and doesn't rely on the readonly support fields anymore. The result are shown in a more intuitive way. Also, the input is debounced and makes less calls to Elasticsearch. * fix: ability to edit the form on error (Fixes #159) When the server side validation fails, the user can now fix what's wrong right there instead of starting over. * refactor: simplify the validators (Fixes #138) * feat: code hosting URL validation The validation now rejects non HTTPS URLs and URLs that don't look like a code hosting organization. (ie. https://github.com/foobar/repo will be rejected, but https://github.com/foobar won't)
italia · May 26, 2021 · 8ee33ca · 8ee33ca
1 parent da239ba
commit 8ee33ca
Show file tree

Hide file tree

Showing 11 changed files with 313 additions and 309 deletions.
diff --git a/public/assets/css/main.css b/public/assets/css/main.css
@@ -27,7 +27,6 @@ label[for="pec"] {
 
 /* hack to have this component working with new bootstrap-italia*/
 #risultatoRicerca {
-  margin-top: 30px !important;
   max-height: 300px;
   overflow: auto;
 }

diff --git a/public/assets/js/main.js b/public/assets/js/main.js
@@ -1,59 +1,78 @@
 const ipaInput = document.querySelector('input#ipa');
-const refInput = document.querySelector('input#nomeReferente');
 const telInput = document.querySelector('input#telReferente');
 const urlInput = document.querySelector('input#url');
 const esUrl = document.querySelector('input#esUrl')
   ? document.querySelector('input#esUrl').value
   : null;
-  
+
 if (ipaInput) {
-  //validation
-  ipaInput.addEventListener('input', function () {
-    ipaInput.setCustomValidity('');
-    ipaInput.checkValidity();
-  });
+  document.querySelector('form#pa-form').addEventListener('submit', function (e) {
+    if (ipaInput.value === '') {
+      document
+        .querySelector('#ricercaAmministrazione')
+        // eslint-disable-next-line quotes
+        .setCustomValidity("Selezionare un'Amministrazione");
 
-  refInput.addEventListener('input', function () {
-    refInput.setCustomValidity('');
-    refInput.checkValidity();
+      e.preventDefault();
+    }
   });
 
   telInput.addEventListener('input', function () {
-    telInput.setCustomValidity('');
-    telInput.checkValidity();
+    const stripped = telInput.value.replace(/\s/g,'');
+
+    const regex = /^\+{0,1}\d{8,15}$/;
+    if (!regex.test(stripped)) {
+      telInput.setCustomValidity('Inserire un numero di telefono valido');
+    } else {
+      telInput.setCustomValidity('');
+    }
   });
 
   urlInput.addEventListener('input', function () {
-    urlInput.setCustomValidity('');
-    urlInput.checkValidity();
-  });
+    const trimmed = urlInput.value.trim();
 
-  ipaInput.addEventListener('invalid', function () {
-    if (ipaInput.value === '')
-      ipaInput.setCustomValidity('Selezionare un\'amministrazione dal campo Ricerca Amministrazione!');
-  });
+    try {
+      const u = new URL(trimmed);
+      if (u.protocol != 'https:') {
+        urlInput.setCustomValidity('Specificare un URL in HTTPS');
+        return;
+      }
 
-  refInput.addEventListener('invalid', function () {
-    if (refInput.value === '')
-      refInput.setCustomValidity('Specificare un referente per l\'amministrazione!');
-  });
+      if (u.host == 'github.com' || u.host == 'gitlab.com' || u.host == 'bitbucket.org') {
+        const orgsRegexp = new RegExp('^/[^/]+?/{0,1}$');
 
-  telInput.addEventListener('keyup', function () {
-    const regex = /^\+?[1-9]\d{1,14}$/;
-    if (!regex.test(telInput.value))
-      telInput.setCustomValidity('Inserire un numero E.164 Valido');
-  });
-  telInput.addEventListener('invalid', function () {
-    if (telInput.value === '')
-      telInput.setCustomValidity('Specificare un numero telefonico per il referente!');
-  });
+        if (orgsRegexp.test(u.pathname)) {
+          urlInput.setCustomValidity('');
+        } else {
+          urlInput.setCustomValidity(
+            'Deve essere un URL di una organizzazione (es. https://github.com/comune-di-reuso)'
+          );
+        }
+        return;
+      }
+    } catch(_) {
+      urlInput.setCustomValidity('Specificare un URL valido');
+      return;
+    }
 
-  urlInput.addEventListener('invalid', function () {
-    if (urlInput.value === '')
-      urlInput.setCustomValidity('Specificare un URL di riferimento!');
+    urlInput.setCustomValidity('');
   });
 }
 
+function debounce(func, wait) {
+  var timeout;
+  return function() {
+    var context = this, args = arguments;
+    clearTimeout(timeout);
+    timeout = setTimeout(
+      function() {
+        timeout = null;
+        func.apply(context, args);
+      }, wait
+    );
+  };
+}
+
 /**
  * Creating dynamic list for autocomplete
  * @param result data from remote call
@@ -63,8 +82,11 @@ function getResultElement(result) {
   return '<li class="result-item" data-ipa="' + result.ipa + '" data-pec="' + result.pec + '" ' +
     'data-description="' + result.description + '" data-office="' + result.office + '">'
     + '<a href="#">' +
-    '    <span class="autocomplete-list-text">\n' +
-    '      <span>' + result.value + '</span>\n' +
+    '    <svg class="icon">' +
+    '      <use xlink:href="/bootstrap-italia/dist/svg/sprite.svg#it-pa"></use>' +
+    '    </svg>' +
+    '    <span class="autocomplete-list-text">' +
+    '      <span>' + result.value + '</span>' +
     '    </span>' +
     '  </a>'
     + '</li>';
@@ -82,10 +104,10 @@ function modelData(result) {
     description: result.description,
     pec: result.pec,
     link: '#',
-    value: result.description
+    value: '<span class="lead">' + result.description + '</span>'
+      + '<br />'
+      + 'Codice iPA: ' + result.ipa
       + '<br />'
-      + '<b>ipa: </b>' + result.ipa
-      + ', <b>pec: </b>'
       + result.pec,
   };
 }
@@ -95,6 +117,8 @@ function modelData(result) {
  * @param data
  */
 function populateAutocompleteBox(data) {
+  $('#search-spinner').addClass('d-none');
+
   let resultsElem = $('#risultatoRicerca');
   if (data.hits.hits.length > 0) {
 
@@ -113,19 +137,13 @@ function populateAutocompleteBox(data) {
     resultsElem.addClass('autocomplete-list-show');
 
     $('.result-item').click(function (e) {
-      $('#ipa').val(this.dataset.ipa);
-      $('label[for=\'ipa\']').addClass('active');
-      $('#nomeAmministrazione').val(this.dataset.description);
-      $('label[for=\'nomeAmministrazione\']').addClass('active');
-      $('#pec').val(this.dataset.pec);
-      $('label[for=\'pec\']').addClass('active');
-      $('#risultatoRicerca').empty();
-      $('#ricercaAmministrazione').val(this.dataset.description);
-      resultsElem.removeClass('autocomplete-list-show');
-      ipaInput.setCustomValidity('');
-      ipaInput.checkValidity();
+      $('input#ipa').val(this.dataset.ipa);
+      $('input#nomeAmministrazione').val(this.dataset.description);
+      $('input#pec').val(this.dataset.pec);
+
+      $('#ricercaAmministrazione').val(this.dataset.description + ' (' + this.dataset.pec + ')');
 
-      //prevent default link action
+      resultsElem.removeClass('autocomplete-list-show');
       e.preventDefault();
     });
   }
@@ -135,13 +153,19 @@ function populateAutocompleteBox(data) {
 /**
  * setting up key listener for autocomplete input box
  */
-$('#ricercaAmministrazione').on('keyup', function (e) {
-  $('#risultatoRicerca').empty();
-
+$('#ricercaAmministrazione').on('keyup', debounce(function (e) {
   if (this.value.length < 2) {
     $('#risultatoRicerca').removeClass('autocomplete-list-show');
+
+    $('input#ipa').val('');
+    $('input#nomeAmministrazione').val('');
+    $('input#pec').val('');
+
     return;
   }
+
+  $('#search-spinner').removeClass('d-none');
+
   if (e.which == 13) {
     e.preventDefault();
   }
@@ -176,10 +200,4 @@ $('#ricercaAmministrazione').on('keyup', function (e) {
     }),
     success: populateAutocompleteBox
   });
-});
-
-//hack to make readonly fields required and validate them
-$('.readonly').on('keydown paste', function (e) {
-  e.preventDefault();
-});
-
+}, 200));
diff --git a/src/email-sent.js b/src/email-sent.js
@@ -7,39 +7,42 @@ const mustache = require('mustache');
 const jwt = require('jsonwebtoken');
 const key = require('./get-jwt-key')();
 const validator = require('./validator');
-const {VALIDATION_OK} = require('./validator-result');
-const getErrorMessage = require('./validation-error-message');
 
 module.exports = async function (request, h) {
-  const referente = request.payload.nomeReferente;
-  const refTel = request.payload.telReferente;
-  const ipa = request.payload.ipa;
+  const referente = request.payload.nomeReferente.trim();
+  const refTel = request.payload.telReferente.replace(/\s/g,'');
+  const ipa = request.payload.ipa.trim();
   const amministrazione = request.payload.description;
-  const url = request.payload.url;
+  const url = request.payload.url.trim();
   const overridePec = (config.email.overrideRecipientAddr
                    && config.email.overrideRecipientAddr.length > 1);
   const pec = overridePec ? config.email.overrideRecipientAddr : request.payload.pec;
 
   const originalPec = request.payload.pec;
 
-  // Server validation
-  let validationResultUrl = validator.url(url);
-  let validationResultPhone = validator.phone(refTel);
-  let validationCheckDups = validator.checkDups(ipa, url);
-  const validationIpaMatchesPec = await validator.ipaMatchesPec(ipa, originalPec);
-
-  if (validationResultUrl != VALIDATION_OK) {
-    let data = {errorMsg: getErrorMessage(validationResultUrl)};
-    return h.view('main-content', data, {layout: 'index'});
-  } else if (validationResultPhone != VALIDATION_OK) {
-    let data = {errorMsg: getErrorMessage(validationResultPhone)};
-    return h.view('main-content', data, {layout: 'index'});
-  } else if (validationCheckDups != VALIDATION_OK) {
-    let data = {errorMsg: getErrorMessage(validationCheckDups)};
-    return h.view('main-content', data, {layout: 'index'});
-  } else if (validationIpaMatchesPec !== VALIDATION_OK) {
-    let data = {errorMsg: getErrorMessage(validationIpaMatchesPec)};
-    return h.view('main-content', data, {layout: 'index'});
+  if (! validator.isValidCodeHostingUrl(url)) {
+    let data = { errorMsg: 'Indirizzo del repository non valido', pa: request.payload };
+    return h.view('main-content', data, { layout: 'index' });
+  }
+  if (! validator.isValidPhoneNumber(refTel)) {
+    let data = { errorMsg: 'Numero di telefono non valido', pa: request.payload };
+    return h.view('main-content', data, { layout: 'index' });
+  }
+  if (validator.isAlreadyOnboarded(ipa, url)) {
+    let data = { errorMsg: 'Questo repository è già presente', pa: request.payload };
+    return h.view('main-content', data, { layout: 'index' });
+  }
+  const ipaMatchesPec = await validator.ipaMatchesPec(ipa, originalPec);
+  if (ipaMatchesPec === null ) {
+    let data = {
+      errorMsg: 'Errore inaspettato nel controllo della validità dei dati, riprovare più tardi',
+      pa: request.payload
+    };
+    return h.view('main-content', data, { layout: 'index' });
+  }
+  if (! ipaMatchesPec) {
+    let data = { errorMsg: 'Nessun Ente con questo codice iPA e PEC', pa: request.payload };
+    return h.view('main-content', data, { layout: 'index' });
   }
 
   if (config.environment == 'dev') {

diff --git a/src/home.js b/src/home.js
@@ -4,5 +4,19 @@ const config = require('./config');
 const appVersion = require('../package.json').version;
 
 module.exports = function (request, h) {
-  return h.view('main-content', { esUrl: config.esUrl, appVersion }, { layout: 'index' });
+  return h.view(
+    'main-content', {
+      appVersion,
+      pa: {
+        esUrl: config.esUrl,
+        ipa: '',
+        pec: '',
+        url: '',
+        description: '',
+        nomeReferente: '',
+        telReferente: '',
+      },
+    }, {
+      layout: 'index'
+    });
 };
diff --git a/src/register-confirm.js b/src/register-confirm.js
@@ -1,7 +1,5 @@
 'use strict';
 const jwt = require('jsonwebtoken');
-const getErrorMessage = require('./validation-error-message.js');
-const { VALIDATION_OK } = require('./validator-result.js');
 const validator = require('./validator.js');
 const key = require('./get-jwt-key.js')();
 
@@ -16,11 +14,10 @@ module.exports = function (request, h) {
   const pec = decoded.pec;
   const amministrazione = decoded.description;
 
-  const validationCheckDups = validator.checkDups(ipa, url);
   let errorMsg = null;
 
-  if (validationCheckDups != VALIDATION_OK) {
-    errorMsg = getErrorMessage(validationCheckDups);
+  if (validator.isAlreadyOnboarded(ipa, url)) {
+    errorMsg = 'Questo repository è già presente';
   }
 
   return h.view(

diff --git a/src/registered.js b/src/registered.js
@@ -6,9 +6,7 @@ const key = require('./get-jwt-key.js')();
 const low = require('lowdb');
 const FileSync = require('lowdb/adapters/FileSync');
 
-const getErrorMessage = require('./validation-error-message');
 const validator = require('./validator');
-const { VALIDATION_OK } = require('./validator-result.js');
 const whitelistFile = 'private/data/whitelist.db.json';
 
 module.exports = function (request, h) {
@@ -30,12 +28,11 @@ module.exports = function (request, h) {
   // Set some defaults (required if your JSON file is empty)
   db.defaults({ registrati: [] }).write();
 
-  const validationCheckDups = validator.checkDups(ipa, url);
-  if (validationCheckDups != VALIDATION_OK) {
+  if (validator.isAlreadyOnboarded(ipa, url)) {
     return h.view(
       'register-confirm',
       {
-        errorMsg: getErrorMessage(validationCheckDups),
+        errorMsg: 'Questo repository è già presente',
         referente,
         refTel,
         ipa,