[BUG] Fixed team feedback bugs #100
Conversation
…method - The value for `min_num_sessions` must be greater than 0 - Updated the toast message for validation age fields
Added test case for Minimum number of sessions error toast
…tring` method - Used optional chaining for `image_modals` - Fixed the MIME type for tsv files
The test uses a mock response to make sure `\n` escape character present in dataset names doesn't make it to the dataset results tsv file
There was a problem hiding this comment.
Thanks a lot for the PR @rmanaem! All looks very good, thanks for figuring out how to test these bugs! I requested changes mostly because I think we should disallow bad session-numbers (i.e. 0) to be entered in the query-form in the first place - and because I think the test assertions should be looked over again.
| @@ -92,9 +92,9 @@ export default { | |||
| tsvRows.push([ | |||
| res.dataset_portal_uri, | |||
| res.dataset_file_path, | |||
| res.dataset_name, | |||
| res.dataset_name.replace('\n', ' '), | |||
There was a problem hiding this comment.
like we said today: I think this is an appropriate fix for the specific problem. I also think that we should find a more generic way of addressing the problem without changing data that we pass to the user. E.g. by encapsulating everything in a string. Maybe we can add a TODO here with a link to the issue / PR to revisit this problem in the near future?
There was a problem hiding this comment.
Could you create an issue with a short description of what you had in mind? I prefer not to have TODOs in code as they tend to go unnoticed.
- Set `min` to 1 for minimum number of sessions field - Removed validation for minimum number of sessions field in `validateQueryForm` method - Updated `QueryForm` component test
- Updated test name - Modified test to assert if `some name` exists in the downloaded results file
|
@rmanaem I don't think you commited any changes. Did you mean to push them still or do you want me to review just based on your comments? |
|
This is weird, see branch history |
|
There we go |
Fixes #95
Fixes #96
See also neurobagel/query-tool#70
Changes proposed in this pull request:
validateQueryFormQueryFormcomponent testMRI Lab Graz: Reading-related functional activity in children with isolated\nspelling deficits and dyslexiabreak the format of thedataset-results.tsvfile #96, to replace\nescape character indataset_namewith a spaceDatasetResultsTSVe2e test for the aboveChecklist
[ENH],[BUG],[DOC],[INFRA],[MAINT])Closes #XXXXFor new features:
For bug fixes: