feat: create legacy FieldParameters table and transfer functionality#413
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements legacy data migration functionality for the NMA_FieldParameters table. It creates the table structure, transfer logic, admin interface, and comprehensive test coverage.
Changes:
- Created database table structure with appropriate foreign key constraints and indexes for legacy field parameters data
- Implemented data transfer logic with batch processing, validation, and idempotent upsert operations
- Added read-only admin interface for viewing migrated field parameters records
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
alembic/versions/c1d2e3f4a5b6_create_nma_field_parameters.py |
Defines Alembic migration to create NMA_FieldParameters table with foreign key to ChemistrySampleInfo and appropriate indexes |
transfers/field_parameters_transfer.py |
Implements FieldParametersTransferer class with validation logic, batch processing, and upsert functionality for migrating data |
tests/test_field_parameters_legacy.py |
Provides comprehensive unit tests verifying table structure, CRUD operations, and foreign key cascade behavior |
admin/views/field_parameters.py |
Creates read-only admin view for browsing field parameters records |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5248564c78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pture_validation_error in multiple transfer files
There was a problem hiding this comment.
Looks good besides the failing tests and that sample value field question. Kelsey is out today, so if @jirhiker you have the availability to update this, please do so your transfer metric changes can be merged in.
Edit: looks like you literally just pushed changes to fix the tests so ignore that.
Somehow, in my haste, @ksmuczynski's FieldParameters PR was not successfully merged into staging. This PR is an attempt to get FieldParameters merged into staging.
this PR also resolves a number of transfer metrics issues