-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
NIFI-12368 Clear versionedComponentId on top-level components of popu… #8025
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks addressing this issue @bbende. The change appears straightforward, so I created a unit test to validate the behavior. In the course of the testing, I was wondering whether the source snippet or the destination versionedComponentId values should be cleared. I created the test based on the behavior you have implemented, but just wanted to verify that is the intent.
@@ -404,9 +404,26 @@ private Set<ControllerServiceDTO> getControllerServices(final Map<PropertyDescri | |||
public FlowSnippetDTO copy(final FlowSnippetDTO snippetContents, final ProcessGroup group, final String idGenerationSeed, boolean isCopy) { | |||
final FlowSnippetDTO snippetCopy = copyContentsForGroup(snippetContents, group.getIdentifier(), null, null, idGenerationSeed, isCopy); | |||
resolveNameConflicts(snippetCopy, group); | |||
removeTopLevelVersionedIds(snippetContents); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this clear the source snippet, as it is doing now, or the copy snippet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, it was meant to clear the copy since that is what will be used to instantiate the components, will update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the adjustment @bbende, looks good! +1 merging
…lated Snippet
Summary
NIFI-12368
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
filesDocumentation