Skip to content

Commit c989879

Browse files
Copilotjdkent
andauthored
Implement preserve_on_clone flag system to fix condition cloning behavior (#1133)
* Initial plan * Update test to validate current condition cloning issue Co-authored-by: jdkent <[email protected]> * Add comprehensive tests for condition cloning behavior and initial fix attempt Co-authored-by: jdkent <[email protected]> * Implement preserve_on_clone flag for condition cloning fix Co-authored-by: jdkent <[email protected]> * Add comprehensive integration test demonstrating successful condition cloning fix Co-authored-by: jdkent <[email protected]> * Refactor tests and fix style issues: separate schema and API tests Co-authored-by: jdkent <[email protected]> * Refactor condition cloning tests for clarity Renamed tests for clarity and removed unused test. * Remove unused import of pytest in test_condition_cloning Removed unused import statement for pytest. * Changes before error encountered Co-authored-by: jdkent <[email protected]> * fix tests to compare the right id ("external_id") * remove redundant test files * fix style --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jdkent <[email protected]> Co-authored-by: James Kent <[email protected]>
1 parent 8edc158 commit c989879

File tree

3 files changed

+470
-16
lines changed

3 files changed

+470
-16
lines changed

store/backend/neurostore/schemas/data.py

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
# nested: serialize nested objects (true or false)
2121
# flat: do not display any relationships
2222
# info: only display info fields
23+
# preserve_on_clone: field metadata to preserve original values when cloning
2324

2425

2526
class BooleanOrString(fields.Field):
@@ -67,13 +68,36 @@ def _modify_schema(self):
6768
schema = self.schema
6869
schema.context = self.context
6970
if self.context.get("clone"):
70-
id_fields = [
71-
field
72-
for field, f_obj in schema._declared_fields.items()
73-
if f_obj.metadata.get("id_field")
74-
]
75-
for f in id_fields:
76-
schema.exclude.add(f)
71+
# Check if this schema has preserve_on_clone set for any id field
72+
has_preserve_on_clone = any(
73+
f_obj.metadata.get("id_field")
74+
and f_obj.metadata.get("preserve_on_clone")
75+
for f_obj in schema._declared_fields.values()
76+
)
77+
78+
if has_preserve_on_clone:
79+
# For schemas with preserve_on_clone,
80+
# include ONLY id fields with preserve_on_clone=True
81+
preserve_fields = [
82+
field
83+
for field, f_obj in schema._declared_fields.items()
84+
if f_obj.metadata.get("id_field")
85+
and f_obj.metadata.get("preserve_on_clone")
86+
]
87+
if preserve_fields:
88+
schema.only = schema.set_class(preserve_fields)
89+
schema.exclude = schema.set_class()
90+
else:
91+
# Normal cloning behavior: exclude id fields without preserve_on_clone
92+
id_fields = [
93+
field
94+
for field, f_obj in schema._declared_fields.items()
95+
if f_obj.metadata.get("id_field")
96+
and not f_obj.metadata.get("preserve_on_clone")
97+
]
98+
for f in id_fields:
99+
schema.exclude.add(f)
100+
77101
if self.context.get("info"):
78102
info_fields = [
79103
field
@@ -132,15 +156,38 @@ class BaseSchema(Schema):
132156
def __init__(self, *args, **kwargs):
133157
exclude = kwargs.get("exclude") or self.opts.exclude
134158
context = kwargs.get("context", {})
135-
# if cloning and not only id, exclude id fields
136-
if context.get("clone") and kwargs.get("only") != ("id",):
137-
id_fields = [
138-
field
139-
for field, f_obj in self._declared_fields.items()
140-
if f_obj.metadata.get("id_field")
141-
]
142-
for f in id_fields:
143-
exclude += (f,)
159+
only = kwargs.get("only")
160+
161+
# if cloning and not only id, exclude id fields (unless preserve_on_clone is True)
162+
if context.get("clone") and only != ("id",):
163+
# Check if this schema has preserve_on_clone set for any id field
164+
has_preserve_on_clone = any(
165+
f_obj.metadata.get("id_field")
166+
and f_obj.metadata.get("preserve_on_clone")
167+
for f_obj in self._declared_fields.values()
168+
)
169+
170+
if has_preserve_on_clone:
171+
# For schemas with preserve_on_clone,
172+
# include ONLY id fields with preserve_on_clone=True
173+
preserve_fields = [
174+
field
175+
for field, f_obj in self._declared_fields.items()
176+
if f_obj.metadata.get("id_field")
177+
and f_obj.metadata.get("preserve_on_clone")
178+
]
179+
if preserve_fields and only is None:
180+
kwargs["only"] = tuple(preserve_fields)
181+
else:
182+
# Normal cloning behavior: exclude id fields
183+
id_fields = [
184+
field
185+
for field, f_obj in self._declared_fields.items()
186+
if f_obj.metadata.get("id_field")
187+
]
188+
for f in id_fields:
189+
exclude += (f,)
190+
144191
if context.get("flat"):
145192
relationships = [
146193
field
@@ -182,6 +229,11 @@ class Meta:
182229
additional = ("name", "description")
183230
allow_none = ("name", "description")
184231

232+
# Override the id field to preserve it during cloning
233+
id = fields.String(
234+
metadata={"info_field": True, "id_field": True, "preserve_on_clone": True}
235+
)
236+
185237

186238
class EntitySchema(BaseDataSchema):
187239
analysis_id = fields.String(data_key="analysis", metadata={"id_field": True})

0 commit comments

Comments
 (0)