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

When opening a customNode's viewblock don't add already existing notes/annotations #9165

Merged
merged 2 commits into from Oct 12, 2018

Conversation

Projects
None yet
3 participants
@mjkkirschner
Copy link
Member

mjkkirschner commented Oct 10, 2018

Purpose

Found that when a custom node is opened and the workspace is not already open in Dynamo - the json view block is deserialized again, and notes and annotations are simply added again... If the workspace is already loaded and these items exist already, don't do anything.

This might even fix existing graphs which have been corrupted in this way, as the guids of the repeated annotations and notes are the same.... ⚠️ !

I will add tests next, one of them will assert this.

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@ColinDayOrg
@alfarok

FYIs

@smangarole

seems to work -
add tests next

@mjkkirschner mjkkirschner requested review from ColinDayOrg and alfarok Oct 10, 2018

@ColinDayOrg

This comment has been minimized.

Copy link
Contributor

ColinDayOrg commented Oct 11, 2018

Code changes LGTM

@alfarok

This comment has been minimized.

Copy link
Contributor

alfarok commented Oct 11, 2018

@mjkkirschner LGTM, pretty nice that this will potentially will fix graphs that have already encountered the issue without additional recourse

@mjkkirschner mjkkirschner merged commit 5083f34 into DynamoDS:master Oct 12, 2018

3 checks passed

EngOps Build EngOps Build Succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment