Merge branch '15685-file-renaming-empty-name'
authorLucas Di Pentima <lucas@di-pentima.com.ar>
Thu, 12 Nov 2020 19:32:07 +0000 (16:32 -0300)
committerLucas Di Pentima <lucas@di-pentima.com.ar>
Thu, 12 Nov 2020 19:32:07 +0000 (16:32 -0300)
Closes #15685

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

18 files changed:
cypress/integration/collection-panel.spec.js
cypress/integration/login.spec.js
src/common/webdav.ts
src/common/xml.ts
src/components/collection-panel-files/collection-panel-files.tsx
src/components/form-dialog/form-dialog.tsx
src/components/rename-dialog/rename-dialog.tsx [deleted file]
src/components/tree/virtual-tree.tsx
src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
src/validators/valid-name.tsx
src/validators/validators.tsx
src/views-components/context-menu/action-sets/collection-files-item-action-set.ts
src/views-components/context-menu/actions/copy-to-clipboard-action.test.tsx
src/views-components/context-menu/actions/file-viewer-action.test.tsx
src/views-components/dialog-forms/copy-collection-dialog.ts
src/views-components/dialog-forms/create-collection-dialog.ts
src/views-components/dialog-forms/update-collection-dialog.ts
src/views-components/rename-file-dialog/rename-file-dialog.tsx

index 404d1c5b04f7ec2b2a17729908b0e6f0130dc60e..63a25a25a92d190669374aaf44aa1985f3c00e1b 100644 (file)
@@ -21,12 +21,12 @@ describe('Collection panel tests', function() {
                 activeUser = this.activeUser;
             }
         );
-    })
+    });
 
     beforeEach(function() {
-        cy.clearCookies()
-        cy.clearLocalStorage()
-    })
+        cy.clearCookies();
+        cy.clearLocalStorage();
+    });
 
     it('shows collection by URL', function() {
         cy.loginAs(activeUser);
@@ -128,8 +128,131 @@ describe('Collection panel tests', function() {
         })
     })
 
+    it('renames a file using valid names', function() {
+        // Creates the collection using the admin token so we can set up
+        // a bogus manifest text without block signatures.
+        cy.createCollection(adminUser.token, {
+            name: `Test collection ${Math.floor(Math.random() * 999999)}`,
+            owner_uuid: activeUser.user.uuid,
+            manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"})
+        .as('testCollection').then(function() {
+            cy.loginAs(activeUser);
+            cy.visit(`/collections/${this.testCollection.uuid}`);
+            const nameTransitions = [
+                ['bar', '&'],
+                ['&', 'foo'],
+                ['foo', '&amp;'],
+                ['&amp;', 'I ❤️ ⛵️'],
+                ['I ❤️ ⛵️', '...']
+            ];
+            nameTransitions.forEach(([from, to]) => {
+                cy.get('[data-cy=collection-files-panel]')
+                    .contains(`${from}`).rightclick();
+                cy.get('[data-cy=context-menu]')
+                    .contains('Rename')
+                    .click();
+                cy.get('[data-cy=form-dialog]')
+                    .should('contain', 'Rename')
+                    .within(() => {
+                        cy.get('input').type(`{selectall}{backspace}${to}`);
+                    });
+                cy.get('[data-cy=form-submit-btn]').click();
+                cy.get('[data-cy=collection-files-panel]')
+                    .should('not.contain', `${from}`)
+                    .and('contain', `${to}`);
+            })
+        });
+    });
+
+    it('renames a file to a different directory', function() {
+        // Creates the collection using the admin token so we can set up
+        // a bogus manifest text without block signatures.
+        cy.createCollection(adminUser.token, {
+            name: `Test collection ${Math.floor(Math.random() * 999999)}`,
+            owner_uuid: activeUser.user.uuid,
+            manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"})
+        .as('testCollection').then(function() {
+            cy.loginAs(activeUser);
+            cy.visit(`/collections/${this.testCollection.uuid}`);
+            // Rename 'bar' to 'subdir/foo'
+            cy.get('[data-cy=collection-files-panel]')
+                .contains('bar').rightclick();
+            cy.get('[data-cy=context-menu]')
+                .contains('Rename')
+                .click();
+            cy.get('[data-cy=form-dialog]')
+                .should('contain', 'Rename')
+                .within(() => {
+                    cy.get('input').type(`{selectall}{backspace}subdir/foo`);
+                });
+            cy.get('[data-cy=form-submit-btn]').click();
+            cy.get('[data-cy=collection-files-panel]')
+                .should('not.contain', 'bar')
+                .and('contain', 'subdir');
+            // Look for the "arrow icon" and expand the "subdir" directory.
+            cy.get('[data-cy=virtual-file-tree] > div > i').click();
+            // Rename 'subdir/foo' to 'baz'
+            cy.get('[data-cy=collection-files-panel]')
+                .contains('foo').rightclick();
+            cy.get('[data-cy=context-menu]')
+                .contains('Rename')
+                .click();
+            cy.get('[data-cy=form-dialog]')
+                .should('contain', 'Rename')
+                .within(() => {
+                    cy.get('input')
+                        .should('have.value', 'subdir/foo')
+                        .type(`{selectall}{backspace}baz`);
+                });
+            cy.get('[data-cy=form-submit-btn]').click();
+            cy.get('[data-cy=collection-files-panel]')
+                .should('contain', 'subdir') // empty dir kept
+                .and('contain', 'baz');
+        });
+    });
+
+    it('tries to rename a file with an illegal names', function() {
+        // Creates the collection using the admin token so we can set up
+        // a bogus manifest text without block signatures.
+        cy.createCollection(adminUser.token, {
+            name: `Test collection ${Math.floor(Math.random() * 999999)}`,
+            owner_uuid: activeUser.user.uuid,
+            manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"})
+        .as('testCollection').then(function() {
+            cy.loginAs(activeUser);
+            cy.visit(`/collections/${this.testCollection.uuid}`);
+            const illegalNamesFromUI = [
+                ['.', "Name cannot be '.' or '..'"],
+                ['..', "Name cannot be '.' or '..'"],
+                ['', 'This field is required'],
+                [' ', 'Leading/trailing whitespaces not allowed'],
+                [' foo', 'Leading/trailing whitespaces not allowed'],
+                ['foo ', 'Leading/trailing whitespaces not allowed'],
+                ['//foo', 'Empty dir name not allowed']
+            ]
+            illegalNamesFromUI.forEach(([name, errMsg]) => {
+                cy.get('[data-cy=collection-files-panel]')
+                    .contains('bar').rightclick();
+                cy.get('[data-cy=context-menu]')
+                    .contains('Rename')
+                    .click();
+                cy.get('[data-cy=form-dialog]')
+                    .should('contain', 'Rename')
+                    .within(() => {
+                        cy.get('input').type(`{selectall}{backspace}${name}`);
+                    });
+                cy.get('[data-cy=form-dialog]')
+                    .should('contain', 'Rename')
+                    .within(() => {
+                        cy.contains(`${errMsg}`);
+                    });
+                cy.get('[data-cy=form-cancel-btn]').click();
+            })
+        });
+    });
+
     it('can correctly display old versions', function() {
-        const colName = `Versioned Collection ${Math.floor(Math.random() * Math.floor(999999))}`;
+        const colName = `Versioned Collection ${Math.floor(Math.random() * 999999)}`;
         let colUuid = '';
         let oldVersionUuid = '';
         // Make sure no other collections with this name exist
index 25c8cd4b8a4179cf9f60ad2e7905e8204fac4788..aeea01cdcfcb8d99fa5f6bac415ffec69dbc0175 100644 (file)
@@ -28,7 +28,7 @@ describe('Login tests', function() {
                 inactiveUser = this.inactiveUser;
             }
         );
-        randomUser.username = `randomuser${Math.floor(Math.random() * Math.floor(999999))}`;
+        randomUser.username = `randomuser${Math.floor(Math.random() * 999999)}`;
         randomUser.password = {
             crypt: 'zpAReoZzPnwmQ',
             clear: 'topsecret',
@@ -89,7 +89,7 @@ describe('Login tests', function() {
         cy.doRequest('PUT', `/arvados/v1/api_client_authorizations/${tokenUuid}`, {
             id: tokenUuid,
             api_client_authorization: JSON.stringify({
-                api_token: `randomToken${Math.floor(Math.random() * Math.floor(999999))}`
+                api_token: `randomToken${Math.floor(Math.random() * 999999)}`
             })
         }, null, activeUser.token, true);
         // Should log the user out.
index 17032768fd00436ef92cb4b63d156d0456ddc442..c4d8acaea2e8a58f090b21d6b4b8dc48a31c0e49 100644 (file)
@@ -75,7 +75,7 @@ export class WebDAV {
             r.open(config.method,
                 `${this.defaults.baseURL
                     ? this.defaults.baseURL+'/'
-                    : ''}${config.url}`);
+                    : ''}${encodeURI(config.url)}`);
             const headers = { ...this.defaults.headers, ...config.headers };
             Object
                 .keys(headers)
@@ -85,14 +85,16 @@ export class WebDAV {
                 r.upload.addEventListener('progress', config.onUploadProgress);
             }
 
+            // This event gets triggered on *any* server response
             r.addEventListener('load', () => {
-                if (r.status === 404) {
+                if (r.status >= 400) {
                     return reject(r);
                 } else {
                     return resolve(r);
                 }
             });
 
+            // This event gets triggered on network errors
             r.addEventListener('error', () => {
                 return reject(r);
             });
index 098f27818a85c6406445f08770231909ea400c62..3c6feb5dce8836a92a08074ba32a768af4ec84fc 100644 (file)
@@ -4,5 +4,16 @@
 
 export const getTagValue = (document: Document | Element, tagName: string, defaultValue: string) => {
     const [el] = Array.from(document.getElementsByTagName(tagName));
-    return decodeURI(el ? el.innerHTML : defaultValue);
+    return decodeURI(el ? htmlDecode(el.innerHTML) : defaultValue);
+};
+
+const htmlDecode = (input: string) => {
+    const out = input.split(' ').map((i) => {
+        const doc = new DOMParser().parseFromString(i, "text/html");
+        if (doc.documentElement !== null) {
+            return doc.documentElement.textContent || '';
+        }
+        return '';
+    });
+    return out.join(' ');
 };
index 29f20be26fbc61a9dab267d5707f62774ee3fa57..a89c3a92c89425c7dac55759f63715d3513ffee6 100644 (file)
@@ -128,8 +128,7 @@ export const CollectionPanelFilesComponent = ({ onItemMenuOpen, onSearchChange,
                     : <div style={{ height: 'calc(100% - 60px)' }}>
                         <FileTree
                             onMenuOpen={(ev, item) => onItemMenuOpen(ev, item, isWritable)}
-                            {...treeProps}
-                            items={treeProps.items} /></div>}
+                            {...treeProps} /></div>}
             </>
         }
     </Card>);
index b37ec68dfb8096fa07187344e5e141121c664fc4..8c847ca48e77acb5ba33670d1cf0e8da4dc64200 100644 (file)
@@ -63,6 +63,7 @@ export const FormDialog = withStyles(styles)((props: DialogProjectProps) =>
             </DialogContent>
             <DialogActions className={props.classes.dialogActions}>
                 <Button
+                    data-cy='form-cancel-btn'
                     onClick={props.closeDialog}
                     className={props.classes.button}
                     color="primary"
diff --git a/src/components/rename-dialog/rename-dialog.tsx b/src/components/rename-dialog/rename-dialog.tsx
deleted file mode 100644 (file)
index 75c25b7..0000000
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright (C) The Arvados Authors. All rights reserved.
-//
-// SPDX-License-Identifier: AGPL-3.0
-
-import * as React from "react";
-import { InjectedFormProps, Field } from "redux-form";
-import { Dialog, DialogTitle, DialogContent, DialogActions, Button, DialogContentText, CircularProgress } from "@material-ui/core";
-import { WithDialogProps } from "~/store/dialog/with-dialog";
-import { TextField } from "../text-field/text-field";
-
-export const RenameDialog = (props: WithDialogProps<string> & InjectedFormProps<{ name: string }>) =>
-    <form>
-        <Dialog open={props.open}>
-            <DialogTitle>{`Rename`}</DialogTitle>
-            <DialogContent>
-                <DialogContentText>
-                    {`Please, enter a new name for ${props.data}`}
-                </DialogContentText>
-                <Field
-                    name='name'
-                    component={TextField}
-                />
-            </DialogContent>
-            <DialogActions>
-                <Button
-                    variant='text'
-                    color='primary'
-                    disabled={props.submitting}
-                    onClick={props.closeDialog}>
-                    Cancel
-                    </Button>
-                <Button
-                    variant='contained'
-                    color='primary'
-                    type='submit'
-                    onClick={props.handleSubmit}
-                    disabled={props.pristine || props.invalid || props.submitting}>
-                    {props.submitting
-                        ? <CircularProgress size={20} />
-                        : 'Ok'}
-                </Button>
-            </DialogActions>
-        </Dialog>
-    </form>;
index 6db3d1e2598517e6706ffc787a25b3fa0f3c8f34..54938969ffded1d38c47cf1172d1fc160a7f1990 100644 (file)
@@ -130,7 +130,7 @@ export const Row =  <T, _>(itemList: VirtualTreeItem<T>[], render: any, treeProp
                 : undefined;
         };
 
-        return <div style={style}>
+        return <div data-cy='virtual-file-tree' style={style}>
             <ListItem button className={listItem}
                 style={{
                     paddingLeft: (level + 1) * levelIndentation,
index 704e299990a055742c5a19fdc5cba2e112bc2de0..7b2b2557aaf26eb2c39a729c05048dc6d7ccabbc 100644 (file)
@@ -121,6 +121,7 @@ export const RENAME_FILE_DIALOG = 'renameFileDialog';
 export interface RenameFileDialogData {
     name: string;
     id: string;
+    path: string;
 }
 
 export const openRenameFileDialog = (data: RenameFileDialogData) =>
@@ -129,7 +130,7 @@ export const openRenameFileDialog = (data: RenameFileDialogData) =>
         dispatch(dialogActions.OPEN_DIALOG({ id: RENAME_FILE_DIALOG, data }));
     };
 
-export const renameFile = (newName: string) =>
+export const renameFile = (newFullPath: string) =>
     async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => {
         const dialog = getDialog<RenameFileDialogData>(getState().dialog, RENAME_FILE_DIALOG);
         const currentCollection = getState().collectionPanel.item;
@@ -138,7 +139,7 @@ export const renameFile = (newName: string) =>
             if (file) {
                 dispatch(startSubmit(RENAME_FILE_DIALOG));
                 const oldPath = getFileFullPath(file);
-                const newPath = getFileFullPath({ ...file, name: newName });
+                const newPath = newFullPath;
                 try {
                     await services.collectionService.moveFile(currentCollection.uuid, oldPath, newPath);
                     dispatch<any>(loadCollectionFiles(currentCollection.uuid));
@@ -146,7 +147,7 @@ export const renameFile = (newName: string) =>
                     dispatch(snackbarActions.OPEN_SNACKBAR({ message: 'File name changed.', hideDuration: 2000 }));
                 } catch (e) {
                     const errors: FormErrors<RenameFileDialogData, string> = {
-                        name: 'Could not rename the file'
+                        path: `Could not rename the file: ${e.responseText}`
                     };
                     dispatch(stopSubmit(RENAME_FILE_DIALOG, errors));
                 }
index da96712398e278f6a45de0cd57a6395a7691ad7b..89bb3f96888203691029b322e486cd98b3e0df3f 100644 (file)
@@ -4,12 +4,12 @@
 
 export const disallowDotName = /^\.{1,2}$/;
 export const disallowSlash = /\//;
-
-const ERROR_MESSAGE = "Name cannot be '.' or '..' or contain '/' characters";
+export const disallowLeadingWhitespaces = /^\s+/;
+export const disallowTrailingWhitespaces = /\s+$/;
 
 export const validName = (value: string) => {
     return [disallowDotName, disallowSlash].find(aRule => value.match(aRule) !== null)
-        ? ERROR_MESSAGE
+        ? "Name cannot be '.' or '..' or contain '/' characters"
         : undefined;
 };
 
@@ -18,3 +18,20 @@ export const validNameAllowSlash = (value: string) => {
         ? "Name cannot be '.' or '..'"
         : undefined;
 };
+
+export const validFileName = (value: string) => {
+    return [
+        disallowLeadingWhitespaces,
+        disallowTrailingWhitespaces
+    ].find(aRule => value.match(aRule) !== null)
+        ? `Leading/trailing whitespaces not allowed on '${value}'`
+        : undefined;
+};
+
+export const validFilePath = (filePath: string) => {
+    const errors = filePath.split('/').map(pathPart => {
+        if (pathPart === "") { return "Empty dir name not allowed"; }
+        return validNameAllowSlash(pathPart) || validFileName(pathPart);
+    });
+    return errors.filter(e => e !== undefined)[0];
+};
\ No newline at end of file
index d9eca97f4c2a85b54a424497f3bce836e1380112..81fed2ccccdb4220822a342a38eb3af10a38a735 100644 (file)
@@ -6,7 +6,7 @@ import { require } from './require';
 import { maxLength } from './max-length';
 import { isRsaKey } from './is-rsa-key';
 import { isRemoteHost } from "./is-remote-host";
-import { validName, validNameAllowSlash } from "./valid-name";
+import { validFilePath, validName, validNameAllowSlash } from "./valid-name";
 
 export const TAG_KEY_VALIDATION = [require, maxLength(255)];
 export const TAG_VALUE_VALIDATION = [require, maxLength(255)];
@@ -21,6 +21,7 @@ export const COLLECTION_PROJECT_VALIDATION = [require];
 
 export const COPY_NAME_VALIDATION = [require, maxLength(255)];
 export const COPY_FILE_VALIDATION = [require];
+export const RENAME_FILE_VALIDATION = [require, validFilePath];
 
 export const MOVE_TO_VALIDATION = [require];
 
index 6ce62ca942c55e738f79b2b47a295bd7cc220d1a..bfbdec610e23eeab39fa094b7a198666573036d1 100644 (file)
@@ -29,7 +29,10 @@ export const collectionFilesItemActionSet: ContextMenuActionSet = readOnlyCollec
         name: "Rename",
         icon: RenameIcon,
         execute: (dispatch, resource) => {
-            dispatch<any>(openRenameFileDialog({ name: resource.name, id: resource.uuid }));
+            dispatch<any>(openRenameFileDialog({
+                name: resource.name,
+                id: resource.uuid,
+                path: resource.uuid.split('/').slice(1).join('/') }));
         }
     },
     {
index 1ada703b88940a4b989f092f5c2fffd539bcae88..83cd03289b1641a8a4d6608131730de314356c70 100644 (file)
@@ -18,7 +18,7 @@ describe('CopyToClipboardAction', () => {
     beforeEach(() => {
         props = {
             onClick: jest.fn(),
-            href: 'https://collections.ardev.roche.com/c=ardev-4zz18-k0hamvtwyit6q56/t=1ha4ykd3w14ed19b2gh3uyjrjup38vsx27x1utwdne0bxcfg5d/LIMS/1.html',
+            href: 'https://collections.example.com/c=zzzzz-4zz18-k0hamvtwyit6q56/t=xxxxxxxx/LIMS/1.html',
         };
     });
 
index fa455defed5b0fe6b509745e85c669f12650b565..75f90d83448d082c5dfdf99c5f93beba628cdabe 100644 (file)
@@ -15,7 +15,7 @@ describe('FileViewerAction', () => {
     beforeEach(() => {
         props = {
             onClick: jest.fn(),
-            href: 'https://collections.ardev.roche.com/c=ardev-4zz18-k0hamvtwyit6q56/t=1ha4ykd3w14ed19b2gh3uyjrjup38vsx27x1utwdne0bxcfg5d/LIMS/1.html',
+            href: 'https://collections.example.com/c=zzzzz-4zz18-k0hamvtwyit6q56/t=xxxxxxx/LIMS/1.html',
         };
     });
 
index 3c8f7ebf537f4ed755ba2f380e9251a3a4f4d768..bc0bb3161011d2ef4a81c4fd7269693b4c480633 100644 (file)
@@ -15,6 +15,7 @@ export const CopyCollectionDialog = compose(
     withDialog(COLLECTION_COPY_FORM_NAME),
     reduxForm<CopyFormDialogData>({
         form: COLLECTION_COPY_FORM_NAME,
+        touchOnChange: true,
         onSubmit: (data, dispatch) => {
             dispatch(copyCollection(data));
         }
index 374b070b22e914f6c9865ab0ca4a770b4b327716..68e6380560e673a270904b3195b2ce1ef6cced9b 100644 (file)
@@ -13,6 +13,7 @@ export const CreateCollectionDialog = compose(
     withDialog(COLLECTION_CREATE_FORM_NAME),
     reduxForm<CollectionCreateFormDialogData>({
         form: COLLECTION_CREATE_FORM_NAME,
+        touchOnChange: true,
         onSubmit: (data, dispatch) => {
             // Somehow an extra field called 'files' gets added, copy
             // the data object to get rid of it.
index cfa52639f4d830b6990cf2f35e9bb6b7f6b90763..021a335b63dd6c0ffd9c0da80a5850bde49d26ed 100644 (file)
@@ -12,6 +12,7 @@ import { updateCollection } from "~/store/workbench/workbench-actions";
 export const UpdateCollectionDialog = compose(
     withDialog(COLLECTION_UPDATE_FORM_NAME),
     reduxForm<CollectionUpdateFormDialogData>({
+        touchOnChange: true,
         form: COLLECTION_UPDATE_FORM_NAME,
         onSubmit: (data, dispatch) => {
             dispatch(updateCollection(data));
index 1a806511523b6e0bc3fabeb1322a72ba615e3fcf..98147acc0acd2abdf4a1b29d6c5672009390bb84 100644 (file)
@@ -11,16 +11,18 @@ import { DialogContentText } from '@material-ui/core';
 import { TextField } from '~/components/text-field/text-field';
 import { RENAME_FILE_DIALOG, RenameFileDialogData, renameFile } from '~/store/collection-panel/collection-panel-files/collection-panel-files-actions';
 import { WarningCollection } from '~/components/warning-collection/warning-collection';
+import { RENAME_FILE_VALIDATION } from '~/validators/validators';
 
 export const RenameFileDialog = compose(
     withDialog(RENAME_FILE_DIALOG),
     reduxForm({
         form: RENAME_FILE_DIALOG,
-        onSubmit: (data: { name: string }, dispatch) => {
-            dispatch<any>(renameFile(data.name));
+        touchOnChange: true,
+        onSubmit: (data: { path: string }, dispatch) => {
+            dispatch<any>(renameFile(data.path));
         }
     })
-)((props: WithDialogProps<RenameFileDialogData> & InjectedFormProps<{ name: string }>) =>
+)((props: WithDialogProps<RenameFileDialogData> & InjectedFormProps<{ name: string, path: string }>) =>
     <FormDialog
         dialogTitle='Rename'
         formFields={RenameDialogFormFields}
@@ -33,9 +35,10 @@ const RenameDialogFormFields = (props: WithDialogProps<RenameFileDialogData>) =>
         {`Please, enter a new name for ${props.data.name}`}
     </DialogContentText>
     <Field
-        name='name'
+        name='path'
         component={TextField}
         autoFocus={true}
+        validate={RENAME_FILE_VALIDATION}
     />
-    <WarningCollection text="Renaming a file will change content address." />
+    <WarningCollection text="Renaming a file will change the collection's content address." />
 </>;