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 { filter, first, takeUntil } from 'rxjs/operators';
import { ContentActionRef } from '@alfresco/adf-extensions';

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

Choose a reason for hiding this comment

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

Same here, you can use only first to filter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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(
filter((event) => event instanceof NavigationStart),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same points here for filter and takeUntil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same answer like above

first(),
takeUntil(this.onDestroy$)
)
)
)
)
.subscribe(() => this.goBack());
}

setActiveTab(tabName: string) {
Expand Down
63 changes: 47 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, NavigationStart, 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 @@ -507,4 +509,33 @@ describe('NodeEffects', () => {
expect(contentService.manageAspects).toHaveBeenCalledWith({ entry: { isFile: false, id: 'folder-node-id' } }, undefined);
}));
});

describe('expandInfoDrawer$', () => {
beforeEach(() => {
spyOn(store, 'dispatch').and.callThrough();
});

it('should call dispatch on store with SetInfoDrawerStateAction when NavigationEnd event occurs', () => {
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
})
);
});

it('should not call dispatch on store with SetInfoDrawerStateAction when different navigation event occurs than NavigationEnd', () => {
Object.defineProperty(router, 'events', {
value: of(new NavigationStart(1, ''))
});

store.dispatch(new ExpandInfoDrawerAction(undefined));
expect(store.dispatch).not.toHaveBeenCalledWith(jasmine.any(SetInfoDrawerStateAction));
});
});
});
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 { filter, 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 @@ -308,6 +308,12 @@ export class NodeEffects {
this.actions$.pipe(
ofType<ExpandInfoDrawerAction>(NodeActionTypes.ExpandInfoDrawer),
map((action) => {
this.router.events
.pipe(
filter((event) => event instanceof NavigationEnd),
first()
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 you don't have to use filter, in this case you could with something like first(event => event instanceof NavigationEnd) which will emit only first value that matches the predicate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, done

)
.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