diff --git a/.eslintrc b/.eslintrc index e61ac443c..2bf19f2fc 100644 --- a/.eslintrc +++ b/.eslintrc @@ -20,7 +20,8 @@ "rules": { "no-bitwise": 0, "@typescript-eslint/no-extraneous-class": ["off"], - "jasmine/new-line-before-expect": 0 + "jasmine/new-line-before-expect": 0, + "jasmine/no-unsafe-spy": ["off"] }, "ignorePatterns": ["node_modules", "dist", "kamu.graphql.interface.ts"] } diff --git a/images/kamu-web-ui/runtime-config.json b/images/kamu-web-ui/runtime-config.json index 722ff5339..611452b1c 100644 --- a/images/kamu-web-ui/runtime-config.json +++ b/images/kamu-web-ui/runtime-config.json @@ -1,4 +1,7 @@ { "apiServerGqlUrl": "http://localhost:8080/graphql", - "loginEnabled": true + "featureFlags": { + "enableLogin": true, + "enableLogout": true + } } \ No newline at end of file diff --git a/src/app/api/auth.api.spec.ts b/src/app/api/auth.api.spec.ts index d703a006c..7e122f926 100644 --- a/src/app/api/auth.api.spec.ts +++ b/src/app/api/auth.api.spec.ts @@ -1,26 +1,22 @@ -import { NavigationService } from "src/app/services/navigation.service"; import { fakeAsync, TestBed, tick } from "@angular/core/testing"; import { Apollo } from "apollo-angular"; import { AuthApi } from "./auth.api"; import { AccountDetailsFragment, FetchAccountInfoDocument, GithubLoginDocument } from "./kamu.graphql.interface"; import { ApolloTestingController, ApolloTestingModule } from "apollo-angular/testing"; import { + mockAccountDetails, mockGithubLoginResponse, mockLogin401Error, mockUserInfoFromAccessToken, TEST_ACCESS_TOKEN, TEST_GITHUB_CODE, } from "./mock/auth.mock"; -import AppValues from "../common/app.values"; -import { MaybeNull } from "../common/app.types"; import { AuthenticationError } from "../common/errors"; import { first } from "rxjs/operators"; describe("AuthApi", () => { let service: AuthApi; - let navigationService: NavigationService; let controller: ApolloTestingController; - let localStorageSetItemSpy: jasmine.Spy; beforeEach(() => { TestBed.configureTestingModule({ @@ -28,9 +24,7 @@ describe("AuthApi", () => { imports: [ApolloTestingModule], }); service = TestBed.inject(AuthApi); - navigationService = TestBed.inject(NavigationService); controller = TestBed.inject(ApolloTestingController); - localStorageSetItemSpy = spyOn(localStorage, "setItem").and.stub(); }); afterEach(() => { @@ -38,10 +32,7 @@ describe("AuthApi", () => { }); function loginViaAccessToken(): void { - service.fetchUserInfoFromAccessToken(TEST_ACCESS_TOKEN).subscribe(() => { - expect(service.isAuthenticated).toBeTrue(); - expect(service.currentUser).toBe(mockUserInfoFromAccessToken.auth.accountInfo); - }); + service.fetchUserInfoFromAccessToken(TEST_ACCESS_TOKEN).subscribe(); const op = controller.expectOne(FetchAccountInfoDocument); expect(op.operation.variables.accessToken).toEqual(TEST_ACCESS_TOKEN); @@ -52,14 +43,7 @@ describe("AuthApi", () => { } function loginFullyViaGithub(): void { - service.fetchUserInfoAndTokenFromGithubCallackCode(TEST_GITHUB_CODE).subscribe(() => { - expect(service.isAuthenticated).toBeTrue(); - expect(service.currentUser).toBe(mockGithubLoginResponse.auth.githubLogin.accountInfo); - expect(localStorageSetItemSpy).toHaveBeenCalledWith( - AppValues.LOCAL_STORAGE_ACCESS_TOKEN, - mockGithubLoginResponse.auth.githubLogin.token.accessToken, - ); - }); + service.fetchUserInfoAndTokenFromGithubCallackCode(TEST_GITHUB_CODE).subscribe(); const op = controller.expectOne(GithubLoginDocument); expect(op.operation.variables.code).toEqual(TEST_GITHUB_CODE); @@ -69,44 +53,30 @@ describe("AuthApi", () => { }); } - function checkUserIsLogged(user: AccountDetailsFragment): void { - expect(service.isAuthenticated).toBeTrue(); - expect(service.currentUser).toEqual(user); - } - it("should be created", () => { expect(service).toBeTruthy(); }); - it("should check user is initially non-authenticated", () => { - expect(service.isAuthenticated).toBeFalse(); - expect(service.currentUser).toBeNull(); - }); - - it("should check user changes via login with alive access token", fakeAsync(() => { - let callbackInvoked = false; - service.onUserChanges.subscribe((user: MaybeNull) => { - callbackInvoked = true; - user ? checkUserIsLogged(user) : fail("User must not be null"); - }); - - loginViaAccessToken(); - tick(); - - expect(callbackInvoked).toBeTrue(); - })); + it("should check full login GraphQL success", fakeAsync(() => { + const accessTokenObtained$ = service + .accessTokenObtained() + .pipe(first()) + .subscribe((token: string) => { + expect(token).toEqual(mockGithubLoginResponse.auth.githubLogin.token.accessToken); + }); - it("should check user changes via full login with github", fakeAsync(() => { - const subscription$ = service.onUserChanges + const accountChanged$ = service + .accountChanged() .pipe(first()) - .subscribe((user: MaybeNull) => { - user ? checkUserIsLogged(user) : fail("User must not be null"); + .subscribe((user: AccountDetailsFragment) => { + expect(user).toEqual(mockAccountDetails); }); loginFullyViaGithub(); tick(); - expect(subscription$.closed).toBeTrue(); + expect(accessTokenObtained$.closed).toBeTrue(); + expect(accountChanged$.closed).toBeTrue(); })); it("should check full login GraphQL failure", fakeAsync(() => { @@ -129,6 +99,30 @@ describe("AuthApi", () => { expect(subscription$.closed).toBeTrue(); })); + it("should check login via access token GraphQL success", fakeAsync(() => { + const accessTokenObtained$ = service + .accessTokenObtained() + .pipe(first()) + .subscribe(() => { + fail("Unexpected call of access token update"); + }); + + const accountChanged$ = service + .accountChanged() + .pipe(first()) + .subscribe((user: AccountDetailsFragment) => { + expect(user).toEqual(mockAccountDetails); + }); + + loginViaAccessToken(); + tick(); + + expect(accessTokenObtained$.closed).toBeFalse(); + accessTokenObtained$.unsubscribe(); + + expect(accountChanged$.closed).toBeTrue(); + })); + it("should check login via access token GraphQL failure", fakeAsync(() => { const subscription$ = service .fetchUserInfoFromAccessToken(TEST_ACCESS_TOKEN) @@ -148,12 +142,4 @@ describe("AuthApi", () => { expect(subscription$.closed).toBeTrue(); })); - - it("should check user logout navigates to home page", () => { - const navigationServiceSpy = spyOn(navigationService, "navigateToHome"); - loginViaAccessToken(); - service.logOut(); - expect(service.currentUser).toBeNull(); - expect(navigationServiceSpy).toHaveBeenCalledWith(); - }); }); diff --git a/src/app/api/auth.api.ts b/src/app/api/auth.api.ts index 752d3bef4..fe9ba3cb4 100644 --- a/src/app/api/auth.api.ts +++ b/src/app/api/auth.api.ts @@ -1,7 +1,6 @@ import { Injectable } from "@angular/core"; import { catchError, map } from "rxjs/operators"; -import { Observable, Subject, throwError } from "rxjs"; -import { NavigationService } from "../services/navigation.service"; +import { Observable, ReplaySubject, Subject, throwError } from "rxjs"; import { AccountDetailsFragment, FetchAccountInfoGQL, @@ -9,41 +8,24 @@ import { GithubLoginGQL, GithubLoginMutation, } from "./kamu.graphql.interface"; -import AppValues from "../common/app.values"; -import { MaybeNull } from "../common/app.types"; import { MutationResult } from "apollo-angular"; -import { isNull } from "lodash"; import { AuthenticationError } from "../common/errors"; @Injectable({ providedIn: "root", }) export class AuthApi { - private user: MaybeNull = null; + constructor(private githubLoginGQL: GithubLoginGQL, private fetchAccountInfoGQL: FetchAccountInfoGQL) {} - private userChanges$: Subject> = new Subject>(); + private accessTokenObtained$: Subject = new ReplaySubject(1); + private accountChanged$: Subject = new ReplaySubject(1); - constructor( - private githubLoginGQL: GithubLoginGQL, - private fetchAccountInfoGQL: FetchAccountInfoGQL, - private navigationService: NavigationService, - ) {} - - public get onUserChanges(): Observable> { - return this.userChanges$.asObservable(); - } - - public get currentUser(): MaybeNull { - return this.user; - } - - public get isAuthenticated(): boolean { - return !isNull(this.user); + public accessTokenObtained(): Observable { + return this.accessTokenObtained$.asObservable(); } - private changeUser(user: MaybeNull) { - this.user = user; - this.userChanges$.next(user); + public accountChanged(): Observable { + return this.accountChanged$.asObservable(); } public fetchUserInfoAndTokenFromGithubCallackCode(code: string): Observable { @@ -51,8 +33,8 @@ export class AuthApi { map((result: MutationResult) => { if (result.data) { const data: GithubLoginMutation = result.data; - localStorage.setItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN, data.auth.githubLogin.token.accessToken); - this.changeUser(data.auth.githubLogin.accountInfo); + this.accessTokenObtained$.next(data.auth.githubLogin.token.accessToken); + this.accountChanged$.next(data.auth.githubLogin.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } @@ -66,7 +48,7 @@ export class AuthApi { map((result: MutationResult) => { if (result.data) { const data: FetchAccountInfoMutation = result.data; - this.changeUser(data.auth.accountInfo); + this.accountChanged$.next(data.auth.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } @@ -74,14 +56,4 @@ export class AuthApi { catchError((e: Error) => throwError(new AuthenticationError([e]))), ); } - - public terminateSession() { - this.changeUser(null); - localStorage.removeItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); - } - - public logOut(): void { - this.terminateSession(); - this.navigationService.navigateToHome(); - } } diff --git a/src/app/api/mock/auth.mock.ts b/src/app/api/mock/auth.mock.ts index a3bb8feab..d2dd002b3 100644 --- a/src/app/api/mock/auth.mock.ts +++ b/src/app/api/mock/auth.mock.ts @@ -1,5 +1,6 @@ import { GraphQLError } from "graphql"; import { AccountDetailsFragment, FetchAccountInfoMutation, GithubLoginMutation } from "../kamu.graphql.interface"; +import AppValues from "src/app/common/app.values"; export const TEST_GITHUB_CODE = "12345"; export const TEST_ACCESS_TOKEN = "someToken"; @@ -9,7 +10,7 @@ export const mockAccountDetails: AccountDetailsFragment = { name: "Test User", email: "test@example.com", // avatarUrl: null, - avatarUrl: "https://avatars.githubusercontent.com/u/11951648?v=4", + avatarUrl: AppValues.DEFAULT_AVATAR_URL, gravatarId: null, }; diff --git a/src/app/app-config.service.ts b/src/app/app-config.service.ts index 93628cb69..321c0ca44 100644 --- a/src/app/app-config.service.ts +++ b/src/app/app-config.service.ts @@ -1,8 +1,21 @@ import { Injectable } from "@angular/core"; +import { AccountDetailsFragment } from "./api/kamu.graphql.interface"; +import { FeatureFlags } from "./common/feature-flags.model"; interface AppConfig { apiServerGqlUrl: string; - loginEnabled: boolean; + featureFlags: { + enableLogin: boolean; + enableLogout: boolean; + }; + loggedUser?: AppConfigLoggedUser; +} + +interface AppConfigLoggedUser { + login: string; + name: string; + email?: string; + avatarUrl?: string; } @Injectable({ @@ -19,12 +32,26 @@ export class AppConfigService { return this.appConfig.apiServerGqlUrl; } - get loginEnabled(): boolean { + get featureFlags(): FeatureFlags { if (!this.appConfig) { this.appConfig = AppConfigService.loadAppConfig(); } - return this.appConfig.loginEnabled; + return this.appConfig.featureFlags; + } + + get loggedUser(): AccountDetailsFragment | null { + if (!this.appConfig) { + this.appConfig = AppConfigService.loadAppConfig(); + } + + if (this.appConfig.loggedUser) { + return { + ...this.appConfig.loggedUser, + } as AccountDetailsFragment; + } else { + return null; + } } private static loadAppConfig(): AppConfig { @@ -33,8 +60,8 @@ export class AppConfigService { request.send(null); const data: AppConfig = JSON.parse(request.responseText) as AppConfig; return { + ...data, apiServerGqlUrl: AppConfigService.toRemoteURL(data.apiServerGqlUrl), - loginEnabled: data.loginEnabled, }; } diff --git a/src/app/app.component.html b/src/app/app.component.html index 98240891b..b8373691a 100644 --- a/src/app/app.component.html +++ b/src/app/app.component.html @@ -1,22 +1,21 @@ diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index f03bd6119..daae3606b 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -23,12 +23,14 @@ import { mockAccountDetails, mockUserInfoFromAccessToken } from "./api/mock/auth import { FetchAccountInfoGQL } from "./api/kamu.graphql.interface"; import { AppHeaderComponent } from "./components/app-header/app-header.component"; import { SpinnerComponent } from "./components/spinner/spinner/spinner.component"; +import { LoggedUserService } from "./auth/logged-user.service"; describe("AppComponent", () => { let component: AppComponent; let fixture: ComponentFixture; let navigationService: NavigationService; let authApi: AuthApi; + let loggedUserService: LoggedUserService; let fetchAccountInfoGQL: FetchAccountInfoGQL; const DEFAULT_SEARCH_QUERY = "defaultSearchQuery"; @@ -53,10 +55,13 @@ describe("AppComponent", () => { }).compileComponents(); routerMock.url = ProjectLinks.URL_HOME; + fixture = TestBed.createComponent(AppComponent); navigationService = TestBed.inject(NavigationService); authApi = TestBed.inject(AuthApi); + loggedUserService = TestBed.inject(LoggedUserService); fetchAccountInfoGQL = TestBed.inject(FetchAccountInfoGQL); + component = fixture.componentInstance; routerMockEventSubject.next(new NavigationEnd(1, ProjectLinks.URL_LOGIN, "")); fixture.detectChanges(); @@ -69,17 +74,19 @@ describe("AppComponent", () => { ProjectLinks.ALL_URLS.filter((url) => !ALL_URLS_WITHOUT_ACCESS_TOKEN.includes(url)).forEach((url: string) => { it(`should call authentification method in onInit for ${url} and trigger token restore`, () => { const someToken = "someToken"; - const authentificationSpy = spyOn(component, "authentification").and.callThrough(); + const performAuthenticationSpy = spyOn(component, "performAuthentication").and.callThrough(); const localStorageGetItemSpy = spyOn(localStorage, "getItem").and.returnValue(someToken); const fetchUserInfoFromAccessTokenSpy = spyOn(authApi, "fetchUserInfoFromAccessToken").and.callFake(() => - of(), + of(void {}), + ); + const isAuthenticatedSpy = spyOnProperty(loggedUserService, "isAuthenticated", "get").and.returnValue( + false, ); - const isAuthenticatedSpy = spyOnProperty(authApi, "isAuthenticated", "get").and.returnValue(false); routerMock.url = url; component.ngOnInit(); - expect(authentificationSpy).toHaveBeenCalledWith(); + expect(performAuthenticationSpy).toHaveBeenCalledWith(); expect(localStorageGetItemSpy).toHaveBeenCalledWith(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); expect(fetchUserInfoFromAccessTokenSpy).toHaveBeenCalledWith(someToken); expect(isAuthenticatedSpy).toHaveBeenCalledWith(); @@ -88,15 +95,15 @@ describe("AppComponent", () => { ALL_URLS_WITHOUT_ACCESS_TOKEN.forEach((url: string) => { it(`should call authentification method in onInit on ${url} without token restore`, () => { - const authentificationSpy = spyOn(component, "authentification").and.callThrough(); + const performAuthenticationSpy = spyOn(component, "performAuthentication").and.callThrough(); const localStorageGetItemSpy = spyOn(localStorage, "getItem").and.stub(); const fetchUserInfoFromAccessTokenSpy = spyOn(authApi, "fetchUserInfoFromAccessToken").and.stub(); - const isAuthenticatedSpy = spyOnProperty(authApi, "isAuthenticated", "get").and.stub(); + const isAuthenticatedSpy = spyOnProperty(loggedUserService, "isAuthenticated", "get").and.stub(); routerMock.url = url; component.ngOnInit(); - expect(authentificationSpy).toHaveBeenCalledWith(); + expect(performAuthenticationSpy).toHaveBeenCalledWith(); expect(localStorageGetItemSpy).not.toHaveBeenCalled(); expect(fetchUserInfoFromAccessTokenSpy).not.toHaveBeenCalled(); expect(isAuthenticatedSpy).not.toHaveBeenCalled(); @@ -110,9 +117,9 @@ describe("AppComponent", () => { expect(component.isMobileView).toEqual(isMobileView()); }); - it("should check call onClickAppLogo method", () => { + it("should check call onAppLogo method", () => { const navigateToSearchSpy = spyOn(navigationService, "navigateToSearch").and.returnValue(); - component.onClickAppLogo(); + component.onAppLogo(); expect(navigateToSearchSpy).toHaveBeenCalledWith(); }); @@ -122,10 +129,10 @@ describe("AppComponent", () => { expect(navigateToDatasetCreateSpy).toHaveBeenCalledWith(); }); - it("should check call onLogOut method", () => { - const logOutSpy = spyOn(authApi, "logOut").and.returnValue(); - component.onLogOut(); - expect(logOutSpy).toHaveBeenCalledWith(); + it("should check call onLogout method", () => { + const logoutSpy = spyOn(loggedUserService, "logout").and.returnValue(); + component.onLogout(); + expect(logoutSpy).toHaveBeenCalledWith(); }); it("should check call onLogin method", () => { @@ -136,7 +143,7 @@ describe("AppComponent", () => { it("should check call onSelectDataset method and navigate to dataset", () => { const navigateToDatasetViewSpy = spyOn(navigationService, "navigateToDatasetView").and.returnValue(); - component.onSelectDataset(mockAutocompleteItems[0]); + component.onSelectedDataset(mockAutocompleteItems[0]); expect(navigateToDatasetViewSpy).toHaveBeenCalledWith({ accountName: mockAutocompleteItems[0].dataset.owner.name, datasetName: mockAutocompleteItems[0].dataset.name as string, @@ -145,13 +152,15 @@ describe("AppComponent", () => { it("should check call onSelectDataset method and navigate to search", () => { const navigateToSearchSpy = spyOn(navigationService, "navigateToSearch").and.returnValue(); - component.onSelectDataset(mockAutocompleteItems[1]); + component.onSelectedDataset(mockAutocompleteItems[1]); expect(navigateToSearchSpy).toHaveBeenCalledWith(mockAutocompleteItems[1].dataset.id as string); }); it("should check call onUserProfile", () => { const navigationServicelSpy = spyOn(navigationService, "navigateToOwnerView").and.returnValue(); - const currentUserSpy = spyOnProperty(authApi, "currentUser", "get").and.returnValue(mockAccountDetails); + const currentUserSpy = spyOnProperty(loggedUserService, "currentlyLoggedInUser", "get").and.returnValue( + mockAccountDetails, + ); component.onUserProfile(); expect(currentUserSpy).toHaveBeenCalledWith(); expect(navigationServicelSpy).toHaveBeenCalledWith(mockAccountDetails.login, AccountTabs.overview); @@ -182,9 +191,9 @@ describe("AppComponent", () => { ); authApi.fetchUserInfoFromAccessToken("someToken").subscribe(); - expect(component.user).toEqual(mockUserInfoFromAccessToken.auth.accountInfo); + expect(component.loggedUserInfo).toEqual(mockUserInfoFromAccessToken.auth.accountInfo); - authApi.terminateSession(); - expect(component.user).toEqual(AppComponent.AnonymousAccountInfo); + loggedUserService.terminateSession(); + expect(component.loggedUserInfo).toEqual(AppComponent.ANONYMOUS_ACCOUNT_INFO); }); }); diff --git a/src/app/app.component.ts b/src/app/app.component.ts index bb8de75ea..d750c23b6 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -7,7 +7,6 @@ import AppValues from "./common/app.values"; import { filter, map } from "rxjs/operators"; import { NavigationEnd, Router, RouterEvent } from "@angular/router"; import { DatasetAutocompleteItem, TypeNames } from "./interface/search.interface"; -import { AuthApi } from "./api/auth.api"; import { ModalService } from "./components/modal/modal.service"; import { BaseComponent } from "./common/base.component"; import ProjectLinks from "./project-links"; @@ -16,6 +15,8 @@ import { MaybeNull } from "./common/app.types"; import _ from "lodash"; import { isMobileView, promiseWithCatch } from "./common/app.helpers"; import { AppConfigService } from "./app-config.service"; +import { FeatureFlags } from "./common/feature-flags.model"; +import { LoggedUserService } from "./auth/logged-user.service"; export const ALL_URLS_WITHOUT_HEADER: string[] = [ProjectLinks.URL_LOGIN, ProjectLinks.URL_GITHUB_CALLBACK]; @@ -28,35 +29,42 @@ export const ALL_URLS_WITHOUT_ACCESS_TOKEN: string[] = [ProjectLinks.URL_LOGIN, changeDetection: ChangeDetectionStrategy.OnPush, }) export class AppComponent extends BaseComponent implements OnInit { - public static readonly AnonymousAccountInfo: AccountDetailsFragment = { + public static readonly ANONYMOUS_ACCOUNT_INFO: AccountDetailsFragment = { login: "", name: AppValues.DEFAULT_USERNAME, }; + public static readonly DEFAULT_FEATURE_FLAGS: FeatureFlags = { + enableLogin: true, + enableLogout: true, + }; + + public readonly APP_LOGO = `/${AppValues.APP_LOGO}`; - public appLogo = `/${AppValues.APP_LOGO}`; public isMobileView = false; public isHeaderVisible = true; - public isLoginEnabled = true; - public user: AccountDetailsFragment = AppComponent.AnonymousAccountInfo; + + public featureFlags: FeatureFlags = AppComponent.DEFAULT_FEATURE_FLAGS; + public loggedUserInfo: AccountDetailsFragment = AppComponent.ANONYMOUS_ACCOUNT_INFO; @HostListener("window:resize") - checkWindowSize(): void { + public checkWindowSize(): void { this.checkView(); } constructor( private router: Router, - private authApi: AuthApi, + private loggedUserService: LoggedUserService, private modalService: ModalService, private navigationService: NavigationService, private appConfigService: AppConfigService, ) { super(); - this.isLoginEnabled = appConfigService.loginEnabled; } public ngOnInit(): void { + this.fetchFeatureFlags(); this.checkView(); + this.trackSubscriptions( this.router.events .pipe( @@ -67,25 +75,26 @@ export class AppComponent extends BaseComponent implements OnInit { this.isHeaderVisible = this.shouldHeaderBeVisible(event.url); }), - this.authApi.onUserChanges.subscribe((user: MaybeNull) => { - this.user = user ? _.cloneDeep(user) : AppComponent.AnonymousAccountInfo; + this.loggedUserService.onLoggedInUserChanges.subscribe((user: MaybeNull) => { + this.loggedUserInfo = user ? _.cloneDeep(user) : AppComponent.ANONYMOUS_ACCOUNT_INFO; }), ); - this.authentification(); + + this.performAuthentication(); } - authentification(): void { + public performAuthentication(): void { if (ALL_URLS_WITHOUT_ACCESS_TOKEN.includes(this.router.url)) { return; - } else { - const accessToken: string | null = localStorage.getItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); - if (typeof accessToken === "string" && !this.authApi.isAuthenticated) { - this.trackSubscription(this.authApi.fetchUserInfoFromAccessToken(accessToken).subscribe()); - return; - } + } else if (!this.loggedUserService.isAuthenticated) { + this.loggedUserService.attemptPreviousAuthentication(); } } + private fetchFeatureFlags(): void { + this.featureFlags = this.appConfigService.featureFlags; + } + private checkView(): void { this.isMobileView = isMobileView(); } @@ -94,7 +103,7 @@ export class AppComponent extends BaseComponent implements OnInit { return !ALL_URLS_WITHOUT_HEADER.some((item) => url.toLowerCase().includes(item)); } - public onSelectDataset(item: DatasetAutocompleteItem): void { + public onSelectedDataset(item: DatasetAutocompleteItem): void { if (item.__typename === TypeNames.datasetType) { this.navigationService.navigateToDatasetView({ accountName: item.dataset.owner.name, @@ -104,7 +113,8 @@ export class AppComponent extends BaseComponent implements OnInit { this.navigationService.navigateToSearch(item.dataset.id as string); } } - public onClickAppLogo(): void { + + public onAppLogo(): void { this.navigationService.navigateToSearch(); } @@ -120,21 +130,27 @@ export class AppComponent extends BaseComponent implements OnInit { this.navigationService.navigateToLogin(); } - public onLogOut(): void { - this.authApi.logOut(); + public onLogout(): void { + this.loggedUserService.logout(); } public onUserProfile(): void { - if (this.authApi.currentUser?.login) { - this.navigationService.navigateToOwnerView(this.authApi.currentUser.login, AccountTabs.overview); + if (this.loggedUserService.currentlyLoggedInUser?.login) { + this.navigationService.navigateToOwnerView( + this.loggedUserService.currentlyLoggedInUser.login, + AccountTabs.overview, + ); } else { throwError(new AuthenticationError([new Error("Login is undefined")])); } } public onUserDatasets(): void { - if (this.authApi.currentUser?.login) { - this.navigationService.navigateToOwnerView(this.authApi.currentUser.login, AccountTabs.datasets); + if (this.loggedUserService.currentlyLoggedInUser?.login) { + this.navigationService.navigateToOwnerView( + this.loggedUserService.currentlyLoggedInUser.login, + AccountTabs.datasets, + ); } else { throwError(new AuthenticationError([new Error("Login is undefined")])); } @@ -159,7 +175,7 @@ export class AppComponent extends BaseComponent implements OnInit { } public onSettings(): void { - if (this.authApi.currentUser?.login) { + if (this.loggedUserService.currentlyLoggedInUser?.login) { this.navigationService.navigateToSettings(); } else { throwError(new AuthenticationError([new Error("Login is undefined")])); diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 7abf30633..58cb7b2e7 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -62,6 +62,7 @@ import { DatasetsTabComponent } from "./auth/account/additional-components/datas import { ClipboardModule } from "@angular/cdk/clipboard"; import { HighlightModule, HIGHLIGHT_OPTIONS } from "ngx-highlightjs"; import { ToastrModule } from "ngx-toastr"; +import { LoggedUserService } from "./auth/logged-user.service"; const Services = [ { @@ -76,6 +77,7 @@ const Services = [ }, Apollo, AuthApi, + LoggedUserService, SearchApi, DatasetApi, HttpLink, diff --git a/src/app/auth/account/account.component.ts b/src/app/auth/account/account.component.ts index 52a8a4114..7ca7d7e42 100644 --- a/src/app/auth/account/account.component.ts +++ b/src/app/auth/account/account.component.ts @@ -4,7 +4,6 @@ import { DatasetSearchOverviewFragment, PageBasedInfo } from "./../../api/kamu.g import { BaseComponent } from "src/app/common/base.component"; import { NavigationService } from "src/app/services/navigation.service"; import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, OnInit, ViewChild } from "@angular/core"; -import { AuthApi } from "src/app/api/auth.api"; import { AccountDetailsFragment } from "src/app/api/kamu.graphql.interface"; import { AccountTabs } from "./account.constants"; import { ActivatedRoute, NavigationEnd, Params, Router, RouterEvent } from "@angular/router"; @@ -13,6 +12,7 @@ import { promiseWithCatch } from "src/app/common/app.helpers"; import { AccountService } from "src/app/services/account.service"; import { DatasetsAccountResponse } from "src/app/interface/dataset.interface"; import { filter, map } from "rxjs/operators"; +import { LoggedUserService } from "../logged-user.service"; @Component({ selector: "app-account", @@ -36,13 +36,13 @@ export class AccountComponent extends BaseComponent implements OnInit { @ViewChild("dropdownMenu") dropdownMenu: ElementRef; constructor( - private authApi: AuthApi, private route: ActivatedRoute, private navigationService: NavigationService, private cdr: ChangeDetectorRef, private router: Router, private modalService: ModalService, private accountService: AccountService, + private loggedUserService: LoggedUserService, ) { super(); } @@ -98,7 +98,7 @@ export class AccountComponent extends BaseComponent implements OnInit { } public get isOwner(): boolean { - return this.authApi.currentUser?.login === this.accountName; + return this.loggedUserService.currentlyLoggedInUser?.login === this.accountName; } public onEditProfile(): void { diff --git a/src/app/auth/authentication.guard.spec.ts b/src/app/auth/authentication.guard.spec.ts index 4e6ec1acd..3a64356dd 100644 --- a/src/app/auth/authentication.guard.spec.ts +++ b/src/app/auth/authentication.guard.spec.ts @@ -1,18 +1,18 @@ import { TestBed } from "@angular/core/testing"; import { ApolloTestingModule } from "apollo-angular/testing"; -import { AuthApi } from "../api/auth.api"; import { AuthenticationGuard } from "./authentication.guard"; +import { LoggedUserService } from "./logged-user.service"; describe("AuthenticationGuard", () => { let guard: AuthenticationGuard; - let authApi: AuthApi; + let loggedUserService: LoggedUserService; beforeEach(() => { TestBed.configureTestingModule({ imports: [ApolloTestingModule], }); guard = TestBed.inject(AuthenticationGuard); - authApi = TestBed.inject(AuthApi); + loggedUserService = TestBed.inject(LoggedUserService); }); it("should be created", () => { @@ -21,7 +21,9 @@ describe("AuthenticationGuard", () => { [true, false].forEach((expectation: boolean) => { it(`should check canActive method when user is${expectation ? "" : "not"} authentiсation`, () => { - const isAuthenticatedSpy = spyOnProperty(authApi, "isAuthenticated", "get").and.returnValue(expectation); + const isAuthenticatedSpy = spyOnProperty(loggedUserService, "isAuthenticated", "get").and.returnValue( + expectation, + ); const result = guard.canActivate(); expect(result).toEqual(expectation); expect(isAuthenticatedSpy).toHaveBeenCalledWith(); diff --git a/src/app/auth/authentication.guard.ts b/src/app/auth/authentication.guard.ts index f19abe91c..06c044524 100644 --- a/src/app/auth/authentication.guard.ts +++ b/src/app/auth/authentication.guard.ts @@ -1,15 +1,16 @@ import { NavigationService } from "src/app/services/navigation.service"; import { Injectable } from "@angular/core"; import { CanActivate } from "@angular/router"; -import { AuthApi } from "../api/auth.api"; +import { LoggedUserService } from "./logged-user.service"; @Injectable({ providedIn: "root", }) export class AuthenticationGuard implements CanActivate { - constructor(private navigationService: NavigationService, private authApi: AuthApi) {} - canActivate(): boolean { - if (!this.authApi.isAuthenticated) { + constructor(private navigationService: NavigationService, private logggedUserService: LoggedUserService) {} + + public canActivate(): boolean { + if (!this.logggedUserService.isAuthenticated) { this.navigationService.navigateToHome(); return false; } diff --git a/src/app/auth/logged-user-service.spec.ts b/src/app/auth/logged-user-service.spec.ts new file mode 100644 index 000000000..5e4b8d411 --- /dev/null +++ b/src/app/auth/logged-user-service.spec.ts @@ -0,0 +1,176 @@ +import { Apollo } from "apollo-angular"; +import { AuthApi } from "../api/auth.api"; +import { LoggedUserService } from "./logged-user.service"; +import { TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { ApolloTestingController, ApolloTestingModule } from "apollo-angular/testing"; +import { NavigationService } from "../services/navigation.service"; +import { + TEST_ACCESS_TOKEN, + TEST_GITHUB_CODE, + mockAccountDetails, + mockGithubLoginResponse, + mockUserInfoFromAccessToken, +} from "../api/mock/auth.mock"; +import { AccountDetailsFragment, FetchAccountInfoDocument, GithubLoginDocument } from "../api/kamu.graphql.interface"; +import { first } from "rxjs/operators"; +import { MaybeNull } from "../common/app.types"; +import AppValues from "../common/app.values"; +import { AppConfigService } from "../app-config.service"; + +describe("LoggedUserService", () => { + describe("Main Test Suite", () => { + let service: LoggedUserService; + let navigationService: NavigationService; + let authApi: AuthApi; + let controller: ApolloTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [AuthApi, Apollo], + imports: [ApolloTestingModule], + }); + service = TestBed.inject(LoggedUserService); + navigationService = TestBed.inject(NavigationService); + authApi = TestBed.inject(AuthApi); + controller = TestBed.inject(ApolloTestingController); + }); + + afterEach(() => { + controller.verify(); + }); + + function checkUserIsLogged(user: AccountDetailsFragment): void { + expect(service.isAuthenticated).toBeTrue(); + expect(service.currentlyLoggedInUser).toEqual(user); + } + + function attemptSuccessfulLoginViaAccessToken(): void { + const localStorageGetItemSpy = spyOn(localStorage, "getItem").and.returnValue(TEST_ACCESS_TOKEN); + + service.attemptPreviousAuthentication(); + + const op = controller.expectOne(FetchAccountInfoDocument); + expect(op.operation.variables.accessToken).toEqual(TEST_ACCESS_TOKEN); + + op.flush({ + data: mockUserInfoFromAccessToken, + }); + + expect(localStorageGetItemSpy).toHaveBeenCalledOnceWith(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); + } + + function loginFullyViaGithub(): void { + authApi.fetchUserInfoAndTokenFromGithubCallackCode(TEST_GITHUB_CODE).subscribe(); + + const op = controller.expectOne(GithubLoginDocument); + expect(op.operation.variables.code).toEqual(TEST_GITHUB_CODE); + + op.flush({ + data: mockGithubLoginResponse, + }); + } + + it("should be created", () => { + expect(service).toBeTruthy(); + }); + + it("should check user is initially non-authenticated", () => { + expect(service.isAuthenticated).toBeFalse(); + expect(service.currentlyLoggedInUser).toBeNull(); + }); + + it("should check user is non-authenticated if no access token exists", fakeAsync(() => { + const localStorageGetItemSpy = spyOn(localStorage, "getItem").and.returnValue(null); + + service.attemptPreviousAuthentication(); + + controller.expectNone(FetchAccountInfoDocument); + expect(service.isAuthenticated).toBeFalse(); + expect(localStorageGetItemSpy).toHaveBeenCalledOnceWith(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); + })); + + it("should check user changes via login with alive access token", fakeAsync(() => { + attemptSuccessfulLoginViaAccessToken(); + tick(); + + const userChanges$ = service.onLoggedInUserChanges + .pipe(first()) + .subscribe((user: MaybeNull) => { + user ? checkUserIsLogged(user) : fail("User must not be null"); + }); + expect(userChanges$.closed).toBeTrue(); + })); + + it("should check user changes via full login", fakeAsync(() => { + const localStorageSetItemSpy = spyOn(localStorage, "setItem"); + + loginFullyViaGithub(); + tick(); + + const userChanges$ = service.onLoggedInUserChanges + .pipe(first()) + .subscribe((user: MaybeNull) => { + user ? checkUserIsLogged(user) : fail("User must not be null"); + }); + + expect(localStorageSetItemSpy).toHaveBeenCalledWith( + AppValues.LOCAL_STORAGE_ACCESS_TOKEN, + mockGithubLoginResponse.auth.githubLogin.token.accessToken, + ); + + expect(userChanges$.closed).toBeTrue(); + })); + + it("should check user logout navigates to home page", fakeAsync(() => { + const navigationServiceSpy = spyOn(navigationService, "navigateToHome"); + + attemptSuccessfulLoginViaAccessToken(); + tick(); + + service.logout(); + + expect(service.currentlyLoggedInUser).toBeNull(); + expect(navigationServiceSpy).toHaveBeenCalledWith(); + })); + + it("should check user terminate session does not navigate to home page", fakeAsync(() => { + const navigationServiceSpy = spyOn(navigationService, "navigateToHome"); + + attemptSuccessfulLoginViaAccessToken(); + tick(); + + service.terminateSession(); + + expect(service.currentlyLoggedInUser).toBeNull(); + expect(navigationServiceSpy).not.toHaveBeenCalled(); + })); + }); + + describe("Custom configuration", () => { + let service: LoggedUserService; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + AuthApi, + Apollo, + { + provide: AppConfigService, + useFactory: () => { + const realService = new AppConfigService(); + spyOnProperty(realService, "loggedUser", "get").and.returnValue(mockAccountDetails); + return realService; + }, + }, + ], + imports: [ApolloTestingModule], + }); + service = TestBed.inject(LoggedUserService); + }); + + it("should use custom configuration's initial user", () => { + expect(service.isAuthenticated).toBeTrue(); + expect(service.currentlyLoggedInUser).toEqual(mockAccountDetails); + }); + }); +}); diff --git a/src/app/auth/logged-user.service.ts b/src/app/auth/logged-user.service.ts new file mode 100644 index 000000000..d844f8c06 --- /dev/null +++ b/src/app/auth/logged-user.service.ts @@ -0,0 +1,79 @@ +import { Injectable } from "@angular/core"; +import { first } from "rxjs/operators"; +import { Observable, ReplaySubject, Subject } from "rxjs"; +import { NavigationService } from "../services/navigation.service"; +import AppValues from "../common/app.values"; +import { MaybeNull } from "../common/app.types"; +import { isNull } from "lodash"; +import { AppConfigService } from "../app-config.service"; +import { AuthApi } from "../api/auth.api"; +import { AccountDetailsFragment } from "../api/kamu.graphql.interface"; +import { UnsubscribeOnDestroyAdapter } from "../common/unsubscribe.ondestroy.adapter"; + +@Injectable({ + providedIn: "root", +}) +export class LoggedUserService extends UnsubscribeOnDestroyAdapter { + private loggedInUser: MaybeNull = null; + + private loggedInUserChanges$: Subject> = new ReplaySubject< + MaybeNull + >(1); + + constructor( + private authApi: AuthApi, + private navigationService: NavigationService, + private appConfigService: AppConfigService, + ) { + super(); + + this.trackSubscriptions( + this.authApi.accessTokenObtained().subscribe((token: string) => this.saveAccessToken(token)), + this.authApi.accountChanged().subscribe((user: AccountDetailsFragment) => this.changeUser(user)), + ); + + this.changeUser(this.appConfigService.loggedUser); + } + + public get onLoggedInUserChanges(): Observable> { + return this.loggedInUserChanges$.asObservable(); + } + + public get currentlyLoggedInUser(): MaybeNull { + return this.loggedInUser; + } + + public get isAuthenticated(): boolean { + return !isNull(this.loggedInUser); + } + + public attemptPreviousAuthentication() { + const accessToken: string | null = localStorage.getItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); + if (typeof accessToken === "string" && !this.isAuthenticated) { + this.authApi.fetchUserInfoFromAccessToken(accessToken).pipe(first()).subscribe(); + } + } + + public logout(): void { + this.terminateSession(); + this.navigationService.navigateToHome(); + } + + public terminateSession() { + this.changeUser(null); + this.resetAccessToken(); + } + + private changeUser(user: MaybeNull) { + this.loggedInUser = user; + this.loggedInUserChanges$.next(user); + } + + private resetAccessToken(): void { + localStorage.removeItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN); + } + + private saveAccessToken(token: string): void { + localStorage.setItem(AppValues.LOCAL_STORAGE_ACCESS_TOKEN, token); + } +} diff --git a/src/app/auth/settings/settings.component.html b/src/app/auth/settings/settings.component.html index 6bde41bb3..bfd0c2493 100644 --- a/src/app/auth/settings/settings.component.html +++ b/src/app/auth/settings/settings.component.html @@ -2,7 +2,7 @@
@@ -99,7 +99,7 @@ Explore - + - {{ isUserLoggedIn() ? userInfo.name : defaultUsername }} + {{ isUserLoggedIn() ? loggedUserInfo.name : defaultUsername }} - + + + -
@@ -203,7 +204,7 @@ @@ -255,10 +256,12 @@
- + + +
@@ -267,8 +270,8 @@
-
diff --git a/src/app/components/app-header/app-header.component.spec.ts b/src/app/components/app-header/app-header.component.spec.ts index b4c99a6ec..85308d73a 100644 --- a/src/app/components/app-header/app-header.component.spec.ts +++ b/src/app/components/app-header/app-header.component.spec.ts @@ -75,11 +75,11 @@ describe("AppHeaderComponent", () => { fixture = TestBed.createComponent(AppHeaderComponent); component = fixture.componentInstance; - component.userInfo = { + component.loggedUserInfo = { login: "", name: AppValues.DEFAULT_USERNAME, }; - component.isLoginEnabled = true; + component.featureFlags = { enableLogin: true, enableLogout: true }; component.isVisible = true; component.isMobileView = false; fixture.detectChanges(); @@ -88,7 +88,7 @@ describe("AppHeaderComponent", () => { }); function loginUser(): void { - component.userInfo = { + component.loggedUserInfo = { login: "ssss", name: "testName", } as AccountDetailsFragment; @@ -130,14 +130,14 @@ describe("AppHeaderComponent", () => { }); it("should emit on click app logo", () => { - const emitterSubscription$ = component.clickAppLogoEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedAppLogo.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "appLogo"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); }); it("should emit on click Login link", () => { - const emitterSubscription$ = component.loginEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedLogin.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "loginHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -149,9 +149,20 @@ describe("AppHeaderComponent", () => { expect(link).toBeUndefined(); }); + it("should not have Login link when login feature is disabled", () => { + component.featureFlags = { + ...component.featureFlags, + enableLogin: false, + }; + fixture.detectChanges(); + + const link = findElementByDataTestId(fixture, "loginHeader"); + expect(link).toBeUndefined(); + }); + it("should emit on click User Datasets link", () => { loginUser(); - const emitterSubscription$ = component.clickUserDatasetsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedUserDatasets.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "userDatasetsHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -159,7 +170,7 @@ describe("AppHeaderComponent", () => { it("should emit on click AddNew link", () => { loginUser(); - const emitterSubscription$ = component.addNewEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedAddNew.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "addNewDatasetHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -167,7 +178,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Your Profile link", () => { loginUser(); - const emitterSubscription$ = component.clickUserProfileEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedUserProfile.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openUserProfileHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -175,7 +186,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Billing link", () => { loginUser(); - const emitterSubscription$ = component.clickBillingEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedBilling.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openBillingPlanHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -183,7 +194,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Analytics link", () => { loginUser(); - const emitterSubscription$ = component.clickAnalyticsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedAnalytics.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openAnalyticsHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -191,7 +202,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Settings link", () => { loginUser(); - const emitterSubscription$ = component.clickSettingsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedSettings.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openSettingsHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -199,14 +210,26 @@ describe("AppHeaderComponent", () => { // TODO: test userNameHeader and it's content - it("should emit on click Signout link", () => { + it("should emit on click Log out link", () => { loginUser(); - const emitterSubscription$ = component.logOutEmitter.pipe(first()).subscribe(); - const link = getElementByDataTestId(fixture, "openSignOutHeader"); + const emitterSubscription$ = component.onClickedLogout.pipe(first()).subscribe(); + const link = getElementByDataTestId(fixture, "openLogoutHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); }); + it("no Log Out link when feature disabled", () => { + loginUser(); + component.featureFlags = { + ...component.featureFlags, + enableLogout: false, + }; + fixture.detectChanges(); + + const link = findElementByDataTestId(fixture, "openLogoutHeader"); + expect(link).toBeUndefined(); + }); + [ "userDatasetsHeader", "addNewDatasetHeader", @@ -215,7 +238,7 @@ describe("AppHeaderComponent", () => { "openAnalyticsHeader", "openSettingsHeader", "userNameHeader", - "openSignOutHeader", + "openLogoutHeader", ].forEach((linkId: string) => { it("should not see header links for logged users when not logged in", () => { const link = findElementByDataTestId(fixture, linkId); @@ -224,7 +247,7 @@ describe("AppHeaderComponent", () => { }); it("should emit on click Help link", () => { - const emitterSubscription$ = component.clickHelpEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedHelp.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openHelpHeader"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -232,7 +255,7 @@ describe("AppHeaderComponent", () => { it("should emit add new second link", () => { loginUser(); - const emitterSubscription$ = component.addNewEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedAddNew.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "addNewBlock"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -261,14 +284,14 @@ describe("AppHeaderComponent", () => { }); it("should emit on click Help link menu", () => { - const emitterSubscription$ = component.clickHelpEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedHelp.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openHelp"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); }); it("should emit on click Login link menu", () => { - const emitterSubscription$ = component.loginEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedLogin.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openLogin"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -280,9 +303,20 @@ describe("AppHeaderComponent", () => { expect(link).toBeUndefined(); }); + it("should not have Login link menu when login feature is disabled", () => { + component.featureFlags = { + ...component.featureFlags, + enableLogin: false, + }; + fixture.detectChanges(); + + const link = findElementByDataTestId(fixture, "openLogin"); + expect(link).toBeUndefined(); + }); + it("should emit on click User Profile link menu", () => { loginUser(); - const emitterSubscription$ = component.clickUserProfileEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedUserProfile.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openUserProfile"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -290,7 +324,7 @@ describe("AppHeaderComponent", () => { it("should emit on click User Datasets link menu", () => { loginUser(); - const emitterSubscription$ = component.clickUserDatasetsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedUserDatasets.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openUserDatasets"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -298,7 +332,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Billing link menu", () => { loginUser(); - const emitterSubscription$ = component.clickBillingEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedBilling.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openBillingPlan"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -306,7 +340,7 @@ describe("AppHeaderComponent", () => { it("should emit on click Analytics link menu", () => { loginUser(); - const emitterSubscription$ = component.clickAnalyticsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedAnalytics.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openAnalytics"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); @@ -314,27 +348,39 @@ describe("AppHeaderComponent", () => { it("should emit on click Settings link menu", () => { loginUser(); - const emitterSubscription$ = component.clickSettingsEmitter.pipe(first()).subscribe(); + const emitterSubscription$ = component.onClickedSettings.pipe(first()).subscribe(); const link = getElementByDataTestId(fixture, "openSettings"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); }); - it("should emit on click Signout link menu", () => { + it("should emit on click logout link menu", () => { loginUser(); - const emitterSubscription$ = component.logOutEmitter.pipe(first()).subscribe(); - const link = getElementByDataTestId(fixture, "openSignOut"); + const emitterSubscription$ = component.onClickedLogout.pipe(first()).subscribe(); + const link = getElementByDataTestId(fixture, "openLogout"); link.click(); expect(emitterSubscription$.closed).toBeTrue(); }); + it("no Log Out link menu when feature disabled", () => { + loginUser(); + component.featureFlags = { + ...component.featureFlags, + enableLogout: false, + }; + fixture.detectChanges(); + + const link = findElementByDataTestId(fixture, "openLogout"); + expect(link).toBeUndefined(); + }); + [ "openUserProfile", "openUserDatasets", "openBillingPlan", "openAnalytics", "openSettings", - "openSignOut", + "openLogout", ].forEach((linkId: string) => { it("should not see header menus links for logged users when not logged in", () => { const link = findElementByDataTestId(fixture, linkId); @@ -363,7 +409,7 @@ describe("AppHeaderComponent", () => { fixture.detectChanges(); // Expect emitter event with hardcoded auto-complete item - const emitterSubscription$ = component.selectDatasetEmitter + const emitterSubscription$ = component.onSelectedDataset .pipe(first()) .subscribe((item: DatasetAutocompleteItem) => { expect(item).toBe(MOCK_AUTOCOMPLETE_ITEM); diff --git a/src/app/components/app-header/app-header.component.ts b/src/app/components/app-header/app-header.component.ts index 2f13dbd35..f0c13552a 100644 --- a/src/app/components/app-header/app-header.component.ts +++ b/src/app/components/app-header/app-header.component.ts @@ -20,6 +20,7 @@ import { AccountDetailsFragment } from "src/app/api/kamu.graphql.interface"; import { NgbTypeaheadSelectItemEvent } from "@ng-bootstrap/ng-bootstrap"; import ProjectLinks from "src/app/project-links"; import { NavigationService } from "src/app/services/navigation.service"; +import { FeatureFlags } from "src/app/common/feature-flags.model"; @Component({ selector: "app-header", @@ -27,27 +28,29 @@ import { NavigationService } from "src/app/services/navigation.service"; changeDetection: ChangeDetectionStrategy.OnPush, }) export class AppHeaderComponent extends BaseComponent implements OnInit { - @Input() public appLogo: string; + public readonly APP_LOGO = AppValues.APP_LOGO; + public readonly DEFAULT_AVATAR_URL = AppValues.DEFAULT_AVATAR_URL; + @Input() public isMobileView: boolean; @Input() public isVisible: boolean; - @Input() public userInfo: AccountDetailsFragment; - @Input() public isLoginEnabled: boolean; - - @Output() public selectDatasetEmitter = new EventEmitter(); - @Output() public addNewEmitter = new EventEmitter(); - @Output() public loginEmitter = new EventEmitter(); - @Output() public logOutEmitter = new EventEmitter(); - @Output() public userProfileEmitter = new EventEmitter(); - @Output() public clickAppLogoEmitter = new EventEmitter(); - @Output() public clickSettingsEmitter = new EventEmitter(); - @Output() public clickHelpEmitter = new EventEmitter(); - @Output() public clickAnalyticsEmitter = new EventEmitter(); - @Output() public clickBillingEmitter = new EventEmitter(); - @Output() public clickUserDatasetsEmitter = new EventEmitter(); - @Output() public clickUserProfileEmitter = new EventEmitter(); + @Input() public loggedUserInfo: AccountDetailsFragment; + @Input() public featureFlags: FeatureFlags; + + @Output() public onSelectedDataset = new EventEmitter(); + @Output() public onClickedAddNew = new EventEmitter(); + @Output() public onClickedLogin = new EventEmitter(); + @Output() public onClickedLogout = new EventEmitter(); + @Output() public onClickedOpenUserInfo = new EventEmitter(); + @Output() public onClickedAppLogo = new EventEmitter(); + @Output() public onClickedSettings = new EventEmitter(); + @Output() public onClickedHelp = new EventEmitter(); + @Output() public onClickedAnalytics = new EventEmitter(); + @Output() public onClickedBilling = new EventEmitter(); + @Output() public onClickedUserDatasets = new EventEmitter(); + @Output() public onClickedUserProfile = new EventEmitter(); @ViewChild("appHeaderMenuButton") - appHeaderMenuButton: ElementRef; + private appHeaderMenuButton: ElementRef; public defaultUsername: string = AppValues.DEFAULT_USERNAME; public isSearchActive = false; @@ -55,7 +58,7 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { public searchQuery = ""; private delayTime: number = AppValues.SHORT_DELAY_MS; - constructor( + public constructor( private appSearchAPI: SearchApi, private route: ActivatedRoute, private router: Router, @@ -64,7 +67,8 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { ) { super(); } - ngOnInit(): void { + + public ngOnInit(): void { this.trackSubscriptions( this.router.events .pipe( @@ -90,7 +94,7 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { } public isUserLoggedIn(): boolean { - return this.userInfo.login.length > 0; + return this.loggedUserInfo.login.length > 0; } public search: OperatorFunction = (text$: Observable) => { @@ -115,7 +119,7 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { public onSelectItem(event: NgbTypeaheadSelectItemEvent): void { this.isSearchActive = false; if (event.item) { - this.selectDatasetEmitter.emit(event.item as DatasetAutocompleteItem); + this.onSelectedDataset.emit(event.item as DatasetAutocompleteItem); setTimeout(() => { const typeaheadInput: HTMLElement | null = document.getElementById("typeahead-http"); if (typeaheadInput) { @@ -163,19 +167,19 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { } public onLogin(): void { - this.loginEmitter.emit(); + this.onClickedLogin.emit(); } - public onLogOut(): void { - this.logOutEmitter.emit(); + public onLogout(): void { + this.onClickedLogout.emit(); } public onAddNew(): void { - this.addNewEmitter.emit(); + this.onClickedAddNew.emit(); } public onOpenUserInfo(): void { - this.userProfileEmitter.emit(); + this.onClickedOpenUserInfo.emit(); } public triggerMenuClick(): void { @@ -186,31 +190,31 @@ export class AppHeaderComponent extends BaseComponent implements OnInit { this.isCollapsedAppHeaderMenu = !this.isCollapsedAppHeaderMenu; } - public onClickAppLogo(): void { - this.clickAppLogoEmitter.emit(); + public onAppLogo(): void { + this.onClickedAppLogo.emit(); } public onHelp(): void { - this.clickHelpEmitter.emit(); + this.onClickedHelp.emit(); } public onSettings(): void { - this.clickSettingsEmitter.emit(); + this.onClickedSettings.emit(); } public onAnalytics(): void { - this.clickAnalyticsEmitter.emit(); + this.onClickedAnalytics.emit(); } public onBilling(): void { - this.clickBillingEmitter.emit(); + this.onClickedBilling.emit(); } public onUserDatasets(): void { - this.clickUserDatasetsEmitter.emit(); + this.onClickedUserDatasets.emit(); } public onUserProfile(): void { - this.clickUserProfileEmitter.emit(); + this.onClickedUserProfile.emit(); } } diff --git a/src/app/components/overview-history-summary-header/overview-history-summary-header.component.html b/src/app/components/overview-history-summary-header/overview-history-summary-header.component.html index 4c3fe7c86..ca31606b9 100644 --- a/src/app/components/overview-history-summary-header/overview-history-summary-header.component.html +++ b/src/app/components/overview-history-summary-header/overview-history-summary-header.component.html @@ -16,7 +16,7 @@ > @sergiimk diff --git a/src/app/components/overview-history-summary-header/overview-history-summary-header.component.ts b/src/app/components/overview-history-summary-header/overview-history-summary-header.component.ts index 41773da4e..e01350212 100644 --- a/src/app/components/overview-history-summary-header/overview-history-summary-header.component.ts +++ b/src/app/components/overview-history-summary-header/overview-history-summary-header.component.ts @@ -14,7 +14,8 @@ export class OverviewHistorySummaryHeaderComponent { @Input() public metadataBlockFragment?: MetadataBlockFragment; @Input() public numBlocksTotal: number; @Input() public datasetName: string; - public appLogo = `/${AppValues.APP_LOGO}`; + + public readonly APP_LOGO = `/${AppValues.APP_LOGO}`; constructor(private navigationService: NavigationService) {} diff --git a/src/app/components/page-not-found/page-not-found.component.html b/src/app/components/page-not-found/page-not-found.component.html index c40bcc56d..92f126bd6 100644 --- a/src/app/components/page-not-found/page-not-found.component.html +++ b/src/app/components/page-not-found/page-not-found.component.html @@ -2,7 +2,7 @@
- Application logo + Application logo

Oops!

404 Not Found

Sorry, an error has occured, Requested page not found!
diff --git a/src/app/components/page-not-found/page-not-found.component.ts b/src/app/components/page-not-found/page-not-found.component.ts index 6151111fd..579694e04 100644 --- a/src/app/components/page-not-found/page-not-found.component.ts +++ b/src/app/components/page-not-found/page-not-found.component.ts @@ -9,7 +9,7 @@ import AppValues from "src/app/common/app.values"; changeDetection: ChangeDetectionStrategy.OnPush, }) export class PageNotFoundComponent { - public appLogo = `/${AppValues.APP_LOGO}`; + public readonly APP_LOGO = `/${AppValues.APP_LOGO}`; constructor(private navigationService: NavigationService) {} diff --git a/src/app/dataset-create/dataset-create.component.spec.ts b/src/app/dataset-create/dataset-create.component.spec.ts index f52279dd2..10f151f28 100644 --- a/src/app/dataset-create/dataset-create.component.spec.ts +++ b/src/app/dataset-create/dataset-create.component.spec.ts @@ -98,7 +98,7 @@ describe("DatasetCreateComponent", () => { const result = await component.onFileSelected(mockEvt as unknown as Event); expect(result).toBe("# You can edit this file\ntest content"); } catch (error) { - console.log(error); + fail(error); } }); @@ -113,7 +113,7 @@ describe("DatasetCreateComponent", () => { const result = await component.onFileSelected(mockEvt as unknown as Event); expect(result).toBe(""); } catch (error) { - console.log(error); + fail(error); } }); diff --git a/src/app/services/error-handler.service.spec.ts b/src/app/services/error-handler.service.spec.ts index 4d4293c9b..91ea2c340 100644 --- a/src/app/services/error-handler.service.spec.ts +++ b/src/app/services/error-handler.service.spec.ts @@ -5,14 +5,14 @@ import { ModalService } from "../components/modal/modal.service"; import { TestBed } from "@angular/core/testing"; import { ErrorHandlerService } from "./error-handler.service"; import { NavigationService } from "./navigation.service"; -import { AuthApi } from "../api/auth.api"; +import { LoggedUserService } from "../auth/logged-user.service"; describe("ErrorHandlerService", () => { let service: ErrorHandlerService; let modalService: ModalService; let navigationService: NavigationService; - const authApiMock = { + const loggedUserServiceMock = { terminateSession: () => { /* Intentionally empty */ }, @@ -20,7 +20,11 @@ describe("ErrorHandlerService", () => { beforeEach(() => { TestBed.configureTestingModule({ - providers: [ModalService, NavigationService, { provide: AuthApi, useValue: authApiMock }], + providers: [ + ModalService, + NavigationService, + { provide: LoggedUserService, useValue: loggedUserServiceMock }, + ], }); service = TestBed.inject(ErrorHandlerService); modalService = TestBed.inject(ModalService); @@ -91,7 +95,7 @@ describe("ErrorHandlerService", () => { it("should log authentication errors and terminate session", () => { const consoleErrorSpy: jasmine.Spy = spyOn(console, "error").and.stub(); - const authApiTerminateSessionSpy: jasmine.Spy = spyOn(authApiMock, "terminateSession").and.stub(); + const authApiTerminateSessionSpy: jasmine.Spy = spyOn(loggedUserServiceMock, "terminateSession").and.stub(); service.handleError(new AuthenticationError([])); expect(consoleErrorSpy).toHaveBeenCalledWith(ErrorTexts.ERROR_UNKNOWN_AUTHENTICATION); expect(authApiTerminateSessionSpy).toHaveBeenCalledWith(); diff --git a/src/app/services/error-handler.service.ts b/src/app/services/error-handler.service.ts index 2f5268d4a..097416b73 100644 --- a/src/app/services/error-handler.service.ts +++ b/src/app/services/error-handler.service.ts @@ -4,18 +4,18 @@ import { ErrorHandler, Injectable, NgZone } from "@angular/core"; import { ModalService } from "../components/modal/modal.service"; import { logError } from "../common/app.helpers"; import { ApolloError } from "@apollo/client/core"; -import { AuthApi } from "../api/auth.api"; +import { LoggedUserService } from "../auth/logged-user.service"; @Injectable({ providedIn: "root", }) export class ErrorHandlerService implements ErrorHandler { - private kamuHandlerError = new KamuErrorHandler(this.navigationService, this.modalService, this.authApi); + private kamuHandlerError = new KamuErrorHandler(this.navigationService, this.modalService, this.loggedUserService); constructor( private modalService: ModalService, private navigationService: NavigationService, - private authApi: AuthApi, + private loggedUserService: LoggedUserService, private ngZone: NgZone, ) {} diff --git a/src/assets/runtime-config.json b/src/assets/runtime-config.json index 722ff5339..611452b1c 100644 --- a/src/assets/runtime-config.json +++ b/src/assets/runtime-config.json @@ -1,4 +1,7 @@ { "apiServerGqlUrl": "http://localhost:8080/graphql", - "loginEnabled": true + "featureFlags": { + "enableLogin": true, + "enableLogout": true + } } \ No newline at end of file