Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ACS-5133] view details button is inactive in expanded view #3603

Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,19 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { AppTestingModule } from '../../testing/app-testing.module';
import { DetailsComponent } from './details.component';
import { ActivatedRoute } from '@angular/router';
import { ActivatedRoute, NavigationStart, Router } from '@angular/router';
import { BehaviorSubject, of, Subject } from 'rxjs';
import { NO_ERRORS_SCHEMA } from '@angular/core';
import { Store } from '@ngrx/store';
import { DefaultProjectorFn, MemoizedSelector, Store } from '@ngrx/store';
import { ContentApiService } from '@alfresco/aca-shared';
import { NavigateToFolder, SetSelectedNodesAction, STORE_INITIAL_APP_DATA } from '@alfresco/aca-shared/store';
import {
AppStore,
isInfoDrawerOpened,
NavigateToFolder,
NavigateToPreviousPage,
SetSelectedNodesAction,
STORE_INITIAL_APP_DATA
} from '@alfresco/aca-shared/store';
import { NodeEntry, PathElement } from '@alfresco/js-api';
import { RouterTestingModule } from '@angular/router/testing';
import { AuthenticationService, PageTitleService } from '@alfresco/adf-core';
Expand Down Expand Up @@ -100,6 +107,7 @@ describe('DetailsComponent', () => {
contentApiService = TestBed.inject(ContentApiService);
contentService = TestBed.inject(ContentService);
store = TestBed.inject(Store);
storeMock.dispatch.calls.reset();

node = {
entry: {
Expand Down Expand Up @@ -242,4 +250,45 @@ describe('DetailsComponent', () => {
component.setActiveTab('permissions');
expect(component.activeTab).not.toBe(2);
});

describe('infoDrawerOpened$ event', () => {
let infoDrawerOpened$: Subject<boolean>;

beforeEach(() => {
infoDrawerOpened$ = new Subject<boolean>();
spyOn(store, 'select').and.callFake((mapFn: MemoizedSelector<AppStore, boolean, DefaultProjectorFn<boolean>>) =>
mapFn === isInfoDrawerOpened ? infoDrawerOpened$ : mockStream
);
});

it('should dispatch store NavigateToPreviousPage by store if info drawer is closed', () => {
component.ngOnInit();

infoDrawerOpened$.next(false);
expect(storeMock.dispatch).toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer is opened', () => {
component.ngOnInit();

infoDrawerOpened$.next(true);
expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer opening state is not changed', () => {
component.ngOnInit();

expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer is closed but there occurred NavigationStart event', () => {
Object.defineProperty(TestBed.inject(Router), 'events', {
value: of(new NavigationStart(1, ''))
});
component.ngOnInit();

infoDrawerOpened$.next(false);
expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@
*/

import { Component, OnDestroy, OnInit, ViewEncapsulation } from '@angular/core';
import { ActivatedRoute } from '@angular/router';
import { ActivatedRoute, NavigationStart } from '@angular/router';
import { ContentApiService, PageComponent, PageLayoutComponent, ToolbarComponent } from '@alfresco/aca-shared';
import { NavigateToFolder, NavigateToPreviousPage, SetSelectedNodesAction } from '@alfresco/aca-shared/store';
import { Subject } from 'rxjs';
import { merge, Subject } from 'rxjs';
import { BreadcrumbModule, ContentService, PermissionManagerModule } from '@alfresco/adf-content-services';
import { CommonModule } from '@angular/common';
import { TranslateModule } from '@ngx-translate/core';
Expand All @@ -37,7 +37,7 @@ import { MatButtonModule } from '@angular/material/button';
import { MetadataTabComponent } from '../info-drawer/metadata-tab/metadata-tab.component';
import { CommentsTabComponent } from '../info-drawer/comments-tab/comments-tab.component';
import { NodeEntry, PathElement } from '@alfresco/js-api';
import { takeUntil } from 'rxjs/operators';
import { first, takeUntil } from 'rxjs/operators';
import { ContentActionRef } from '@alfresco/adf-extensions';

@Component({
Expand Down Expand Up @@ -99,6 +99,20 @@ export class DetailsComponent extends PageComponent implements OnInit, OnDestroy
.subscribe((aspectActions) => {
this.aspectActions = aspectActions;
});
this.infoDrawerOpened$
.pipe(
first((opened) => !opened),
takeUntil(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you don't need takeUntil when you're using first as after emitting the first value first then completes according to rxjs docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but what in case if somebody will exit page before any value will be emitted? Then first won't be executed but still we would need to cancel subscription

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right good point since infoDrawerOpened$ comes from the store so it won't complete when user will exit the page

merge(
this.onDestroy$,
this.router.events.pipe(
first((event) => event instanceof NavigationStart),
takeUntil(this.onDestroy$)
)
)
)
)
.subscribe(() => this.goBack());
}

setActiveTab(tabName: string) {
Expand Down
66 changes: 50 additions & 16 deletions projects/aca-content/src/lib/store/effects/node.effects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,38 @@
* from Hyland Software. If not, see <http://www.gnu.org/licenses/>.
*/

import { TestBed, fakeAsync, tick } from '@angular/core/testing';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { AppTestingModule } from '../../testing/app-testing.module';
import { NodeEffects } from './node.effects';
import { EffectsModule } from '@ngrx/effects';
import { Store } from '@ngrx/store';
import { ContentManagementService } from '../../services/content-management.service';
import {
SharedStoreModule,
ShareNodeAction,
SetSelectedNodesAction,
UnshareNodesAction,
PurgeDeletedNodesAction,
RestoreDeletedNodesAction,
DeleteNodesAction,
UndoDeleteNodesAction,
CopyNodesAction,
CreateFolderAction,
DeleteNodesAction,
EditFolderAction,
CopyNodesAction,
MoveNodesAction,
UnlockWriteAction,
ExpandInfoDrawerAction,
FullscreenViewerAction,
PrintFileAction,
SetCurrentFolderAction,
ManageAspectsAction,
ManagePermissionsAction,
ShowLoaderAction
MoveNodesAction,
PrintFileAction,
PurgeDeletedNodesAction,
RestoreDeletedNodesAction,
SetCurrentFolderAction,
SetInfoDrawerStateAction,
SetSelectedNodesAction,
SharedStoreModule,
ShareNodeAction,
ShowLoaderAction,
UndoDeleteNodesAction,
UnlockWriteAction,
UnshareNodesAction
} from '@alfresco/aca-shared/store';
import { RenditionService } from '@alfresco/adf-content-services';
import { ViewerEffects } from './viewer.effects';
import { Router } from '@angular/router';
import { NavigationEnd, Router } from '@angular/router';
import { of } from 'rxjs';
import { MatDialogModule } from '@angular/material/dialog';
import { MatSnackBarModule } from '@angular/material/snack-bar';
Expand Down Expand Up @@ -403,6 +405,21 @@ describe('NodeEffects', () => {

expect(router.navigate).not.toHaveBeenCalled();
});

it('should call dispatch on store with SetInfoDrawerStateAction when NavigationEnd event occurs', () => {
spyOn(store, 'dispatch').and.callThrough();
Object.defineProperty(router, 'events', {
value: of(new NavigationEnd(1, '', ''))
});

store.dispatch(new ManagePermissionsAction(null));
expect(store.dispatch).toHaveBeenCalledWith(jasmine.any(SetInfoDrawerStateAction));
expect(store.dispatch).toHaveBeenCalledWith(
jasmine.objectContaining({
payload: true
})
);
});
});

describe('printFile$', () => {
Expand Down Expand Up @@ -507,4 +524,21 @@ describe('NodeEffects', () => {
expect(contentService.manageAspects).toHaveBeenCalledWith({ entry: { isFile: false, id: 'folder-node-id' } }, undefined);
}));
});

describe('expandInfoDrawer$', () => {
it('should call dispatch on store with SetInfoDrawerStateAction when NavigationEnd event occurs', () => {
spyOn(store, 'dispatch').and.callThrough();
Object.defineProperty(router, 'events', {
value: of(new NavigationEnd(1, '', ''))
});

store.dispatch(new ExpandInfoDrawerAction(undefined));
expect(store.dispatch).toHaveBeenCalledWith(jasmine.any(SetInfoDrawerStateAction));
expect(store.dispatch).toHaveBeenCalledWith(
jasmine.objectContaining({
payload: true
})
);
});
});
});
13 changes: 9 additions & 4 deletions projects/aca-content/src/lib/store/effects/node.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

import { Actions, ofType, createEffect } from '@ngrx/effects';
import { Injectable } from '@angular/core';
import { map, take } from 'rxjs/operators';
import { first, map, take } from 'rxjs/operators';
import { Store } from '@ngrx/store';
import {
AppStore,
Expand All @@ -50,12 +50,12 @@ import {
ExpandInfoDrawerAction,
ManageRulesAction,
ShowLoaderAction,
ToggleInfoDrawerAction,
SetInfoDrawerStateAction,
NavigateUrlAction
} from '@alfresco/aca-shared/store';
import { ContentManagementService } from '../../services/content-management.service';
import { RenditionService } from '@alfresco/adf-content-services';
import { Router } from '@angular/router';
import { NavigationEnd, Router } from '@angular/router';

@Injectable()
export class NodeEffects {
Expand Down Expand Up @@ -284,6 +284,9 @@ export class NodeEffects {
this.actions$.pipe(
ofType<ManagePermissionsAction>(NodeActionTypes.ManagePermissions),
map((action) => {
this.router.events
.pipe(first((event) => event instanceof NavigationEnd))
.subscribe(() => this.store.dispatch(new SetInfoDrawerStateAction(true)));
if (action?.payload) {
const route = 'personal-files/details';
this.store.dispatch(new NavigateUrlAction([route, action.payload.entry.id, 'permissions'].join('/')));
Expand All @@ -308,6 +311,9 @@ export class NodeEffects {
this.actions$.pipe(
ofType<ExpandInfoDrawerAction>(NodeActionTypes.ExpandInfoDrawer),
map((action) => {
this.router.events
.pipe(first((event) => event instanceof NavigationEnd))
.subscribe(() => this.store.dispatch(new SetInfoDrawerStateAction(true)));
if (action?.payload) {
const route = 'personal-files/details';
this.router.navigate([route, action.payload.entry.id], {
Expand All @@ -330,7 +336,6 @@ export class NodeEffects {
}
});
}
this.store.dispatch(new ToggleInfoDrawerAction());
})
),
{ dispatch: false }
Expand Down
Loading