⚡ Only patch descendant visibility if needed (also adds dry_run)

README.md

@@ -115,7 +115,7 @@ d2 = find_descendants_by_filter(
 # List genomic files with contributions from these biospecimens that also have
 # contributions from biospecimens that aren't these
 promiscuous_gs = find_gfs_with_extra_contributors(
-  db_url or api_url, ["BS_11111111", "BS_22222222", "BS_33333333"]
+    db_url or api_url, ["BS_11111111", "BS_22222222", "BS_33333333"]
 )
 ```
 
@@ -124,7 +124,7 @@ promiscuous_gs = find_gfs_with_extra_contributors(
 # Genomic files receive the specified acl.
 # This and unhide_descendants_by_filter are not symmetrical.
 hide_descendants_by_filter(
-  api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": True}, gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url
+    api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": True}, gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url, dry_run=False
 )
 ```
 
@@ -133,7 +133,9 @@ hide_descendants_by_filter(
 # genomic files with additional contributing specimens if those specimens will remain
 # hidden.
 # This and hide_descendants_by_filter are not symmetrical.
-unhide_descendants_by_filter(api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": False}, db_url=db_url)
+unhide_descendants_by_filter(
+    api_url, "families", {"study_id": "SD_DYPMEHHF", "visible": False}, db_url=db_url, dry_run=False
+)
 ```
 
 `descendants.py` also provides wrapper functions hiding/unhiding descendants by KF ID(s):
@@ -143,15 +145,17 @@ unhide_descendants_by_filter(api_url, "families", {"study_id": "SD_DYPMEHHF", "v
 # specified acl.
 # This and unhide_descendants_by_kfids are not symmetrical.
 hide_descendants_by_kfids(
-  api_url, "families", ["FM_12345678", "FM_87654321"], gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url
+    api_url, "families", ["FM_12345678", "FM_87654321"], gf_acl=["SD_DYPMEHHF", "phs001436.c999"], db_url=db_url, dry_run=False
 )
 ```
 
 ```Python
 # Unhide these families and all of their descendants except for genomic files with
 # additional contributing specimens if those specimens will remain hidden.
 # This and hide_descendants_by_kfids are not symmetrical.
-unhide_descendants_by_kfids(api_url, "families", ["FM_12345678", "FM_87654321"], db_url=db_url)
+unhide_descendants_by_kfids(
+    api_url, "families", ["FM_12345678", "FM_87654321"], db_url=db_url, dry_run=False
+)
 ```
 
 #### [dataservice/patch.py](kf_utils/dataservice/patch.py) - Rapid patch submission
@@ -167,16 +171,16 @@ host = "http://localhost:5000"
 
 # Patch the given KFIDs with the given changes
 patches = {
-  "PT_12345678": {"visible": True},
-  "BS_99999999": {"participant_id": "PT_12345678", "visible": False}
+    "PT_12345678": {"visible": True},
+    "BS_99999999": {"participant_id": "PT_12345678", "visible": False}
 }
 send_patches(host, patches)
 ```
 
 ```Python
 things = [
-  {"kf_id": "PT_11223344", "visible": True},
-  {"kf_id": "PT_22334455", "visible": False}
+    {"kf_id": "PT_11223344", "visible": True},
+    {"kf_id": "PT_22334455", "visible": False}
 ]
 
 def my_patch_func(thing):

kf_utils/dataservice/descendants.py

@@ -7,8 +7,7 @@
 
 import psycopg2
 import psycopg2.extras
-
-from kf_utils.dataservice.patch import hide_kfids, unhide_kfids
+from kf_utils.dataservice.patch import hide_entities, unhide_entities
 from kf_utils.dataservice.scrape import yield_entities
 
 
@@ -431,7 +430,7 @@ def find_descendants_by_filter(
 
 
 def hide_descendants_by_filter(
-    api_url, endpoint, filter, gf_acl=None, db_url=None
+    api_url, endpoint, filter, gf_acl=None, db_url=None, dry_run=False
 ):
     """
     Be aware that this and unhide_descendants_by_filter are not symmetrical.
@@ -442,13 +441,22 @@ def hide_descendants_by_filter(
     change.
     """
     desc = find_descendants_by_filter(
-        api_url, endpoint, filter, False, db_url=db_url
+        api_url,
+        endpoint,
+        filter,
+        ignore_gfs_with_hidden_external_contribs=False,
+        kfids_only=False,
+        db_url=db_url,
     )
-    for v in desc.values():
-        hide_kfids(api_url, v, gf_acl)
+    changed = []
+    for es in desc.values():
+        changed.extend(hide_entities(api_url, es.values(), gf_acl, dry_run))
+    return changed
 
 
-def unhide_descendants_by_filter(api_url, endpoint, filter, db_url=None):
+def unhide_descendants_by_filter(
+    api_url, endpoint, filter, db_url=None, dry_run=False
+):
     """
     Be aware that this and hide_descendants_by_filter are not symmetrical.
 
@@ -458,14 +466,21 @@ def unhide_descendants_by_filter(api_url, endpoint, filter, db_url=None):
     change.
     """
     desc = find_descendants_by_filter(
-        api_url, endpoint, filter, True, db_url=db_url
+        api_url,
+        endpoint,
+        filter,
+        ignore_gfs_with_hidden_external_contribs=True,
+        kfids_only=False,
+        db_url=db_url,
     )
-    for v in desc.values():
-        unhide_kfids(api_url, v)
+    changed = []
+    for es in desc.values():
+        changed.extend(unhide_entities(api_url, es.values(), dry_run))
+    return changed
 
 
 def hide_descendants_by_kfids(
-    api_url, endpoint, kfids, gf_acl=None, db_url=None
+    api_url, endpoint, kfids, gf_acl=None, db_url=None, dry_run=False
 ):
     """
     Be aware that this and unhide_descendants_by_kfids are not symmetrical.
@@ -475,12 +490,22 @@ def hide_descendants_by_kfids(
     If you anticipate needing symmetrical behavior, keep a record of what you
     change.
     """
-    desc = find_descendants_by_kfids(db_url or api_url, endpoint, kfids, False)
-    for v in desc.values():
-        hide_kfids(api_url, v, gf_acl)
+    desc = find_descendants_by_kfids(
+        db_url or api_url,
+        endpoint,
+        kfids,
+        ignore_gfs_with_hidden_external_contribs=False,
+        kfids_only=False,
+    )
+    changed = []
+    for es in desc.values():
+        changed.extend(hide_entities(api_url, es.values(), gf_acl, dry_run))
+    return changed
 
 
-def unhide_descendants_by_kfids(api_url, endpoint, kfids, db_url=None):
+def unhide_descendants_by_kfids(
+    api_url, endpoint, kfids, db_url=None, dry_run=False
+):
     """
     Be aware that this and hide_descendants_by_kfids are not symmetrical.
 
@@ -489,6 +514,14 @@ def unhide_descendants_by_kfids(api_url, endpoint, kfids, db_url=None):
     If you anticipate needing symmetrical behavior, keep a record of what you
     change.
     """
-    desc = find_descendants_by_kfids(db_url or api_url, endpoint, kfids, True)
-    for v in desc.values():
-        unhide_kfids(api_url, v)
+    desc = find_descendants_by_kfids(
+        db_url or api_url,
+        endpoint,
+        kfids,
+        ignore_gfs_with_hidden_external_contribs=True,
+        kfids_only=False,
+    )
+    changed = []
+    for es in desc.values():
+        changed.extend(unhide_entities(api_url, es.values(), dry_run))
+    return changed

tests/integration/test_dataservice_descendants.py

@@ -1,9 +1,9 @@
-from kf_utils.dataservice.descendants import find_descendants_by_kfids, find_descendants_by_filter
+from kf_utils.dataservice.descendants import find_descendants_by_kfids, find_descendants_by_filter, unhide_descendants_by_kfids, hide_descendants_by_kfids
 from tests.conftest import DATASERVICE_URL, populate_data
 
 
 def test_by_kfids_simple(dataservice_setup):
-    n = 4
+    n = 2
     populate_data(n)
 
     desc = find_descendants_by_kfids(
@@ -18,7 +18,7 @@ def test_by_kfids_simple(dataservice_setup):
 
     desc = find_descendants_by_kfids(
         DATASERVICE_URL, "participants", [
-            "PT_10111111", "PT_11111111", "PT_12111111", "PT_13111111"
+            "PT_10111111", "PT_11111111"
         ],
         ignore_gfs_with_hidden_external_contribs=False,
         kfids_only=False
@@ -36,7 +36,7 @@ def test_by_kfids_simple(dataservice_setup):
 
 
 def test_by_filter_simple(dataservice_setup):
-    n = 4
+    n = 2
     populate_data(n)
     desc = find_descendants_by_filter(
         DATASERVICE_URL, "participants", {"study_id": "SD_11111111"},
@@ -47,3 +47,18 @@ def test_by_filter_simple(dataservice_setup):
     assert len(desc["biospecimens"]) == n*n
     # parent level should be populated
     assert isinstance(desc["participants"]["PT_11111111"], dict)
+
+
+def test_already_done(dataservice_setup):
+    n = 2
+    all_kfids = populate_data(n)
+    sdids = ["SD_11111111"]
+
+    changed = unhide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids)
+    assert not changed
+
+    changed = hide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids)
+    assert len(changed) == 1 + n + n*n
+
+    changed = unhide_descendants_by_kfids(DATASERVICE_URL, "studies", sdids)
+    assert len(changed) == 1 + n + n*n