NIFI-3785: Added feature to move controller services between process …

[Freedom9339]

…groups with the new UI

Summary

NIFI-3785: Added an option on the controller services grid that moves a controller service to a specified process group. The controller service can be moved to the parent process group or a child process group. However, it can only be moved one level at a time. If there are any scope conflicts with referencing components, the move will fail displaying the reason.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• [X] Apache NiFi Jira issue created

Pull Request Tracking

• [X] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• [X] Pull Request based on current revision of the main branch
• [X] Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• [X] Build completed using mvn clean install -P contrib-check
  • [X] JDK 21

Licensing

• [ ] New dependencies are compatible with the Apache License 2.0 according to the License Policy
• [ ] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• [ ] Documentation formatting appears as expected in rendered files

[Freedom9339]

@markap14 @exceptionfactory I've resubmitted this PR with the changes done to the new UI. The backend is the same as it was in the closed PR(https://github.com/apache/nifi/pull/7734), but the UI changes were done against the new UI.

[Freedom9339]

@markobean

[exceptionfactory]

Thanks for putting in the work to refactor this for the new UI @Freedom9339.

Tagging @mcgilman and @rfellows for potential review.

[mcgilman]

Thanks for the mention @exceptionfactory! I'll have a look...

[Freedom9339]

@mcgilman Just checking in. Any updates on this?

[mcgilman]

@mcgilman Just checking in. Any updates on this?

Thanks for the ping. Sorry for the delay, I'll try to get some more eyes on this PR this week.

[Freedom9339]

@mcgilman Sorry the pinging again. Just want to make sure this doesn't fall through the cracks.

[mcgilman]

@mcgilman Sorry the pinging again. Just want to make sure this doesn't fall through the cracks.

I just pulled down the latest. After rebasing against current main there were build failures due to changes that have landed after your PR was created. So I tried to run the PR as-is without rebasing and I'm still seeing issues. When running through the dev server it fails to serve with

Application bundle generation failed. [13.461 seconds]

✘ [ERROR] Undefined function. ╷ 109 │ $surface-light-palette: mat.define-palette($surface-palette, 600, 100, 900); │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ╵ libs/shared/src/assets/themes/supplemental.scss 109:25 @import apps/nifi/src/styles.scss 62:9 root stylesheet [plugin angular-sass]

angular:styles/global:styles:1:8:
  1 │ @import 'apps/nifi/src/styles.scss';

I then tried running with the built application but I'm seeing this error in dev tools when attempting to open the MoveControllerService Component.

TypeError: g is not iterable
    at chunk-VIZEOFFB.js:70:66450
    at Array.forEach (<anonymous>)
    at o.loadChildOptions (chunk-VIZEOFFB.js:70:66333)
    at new o (chunk-VIZEOFFB.js:70:65423)
    at o.ɵfac [as factory] (chunk-VIZEOFFB.js:70:67428)
    at hn (chunk-ZKVQE3GD.js:7:23746)
    at TC (chunk-ZKVQE3GD.js:7:63202)
    at mn.create (chunk-ZKVQE3GD.js:7:61795)
    at Vg.createComponent (chunk-ZKVQE3GD.js:7:64954)
    at t.attachComponentPortal (chunk-6XYXU2WR.js:1:110675)

Once the PR is updated I should be able to get some more eyes on it quickly.

[Freedom9339]

@mcgilman I fixed the issue caused by rebasing and pushed. Thank You!

[Freedom9339]

@mcgilman I've completed the change/fixes requested. Thank You!

[Freedom9339]

@mcgilman Sorry, I just realized my latest commit didn't go through. It's there now.

[Freedom9339]

@mcgilman Any update on this? I just did a fresh rebase.

[Freedom9339]

@mcgilman Thank you for the review. I've made the requested changes. Thank You.

[Freedom9339]

@mcgilman Sorry for the delay. I addressed the issues you mentioned and added more user validation to not expose any unauthorized components. I've tested every scenario I could think of. Please let me know if there is one I missed.

[Freedom9339]

@mcgilman Any update on this? I've just rebased to fix merge conflicts.

[Freedom9339]

@mcgilman Thank you for the review. I've made the changed requested.

[Freedom9339]

@mcgilman Any new updates on this?

[mcgilman]

@Freedom9339 In my last review I mentioned the UI portions were looking good but I will take a peek at your latest updates. I also provided some initial feedback on some of the back end changes but we'll want some additional eyes on those from someone more familiar with those portions of the code base.

[Freedom9339]

@exceptionfactory I've added the replicating of the move-options end point as well as merging the responses. Please let me know if this is being placed on hold or if there is anything else I need to do.

[joewitt]

Given how long this has been in process and the recent rounds of feedback I think we should close this.

The effort that has gone into this contribution is very impressive as is the depth of reviews from very experienced members of the community. But this is too big/too complex and needs to be very consistent to land.

[exceptionfactory]

I concur @joewitt.

Thanks again to all for the effort that has gone into this, but it needs to be revisited and implemented separately. This pull request and associated reviews provide helpful background on the effort involved.