Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue multiple dummy nodes formed #1200

Merged
merged 10 commits into from
Oct 7, 2024
Merged

Conversation

himanshudube97
Copy link
Contributor

@himanshudube97 himanshudube97 commented Sep 21, 2024

Issues->

  1. Click on source node (table ) then click on operational node, the column data in the operational form would dissapper.
    reason-> It was a dummy operational node (and action was edit), so api call wont fetch any data with that nodeid.
    sol-> If its an operational dummy node, keep it in create mode (not edit) and use source node id for api call.

  2. Click on source node then any operation from operations table, a dummy node appears. Now again click source node (table) and choose another operation , a new dummy node appears and so on.
    reason-> No code to check if the dummy node created earlier is being used or not.
    sol-> If user clicks on source node , then remove the dummy node. This ensures we dont have multiple dummy node on the canvas.

Before: ->
link1

After:->
link2

Copy link

codecov bot commented Sep 21, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 33 lines in your changes missing coverage. Please review.

Project coverage is 70.18%. Comparing base (06841d2) to head (a11ee00).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...ow/FlowEditor/Components/OperationConfigLayout.tsx 27.27% 16 Missing ⚠️
...TransformWorkflow/FlowEditor/Components/Canvas.tsx 75.00% 1 Missing ⚠️
...ponents/OperationPanel/Forms/AggregationOpForm.tsx 88.88% 1 Missing ⚠️
...mponents/OperationPanel/Forms/ArithmeticOpForm.tsx 88.88% 1 Missing ⚠️
...Components/OperationPanel/Forms/CaseWhenOpForm.tsx 88.88% 1 Missing ⚠️
...mponents/OperationPanel/Forms/CastColumnOpForm.tsx 91.66% 1 Missing ⚠️
...Components/OperationPanel/Forms/CoalesceOpForm.tsx 88.88% 1 Missing ⚠️
...mponents/OperationPanel/Forms/DropColumnOpForm.tsx 95.00% 1 Missing ⚠️
...ponents/OperationPanel/Forms/FlattenJsonOpForm.tsx 90.90% 1 Missing ⚠️
...nents/OperationPanel/Forms/GenericColumnOpForm.tsx 88.88% 1 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1200      +/-   ##
==========================================
+ Coverage   69.45%   70.18%   +0.72%     
==========================================
  Files          97       97              
  Lines        6974     6987      +13     
  Branches     1667     1634      -33     
==========================================
+ Hits         4844     4904      +60     
+ Misses       2090     2043      -47     
  Partials       40       40              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@himanshudube97 himanshudube97 self-assigned this Sep 21, 2024
@himanshudube97 himanshudube97 linked an issue Sep 21, 2024 that may be closed by this pull request
@himanshudube97 himanshudube97 marked this pull request as draft September 28, 2024 18:31
@himanshudube97 himanshudube97 marked this pull request as ready for review October 2, 2024 06:17
@himanshudube97
Copy link
Contributor Author

Issues->

  1. When clicked on a dummy operational node the data in the right form would go empty.
  2. If there was a dummy node created and then again we click on the parent node (source /operational) , another dummy node would comeup on screen. So this way multiple dummy nodes were there.

Changes made->

  1. added a parent node as a field in the dummy node. So that we know from which parent that node is created. This works for both if the parent node is a sourceNode(table) or an operationalNode.

  2. Also a custom hook useOpForm has been added. Right now it is returning the nodeData and the parentNode. In next iterations we will add the inital useeffect that fetches data (common code in all components).

  3. In operationalConfigLayout, code to check if there is a dummy node and user clicks on some other node, than the dummy node is deleted.

  4. In Canvas.tsx file its a minor ui change of the top run workflow button.

Comman Changes in all forms:->

  1. When the forms are rendering, and if the node is dummy node, then useEffect don't call the apis.

  2. While Saving:
    a. checking if the node is a dummy node or a real node (saved in the db).
    b. if its a real node it have a id (saved in db), works fine, else if its a dummy node, we take id and other data from its parent, and mark this operation as create not edit (for dummy nodes).

@himanshudube97
Copy link
Contributor Author

Issues still remaining->

  1. Cases where the dummy node is a source node (table), eg join operation. When clicked on the dummy source node created , it resets the form.

@Ishankoradia Ishankoradia self-requested a review October 7, 2024 05:13
@Ishankoradia Ishankoradia merged commit 2f758eb into main Oct 7, 2024
4 of 5 checks passed
@Ishankoradia Ishankoradia deleted the issue-multiple-dummy-nodes-formed branch October 7, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cast form lagging/glitching
2 participants