fix: LSDV-4726: DateTime min/max (#1348)

Add red highlight for invalid dates. Validate all (global and per-region) dates before submit.

* Check `validate()` in every tag, not only required

Labels' validate() was broken anyway, so better to remove it.
And TimeSeriesLabels` required could not be checked because that's
not a classification.

* Add `validate()` for DateTime

Check per-regions as well.
It's important to go through results and check their values.
And because values are formatted but min/max is in ISO, we have to
convert values to ISO.

* Add little comments

* Fix Required — validate only if required=true

* Fix linting

* Fix validate() result — should be true/false only

* Add test for DateTime validation; fix current one

* Fix ISO conversion with shifted timezones; fix tests

* Fix min/max checks for only=year

Author: hlomzik

e2e/codecept.conf.js

@@ -29,6 +29,10 @@ module.exports.config = {
       chromium: process.env.CHROMIUM_EXECUTABLE_PATH ? {
         executablePath: process.env.CHROMIUM_EXECUTABLE_PATH,
       } : {},
+      // to test date shifts because of timezone. (see date-time.test.js)
+      // Paris is in +1/+2 timezone, so date with midnight (00:00)
+      // will be always in previous day in ISO
+      timezoneId: 'Europe/Paris',
       trace: false,
       keepTraceForPassedTests: false,
     },

e2e/fragments/AtDateTime.ts

@@ -0,0 +1,54 @@
+const { I } = inject();
+
+module.exports = {
+  _dateInputId: '__test_date_format__',
+  /**
+   * Browsers use system date format for input[type=date], not locale,
+   * and inputs don't provide any API to get this format or user input,
+   * only the final value in ISO format.
+   * So we fill in some specific input and check the result.
+   * Depending on system settings, entered digits can go to different places,
+   * generating one of 4 possible date formats —year can only be at the beginning
+   * or at the end.
+   * We focus only on formats with full numeric day, month and year (DD and YYYY).
+   * @returns {Promise<string>} date format of y, m, d (e.g. 'ymd')
+   */
+  async detectDateFormat() {
+    // create invisible date input
+    await I.executeScript(({ id }) => {
+      const date = document.createElement('input');
+
+      date.type = 'date';
+      date.id = id;
+      Object.assign(date.style, {
+        position: 'absolute',
+        top: 0,
+        opacity: 0,
+      });
+
+      document.body.appendChild(date);
+    }, { id: this._dateInputId });
+
+    I.fillField(`#${this._dateInputId}`, '01020304');
+
+    const format = await I.executeScript(({ id }) => {
+      const date = document.getElementById(id) as HTMLInputElement;
+      const value = date.value; // always ISO format
+      
+      switch (value) {
+        case '0102-04-03': return 'ydm';
+        case '0102-03-04': return 'ymd';
+        case '0304-02-01': return 'dmy';
+        case '0304-01-02': return 'mdy';
+        default: return 'ymd';
+      }
+    }, { id: this._dateInputId });
+
+    // remove this input
+    await I.executeScript(({ id }) => {
+      (document.getElementById(id) as HTMLInputElement).remove();
+    }, { id: this._dateInputId });
+
+    return format;
+  },
+};

e2e/helpers/DateTime.ts

@@ -0,0 +1,17 @@
+/**
+ * Simple date formatter
+ * @param {string} value date in ISO format
+ * @param {string} format combinations of y, m, d (e.g. 'ymd')
+ * @returns {string} formatted date
+ */
+export const formatDateValue = (value: string, format: string) => {
+  const [y, m, d] = value.split('-');
+  let text = '';
+
+  for (const char of format) {
+    if (char === 'y') text += y;
+    if (char === 'm') text += m;
+    if (char === 'd') text += d;
+  }
+  return text;
+};

e2e/tests/date-time.test.js

@@ -1,6 +1,6 @@
-const { serialize, selectText } = require('./helpers');
-
 const assert = require('assert');
+const { formatDateValue } = require('../helpers/DateTime');
+const { serialize, selectText } = require('./helpers');
 
 Feature('Date Time');
 
@@ -12,61 +12,160 @@ const config = `<View>
   <Label value="event" background="orange"/>
 </Labels>
 <Text name="text" value="$text"/>
+<DateTime name="created" toName="text" required="true" only="date" format="%d.%m.%Y" min="1988-01-13" max="1999-12-31"/>
 <View visibleWhen="region-selected">
   <Header>Date in this fragment, required, stored as ISO date</Header>
   <DateTime name="date" toName="text" perRegion="true" only="date" required="true" format="%Y-%m-%d"/>
   <Header>Year this happened, but stored also as ISO date</Header>
-  <DateTime name="year" toName="text" perRegion="true" only="year" format="%Y-%m-%d"/>
+  <DateTime name="year" toName="text" perRegion="true" only="year" format="%Y-%m-%d" min="2020" max="2022"/>
 </View>
 </View>
 `;
 
-
-
 const data = {
   text: 'Albert Einstein (/ˈaɪnstaɪn/ EYEN-styne;[6] German: [ˈalbɛʁt ˈʔaɪnʃtaɪn] (listen); 14 March 1879 – 18 April 1955) was a German-born theoretical physicist,[7] widely acknowledged to be one of the greatest and most influential physicists of all time. Einstein is best known for developing the theory of relativity, but he also made important contributions to the development of the theory of quantum mechanics. Relativity and quantum mechanics are together the two pillars of modern physics.[3][8] His mass–energy equivalence formula E = mc2, which arises from relativity theory, has been dubbed "the world\'s most famous equation".[9] His work is also known for its influence on the philosophy of science.[10][11] He received the 1921 Nobel Prize in Physics "for his services to theoretical physics, and especially for his discovery of the law of the photoelectric effect",[12] a pivotal step in the development of quantum theory. His intellectual achievements and originality resulted in "Einstein" becoming synonymous with "genius".[13]',
 };
 
-const annotations = [
-  { label: 'birth', rangeStart: 83, rangeEnd: 96, text: '14 March 1879', date: '03141879', dateValue: '1879-03-14', year: '2022' },
-  { label: 'death', rangeStart: 99, rangeEnd: 112, text: '18 April 1955', date: '04181955', dateValue: '1955-04-18', year: '2021' },
-  { label: 'event', rangeStart: 728, rangeEnd: 755, text: '1921 Nobel Prize in Physics', date: '10101921', dateValue: '1921-10-10', year: '2020' },
+const createdDate = {
+  incorrectMin: '1988-01-12',
+  correctMin: '1988-01-13',
+  incorrectMax: '2000-01-01',
+  correctMax: '1999-12-31',
+  result: '31.12.1999',
+};
+
+const regions = [
+  { label: 'birth', rangeStart: 83, rangeEnd: 96, text: '14 March 1879', dateValue: '1879-03-14', year: '2022' },
+  { label: 'death', rangeStart: 99, rangeEnd: 112, text: '18 April 1955', dateValue: '1955-04-18', year: '2021' },
+  { label: 'event', rangeStart: 728, rangeEnd: 755, text: '1921 Nobel Prize in Physics', dateValue: '1921-10-10', year: '2020' },
 ];
 
 const params = { config, data };
 
-Scenario('Check DateTime holds state between annotations and saves result', async function({ I, LabelStudio }) {
-
+Scenario('Check DateTime holds state between annotations and saves result', async function({ I, AtDateTime, AtLabels, AtSidebar, LabelStudio }) {
   I.amOnPage('/');
 
   LabelStudio.init(params);
 
-  annotations.forEach(annotation => {
-    I.click(locate('span').withText(annotation.label));
-    I.executeScript(selectText, {
+  // detect format used for html5 date inputs
+  const format = await AtDateTime.detectDateFormat();
+
+  I.say(`System format is ${format}`);
+
+  ////// GLOBAL
+  I.say('Check validation of required global date control');
+  I.updateAnnotation();
+  I.see('DateTime "created" is required');
+  I.click('OK');
+
+  const checks = {
+    incorrect: [
+      [createdDate.incorrectMin, 'min date is 1988-01-13'],
+      [createdDate.incorrectMax, 'max date is 1999-12-31'],
+    ],
+    correct: [
+      [createdDate.correctMin],
+      [createdDate.correctMax],
+    ],
+  };
+
+  for (const [incorrect, error] of checks.incorrect) {
+    I.fillField('input[type=date]', formatDateValue(incorrect, format));
+    I.updateAnnotation();
+    I.see('is not valid');
+    I.see(error);
+    I.click('OK');
+    assert.strictEqual(await I.grabCssPropertyFrom('[type=date]', 'border-color'), 'rgb(255, 0, 0)');
+  }
+
+  for (const [correct] of checks.correct) {
+    I.fillField('input[type=date]', formatDateValue(correct, format));
+    I.updateAnnotation();
+    I.dontSee('Warning');
+    I.dontSee('is not valid');
+  }
+
+  // this value will be asserted at the end
+  I.fillField('input[type=date]', formatDateValue(createdDate.correctMax, format));
+
+  ////// PER-REGION
+  I.say('Create regions but leave dates empty');
+  for (const region of regions) {
+    AtLabels.clickLabel(region.label);
+    AtLabels.seeSelectedLabel(region.label);
+    await I.executeScript(selectText, {
       selector: '.lsf-htx-richtext',
-      rangeStart: annotation.rangeStart,
-      rangeEnd: annotation.rangeEnd,
+      rangeStart: region.rangeStart,
+      rangeEnd: region.rangeEnd,
     });
-    I.click(locate('li').withText(annotation.text));
-    I.fillField('input[type=date]', annotation.date);
-    I.selectOption('select[name=year-year]', annotation.year);
-    I.click(locate('li').withText(annotation.text));
+    I.pressKey('Escape');
+    // to prevent from double-click region handling (its timeout is 0.45s)
+    I.wait(0.5);
+  }
+
+  I.say('Try to submit and observe validation errors about per-regions');
+  I.updateAnnotation();
+  I.see('DateTime "date" is required');
+  I.click('OK');
+
+  // invalid region is selected on validation to reveal per-region control with error
+  AtSidebar.seeSelectedRegion(regions[0].label);
+  I.fillField('input[name=date-date]', formatDateValue(regions[0].dateValue, format));
+  I.updateAnnotation();
+  // next region with empty required date is selected and error is shown
+  I.see('DateTime "date" is required');
+  I.click('OK');
+  AtSidebar.seeSelectedRegion(regions[1].label);
+
+  I.say('Fill all per-region date fields and check it\'s all good');
+  regions.forEach(region => {
+    I.click(locate('li').withText(region.text));
+    I.fillField('input[name=date-date]', formatDateValue(region.dateValue, format));
+  });
+
+  I.click(locate('li').withText(regions[0].text));
+  // less than min
+  I.selectOption('select[name=year-year]', '1999');
+  assert.strictEqual('', await I.grabValueFrom('select[name=year-year]'));
+  // more than max
+  I.selectOption('select[name=year-year]', '2023');
+  assert.strictEqual('', await I.grabValueFrom('select[name=year-year]'));
+  // exactly the same as max, should be correct
+  I.selectOption('select[name=year-year]', '2022');
+  assert.strictEqual('2022', await I.grabValueFrom('select[name=year-year]'));
+  I.pressKey('Escape');
+
+  regions.forEach(region => {
+    I.click(locate('li').withText(region.text));
+    I.selectOption('select[name=year-year]', region.year);
   });
 
-  annotations.forEach(annotation => {
-    I.click(locate('li').withText(annotation.text));
-    I.seeInField('input[type=date]', annotation.dateValue);
-    I.seeInField('select[name=year-year]', annotation.year);
+  I.updateAnnotation();
+  I.dontSee('Warning');
+  I.dontSee('is required');
 
+  regions.forEach(region => {
+    I.click(locate('li').withText(region.text));
+    // important to see that per-regions change their values
+    I.seeInField('input[name=date-date]', region.dateValue);
+    I.seeInField('select[name=year-year]', region.year);
   });
+
   const results = await I.executeScript(serialize);
 
-  results.filter(result => result.value.labels).forEach((result, index) => {
-    const input = annotations[index];
-    const expected = { end: input.rangeEnd, labels: [input.label], start: input.rangeStart, text: input.text };
+  results.filter(result => result.value.start).forEach(result => {
+    const input = regions.find(reg => reg.text === result.value.text);
+    const expected = { end: input.rangeEnd, start: input.rangeStart, text: input.text };
+
+    switch (result.from_name) {
+      case 'label': expected.labels = [input.label]; break;
+      case 'date': expected.datetime = input.dateValue; break;
+      // year is formatted in config to be an ISO date
+      case 'year': expected.datetime = input.year + '-01-01'; break;
+    }
 
-    assert.deepEqual(result.value, expected);
+    assert.deepStrictEqual(result.value, expected);
   });
 
+  assert.strictEqual(results[0].value.datetime, createdDate.result);
 });

src/mixins/Required.js

@@ -7,12 +7,15 @@ const RequiredMixin = types
   })
   .actions(self => ({
     validate() {
+      if (!self.required) return true;
+
       if (self.perregion) {
         // validating when choices labeling is done per region,
         // for example choice may be required to be selected for
         // every bbox
         const objectTag = self.annotation.names.get(self.toname);
 
+        // if regions don't meet visibility conditions skip validation
         for (const reg of objectTag.regs) {
           const s = reg.results.find(s => s.from_name === self);
 

src/regions/Result.js

@@ -159,12 +159,13 @@ const Result = types
         if (label && !self.area.hasLabel(label)) return false;
       }
 
+      // picks leaf's (last item in a path) value for Taxonomy or usual Choice value for Choices
       const innerResults = (r) =>
         r.map(s => Array.isArray(s) ? s.at(-1) : s);
 
       const isChoiceSelected = () => {
         const tagName = control.whentagname;
-        const choiceValues = control.whenchoicevalue ? control.whenchoicevalue.split(',') : null;
+        const choiceValues = control.whenchoicevalue?.split(',') ?? null;
         const results = self.annotation.results.filter(r => ['choices', 'taxonomy'].includes(r.type) && r !== self);
 
         if (tagName) {

src/stores/Annotation/Annotation.js

@@ -465,16 +465,14 @@ export const Annotation = types
       let ok = true;
 
       self.traverseTree(function(node) {
-        if (node.required === true) {
-          ok = node.validate();
-          if (ok === false) {
-            ok = false;
-            return TRAVERSE_STOP;
-          }
+        ok = node.validate?.();
+        if (ok === false) {
+          return TRAVERSE_STOP;
         }
       });
 
-      return ok;
+      // should be true or false
+      return ok ?? true;
     },
 
     traverseTree(cb) {