phac-nml / irida

Canada’s Integrated Rapid Infectious Disease Analysis Platform for Genomic Epidemiology
https://irida.ca
Apache License 2.0
40 stars 31 forks source link

java.lang.IllegalArgumentException on sample/<id>/edit #28

Closed glwinsor closed 5 years ago

glwinsor commented 6 years ago

I came across this exception when trying to add a date collected to a sample. The backend is expecting a java.util.Date but is receiving a String. My gut feeling is that there should be a calendar plugin for this field in order to make sure a Date is provided but it should also be able to take a string since at many times (as in my example), only the year or year-month may be provided.

05 Apr 2018 14:11:05,279 ERROR ca.corefacility.bioinformatics.irida.ria.web.errors.ExceptionHandlerController:117 - Failed to convert value of type 'java.lang.String' to required type 'java.util.Date';

nested exception is org.springframework.core.convert.ConversionFailedException: Failed to convert from type java.lang.String to type @org.springframework.web.bind.annotation.RequestParam @org.springframework.format.annotation.DateTimeFormat java.util.Date for value '2007';

nested exception is java.lang.IllegalArgumentException: Invalid format: "2007" is too short org.springframework.web.method.annotation.MethodArgumentTypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'java.util.Date';

nested exception is org.springframework.core.convert.ConversionFailedException: Failed to convert from type java.lang.String to type @org.springframework.web.bind.annotation.RequestParam @org.springframework.format.annotation.DateTimeFormat java.util.Date for value '2007';

nested exception is java.lang.IllegalArgumentException: Invalid format: "2007" is too short

joshsadam commented 6 years ago

Hi @glwinsor I am assuming that you are adding this through the Sample > Details > Edit page. That is another one of those pages that really needs an overhaul. Using a date picker is a really great option there. A date picker usually requires day, month, and year - why are you only adding a year here?

Thanks again for all this feedback, it is really going to help make the platform so much better!

glwinsor commented 6 years ago

Hi @joshsadam That's correct - I've been adding it through Sample -> Details -> Edit. I'm currently testing BioSamples associated with a Salmonella enterica serovar Heidelberg dataset already in NCBI and all that is given for that field is 2017. NCBI doesn't enforce datatypes or minimal information with these fields and you'll often see all kinds of values which are not that great. For users who are entering this metadata for the first time, it would be great to have them enter a complete date but I wouldn't be surprised if they only have the year or year+month. That's broadly speaking for all NCBI submissions of course and I imagine that for epidemiological investigations it's possible that someone is recording the full date.

Public-Health-Bioinformatics commented 6 years ago

And additionally there are many cases where sample owners will only provide year or year+month to public repositories for data privacy reasons. If field remains a date time, then in theory we should have a second field for date precision - usually day, but could be month or year. E.g. 2017/01/01, accompanied by "year" precision, means ignore day granularity. For data entry one would want to allow just 2017. Date-pickers should allow just year or year/month entry for this kind of field. This approach allows appropriate bucketing by SQL query. Having a string field store dates is passable but then requires data conversion in order to query by date range.

tom114 commented 5 years ago

Edit page has been fixed by adding a date picker. The further discussion for allowing less date resolution can be opened in another issue if needed.