pdokoupil / EasyStudy

6 stars 9 forks source link

Change pVal comparsion to strict to fix checkboxes in fastcompare #6

Open luk27official opened 3 months ago

luk27official commented 3 months ago

Again, this applies to ndbi021, unsure about other branches. During the implementation of a new plugin to fastcompare, I have used the ParameterType.BOOL for some parameters of the algorithm. The problem is that in the current fastcompare template file, the following snippet of code "validates" the parameters.

https://github.com/pdokoupil/EasyStudy/blob/ac6ac7e4372e2f1cf707f9d439f159853fd61ed2/server/plugins/fastcompare/templates/fastcompare_create.html#L480-L494

In JavaScript, an unchecked checkbox results let pVal = this.selectedAlgorithmsParameters[i][j].value; in pVal = false (which is correct), but then the validation fails here: result = result && pVal != null && pVal != ''; as pval != '' returns the wrong value, because false is a valid value for a checkbox (see the following image).

image

So, what all of this means is that the study cannot be created and there is no actual error description and I had no idea what's wrong. This pull request makes the comparsion strict which fixes the checkboxes problem.

To be honest, I'm not sure which of the comparsions should be !== instead of != (resp. === vs ==) in the entire file and if there are any similar issues. I don't have time to go through this, maybe using some linter would help.

pdokoupil commented 3 months ago

Right, this seems to be a bug

To be honest, I'm not sure which of the comparisons should be !== instead of != (resp. === vs ==) in the entire file and if there are any similar issues. I don't have time to go through this, maybe using some linter would help.

Honestly, these templates are messy and I do realize that. I suspect there will be many misplaced != '' (there is a somewhat complicated mix of checks for '', null, and undefined). I believe that at least all of the pVal != '' should have been pVal !== '' (there should be at least three occurrences, one for algorithm parameters, another for data loader parameters, and the last one for elicitation method parameters).

At some point (once I have enough time) I will try to revisit most of them