CoreMedia / content-sync-example

Other
2 stars 4 forks source link

Provide ability to select all content in folder (in the selection tree) with one click #3

Open blackappsolutions opened 3 years ago

nchieffo commented 3 years ago

Clarification: add a checkbox also for folders, and once the user selects the folder, all content and sub-folders gets selected

cmfgi commented 3 years ago

Actually that was implemented before we released this feature, but was decided to remove it (checkbox was also enabled on directory level, which was performing the add). Reason is that the content-sync-example is following the purpose to provide a rudimentary "partial sync, and is not respecting any edge cases (list of edge cases is too long to support all customer requirements), but can be implemented by enhancing the solution. Additionally, allowing the user to select all contents of a folder plus the recursion flag (with an higher value) may lead to a full-sync functionality which is in any case, not recommended (new WFS will fit in that case much better). if you remove the "if" statement in: https://github.com/CoreMedia/content-sync-example/blob/master/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as

addCheckBox

the checkbox will also appear on Folders. Not sure if it's recursively adding all values to the targetTree, but at least the checkbox will appear.

Will discuss that topic internally, but I am not confident that this feature will find the way into this labs project.

blackappsolutions commented 3 years ago

Hi @cmfgi, thanks for the reply. You said

new WFS will fit in that case much better

Does this mean WFS will get a major update or re-built?

cmfgi commented 3 years ago

No, the idea in general is, if necessary, to have more than one workflows. Means, if there is a need for a different workflow type other than partial, you are able to:

The content-sync-example ui can be extended to support more than one WF. But sorry, was my typo to write WFS instead of WF ;)

nchieffo commented 3 years ago

For reference, this is a patch to also enable checkbox on folders

Index: modules/extensions/coremedia-content-sync/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/modules/extensions/coremedia-content-sync/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as b/modules/extensions/coremedia-content-sync/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as
--- a/modules/extensions/coremedia-content-sync/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as    (revision 8fe06a2e67ea4bc8c4968c43fd6932982904d8b8)
+++ b/modules/extensions/coremedia-content-sync/studio-client/src/main/joo/com/coremedia/blueprint/contentsync/studio/tree/ContentSyncSourceTreePanelBase.as    (date 1621407224493)
@@ -89,6 +89,15 @@
       });
     }
     modelBean.set(ContentSyncConstants.CONTENT_LIST_BEAN_PROPERTY, allSelectedContents);
+
+    if (!obj.leaf) {
+      ev.expand(false, function (nodes:Array) {
+        nodes.forEach(function (node:FolderTreeNode) {
+          node.set("checked", isChecked);
+          fireEvent(CHECK_CHANGE, node);
+        });
+      });
+    }
   }

   private function resolveAndAddReferences(parentFolderTreeNode:FolderTreeNode):void {
@@ -111,9 +120,7 @@
   }

   private static function addCheckBox(parent:*, node:*):void {
-    if (parseInt(node.data.id) % 2 == 0) {
-      node.data.checked = false;
-    }
+    node.data.checked = false;
   }

   public override function destroy(...params):void {

Index: modules/extensions/coremedia-content-sync/workflow-server/src/main/java/com/coremedia/blueprint/contentsync/workflow/action/CreateAndLinkContents.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/modules/extensions/coremedia-content-sync/workflow-server/src/main/java/com/coremedia/blueprint/contentsync/workflow/action/CreateAndLinkContents.java b/modules/extensions/coremedia-content-sync/workflow-server/src/main/java/com/coremedia/blueprint/contentsync/workflow/action/CreateAndLinkContents.java
--- a/modules/extensions/coremedia-content-sync/workflow-server/src/main/java/com/coremedia/blueprint/contentsync/workflow/action/CreateAndLinkContents.java    (revision 8c71df0126fd40a140431f14d9a4f59624119881)
+++ b/modules/extensions/coremedia-content-sync/workflow-server/src/main/java/com/coremedia/blueprint/contentsync/workflow/action/CreateAndLinkContents.java    (date 1621416908004)
@@ -247,7 +247,7 @@
       // update local content, if it does exist
       LOG.debug("setting properties on local content with id {} for remote content with id {}",
               localContent.getId(), remoteContent.getNumericId());
-      if (!localContent.isCheckedOut()) {
+      if (!localContent.isCheckedOut() && !localContent.isFolder()) {
         localContent.checkOut();
       }
       localContent.setProperties(properties);
cmfgi commented 3 years ago

Hi @nchieffo , actually to enable the functionality is not the problem, because as mentioned, it is more to prevent a full-sync via the partial-sync. And to be honest, my personal thinking is more going into the direction to reduce the capabilities of the partial sync in terms of amount (maybe a limit of 100). Will discuss it internally, but with the scope of the MVP and the fact that this project is limited to the topic of an "example", I am not confident that this will find it's way into the codebase.