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

DPL-754 Add new field priority_level to sample table #527

Merged

Conversation

Sangeetha-Bheeman
Copy link
Contributor

Closes #

Changes proposed in this pull request:

  • ...

@Sangeetha-Bheeman Sangeetha-Bheeman changed the title Add new fields to long_read_qc_results for tol DPL-754 Add new fields to long_read_qc_results for tol Jul 24, 2023
@stevieing stevieing requested review from stevieing and emrojo July 25, 2023 11:04
Copy link
Contributor

@stevieing stevieing left a comment

Choose a reason for hiding this comment

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

Could you update the tests please? to ensure the fields are included in the json. https://github.com/sanger/unified_warehouse/blob/develop/spec/models/pac_bio_run_spec.rb

class AddFieldsToLongReadQcResults < ActiveRecord::Migration[7.0]
def change
change_table :long_read_qc_result, bulk: true do |t|
t.string :priority_level, comment: 'Priority level eg Medium, High etc'
Copy link
Contributor

Choose a reason for hiding this comment

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

Again @emrojo I think we need to check that these are relevant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Update from meeting 27/07: date_required_by and reason_for_priority are no longer required and can be removed from this schema. priority_level is not a qc_result attribute but should be moved to sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added the migration to add priority_level to sample table.

@Sangeetha-Bheeman Sangeetha-Bheeman changed the title DPL-754 Add new fields to long_read_qc_results for tol DPL-754 Add new field priority_level to sample table Aug 2, 2023
@@ -5,6 +5,7 @@ on:
branches:
- master
- develop
- dpl-754-add-fields-to-long-read-qc-results
Copy link
Contributor

Choose a reason for hiding this comment

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

This line won't be needed

emrojo and others added 2 commits September 4, 2023 15:35
Co-authored-by: Harriet Craven <harrietc52@users.noreply.github.com>
@Sangeetha-Bheeman Sangeetha-Bheeman merged commit 962773b into develop Sep 4, 2023
6 checks passed
@Sangeetha-Bheeman Sangeetha-Bheeman deleted the dpl-754-add-fields-to-long-read-qc-results branch September 4, 2023 14:52
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.

DPL-754 Traction to receive QC data endpoint from queue
4 participants