From 213ad211c637c0bedda3e936c9c992b38da06d36 Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:56:00 +0300 Subject: [PATCH 1/8] PER-9881: The location picker never closes Change the focus event to keydown event so the location picker closes when the user presses done. --- .../file-browser/components/sidebar/sidebar.component.html | 2 +- .../file-browser/components/sidebar/sidebar.component.ts | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.html b/src/app/file-browser/components/sidebar/sidebar.component.html index e6404fea7..0d761a33f 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.html +++ b/src/app/file-browser/components/sidebar/sidebar.component.html @@ -161,7 +161,7 @@ class="sidebar-item-content" (click)="onLocationClick()" tabindex="0" - (focus)="onLocationClick()" + (keydown)="onLocationEnterPress($event)" [class.can-edit]="canEdit" > diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index 04e720710..846ad437b 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -160,6 +160,7 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { async onFinishEditing(property: KeysOfType, value: string) { this.editService.saveItemVoProperty(this.selectedItem, property, value); this.cdr.detectChanges(); + console.log('here'); } onLocationClick() { @@ -168,6 +169,12 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { } } + onLocationEnterPress(e: KeyboardEvent): void { + if (this.canEdit && e.key === 'Enter') { + this.editService.openLocationDialog(this.selectedItem); + } + } + onShareClick() { this.editService.openShareDialog(this.selectedItem); } From 7a0fa30987a608b9f9f70565e37ec9e0f88a3855 Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Thu, 17 Oct 2024 14:03:52 +0300 Subject: [PATCH 2/8] removed console log --- src/app/file-browser/components/sidebar/sidebar.component.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index 846ad437b..92d2af799 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -160,7 +160,6 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { async onFinishEditing(property: KeysOfType, value: string) { this.editService.saveItemVoProperty(this.selectedItem, property, value); this.cdr.detectChanges(); - console.log('here'); } onLocationClick() { From 84540a63134353c8905d549702b806ac41f01123 Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:25:30 +0300 Subject: [PATCH 3/8] add tests --- .../sidebar/sidebar.component.spec.ts | 107 ++++++++++++++---- .../components/sidebar/sidebar.component.ts | 6 +- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index c316f8454..89c341d0d 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -1,25 +1,82 @@ -// import { async, ComponentFixture, TestBed } from '@angular/core/testing'; - -// import { SidebarComponent } from './sidebar.component'; - -// describe('SidebarComponent', () => { -// let component: SidebarComponent; -// let fixture: ComponentFixture; - -// beforeEach(async(() => { -// TestBed.configureTestingModule({ -// declarations: [ SidebarComponent ] -// }) -// .compileComponents(); -// })); - -// beforeEach(() => { -// fixture = TestBed.createComponent(SidebarComponent); -// component = fixture.componentInstance; -// fixture.detectChanges(); -// }); - -// it('should create', () => { -// expect(component).toBeTruthy(); -// }); -// }); +/* @format */ +import { Shallow } from 'shallow-render'; +import { FileBrowserComponentsModule } from '@fileBrowser/file-browser-components.module'; +import { DataService } from '@shared/services/data/data.service'; +import { EditService } from '@core/services/edit/edit.service'; +import { AccountService } from '@shared/services/account/account.service'; +import { ArchiveVO, RecordVO } from '@models/index'; +import { of } from 'rxjs'; +import { SidebarComponent } from './sidebar.component'; + +const mockDataService = { + selectedItems$: () => + of( + new Set([ + new RecordVO({ + accessRole: 'access.role.owner', + }), + ]), + ), +}; + +const mockEditService = { + openLocationDialog: (record) => {}, +}; + +class MockAccountService { + getArchive() { + return new ArchiveVO({}); + } + checkMinimumArchiveAccess() { + return true; + } + checkMinimumAccess() { + return true; + } +} + +describe('SidebarComponent', () => { + let shallow: Shallow; + + beforeEach(() => { + shallow = new Shallow(SidebarComponent, FileBrowserComponentsModule) + .provideMock({ + provide: DataService, + useValue: mockDataService, + }) + .provide({ + provide: EditService, + useValue: mockEditService, + }) + .provideMock({ + provide: AccountService, + useClass: MockAccountService, + }); + }); + + it('should create', async () => { + const { instance } = await shallow.render(); + + expect(instance).toBeTruthy(); + }); + + it('should open location dialog on Enter key press if editable', async () => { + const { instance, fixture, inject } = await shallow.render(); + + inject(AccountService); + + instance.canEdit = true; + instance.selectedItem = new RecordVO({ + accessRole: 'access.role.owner', + }); + instance.selectedItems = [instance.selectedItem]; + fixture.detectChanges(); + + const mockEvent = new KeyboardEvent('keydown', { key: 'Enter' }); + instance.onLocationEnterPress(mockEvent); + + expect(mockEditService.openLocationDialog).toHaveBeenCalledWith( + instance.selectedItem, + ); + }); +}); diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index 92d2af799..f2d7bfad4 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -51,10 +51,10 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { this.subscriptions.push( this.dataService.selectedItems$().subscribe(async (selectedItems) => { - if (!selectedItems.size) { + if (!selectedItems?.size) { this.selectedItem = this.dataService.currentFolder; this.selectedItems = null; - } else if (selectedItems.size === 1) { + } else if (selectedItems?.size === 1) { this.selectedItem = Array.from(selectedItems.keys())[0]; this.selectedItems = null; } else { @@ -132,7 +132,7 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { !viewOnly && this.accountService.checkMinimumArchiveAccess(AccessRole.Editor); - if (items.length !== 1) { + if (items?.length !== 1) { this.canShare = false; } else { this.canShare = From e863b0380727d5e1a6ab3d80e2be6f4647d0628b Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:32:41 +0300 Subject: [PATCH 4/8] add tests --- .../file-browser/components/sidebar/sidebar.component.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index 89c341d0d..7b81fa3d2 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -8,6 +8,8 @@ import { ArchiveVO, RecordVO } from '@models/index'; import { of } from 'rxjs'; import { SidebarComponent } from './sidebar.component'; +const itemsArray = [new RecordVO({}), new RecordVO({})]; + const mockDataService = { selectedItems$: () => of( @@ -17,6 +19,7 @@ const mockDataService = { }), ]), ), + fetchFullItems: (itemsArray) => {}, }; const mockEditService = { From 373d64d256beea2f919f5c74cffd153f8c6ac490 Mon Sep 17 00:00:00 2001 From: Natalie Martin Date: Tue, 22 Oct 2024 09:44:37 -0700 Subject: [PATCH 5/8] Fix SidebarComponent tests A Pipe was being mocked by shallow-render when we didn't want it to be. Add a `dontMock` section with the `asRecord` pipe provided. Also add a null check to `selectedItem.FileVOs`, which seemed to be triggering issues in tests. Clean up some of the code formatting and unnecessary lines in the SidebarComponent tests while we're there. --- .../sidebar/sidebar.component.spec.ts | 35 ++++++++----------- .../components/sidebar/sidebar.component.ts | 1 + 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index 7b81fa3d2..b3ebed76c 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -6,10 +6,9 @@ import { EditService } from '@core/services/edit/edit.service'; import { AccountService } from '@shared/services/account/account.service'; import { ArchiveVO, RecordVO } from '@models/index'; import { of } from 'rxjs'; +import { RecordCastPipe } from '@shared/pipes/cast.pipe'; import { SidebarComponent } from './sidebar.component'; -const itemsArray = [new RecordVO({}), new RecordVO({})]; - const mockDataService = { selectedItems$: () => of( @@ -19,11 +18,11 @@ const mockDataService = { }), ]), ), - fetchFullItems: (itemsArray) => {}, + fetchFullItems: (_) => {}, }; const mockEditService = { - openLocationDialog: (record) => {}, + openLocationDialog: (_) => {}, }; class MockAccountService { @@ -47,14 +46,15 @@ describe('SidebarComponent', () => { provide: DataService, useValue: mockDataService, }) - .provide({ + .provideMock({ provide: EditService, useValue: mockEditService, }) .provideMock({ provide: AccountService, useClass: MockAccountService, - }); + }) + .dontMock(RecordCastPipe); }); it('should create', async () => { @@ -64,22 +64,17 @@ describe('SidebarComponent', () => { }); it('should open location dialog on Enter key press if editable', async () => { - const { instance, fixture, inject } = await shallow.render(); - - inject(AccountService); - - instance.canEdit = true; - instance.selectedItem = new RecordVO({ - accessRole: 'access.role.owner', - }); - instance.selectedItems = [instance.selectedItem]; - fixture.detectChanges(); + const { instance } = await shallow.render(); - const mockEvent = new KeyboardEvent('keydown', { key: 'Enter' }); - instance.onLocationEnterPress(mockEvent); + const locationDialogSpy = spyOn( + mockEditService, + 'openLocationDialog', + ).and.callThrough(); - expect(mockEditService.openLocationDialog).toHaveBeenCalledWith( - instance.selectedItem, + instance.onLocationEnterPress( + new KeyboardEvent('keydown', { key: 'Enter' }), ); + + expect(locationDialogSpy).toHaveBeenCalledWith(instance.selectedItem); }); }); diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index f2d7bfad4..b4aadd910 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -93,6 +93,7 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { } if ( this.selectedItem instanceof RecordVO && + this.selectedItem.FileVOs && this.selectedItem.FileVOs[0] ) { this.originalFileExtension = this.selectedItem.FileVOs.find( From 79e3b8ae9f09c32fc5bd8708259dd51db67afa6d Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Wed, 23 Oct 2024 13:37:44 +0300 Subject: [PATCH 6/8] added more tests --- .../sidebar/sidebar.component.spec.ts | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index b3ebed76c..dd40702fa 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -77,4 +77,62 @@ describe('SidebarComponent', () => { expect(locationDialogSpy).toHaveBeenCalledWith(instance.selectedItem); }); + + it('should set currentTab correctly when setCurrentTab is called', async () => { + const { instance, fixture } = await shallow.render(); + + instance.setCurrentTab('info'); + fixture.detectChanges(); + + expect(instance.currentTab).toBe('info'); + + instance.isRootFolder = false; + instance.isPublicItem = false; + instance.setCurrentTab('sharing'); + fixture.detectChanges(); + + expect(instance.currentTab).toBe('sharing'); + }); + + it('should call editService.openLocationDialog when onLocationClick is called if editable', async () => { + const { instance, inject } = await shallow.render(); + const editService = inject(EditService); + spyOn(editService, 'openLocationDialog'); + + instance.canEdit = true; + instance.selectedItem = new RecordVO({}); + + instance.onLocationClick(); + + expect(editService.openLocationDialog).toHaveBeenCalledWith(instance.selectedItem); + }); + + + it('should correctly update canEdit and canShare when checkPermissions is called', async () => { + const { instance } = await shallow.render(); + + instance.selectedItem = new RecordVO({ + accessRole: 'access.role.editor', + }); + instance.selectedItems = [instance.selectedItem]; + instance.isRootFolder = false; + instance.isPublicItem = false; + + instance.checkPermissions(); + + expect(instance.canEdit).toBe(true); + expect(instance.canShare).toBe(true); + + instance.selectedItem = new RecordVO({ + accessRole: 'access.role.viewer', + }); + instance.selectedItems = [instance.selectedItem]; + instance.isRootFolder = false; + instance.isPublicItem = false; + + instance.checkPermissions(); + + expect(instance.canEdit).toBe(false); + expect(instance.canShare).toBe(true); + }); }); From d0749750bfb06f74ff2be4aeb640bc71af000b88 Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Wed, 23 Oct 2024 13:41:24 +0300 Subject: [PATCH 7/8] prettier --- .../components/sidebar/sidebar.component.spec.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts index dd40702fa..b8178cc6e 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.spec.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.spec.ts @@ -98,15 +98,16 @@ describe('SidebarComponent', () => { const { instance, inject } = await shallow.render(); const editService = inject(EditService); spyOn(editService, 'openLocationDialog'); - + instance.canEdit = true; - instance.selectedItem = new RecordVO({}); - + instance.selectedItem = new RecordVO({}); + instance.onLocationClick(); - - expect(editService.openLocationDialog).toHaveBeenCalledWith(instance.selectedItem); + + expect(editService.openLocationDialog).toHaveBeenCalledWith( + instance.selectedItem, + ); }); - it('should correctly update canEdit and canShare when checkPermissions is called', async () => { const { instance } = await shallow.render(); From e307391e076553dad2a1bb6dba7feb2c64f0ec52 Mon Sep 17 00:00:00 2001 From: crisnicandrei <62384997+crisnicandrei@users.noreply.github.com> Date: Wed, 23 Oct 2024 15:28:00 +0300 Subject: [PATCH 8/8] Revert changes --- .../file-browser/components/sidebar/sidebar.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/file-browser/components/sidebar/sidebar.component.ts b/src/app/file-browser/components/sidebar/sidebar.component.ts index b4aadd910..91544a562 100644 --- a/src/app/file-browser/components/sidebar/sidebar.component.ts +++ b/src/app/file-browser/components/sidebar/sidebar.component.ts @@ -51,10 +51,10 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { this.subscriptions.push( this.dataService.selectedItems$().subscribe(async (selectedItems) => { - if (!selectedItems?.size) { + if (!selectedItems.size) { this.selectedItem = this.dataService.currentFolder; this.selectedItems = null; - } else if (selectedItems?.size === 1) { + } else if (selectedItems.size === 1) { this.selectedItem = Array.from(selectedItems.keys())[0]; this.selectedItems = null; } else { @@ -133,7 +133,7 @@ export class SidebarComponent implements OnDestroy, HasSubscriptions { !viewOnly && this.accountService.checkMinimumArchiveAccess(AccessRole.Editor); - if (items?.length !== 1) { + if (items.length !== 1) { this.canShare = false; } else { this.canShare =