Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: dont crash when loading biobanks fails #4563

Merged
merged 11 commits into from
Dec 18, 2024

Conversation

chinook25
Copy link
Contributor

@chinook25 chinook25 commented Dec 13, 2024

What are the main changes you did

  • closes GCC#821

How to test

  • load an outdated biobank model
  • go to the app
  • see that instead of having an infinite spinner and console error, it now informs the user an error has occured and 0 biobanks where found.

Checklist

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@chinook25 chinook25 marked this pull request as ready for review December 17, 2024 13:21
Copy link
Member

@esthervanenckevort esthervanenckevort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally it seems to work, someone else needs to review the frontend code.
One thing that might be useful is to add an option to clear all filters if it hangs during a query. (Example: Diagnosis available, match all, and start selecting entire chapters in ICD until this message pops up)

@chinook25
Copy link
Contributor Author

chinook25 commented Dec 17, 2024

Functionally it seems to work, someone else needs to review the frontend code. One thing that might be useful is to add an option to clear all filters if it hangs during a query. (Example: Diagnosis available, match all, and start selecting entire chapters in ICD until this message pops up)

Doesn't the clear filters button stay visible (edit: it doesn't: suggest to have an new ticket for that story)? Also, I can't get it to hang the way you describe. On the test server it just executes the query. Even when I also start adding the orpha codes (though it takes a few seconds to execute and the app gets very slow).

Copy link
Contributor

@connoratrug connoratrug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments
strongly ( but not blocking) suggest to add a test

@chinook25 chinook25 merged commit 7264d23 into master Dec 18, 2024
7 checks passed
@chinook25 chinook25 deleted the fix/dont-let-errors-crash-directory branch December 18, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants