[ACA-4557] Upload Dialog fixes and refactoring (#7507)

* fix random errors for versioning

* remove node deletion on upload cancel

* fix after rebase

* restore the "complete" button

* simplify e2e

* delete obsolte test

e2e/content-services/upload/upload-dialog.e2e.ts

@@ -118,17 +118,6 @@ describe('Upload component', () => {
         await uploadDialog.dialogIsNotDisplayed();
     });
 
-    it('[C260168] Should be possible to cancel upload using dialog icon', async () => {
-        await contentServicesPage.uploadFile(pdfFileModel.location);
-        await contentServicesPage.checkContentIsDisplayed(pdfFileModel.name);
-        await uploadDialog.removeUploadedFile(pdfFileModel.name);
-        await uploadDialog.fileIsCancelled(pdfFileModel.name);
-        await expect(await uploadDialog.getTitleText()).toEqual('Upload canceled');
-        await uploadDialog.clickOnCloseButton();
-        await uploadDialog.dialogIsNotDisplayed();
-        await contentServicesPage.checkContentIsNotDisplayed(pdfFileModel.name);
-    });
-
     it('[C260176] Should remove files from upload dialog box when closed', async () => {
         await contentServicesPage.uploadFile(pngFileModelTwo.location);
         await contentServicesPage.checkContentIsDisplayed(pngFileModelTwo.name);

e2e/content-services/upload/uploader-component.e2e.ts

@@ -249,12 +249,6 @@ describe('Upload component', () => {
         await uploadToggles.enableExtensionFilter();
         await browser.sleep(1000);
         await uploadToggles.addExtension('.docx');
-        await contentServicesPage.uploadFile(docxFileModel.location);
-        await contentServicesPage.checkContentIsDisplayed(docxFileModel.name);
-        await uploadDialog.removeUploadedFile(docxFileModel.name);
-        await uploadDialog.fileIsCancelled(docxFileModel.name);
-        await uploadDialog.clickOnCloseButton();
-        await uploadDialog.dialogIsNotDisplayed();
         await contentServicesPage.uploadFile(pngFileModel.location);
         await contentServicesPage.checkContentIsNotDisplayed(pngFileModel.name);
         await uploadDialog.dialogIsNotDisplayed();
@@ -268,14 +262,6 @@ describe('Upload component', () => {
 
         const dragAndDropArea = element.all(by.css('adf-upload-drag-area div')).first();
 
-        await DropActions.dropFile(dragAndDropArea, docxFileModel.location);
-        await contentServicesPage.checkContentIsDisplayed(docxFileModel.name);
-
-        await uploadDialog.removeUploadedFile(docxFileModel.name);
-        await uploadDialog.fileIsCancelled(docxFileModel.name);
-        await uploadDialog.clickOnCloseButton();
-        await uploadDialog.dialogIsNotDisplayed();
-
         await DropActions.dropFile(dragAndDropArea, pngFileModel.location);
         await contentServicesPage.checkContentIsNotDisplayed(pngFileModel.name);
         await uploadDialog.dialogIsNotDisplayed();

lib/content-services/src/lib/upload/components/file-uploading-dialog.component.html

@@ -61,7 +61,6 @@
                 <ng-template let-file="$implicit">
                      <adf-file-uploading-list-row
                         [file]="file"
-                        (remove)="uploadList.removeFile(file)"
                         (cancel)="uploadList.cancelFile(file)">
                     </adf-file-uploading-list-row>
                 </ng-template>

lib/content-services/src/lib/upload/components/file-uploading-list-row.component.html

@@ -27,7 +27,7 @@
         (keyup.enter)="onCancel(file)"
         (click)="onCancel(file)"
         data-automation-id="cancel-upload-progress"
-        *ngIf="file.status === FileUploadStatus.Progress || file.status === FileUploadStatus.Starting"
+        *ngIf="isUploading()"
         [attr.aria-label]="'ADF_FILE_UPLOAD.ARIA-LABEL.CANCEL_FILE_UPLOAD' | translate: { file: file.name }"
         class="adf-file-uploading-row__group adf-file-uploading-row__group--toggle"
         title="{{ 'ADF_FILE_UPLOAD.BUTTON.CANCEL_FILE' | translate }}">
@@ -45,25 +45,19 @@
     <button mat-icon-button
         adf-toggle-icon
         #toggleIcon="toggleIcon"
-        *ngIf="file.status === FileUploadStatus.Complete && !isUploadVersion()"
-        (click)="onRemove(file)"
+        *ngIf="isUploadComplete()"
         class="adf-file-uploading-row__group"
         [attr.aria-label]="'ADF_FILE_UPLOAD.ARIA-LABEL.REMOVE_FILE' | translate: { file: file.name }"
         title="{{ 'ADF_FILE_UPLOAD.BUTTON.REMOVE_FILE' | translate }}">
 
-        <mat-icon *ngIf="!toggleIcon.isToggled"
+        <mat-icon
             class="adf-file-uploading-row__status adf-file-uploading-row__status--done">
             check_circle
         </mat-icon>
-
-        <mat-icon *ngIf="toggleIcon.isToggled"
-            class="adf-file-uploading-row__action adf-file-uploading-row__action--remove">
-            remove_circle
-        </mat-icon>
     </button>
 
     <div
-        *ngIf="file.status === FileUploadStatus.Complete && isUploadVersion()"
+        *ngIf="isUploadVersionComplete()"
         class="adf-file-uploading-row__file-version"
         [attr.aria-label]="'ADF_FILE_UPLOAD.STATUS.FILE_DONE_STATUS' | translate"
         role="status"
@@ -79,7 +73,7 @@
         adf-toggle-icon
         #toggleIconCancel="toggleIcon"
         mat-icon-button
-        *ngIf="file.status === FileUploadStatus.Pending"
+        *ngIf="canCancelUpload()"
         (click)="onCancel(file)"
         data-automation-id="cancel-upload-queue"
         class="adf-file-uploading-row__group"
@@ -101,7 +95,7 @@
     <div
         tabindex="0"
         role="status"
-        *ngIf="file.status === FileUploadStatus.Error"
+        *ngIf="isUploadError()"
         class="adf-file-uploading-row__block adf-file-uploading-row__status--error">
         <mat-icon mat-list-icon
             [attr.aria-label]="'ADF_FILE_UPLOAD.ARIA-LABEL.UPLOAD_FILE_ERROR' | translate: { error: file.errorCode | adfFileUploadError }"

lib/content-services/src/lib/upload/components/file-uploading-list-row.component.spec.ts

@@ -50,13 +50,6 @@ describe('FileUploadingListRowComponent', () => {
 
             expect(component.cancel.emit).toHaveBeenCalledWith(file);
         });
-
-        it('should emit remove event', () => {
-            spyOn(component.remove, 'emit');
-            component.onRemove(component.file);
-
-            expect(component.remove.emit).toHaveBeenCalledWith(file);
-        });
     });
 
     it('should render node version when upload a version file', () => {
@@ -71,20 +64,6 @@ describe('FileUploadingListRowComponent', () => {
         ).textContent).toContain('1');
     });
 
-    it('should not emit remove event on a version file', () => {
-        spyOn(component.remove, 'emit');
-        component.file = new FileModel({ name: 'fake-name' } as File);
-        component.file.options = { newVersion: true };
-        component.file.data = { entry: { properties: { 'cm:versionLabel': '1' } } };
-        component.file.status = FileUploadStatus.Complete;
-
-        fixture.detectChanges();
-        const uploadCompleteIcon = document.querySelector('.adf-file-uploading-row__file-version .adf-file-uploading-row__status--done');
-        uploadCompleteIcon.dispatchEvent(new MouseEvent('click'));
-
-        expect(component.remove.emit).not.toHaveBeenCalled();
-    });
-
     it('should show cancel button when upload is in progress', async () => {
         component.file = new FileModel({ name: 'fake-name' } as File);
         component.file.data = { entry: { properties: { 'cm:versionLabel': '1' } } };

lib/content-services/src/lib/upload/components/file-uploading-list-row.component.ts

@@ -29,22 +29,12 @@ export class FileUploadingListRowComponent {
     file: FileModel;
 
     @Output()
-    cancel: EventEmitter<FileModel> = new EventEmitter<FileModel>();
-
-    @Output()
-    remove: EventEmitter<FileModel> = new EventEmitter<FileModel>();
-
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    FileUploadStatus = FileUploadStatus;
+    cancel = new EventEmitter<FileModel>();
 
     onCancel(file: FileModel): void {
         this.cancel.emit(file);
     }
 
-    onRemove(file: FileModel): void {
-        this.remove.emit(file);
-    }
-
     showCancelledStatus(): boolean {
         return this.file.status === FileUploadStatus.Cancelled ||
             this.file.status === FileUploadStatus.Aborted ||
@@ -72,4 +62,24 @@ export class FileUploadingListRowComponent {
             this.file.data.entry.properties['cm:versionLabel']
         );
     }
+
+    canCancelUpload(): boolean {
+        return this.file && this.file.status === FileUploadStatus.Pending;
+    }
+
+    isUploadError(): boolean {
+        return this.file && this.file.status === FileUploadStatus.Error;
+    }
+
+    isUploading(): boolean {
+        return this.file && (this.file.status === FileUploadStatus.Progress || this.file.status === FileUploadStatus.Starting);
+    }
+
+    isUploadComplete(): boolean {
+        return this.file.status === FileUploadStatus.Complete && !this.isUploadVersion();
+    }
+
+    isUploadVersionComplete(): boolean {
+        return this.file && (this.file.status === FileUploadStatus.Complete && this.isUploadVersion());
+    }
 }

lib/content-services/src/lib/upload/components/file-uploading-list.component.spec.ts

@@ -17,7 +17,7 @@
 
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { TranslationService, FileUploadStatus, NodesApiService, UploadService, setupTestBed, FileModel } from '@alfresco/adf-core';
-import { of, throwError } from 'rxjs';
+import { of } from 'rxjs';
 import { FileUploadingListComponent } from './file-uploading-list.component';
 import { ContentTestingModule } from '../../testing/content.testing.module';
 import { TranslateModule } from '@ngx-translate/core';
@@ -77,91 +77,6 @@ describe('FileUploadingListComponent', () => {
         });
     });
 
-    describe('removeFile()', () => {
-        it('should change file status when api returns success', () => {
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(of(file));
-
-            component.removeFile(file);
-            fixture.detectChanges();
-
-            expect(file.status).toBe(FileUploadStatus.Deleted);
-        });
-
-        it('should change file status when api returns error', () => {
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(throwError(file));
-
-            component.removeFile(file);
-            fixture.detectChanges();
-
-            expect(file.status).toBe(FileUploadStatus.Error);
-        });
-
-        it('should call uploadService on error', () => {
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(throwError(file));
-
-            component.removeFile(file);
-            fixture.detectChanges();
-
-            expect(uploadService.cancelUpload).toHaveBeenCalled();
-        });
-
-        it('should call uploadService on success', () => {
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(of(file));
-
-            component.removeFile(file);
-            fixture.detectChanges();
-
-            expect(uploadService.cancelUpload).toHaveBeenCalled();
-        });
-
-        it('should set `Deleted` status on file version instances when original is removed', () => {
-            component.files = [
-                {
-                    data: {
-                        entry: { id: 'nodeId' }
-                    },
-                    name: 'file',
-                    status: FileUploadStatus.Complete,
-                    options: {
-                        newVersion: false
-                    }
-                } as FileModel,
-                {
-                    data: {
-                        entry: { id: 'nodeId' }
-                    },
-                    name: 'file_v1',
-                    status: FileUploadStatus.Complete,
-                    options: {
-                        newVersion: true
-                    }
-                } as FileModel
-            ];
-
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(of(component.files[0]));
-
-            component.removeFile(component.files[0]);
-            fixture.detectChanges();
-
-            expect(nodesApiService.deleteNode).toHaveBeenCalledTimes(1);
-            expect(component.files[0].status).toBe(FileUploadStatus.Deleted);
-            expect(component.files[1].status).toBe(FileUploadStatus.Deleted);
-        });
-
-        describe('Events', () => {
-
-            it('should throw an error event if delete file goes wrong', (done) => {
-                spyOn(nodesApiService, 'deleteNode').and.returnValue(throwError(file));
-
-                component.error.subscribe(() => {
-                    done();
-                });
-
-                component.removeFile(file);
-            });
-        });
-    });
-
     describe('cancelAllFiles()', () => {
         beforeEach(() => {
             component.files = [
@@ -194,15 +109,6 @@ describe('FileUploadingListComponent', () => {
             expect(uploadService.cancelUpload).not.toHaveBeenCalled();
         });
 
-        it('should call deleteNode when there are completed uploads', () => {
-            spyOn(nodesApiService, 'deleteNode').and.returnValue(of({}));
-
-            component.files[0].status = FileUploadStatus.Complete;
-            component.cancelAllFiles();
-
-            expect(nodesApiService.deleteNode).toHaveBeenCalled();
-        });
-
         it('should call uploadService when there are uploading files', () => {
             spyOn(nodesApiService, 'deleteNode').and.returnValue(of({}));
 

lib/content-services/src/lib/upload/components/file-uploading-list.component.ts

@@ -18,7 +18,6 @@
 import {
     FileModel,
     FileUploadStatus,
-    NodesApiService,
     TranslationService,
     UploadService
 } from '@alfresco/adf-core';
@@ -30,8 +29,6 @@ import {
     TemplateRef,
     EventEmitter
 } from '@angular/core';
-import { Observable, forkJoin, of } from 'rxjs';
-import { map, catchError } from 'rxjs/operators';
 
 @Component({
     selector: 'adf-file-uploading-list',
@@ -39,9 +36,6 @@ import { map, catchError } from 'rxjs/operators';
     styleUrls: ['./file-uploading-list.component.scss']
 })
 export class FileUploadingListComponent {
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    FileUploadStatus = FileUploadStatus;
-
     @ContentChild(TemplateRef)
     template: any;
 
@@ -54,7 +48,6 @@ export class FileUploadingListComponent {
 
     constructor(
         private uploadService: UploadService,
-        private nodesApi: NodesApiService,
         private translateService: TranslationService) {
     }
 
@@ -81,41 +74,27 @@ export class FileUploadingListComponent {
      * @memberOf FileUploadingListComponent
      */
     removeFile(file: FileModel): void {
-        this.deleteNode(file).subscribe(() => {
-            if (file.status === FileUploadStatus.Error) {
-                this.notifyError(file);
-            }
+        if (file.status === FileUploadStatus.Error) {
+            this.notifyError(file);
+        }
 
+        if (this.isUploadingFile(file)) {
             this.cancelNodeVersionInstances(file);
             this.uploadService.cancelUpload(file);
-        });
+        }
+
+        this.files = this.files.filter(entry => entry !== file);
     }
 
     /**
      * Calls the appropriate methods for each file, depending on state
      */
     cancelAllFiles(): void {
-        const deletedFiles: Observable<FileModel>[] = [];
+        const filesToCancel = this.files.filter(file => this.isUploadingFile(file));
 
-        this.files.forEach((file) => {
-            if (this.isUploadingFile(file)) {
-                this.uploadService.cancelUpload(file);
-            } else if (file.status === FileUploadStatus.Complete) {
-                deletedFiles.push(this.deleteNode(file));
-            }
-        });
-
-        forkJoin(...deletedFiles).subscribe((files: FileModel[]) => {
-            const errors = files.filter(
-                (file) => file.status === FileUploadStatus.Error
-            );
-
-            if (errors.length) {
-                this.notifyError(...errors);
-            }
-
-            this.uploadService.cancelUpload(...files);
-        });
+        if (filesToCancel.length > 0) {
+            this.uploadService.cancelUpload(...filesToCancel);
+        }
     }
 
     /**
@@ -149,21 +128,6 @@ export class FileUploadingListComponent {
         );
     }
 
-    private deleteNode(file: FileModel): Observable<FileModel> {
-        const { id } = file.data.entry;
-
-        return this.nodesApi.deleteNode(id, { permanent: true }).pipe(
-            map(() => {
-                file.status = FileUploadStatus.Deleted;
-                return file;
-            }),
-            catchError(() => {
-                file.status = FileUploadStatus.Error;
-                return of(file);
-            })
-        );
-    }
-
     private cancelNodeVersionInstances(file: FileModel) {
         this.files
             .filter(

lib/core/datatable/components/datatable-cell/datatable-cell.component.ts

@@ -90,7 +90,7 @@ export class DataTableCellComponent implements OnInit, OnDestroy {
         this.alfrescoApiService.nodeUpdated
             .pipe(takeUntil(this.onDestroy$))
             .subscribe(node => {
-                if (this.row) {
+                if (this.row && node && node.id) {
                     if (this.row['node'].entry.id === node.id) {
                         this.row['node'].entry = node;
                         this.row['cache'][this.column.key] = this.column.key.split('.').reduce((source, key) => source ? source[key] : '', node);

lib/core/services/upload.service.ts

@@ -186,7 +186,10 @@ export class UploadService {
                 promise.next();
             } else {
                 const performAction = this.getAction(file);
-                performAction();
+
+                if (performAction) {
+                    performAction();
+                }
             }
         });
     }