bugfix[DYN-6189]: remove duplicated code block node#14373
bugfix[DYN-6189]: remove duplicated code block node#14373QilongTang merged 3 commits intoDynamoDS:masterfrom
Conversation
| NodeFactory.AddTypeFactoryAndLoader(outputData.Type); | ||
| NodeFactory.AddAlsoKnownAs(outputData.Type, outputData.AlsoKnownAs); | ||
|
|
||
| SearchModel?.Add(new CodeBlockNodeSearchElement(cbnData, LibraryServices)); |
There was a problem hiding this comment.
@Enzo707 after this change, can you test searching for code block node and see if it still works?
There was a problem hiding this comment.
@QilongTang if you look at the gif it still appearing in library search and seems to work fine. There's any other place to look at?
There was a problem hiding this comment.
yup, can you take a look at the in canvas search as well, just to make sure?
There was a problem hiding this comment.
@QilongTang it's failing within canvas search... I'm taking a look
There was a problem hiding this comment.
@QilongTang sorry for the delay, please take a look.
| { | ||
| AddNodeTypeToSearchIndex(ele, iDoc); | ||
| } | ||
| AddNodeTypeToSearchIndex(symbolSearchElement, iDoc); |
There was a problem hiding this comment.
@RobertGlobant20 Didn't we add the check intentionally? Should we merge this only after we have the corresponding unit test added?
There was a problem hiding this comment.
If you are refering to the check:
if (ele != null)
I think there is no need to check it due that we are not using the AddNodeTypeToSearch(outputData); method anymore (that could return null) and if you see the code some lines above you will see that those instances are created (see screenshot below).
The unit test only validates that the Input, Output and Code Block nodes are found with the Lucene Search results then it won't catch if they are duplicated visually in Library (As far as I remember Lucene Search doesn't return duplicated items), then I think this change can be merged

Purpose
This PR is for removing the duplicated "code block" node in Library.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
There was a double indexing statement of the code block node within DynamoModel
Reviewers
@QilongTang
@RobertGlobant20
FYIs
@avidit