Merge pull request #604 from kids-first/visibility-reason

✨ Add visibility reason and comment columns

dataservice/api/common/model.py

@@ -14,6 +14,11 @@
 COMMON_ENUM = {"Not Reported", "Not Applicable", "Not Allowed To Collect",
                "Not Available", "Reported Unknown"}
 
+VISIBILITY_REASON_ENUM = {
+    "Ready For Release", "Pre-Release", "Sample Issue",
+    "Consent Hold", "Sequencing Quality Issue", "Other", "Unknown"
+}
+
 
 class KfId(types.TypeDecorator):
     """
@@ -203,3 +208,11 @@ class Base(IDMixin, TimestampMixin):
                         nullable=False,
                         server_default='true',
                         doc='Flags visibility of data from the dataservice')
+    visibility_reason = db.Column(
+        db.Text(),
+        doc='Gives justification for the value in the visible column'
+    )
+    visibility_comment = db.Column(
+        db.Text(),
+        doc='Additional details for the visibility reason'
+    )

dataservice/api/common/schemas.py

@@ -15,7 +15,9 @@
 from flask_marshmallow import Schema
 from dataservice.api.common.pagination import Pagination, After
 from dataservice.api.common.validation import validate_kf_id
+from dataservice.api.common.model import VISIBILITY_REASON_ENUM
 from dataservice.extensions import db
+
 AVAILABILITY_ENUM = {'Immediate Download',
                      'Cold Storage'}
 
@@ -55,6 +57,14 @@ def wrap_pre(self, data, many):
     def valid(self, value):
         validate_kf_id(self.Meta.model.__prefix__, value)
 
+    @validates('visibility_reason')
+    def validate_visibility_reason(self, value):
+        if value and (value not in VISIBILITY_REASON_ENUM):
+            raise ValidationError(
+                'Not a valid choice. Must be one of: {}'.format(
+                    ', '.join([str(item) for item in VISIBILITY_REASON_ENUM]))
+            )
+
     @post_dump(pass_many=True)
     def wrap_envelope(self, data, many):
         """

migrations/versions/16e588cc6b85_.py

@@ -0,0 +1,155 @@
+"""
+Add visibility_reason and visibility_comment columns to capture
+justification for visibility value
+
+Revision ID: 16e588cc6b85
+Revises: 475214ed802d
+Create Date: 2023-03-16 13:15:59.929140
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '16e588cc6b85'
+down_revision = '475214ed802d'
+branch_labels = None
+depends_on = None
+
+
+def add_visibility_reason():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('alias_group', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('biospecimen', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('biospecimen_diagnosis', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('biospecimen_genomic_file', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('cavatica_app', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('diagnosis', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('family', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('family_relationship', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('genomic_file', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('investigator', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('outcome', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('participant', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('phenotype', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('read_group', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('read_group_genomic_file', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('sequencing_center', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('sequencing_experiment', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('sequencing_experiment_genomic_file', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    op.add_column('study', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('study_file', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('task', sa.Column('visibility_reason',
+                  sa.Text(), nullable=True))
+    op.add_column('task_genomic_file', sa.Column(
+        'visibility_reason', sa.Text(), nullable=True))
+    # ### end Alembic commands ###
+
+
+def remove_visibility_reason():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('task_genomic_file', 'visibility_reason')
+    op.drop_column('task', 'visibility_reason')
+    op.drop_column('study_file', 'visibility_reason')
+    op.drop_column('study', 'visibility_reason')
+    op.drop_column('sequencing_experiment_genomic_file', 'visibility_reason')
+    op.drop_column('sequencing_experiment', 'visibility_reason')
+    op.drop_column('sequencing_center', 'visibility_reason')
+    op.drop_column('read_group_genomic_file', 'visibility_reason')
+    op.drop_column('read_group', 'visibility_reason')
+    op.drop_column('phenotype', 'visibility_reason')
+    op.drop_column('participant', 'visibility_reason')
+    op.drop_column('outcome', 'visibility_reason')
+    op.drop_column('investigator', 'visibility_reason')
+    op.drop_column('genomic_file', 'visibility_reason')
+    op.drop_column('family_relationship', 'visibility_reason')
+    op.drop_column('family', 'visibility_reason')
+    op.drop_column('diagnosis', 'visibility_reason')
+    op.drop_column('cavatica_app', 'visibility_reason')
+    op.drop_column('biospecimen_genomic_file', 'visibility_reason')
+    op.drop_column('biospecimen_diagnosis', 'visibility_reason')
+    op.drop_column('biospecimen', 'visibility_reason')
+    op.drop_column('alias_group', 'visibility_reason')
+    # ### end Alembic commands ###
+
+def add_visibility_comment():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('alias_group', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('biospecimen', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('biospecimen_diagnosis', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('biospecimen_genomic_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('cavatica_app', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('diagnosis', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('family', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('family_relationship', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('genomic_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('investigator', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('outcome', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('participant', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('phenotype', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('read_group', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('read_group_genomic_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('sequencing_center', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('sequencing_experiment', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('sequencing_experiment_genomic_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('study', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('study_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('task', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    op.add_column('task_genomic_file', sa.Column('visibility_comment', sa.Text(), nullable=True))
+    # ### end Alembic commands ###
+
+
+def remove_visibility_comment():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('task_genomic_file', 'visibility_comment')
+    op.drop_column('task', 'visibility_comment')
+    op.drop_column('study_file', 'visibility_comment')
+    op.drop_column('study', 'visibility_comment')
+    op.drop_column('sequencing_experiment_genomic_file', 'visibility_comment')
+    op.drop_column('sequencing_experiment', 'visibility_comment')
+    op.drop_column('sequencing_center', 'visibility_comment')
+    op.drop_column('read_group_genomic_file', 'visibility_comment')
+    op.drop_column('read_group', 'visibility_comment')
+    op.drop_column('phenotype', 'visibility_comment')
+    op.drop_column('participant', 'visibility_comment')
+    op.drop_column('outcome', 'visibility_comment')
+    op.drop_column('investigator', 'visibility_comment')
+    op.drop_column('genomic_file', 'visibility_comment')
+    op.drop_column('family_relationship', 'visibility_comment')
+    op.drop_column('family', 'visibility_comment')
+    op.drop_column('diagnosis', 'visibility_comment')
+    op.drop_column('cavatica_app', 'visibility_comment')
+    op.drop_column('biospecimen_genomic_file', 'visibility_comment')
+    op.drop_column('biospecimen_diagnosis', 'visibility_comment')
+    op.drop_column('biospecimen', 'visibility_comment')
+    op.drop_column('alias_group', 'visibility_comment')
+    # ### end Alembic commands ###
+
+def upgrade():
+    add_visibility_reason()
+    add_visibility_comment()
+
+def downgrade():
+    remove_visibility_reason()
+    remove_visibility_comment()

tests/test_api.py

@@ -9,6 +9,7 @@
     ENTITY_PARAMS,
     _add_foreign_keys
 )
+from dataservice.api.common.model import VISIBILITY_REASON_ENUM
 
 
 class TestAPI:
@@ -294,6 +295,48 @@ def test_bad_input(self, client, entities, endpoint, invalid_params,
         assert body['_status']['code'] == 400
         assert 'could not {} '.format(action) in body['_status']['message']
 
+    @pytest.mark.parametrize('reason', VISIBILITY_REASON_ENUM)
+    @pytest.mark.parametrize('endpoint',
+                             [endpoint for endpoint in ENDPOINTS])
+    def test_visibility_reason(self, client, entities, endpoint, reason):
+        """ Tests inputs to visibility_reason field """
+        # Setup inputs
+        inputs = ENTITY_PARAMS['fields'][endpoint].copy()
+        model_cls = ENDPOINT_ENTITY_MAP.get(endpoint)
+        entity = entities.get(model_cls)[0]
+        _add_foreign_keys(inputs, entity)
+
+        # Send request with bad value
+        kf_id = entity.kf_id
+        url = '{}/{}'.format(endpoint, kf_id)
+
+        inputs = {"visibility_reason": reason}
+        resp = client.patch(url, data=json.dumps(inputs),
+                            headers={'Content-Type': 'application/json'})
+
+        body = json.loads(resp.data.decode('utf-8'))
+        assert body['_status']['code'] == 200
+
+    @pytest.mark.parametrize('endpoint',
+                             [endpoint for endpoint in ENDPOINTS])
+    def test_bad_visibility_reason(self, client, entities, endpoint):
+        """ Tests bad inputs to visibility_reason field """
+        # Setup inputs
+        inputs = ENTITY_PARAMS['fields'][endpoint].copy()
+        model_cls = ENDPOINT_ENTITY_MAP.get(endpoint)
+        entity = entities.get(model_cls)[0]
+        _add_foreign_keys(inputs, entity)
+        url = endpoint
+
+        # Send request with bad value
+        inputs.update({"visibility_reason": "foobar"})
+        resp = client.post(url, data=json.dumps(inputs),
+                           headers={'Content-Type': 'application/json'})
+
+        body = json.loads(resp.data.decode('utf-8'))
+        assert body['_status']['code'] == 400
+        assert 'Must be one of' in body['_status']['message']
+
     @pytest.mark.parametrize('method', ['POST'])
     @pytest.mark.parametrize('endpoint, field',
                              [