mWater / mwater-forms

Forms controls for mWater
GNU Lesser General Public License v3.0
3 stars 5 forks source link

Survey imports with roster validation #242

Closed Autio closed 3 years ago

Autio commented 3 years ago

Survey validations seem to be giving unexpected errors during imports.

To replicate, try to import this file https://drive.google.com/file/d/14zmVkJ9Rd-_AV9fs-FGjZaqK-2a7mGQE/view?usp=sharing to this survey: https://portal.mwater.co/#/forms/92ab281b5a8c45f181f7f7c5db8382c6/share

Chris Kingsley reports:

I am running into an issue where it appears that some data validation that I have added to a roster question is preventing the upload. I've duplicated the survey and provided a subset of the sample data to show the issue. https://portal.mwater.co/#/forms/92ab281b5a8c45f181f7f7c5db8382c6/responses

When I try to upload the excel file, I get the data validation indicating that one of the dates is somehow not in the past.

If I remove that validation condition, then I run into an issue with the Head of Household question. There should be one and only one registered head of household, which is the case in the Excel. I use two validation conditions to ensure this, and both of them seem to get triggered when I upload this Excel.

I'm able to replicate this. I see this with the DoB field in the roster - the responses look fine but the importer gives an error on the validation. And removing that validation then causes the head of household question to error out, even though again the data looks fine. Something more widespread may be wrong with (advanced) validations and rosters then?

broncha commented 3 years ago

PromiseExprEvaluator in expressions requires context.rows to evaluate aggregate expressions. This advanced validation contains count where op which requires all roster rows to be passed. Which we are not doing at the moment.

AnswerValidator in forms needs to pass along context.rows to PromiseExprEvaluator But the problem is AnswerValidator is validating a single question inside a roster, and has no clue if its a part of a roster. We could build context.rows in ResponseDataValidator but what happens when there are more than 1 rosters in a survey?

@grassick any ideas? In this particular case, when evaluating advanced validations in one of the roster questions, PromiseEvxprEvaluator::evaluteAggrOp returns null for count where op

Here is one. of the 2 advanced validation expr in one of the roster question

{
    "expr": {
      "op": "<",
      "type": "op",
      "exprs": [
        {
          "expr": {
            "op": "count where",
            "type": "op",
            "exprs": [
              {
                "op": "= any",
                "type": "op",
                "exprs": [
                  {
                    "type": "field",
                    "table": "responses:92ab281b5a8c45f181f7f7c5db8382c6:roster:2c6baf0f64554bd5a49b71a1dd5b6703",
                    "column": "data:80967e7fc5ae4258861dc58cb3faa753:value"
                  },
                  {
                    "type": "literal",
                    "value": [
                      "njHfVwq"
                    ],
                    "valueType": "enumset"
                  }
                ],
                "table": "responses:92ab281b5a8c45f181f7f7c5db8382c6:roster:2c6baf0f64554bd5a49b71a1dd5b6703"
              }
            ],
            "table": "responses:92ab281b5a8c45f181f7f7c5db8382c6:roster:2c6baf0f64554bd5a49b71a1dd5b6703"
          },
          "type": "scalar",
          "joins": [
            "response",
            "data:2c6baf0f64554bd5a49b71a1dd5b6703"
          ],
          "table": "responses:92ab281b5a8c45f181f7f7c5db8382c6:roster:2c6baf0f64554bd5a49b71a1dd5b6703"
        },
        {
          "type": "literal",
          "value": 2,
          "valueType": "number"
        }
      ],
      "table": "responses:92ab281b5a8c45f181f7f7c5db8382c6:roster:2c6baf0f64554bd5a49b71a1dd5b6703"
    },
    "message": {
      "en": "Please choose only 1 head of household",
      "_base": "en"
    }
  },
grassick commented 3 years ago

That's odd... The expression first drills back up to response and then back down to the roster. Looks like the code is missing to go back into roster and get a list of rows. I'll see what I can do!

grassick commented 3 years ago

@broncha Was a bug in passing the response row to roster entries in AnswerValidator