DPL-717: Don't filter for fit to pick #753
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #753 +/- ##
===========================================
- Coverage 92.33% 92.28% -0.05%
===========================================
Files 107 107
Lines 3287 3320 +33
Branches 261 267 +6
===========================================
+ Hits 3035 3064 +29
- Misses 208 210 +2
- Partials 44 46 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
So that they can be joined with a + operator during tests
lighthouse/routes/common/plates.py
Outdated
| body = create_post_body(barcode, plate_type, fit_to_pick_samples) | ||
| plate_configs = app.config["SS_PLATE_CONFIG"] | ||
| if plate_type not in plate_configs: | ||
| return bad_request(f"POST request 'type' must be from the list: {', '.join(plate_configs)}") |
There was a problem hiding this comment.
plate_configs is a dict, not a list
There was a problem hiding this comment.
Yes perhaps I should be more explicit, though in Python, any iteration of a dictionary without specifying a method on it just iterates the keys.
| """ | ||
| try: | ||
| samples_collection: Collection = cast(Eve, app).data.driver.db.samples | ||
| query = {FIELD_LH_SOURCE_PLATE_UUID: source_plate_uuid} |
There was a problem hiding this comment.
maybe throw exception if source_plate_uuid is null, because the query {"lh_source_plate_uuid: null} is valid, but we dont want that to happen.
There was a problem hiding this comment.
It can't happen because the type hinting explicitly specifies that the value is a string that cannot be None but I will add the check because I think this is something that could easily slip up in future. I can't add a test for it because mypy will complain that I'm passing a None value to a method that doesn't accept None.
When I remove the None check before calling this method, mypy fails the checks with the following:
| with app.app_context(): | ||
| result = get_all_samples_for_source_plate("source plate uuid does not exist") | ||
|
|
||
| assert result == [] |
There was a problem hiding this comment.
We can add a test when the argument is None
There was a problem hiding this comment.
I've done this and pushed up the change. I think we probably should revert that last commit though. Look at it and tell me what you think. mypy is already protecting from this issue occurring through the typing system.
Note that we are ignoring the type hinting which already prevents a None value being submitted, but this belt and braces. Because mypy won't normally let you pass None as a value, we have to suppress mypy during testing so that we can force a None value in.
Closes #740
Changes proposed in this pull request: