[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7353: NIFI-11658 Streamline using single parameter context for nested PGs

["exceptionfactory (via GitHub)" <gi...@apache.org>]

exceptionfactory commented on code in PR #7353:
URL: https://github.com/apache/nifi/pull/7353#discussion_r1242201390


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/entity/ProcessGroupUpdateStrategy.java:
##########
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.api.entity;
+
+public enum ProcessGroupUpdateStrategy {
+    UPDATE_PROCESS_GROUP_ONLY,
+    UPDATE_PROCESS_GROUP_WITH_DESCENDANTS

Review Comment:
   Instead of repeating the `UPDATE_PROCESS_GROUP` prefix, recommend changing the naming to be more concise. What about `CURRENT_GROUP` and `CURRENT_GROUP_WITH_CHILDREN`? Much of User Guide documentation mentions "Child Process Groups".



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java:
##########
@@ -1209,9 +1209,10 @@ Set<DocumentedTypeDTO> getControllerServiceTypes(final String serviceType, final
      * Gets all process groups in the specified parent group.
      *
      * @param parentGroupId The id of the parent group
-     * @return process group
+     * @param includeDescendants if process groups from descendant groups should be included
+     * @return List of process groups
      */
-    Set<ProcessGroupEntity> getProcessGroups(String parentGroupId);
+    Set<ProcessGroupEntity> getProcessGroups(String parentGroupId, boolean includeDescendants);

Review Comment:
   Instead of using a `boolean` here, why not pass the `ProcessGroupUpdateStrategy`?



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/ProcessGroupDAO.java:
##########
@@ -59,9 +59,10 @@ public interface ProcessGroupDAO {
      * Gets all of the process groups.
      *
      * @param parentGroupId The parent group id
+     * @param includeDescendants if process groups from descendant groups should be included
      * @return The process groups
      */
-    Set<ProcessGroup> getProcessGroups(String parentGroupId);
+    Set<ProcessGroup> getProcessGroups(String parentGroupId, boolean includeDescendants);

Review Comment:
   See above note on using the `ProcessGroupUpdateStrategy` enum.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org