Fix pending issues 17751 (#17948)

* #17751 make copy folder report errors like move folder * #17751 organize imports
dotCMS · Feb 4, 2020 · 11cef40 · 11cef40
1 parent 37787d2
commit 11cef40
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 35 deletions.
diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/browser/ajax/BrowserAjax.java b/dotCMS/src/main/java/com/dotmarketing/portlets/browser/ajax/BrowserAjax.java
@@ -5,7 +5,6 @@
 import static com.dotmarketing.business.PermissionAPI.PERMISSION_READ;
 import static com.dotmarketing.business.PermissionAPI.PERMISSION_WRITE;
 
-import com.dotcms.business.WrapInTransaction;
 import com.dotcms.contenttype.business.ContentTypeAPI;
 import com.dotcms.contenttype.exception.NotFoundInDbException;
 import com.dotcms.rendering.velocity.viewtools.BrowserAPI;
@@ -14,7 +13,6 @@
 import com.dotmarketing.beans.Host;
 import com.dotmarketing.beans.Identifier;
 import com.dotmarketing.beans.Inode;
-import com.dotmarketing.beans.MultiTree;
 import com.dotmarketing.beans.WebAsset;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.CacheLocator;
@@ -730,49 +728,60 @@ public Map<String, Object> renameFolder (String inode, String newName) throws Do
      * @return Confirmation message
      * @throws Exception
      */
-    public boolean copyFolder ( String inode, String newFolder ) throws Exception {
+    public String copyFolder ( String inode, String newFolder ) throws Exception {
 
-        HttpServletRequest req = WebContextFactory.get().getHttpServletRequest();
-        User user = getUser( req );
+        HttpServletRequest request = WebContextFactory.get().getHttpServletRequest();
+        User user = getUser( request );
 
         UserWebAPI userWebAPI = WebAPILocator.getUserWebAPI();
         HostAPI hostAPI = APILocator.getHostAPI();
 
+		final Locale requestLocale       = request.getLocale();
+		final String successString       = UtilMethods.escapeSingleQuotes(LanguageUtil.get(requestLocale, "Folder-copied"));
+		final String errorTryToMoveFolderToItself       = UtilMethods.escapeSingleQuotes(LanguageUtil.get(requestLocale, "Folder-copied-to-itself"));
+		final String errorTryToMoveFolderToChild       = UtilMethods.escapeSingleQuotes(LanguageUtil.get(requestLocale, "Folder-copied-to-children"));
+
         //Searching for the folder to copy
         Folder folder = APILocator.getFolderAPI().find( inode, user, false );
 
-        if ( !folderAPI.exists( newFolder ) ) {
+        try {
+			if ( !folderAPI.exists( newFolder ) ) {
 
-            Host parentHost = hostAPI.find( newFolder, user, !userWebAPI.isLoggedToBackend( req ) );
+				Host parentHost = hostAPI.find( newFolder, user, !userWebAPI.isLoggedToBackend( request ) );
 
-            if ( !permissionAPI.doesUserHavePermission( folder, PERMISSION_WRITE, user ) || !permissionAPI.doesUserHavePermission( parentHost, PERMISSION_WRITE, user ) ) {
-                throw new DotRuntimeException( "The user doesn't have the required permissions." );
-            }
+				if ( !permissionAPI.doesUserHavePermission( folder, PERMISSION_WRITE, user ) || !permissionAPI.doesUserHavePermission( parentHost, PERMISSION_WRITE, user ) ) {
+					throw new DotRuntimeException( "The user doesn't have the required permissions." );
+				}
 
-            folderAPI.copy( folder, parentHost, user, false );
-            refreshIndex( null, parentHost, folder );
-        } else {
+				folderAPI.copy( folder, parentHost, user, false );
+				refreshIndex( null, parentHost, folder );
+			} else {
 
-            Folder parentFolder = APILocator.getFolderAPI().find( newFolder, user, false );
+				Folder parentFolder = APILocator.getFolderAPI().find( newFolder, user, false );
 
-            if ( !permissionAPI.doesUserHavePermission( folder, PermissionAPI.PERMISSION_WRITE, user ) || !permissionAPI.doesUserHavePermission( parentFolder, PERMISSION_WRITE, user ) ) {
-                throw new DotRuntimeException( "The user doesn't have the required permissions." );
-            }
+				if ( !permissionAPI.doesUserHavePermission( folder, PermissionAPI.PERMISSION_WRITE, user ) || !permissionAPI.doesUserHavePermission( parentFolder, PERMISSION_WRITE, user ) ) {
+					throw new DotRuntimeException( "The user doesn't have the required permissions." );
+				}
 
-            if ( parentFolder.getInode().equalsIgnoreCase( folder.getInode() ) ) {
-                //Trying to move a folder over itself
-                return false;
-            }
-            if ( folderAPI.isChildFolder( parentFolder, folder ) ) {
-                //Trying to move a folder over one of its children
-                return false;
-            }
+				if ( parentFolder.getInode().equalsIgnoreCase( folder.getInode() ) ) {
+					//Trying to move a folder over itself
+					return errorTryToMoveFolderToItself;
+				}
+				if ( folderAPI.isChildFolder( parentFolder, folder ) ) {
+					//Trying to move a folder over one of its children
+					return errorTryToMoveFolderToChild;
+				}
 
-            folderAPI.copy( folder, parentFolder, user, false );
-            refreshIndex(parentFolder, null, folder );
-        }
+				folderAPI.copy( folder, parentFolder, user, false );
+				refreshIndex(parentFolder, null, folder );
+			}
+		} catch(InvalidFolderNameException e ) {
+			Logger.error(this, "Error copying folder with id:" + folder.getInode() + " into folder with id:"
+					+ newFolder + ". Error: " + e.getMessage());
+			return e.getLocalizedMessage();
+		}
 
-        return true;
+        return successString;
     }
 
     /**
@@ -799,7 +808,11 @@ public String moveFolder (final String folderId, final String newFolderId) throw
 
             	return errorString;
 			}
-        } catch (Exception e) {
+        } catch(InvalidFolderNameException e ) {
+			Logger.error(this, "Error moving folder with id:" + folderId + " into folder with id:"
+					+ newFolderId + ". Error: " + e.getMessage());
+			return e.getLocalizedMessage();
+		}catch (Exception e) {
         	Logger.error(this, "Error moving folder with id:" + folderId + " into folder with id:"
 					+ newFolderId + ". Error: " + e.getMessage(), e);
             return e.getLocalizedMessage();

diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties
@@ -1497,6 +1497,8 @@ Fix-Assets-Inconsistencies = Fix Assets Inconsistencies
 Flush-All-Caches = All Caches
 Flush-Cache-Button = Flush
 Folder-copied = Folder copied
+Folder-copied-to-itself = Error while copying Folder. Trying to copy Folder to itself.
+Folder-copied-to-children = Error while copying Folder. Trying to copy Folder to its child.
 Folder-moved = Folder moved
 Folder-Related-Assets = Folder Related Assets
 folder.archive.selected.contentlet = Are you sure you want to archive the selected content?

diff --git a/dotCMS/src/main/webapp/html/portlet/ext/browser/view_browser_js_inc.jsp b/dotCMS/src/main/webapp/html/portlet/ext/browser/view_browser_js_inc.jsp
@@ -1513,12 +1513,12 @@ Structure defaultFileAssetStructure = CacheLocator.getContentTypeCache().getStru
     }
 
     function copyFolderCallback (response) {
-        if (!response) {
-            reloadContent ();
-            showDotCMSErrorMessage('<%= UtilMethods.escapeSingleQuotes(LanguageUtil.get(pageContext, "Failed-to-copy-another-folder-with-the-same-name-already-exists-in-the-destination")) %>');
-        } else {
+        if(response == '<%= UtilMethods.escapeSingleQuotes(LanguageUtil.get(pageContext, "Folder-copied")) %>'){
             BrowserAjax.getTree(null, initializeTree);
-            showDotCMSSystemMessage('<%= UtilMethods.escapeSingleQuotes(LanguageUtil.get(pageContext, "Folder-copied")) %>');
+            showDotCMSSystemMessage(response);
+        } else {
+            reloadContent ();
+            showDotCMSErrorMessage(response);
         }
     }