Fix chart export stripping flow node data (ToolboxFunction, Constant)#3041
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3041 +/- ##
=======================================
Coverage 85.92% 85.92%
=======================================
Files 470 470
Lines 43158 43159 +1
=======================================
+ Hits 37085 37086 +1
Misses 6073 6073
🚀 New features to boost your workflow:
|
ChartResponse.as_request_resource() was validating via the base class with extra="ignore", which silently dropped all unmodeled fields in FlowData nodes (selectedOperation, parameterValues, name, value, etc.). Fusion reads these fields to render calculations, causing a TypeError at node.data.selectedOperation.op on migrated charts. Fix: dump only response-only fields explicitly, then validate the result with extra="allow" to preserve all nested node data opaquely. ## Bump - [x] Patch - [ ] Minor - [ ] Major ## Changelog ### Fixed - Fixed chart export/import stripping flow node data (`selectedOperation`, `parameterValues` for `ToolboxFunction` nodes; `name`, `value` for `Constant` nodes), which caused Fusion to crash with `TypeError: Cannot read properties of undefined (reading 'op')` after a Toolkit round-trip.
70ef7e4 to
0bd4b01
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the as_request_resource method in the Chart resource class to serialize the model to JSON while excluding metadata fields (created_time, last_updated_time, and owner_id), and then validate it back into a ChartRequest. This ensures that nested flow node data within the workflow collection is preserved. A unit test has been added to verify this behavior. I have no feedback to provide as there are no review comments.
Bump
Changelog
Fixed
selectedOperation,parameterValuesforToolboxFunctionnodes;name,valueforConstantnodes), which caused Fusion to crash withTypeError: Cannot read properties of undefined (reading 'op')after a Toolkit round-trip.Summary
ChartResponse.as_request_resource()was calling the base class implementation which usedextra="ignore"when validating the response into a request. This silently dropped any field not declared in Toolkit'sFlowDataPydantic model — includingselectedOperation,parameterValues(forToolboxFunctionnodes) andname,value(forConstantnodes).TypeError: Cannot read properties of undefined (reading 'op')becausenode.data.selectedOperationwasundefined.as_request_resource()inChartResponseto dump only the known response-only fields (created_time,last_updated_time,owner_id) explicitly, then validate withextra="allow"so all nested node data is preserved opaquely.Test plan
test_chart_response_to_request_preserves_flow_node_data— new regression test verifying thatConstant(name,value) andToolboxFunction(selectedOperation,parameterValues) node data survive the response→request conversion, and that response-only fields are excludedTestChartIOtests continue to pass