Merge pull request #1319 from Sage-Bionetworks/synpy-1770

[SYNPY-1770] fix variable mutation in get_conditional_properties

Author: linglp

synapseclient/extensions/curator/schema_generation.py

@@ -4994,6 +4994,7 @@ def get_conditional_properties(
         if self.current_node is None:
             raise ValueError("Current node is None")
         conditional_properties: list[tuple[str, str]] = []
+
         for value in self._reverse_dependencies[self.current_node.name]:
             if value in self._valid_values_map:
                 properties = sorted(self._valid_values_map[value])
@@ -5002,8 +5003,14 @@ def get_conditional_properties(
                         watched_property = self.dmge.get_nodes_display_names(
                             [watched_property]
                         )[0]
-                        value = self.dmge.get_nodes_display_names([value])[0]
-                    conditional_properties.append((watched_property, value))
+                        display_name_value = self.dmge.get_nodes_display_names([value])[
+                            0
+                        ]
+                        conditional_properties.append(
+                            (watched_property, display_name_value)
+                        )
+                    else:
+                        conditional_properties.append((watched_property, value))
         return conditional_properties
 
     def _update_valid_values_map(

tests/unit/synapseclient/extensions/unit_test_create_json_schema.py

@@ -845,6 +845,32 @@ def test_get_conditional_properties(self, dmge: DataModelGraphExplorer) -> None:
         # THEN the current node should have conditional properties
         assert gts.get_conditional_properties() == [("Diagnosis", "Cancer")]
 
+    def test_get_conditional_properties_multiple_watched_properties(
+        self, dmge: DataModelGraphExplorer
+    ) -> None:
+        """Test GraphTraversalState.get_conditional_properties with multiple watched properties.
+
+        This test covers a bug where the 'value' variable was being mutated inside
+        the inner loop when converting to display names.
+
+        """
+        # GIVEN a GraphTraversalState instance where
+        # - CancerType has a reverse dependency of FamilyHistory
+        # - FamilyHistory is a valid value of MULTIPLE attributes
+        gts = GraphTraversalState(dmge, "Patient", logger=Mock())
+        gts._nodes_to_process = ["CancerType"]
+
+        # Use FamilyHistory because its display name ("Family History") differs from class label
+        gts._reverse_dependencies = {"CancerType": ["FamilyHistory"]}
+        # FamilyHistory triggers multiple watched properties - this is key to triggering the bug
+        gts._valid_values_map = {"FamilyHistory": ["Sex", "YearofBirth"]}
+        # WHEN using move_to_next_node
+        gts.move_to_next_node()
+        result = gts.get_conditional_properties()
+        assert len(result) == 2
+        assert ("Year of Birth", "Family History") in result
+        assert ("Sex", "Family History") in result
+
     def test_update_valid_values_map(self, dmge: DataModelGraphExplorer) -> None:
         """Test GraphTraversalState._update_valid_values_map"""
         # GIVEN a GraphTraversalState instance