Merge pull request #672 from kids-first/allow-nullables-sample-rel

♻️ Allow parent or child sample to be null

dataservice/api/sample_relationship/models.py

@@ -32,18 +32,23 @@ class SampleRelationship(db.Model, Base):
     parent_id = db.Column(
         KfId(),
         db.ForeignKey('sample.kf_id'),
-        nullable=False,
+        nullable=True,
         doc='kf_id of one sample in the relationship')
 
     child_id = db.Column(
         KfId(),
         db.ForeignKey('sample.kf_id'),
-        nullable=False,
+        nullable=True,
         doc='kf_id of the other sample in the relationship')
 
-    external_parent_id = db.Column(db.Text())
-
-    external_child_id = db.Column(db.Text())
+    external_parent_id = db.Column(
+        db.Text(),
+        doc='Name given to parent sample by contributor'
+    )
+    external_child_id = db.Column(
+        db.Text(),
+        doc='Name given to child sample by contributor'
+    )
 
     notes = db.Column(
         db.Text(),
@@ -119,35 +124,47 @@ def validate_sample_relationship(target):
     if not target:
         return
 
-    # Get samples in relationship by id
-    parent = Sample.query.get(target.parent_id)
-    child = Sample.query.get(target.child_id)
+    parent_id = target.parent_id
+    child_id = target.child_id
 
-    # Check that both are existing samples
-    if not (parent and child):
+    # Check that at least 1 sample ID is non-null
+    if not (parent_id or child_id):
         raise DatabaseValidationError(
             SampleRelationship.__tablename__,
             "modify",
-            "Either parent sample or child sample or both does not exist"
+            "Both parent_id and child_id cannot be null"
         )
 
+    # If a parent_id or child_id is not-null then check that the ID
+    # refers to an existing sample
+    for sample_id in [parent_id, child_id]:
+        if sample_id:
+            sample = Sample.query.get(sample_id)
+            if not sample:
+                raise DatabaseValidationError(
+                    SampleRelationship.__tablename__,
+                    "modify",
+                    f"Either parent sample {target.parent_id} or "
+                    f"child sample {target.child_id} or both does not exist"
+                )
+
     # Check for reverse relation
     sr = SampleRelationship.query.filter_by(
-        parent_id=child.kf_id,
-        child_id=parent.kf_id,
+        parent_id=child_id,
+        child_id=parent_id,
     ).first()
 
     if sr:
         raise DatabaseValidationError(
             SampleRelationship.__tablename__,
             "modify",
-            f"Reverse relationship, Parent: {target.parent.kf_id} -> Child: "
-            f"{target.child.kf_id}, not allowed since the SampleRelationship, "
+            f"Reverse relationship, Parent: {parent_id} -> Child: "
+            f"{child_id}, not allowed since the SampleRelationship, "
             f"Parent: {sr.parent_id} -> Child: {sr.child_id}, already exists"
         )
 
     # Check for parent = child
-    if target.parent_id == target.child_id:
+    if parent_id == child_id:
         raise DatabaseValidationError(
             SampleRelationship.__tablename__,
             "modify",

migrations/versions/c2f77debb22d_allow_null_parent_sample.py

@@ -0,0 +1,45 @@
+"""
+1.23.0 Allow parent or child sample in sample relationship to be null
+
+This allows us to enter a row where the parent is null and child is not which
+represents a root of a sample tree
+
+Validation ensures that you cannot enter a sample relationship where both
+the parent and child sample in the relationship are null
+
+Revision ID: c2f77debb22d
+Revises: f07d043290d0
+Create Date: 2024-06-11 13:05:50.923693
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'c2f77debb22d'
+down_revision = 'f07d043290d0'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('sample_relationship', 'child_id',
+                    existing_type=sa.VARCHAR(length=11),
+                    nullable=True)
+    op.alter_column('sample_relationship', 'parent_id',
+                    existing_type=sa.VARCHAR(length=11),
+                    nullable=True)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('sample_relationship', 'parent_id',
+                    existing_type=sa.VARCHAR(length=11),
+                    nullable=False)
+    op.alter_column('sample_relationship', 'child_id',
+                    existing_type=sa.VARCHAR(length=11),
+                    nullable=False)
+    # ### end Alembic commands ###

tests/sample_relationship/test_sample_relationship_models.py

@@ -144,6 +144,55 @@ def test_foreign_key_constraint(self):
             db.session.commit()
         assert "does not exist" in str(e.value)
 
+    def test_null_parent_allowed(self):
+        """
+        Test that we can represent a root of a sample tree: a null parent
+        with a non-null child
+        """
+        _, rels = create_relationships()
+
+        pid = rels[0].parent.participant_id
+        sample = Sample(
+            participant_id=pid, external_id="foo"
+        )
+        db.session.add(sample)
+        db.session.commit()
+
+        # Create sample relationship
+        data = {
+            "parent_id": None,
+            "child_id": sample.kf_id,
+            "external_parent_id": None,
+            "external_child_id": "foo",
+        }
+        r = SampleRelationship(**data)
+
+        # Add to db
+        db.session.add(r)
+        db.session.commit()
+
+        assert len(rels) + 1 == SampleRelationship.query.count()
+
+    def test_null_parent_and_child_not_allowed(self):
+        """
+        Test that we cannot create a relationship with a null parent and
+        child id
+        """
+        # Create sample relationship
+        data = {
+            "parent_id": None,
+            "child_id": None,
+            "external_parent_id": None,
+            "external_child_id": None,
+        }
+        r = SampleRelationship(**data)
+
+        # Add to db
+        db.session.add(r)
+        with pytest.raises(DatabaseValidationError) as e:
+            db.session.commit()
+        assert "cannot be null" in str(e.value)
+
     def test_query_all_relationships(self):
         """
         Test the class method query_all_relationships on SampleRelationship