mxcube / mxcubeweb

MXCuBE-Web
http://mxcube.github.io/mxcubeweb/
GNU Lesser General Public License v3.0
20 stars 37 forks source link

mixup of strings and numbers in GenericTaskForm #1231

Closed elmjag closed 1 month ago

elmjag commented 2 months ago

I have added a new queue entry. When launching a task for that queue entry, you first enter parameters in it's task form. When trying to edit for example it's exposure parameter, I get the errors bellow:

Screenshot from 2024-05-02 16-18-17

It seems that UI prefils default values for some standard parameters, such as Energy and Resolution. The appear to come from the the redux state at taskForm.defaultParameters..schema.user_collection_parameters.properties.

Screenshot from 2024-05-02 16-20-26

The default values for energy and resolution are of string type. Which is wrong type according to the queue entry model. Model specify them as floats.

As far as I understand, some of the default parameters are converted to string here:

https://github.com/mxcube/mxcubeweb/blob/develop/ui/src/components/Tasks/GenericTaskForm.js#L381

or more specifically, the toFixed() function converts floats to string here:

https://github.com/mxcube/mxcubeweb/blob/develop/ui/src/components/Tasks/fields.js#L88

We need to make sure that 'must be number' error are not displayed, as long as the value in the text box is a valid number.

marcus-oscarsson commented 2 months ago

I don't have the time to look at this right away and Ill be off next week but in the mean time, just a few pointers; the toFixed function https://github.com/mxcube/mxcubeweb/blob/develop/ui/src/components/Tasks/fields.js#L88 should return a Number

The issue is perhaps that the default parameters are string for resolution and energy, exp_time and num_images looks good. Its a bit odd that they have different types, the properties of the user_collection_parameters should come from the Pydantic model, how does it look like ?

elmjag commented 1 month ago

toFixed() actually returns a string, from my web browser console:

  > const foo = Number.parseFloat("1.234").toFixed(2)
  < undefined
  > typeof(foo)
  < 'string'

I have tested converting the return value of https://github.com/mxcube/mxcubeweb/blob/develop/ui/src/components/Tasks/fields.js#L88 to a number. It seems to work fine.

If it supposed to be a number, I guess that is the solution to the this issue. I'll make a PR, perhaps next week :)

elmjag commented 1 month ago

fixed in this commit: https://github.com/mxcube/mxcubeweb/commit/fffba99371825dc89b772788981ad052c8d88723