-
Notifications
You must be signed in to change notification settings - Fork 34
Add validations for location reports to limit labware count #5299
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
base: develop
Are you sure you want to change the base?
Add validations for location reports to limit labware count #5299
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5299 +/- ##
===========================================
+ Coverage 87.06% 87.15% +0.08%
===========================================
Files 1447 1452 +5
Lines 32289 32544 +255
Branches 3375 3413 +38
===========================================
+ Hits 28114 28364 +250
- Misses 4162 4167 +5
Partials 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tions-for-location-reports
…tions-for-location-reports
andrewsparkes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just asking about the limit number. Else looks good.
| def search_for_labware_by_selection | ||
| params = { faculty_sponsor_ids:, study_id:, start_date:, end_date:, plate_purpose_ids:, barcodes: } | ||
| count = Labware.search_for_count_of_labware(params) | ||
| if count > configatron.fetch(:location_reports_fetch_count_max, 500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 500 a number discussed with the users?
I would think, as an example, that a standard -80 freezer will hold significantly more than 500 labwares.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe find out what location(s) they typically want a report on and check the counts in those.
Closes #5205
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version