Extend kit creation code, barcodes.kit table for daklapack kits #356
Conversation
… a kit with it, using refactored innards of create_kits. Update tests accordingly
…code that creates kits to fill it (with either explicitly supplied values or otherwise with kit uuids). updated tests accordingly
…e-api into daklapack_api � Conflicts: � microsetta_private_api/db/patches/0076.sql
…-private-api into daklapack_db_change
…e-api into daklapack_db_change
| ALTER TABLE barcodes.kit RENAME COLUMN fedex_tracking TO outbound_fedex_tracking; | ||
| ALTER TABLE barcodes.kit ADD COLUMN inbound_fedex_tracking varchar NULL; | ||
| ALTER TABLE barcodes.kit ADD COLUMN box_id varchar NULL; | ||
| UPDATE barcodes.kit SET box_id = kit_uuid; |
There was a problem hiding this comment.
Should we also drop the column kit_uuid?
There was a problem hiding this comment.
Not sure I understand the thinking here. In future, some kits will have a box_id that is distinct from their kit_uuid; in those cases, that box_id will be generated by Daklapack. Nonetheles, for all kits we definitely need a unique kit id that we can use as a primary key, and I argue it would be too risky to use an externally generated id (like the Daklapack-created box_id) as our internal primary key. Therefore I think we need a kit_uuid for each kit even in cases where that kit also has a (different) box_id.
| kit_names = self._generate_novel_kit_names(number_of_kits, kit_prefix) | ||
| kit_name_and_barcode_tuples_list, new_barcodes = \ | ||
| self._generate_novel_barcodes( |
There was a problem hiding this comment.
@dhakim87, do you think these should use lock?
There was a problem hiding this comment.
Not sure- I don't think they need it? What resource do you think needs to be locked/what other operation would this conflict with?
I think the postgres lock mechanism should be thought of as using synchronized/lock blocks in java or C#: we need lock access to particular resource when there are multiple workflows that all need to read and then write based on what was snapshotted, and all access to that resource must be locked.
There was a problem hiding this comment.
Okay, so assuming we rarely if ever expect concurrent transactions, and the likelihood of a conflict in ID generation is low, then we should be good
| "(kit_id) " | ||
| "VALUES (%s)", [(n, ) for n in names]) | ||
| "(kit_uuid, kit_id, box_id) " | ||
| "VALUES (%s, %s, %s)", barcode_kit_inserts) |
There was a problem hiding this comment.
Do the fedex tracking numbers come later?
There was a problem hiding this comment.
@wasade Good catch! I had failed to unshelve one set of changes for this PR [facepalm], which bring in address and fedex tracking numbers ... they are here now :)
Refactor most of
create_kitsfunction inadmin_repo.pyinto private functions that can be used by bothcreate_kitsand a newcreate_kit(note singular :) function that takes in barcodes, box id, and kit name specified by Daklapack (instead of generating them internally). Extendbarcodes.kittable to contain a box id separate from a kit id as well as two different fedex tracking numbers (instead of one).