Skip to content

Commit ca5e8c0

Browse files
authored
Remove existingVersion and retain resourceId for charts and dataGrid containers in Canvas (#2276)
1 parent 53fbbee commit ca5e8c0

File tree

4 files changed

+82
-5
lines changed

4 files changed

+82
-5
lines changed

cognite_toolkit/_cdf_tk/storageio/_applications.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ def _populate_id_cache(self, data_chunk: Sequence[IndustrialCanvas]) -> None:
214214
self.client.lookup.files.external_id(list(file_ids))
215215

216216
def _dump_resource(self, canvas: IndustrialCanvas) -> dict[str, JsonVal]:
217-
dumped = canvas.as_write().dump()
217+
dumped = canvas.as_write().dump(keep_existing_version=False)
218218
references = dumped.get("containerReferences", [])
219219
if not isinstance(references, list):
220220
return dumped
@@ -230,10 +230,18 @@ def _dump_resource(self, canvas: IndustrialCanvas) -> dict[str, JsonVal]:
230230
properties = source["properties"]
231231
if not isinstance(properties, dict):
232232
continue
233+
reference_type = properties.get("containerReferenceType")
234+
if (
235+
reference_type
236+
in {
237+
"charts",
238+
"dataGrid",
239+
}
240+
): # These container reference types are special cases with a resourceId statically set to -1, which is why we skip them
241+
continue
233242
resource_id = properties.pop("resourceId", None)
234243
if not isinstance(resource_id, int):
235244
continue
236-
reference_type = properties.get("containerReferenceType")
237245
if reference_type == "asset":
238246
external_id = self.client.lookup.assets.external_id(resource_id)
239247
elif reference_type == "timeseries":

tests/test_unit/conftest.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]:
242242
},
243243
"space": "IndustrialCanvasInstanceSpace",
244244
"version": 14,
245+
"existingVersion": 14,
245246
}
246247
],
247248
"containerReferences": [
@@ -270,6 +271,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]:
270271
},
271272
"space": "IndustrialCanvasInstanceSpace",
272273
"version": 1,
274+
"existingVersion": 1,
273275
},
274276
{
275277
"createdTime": 1751540275336,
@@ -296,6 +298,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]:
296298
},
297299
"space": "IndustrialCanvasInstanceSpace",
298300
"version": 1,
301+
"existingVersion": 1,
299302
},
300303
{
301304
"createdTime": 1751540544349,
@@ -322,6 +325,61 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]:
322325
},
323326
"space": "IndustrialCanvasInstanceSpace",
324327
"version": 4,
328+
"existingVersion": 4,
329+
},
330+
{
331+
"createdTime": 1751540600000,
332+
"externalId": "495af88f-fe1d-403d-91b1-76ef9f80f265_chart-ref-1",
333+
"instanceType": "node",
334+
"lastUpdatedTime": 1751540600000,
335+
"properties": {
336+
"cdf_industrial_canvas": {
337+
"ContainerReference/v2": {
338+
"chartsId": "my-chart-id",
339+
"containerReferenceType": "charts",
340+
"height": 400,
341+
"id": "chart-ref-1",
342+
"label": "My Chart",
343+
"maxHeight": None,
344+
"maxWidth": None,
345+
"resourceId": -1,
346+
"resourceSubId": None,
347+
"width": 600,
348+
"x": 100,
349+
"y": 100,
350+
}
351+
}
352+
},
353+
"space": "IndustrialCanvasInstanceSpace",
354+
"version": 1,
355+
"existingVersion": 1,
356+
},
357+
{
358+
"createdTime": 1751540700000,
359+
"externalId": "495af88f-fe1d-403d-91b1-76ef9f80f265_datagrid-ref-1",
360+
"instanceType": "node",
361+
"lastUpdatedTime": 1751540700000,
362+
"properties": {
363+
"cdf_industrial_canvas": {
364+
"ContainerReference/v2": {
365+
"chartsId": None,
366+
"containerReferenceType": "dataGrid",
367+
"height": 300,
368+
"id": "datagrid-ref-1",
369+
"label": "My Data Grid",
370+
"maxHeight": None,
371+
"maxWidth": None,
372+
"resourceId": -1,
373+
"resourceSubId": None,
374+
"width": 800,
375+
"x": 200,
376+
"y": 200,
377+
}
378+
}
379+
},
380+
"space": "IndustrialCanvasInstanceSpace",
381+
"version": 1,
382+
"existingVersion": 1,
325383
},
326384
],
327385
}

tests/test_unit/test_cdf_tk/test_commands/test_migration_cmd/test_migrate_canvas.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ def test_migrate_canvas_happy_path(
4747
backup = client.canvas.industrial.create.call_args[0][0]
4848
assert isinstance(backup, IndustrialCanvasApply)
4949

50-
assert len(update.fdm_instance_container_references) == len(canvas.container_references)
50+
# Only asset-centric container references should be migrated (not charts/dataGrid)
51+
migratable_refs = [
52+
ref for ref in canvas.container_references if ref.container_reference_type not in {"charts", "dataGrid"}
53+
]
54+
assert len(update.fdm_instance_container_references) == len(migratable_refs)
5155
assert len(backup.fdm_instance_container_references) == 0
5256

5357
def test_migrate_canvas_missing(self) -> None:

tests/test_unit/test_cdf_tk/test_storageio/test_applications.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,12 @@ def test_download_iterable(
146146
class TestCanvasIO:
147147
def test_download_iterable(self, asset_centric_canvas: tuple[IndustrialCanvas, NodeList[InstanceSource]]) -> None:
148148
canvas, _ = asset_centric_canvas
149-
ids = [container_ref.resource_id for container_ref in canvas.container_references or []]
149+
# Exclude charts/dataGrid types which have resourceId=-1 by design
150+
ids = [
151+
container_ref.resource_id
152+
for container_ref in canvas.container_references or []
153+
if container_ref.container_reference_type not in {"charts", "dataGrid"}
154+
]
150155
assert len(ids) > 0, "Test canvas must have container references for this test to be valid."
151156
mapping = {id_: f"external_id_{no}" for no, id_ in enumerate(ids)}
152157
reverse = {v: k for k, v in mapping.items()}
@@ -186,8 +191,10 @@ def lookup(external_id: str | list[str]) -> int | list[int]:
186191
json_str = json.dumps(json_format)
187192
internal_id_in_json = [id_ for id_ in ids if str(id_) in json_str]
188193
assert len(internal_id_in_json) == 0, "Internal IDs should not be present in the JSON export."
194+
assert "existingVersion" not in json_str, "existingVersion should not be present in the JSON export."
189195
restored_canvases = io.json_chunk_to_data([("line 1", item) for item in json_format])
190196

191197
assert len(restored_canvases) == 1
192198
restored_canvas = restored_canvases[0]
193-
assert restored_canvas.item.dump() == canvas.as_write().dump()
199+
# The restored canvas should match the original without existingVersion fields
200+
assert restored_canvas.item.dump() == canvas.as_write().dump(keep_existing_version=False)

0 commit comments

Comments
 (0)