From d6477de2aec9b23445b758d997cb8ee96d46fbcf Mon Sep 17 00:00:00 2001 From: Sergei Zaychenko Date: Fri, 25 Aug 2023 09:28:11 -0700 Subject: [PATCH] Interpreting login instructions at startup Auth login API publishes custom method, driven by app config. Account component shows full name and avatar, if known. GraphQL caching by User customized to use name instead of id --- src/app/api/account.api.ts | 1 + src/app/api/auth.api.ts | 30 +++++------ src/app/api/dataset.api.spec.ts | 18 +++---- src/app/api/mock/auth.mock.ts | 17 +++++- src/app/api/mock/dataset.mock.ts | 1 - src/app/app-config.model.ts | 15 ++++++ src/app/app-config.service.ts | 28 ++-------- src/app/app.component.ts | 6 +-- src/app/app.module.ts | 12 ++++- src/app/auth/account/account.component.html | 4 +- .../auth/account/account.component.spec.ts | 4 +- src/app/auth/account/account.component.ts | 52 +++++++++++++------ .../datasets-tab/datasets-tab.component.ts | 16 +++--- src/app/auth/guards/login.guard.spec.ts | 4 +- src/app/auth/logged-user-service.spec.ts | 23 +++++--- src/app/auth/logged-user.service.ts | 11 ++-- src/app/common/feature-flags.model.ts | 4 -- .../app-header/app-header.component.ts | 4 +- 18 files changed, 147 insertions(+), 103 deletions(-) create mode 100644 src/app/app-config.model.ts delete mode 100644 src/app/common/feature-flags.model.ts diff --git a/src/app/api/account.api.ts b/src/app/api/account.api.ts index 5f0d58b81..d6510e798 100644 --- a/src/app/api/account.api.ts +++ b/src/app/api/account.api.ts @@ -6,6 +6,7 @@ import { AccountDetailsFragment } from "./kamu.graphql.interface"; @Injectable({ providedIn: "root" }) export class AccountApi { + // TODO: issue a GraphQL query public getAccountInfoByName(name: string): Observable { return of({ ...mockAccountDetails, login: name }); } diff --git a/src/app/api/auth.api.ts b/src/app/api/auth.api.ts index d51cb7408..00b2ca950 100644 --- a/src/app/api/auth.api.ts +++ b/src/app/api/auth.api.ts @@ -36,20 +36,21 @@ export class AuthApi { public fetchUserInfoAndTokenFromPasswordLogin(login: string, password: string): Observable { const passwordCredentials: PasswordLoginCredentials = { login, password }; - return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_PASSWORD, passwordCredentials); + return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_PASSWORD, JSON.stringify(passwordCredentials)); } public fetchUserInfoAndTokenFromGithubCallackCode(code: string): Observable { const githubCredentials: GithubLoginCredentials = { code }; - return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_GITHUB, githubCredentials); + return this.fetchUserInfoAndTokenFromLoginMethod(LOGIN_METHOD_GITHUB, JSON.stringify(githubCredentials)); } - public fetchUserInfoFromAccessToken(accessToken: string): Observable { - return this.fetchAccountInfoGQL.mutate({ accessToken }).pipe( - map((result: MutationResult) => { + public fetchUserInfoAndTokenFromLoginMethod(loginMethod: string, loginCredentialsJson: string): Observable { + return this.loginGQL.mutate({ login_method: loginMethod, login_credentials_json: loginCredentialsJson }).pipe( + map((result: MutationResult) => { if (result.data) { - const data: FetchAccountInfoMutation = result.data; - this.accountChanged$.next(data.auth.accountInfo); + const data: LoginMutation = result.data; + this.accessTokenObtained$.next(data.auth.login.accessToken); + this.accountChanged$.next(data.auth.login.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } @@ -58,17 +59,12 @@ export class AuthApi { ); } - private fetchUserInfoAndTokenFromLoginMethod( - loginMethod: string, - loginCredentials: TCredentials, - ): Observable { - const loginCredentialsJson: string = JSON.stringify(loginCredentials); - return this.loginGQL.mutate({ login_method: loginMethod, login_credentials_json: loginCredentialsJson }).pipe( - map((result: MutationResult) => { + public fetchUserInfoFromAccessToken(accessToken: string): Observable { + return this.fetchAccountInfoGQL.mutate({ accessToken }).pipe( + map((result: MutationResult) => { if (result.data) { - const data: LoginMutation = result.data; - this.accessTokenObtained$.next(data.auth.login.accessToken); - this.accountChanged$.next(data.auth.login.accountInfo); + const data: FetchAccountInfoMutation = result.data; + this.accountChanged$.next(data.auth.accountInfo); } else { throw new AuthenticationError(result.errors ?? []); } diff --git a/src/app/api/dataset.api.spec.ts b/src/app/api/dataset.api.spec.ts index 7fac55d52..6412661f7 100644 --- a/src/app/api/dataset.api.spec.ts +++ b/src/app/api/dataset.api.spec.ts @@ -4,7 +4,6 @@ import { mockGetMetadataBlockQuery, TEST_BLOCK_HASH, TEST_DATASET_NAME, - TEST_USER_NAME, } from "./mock/dataset.mock"; import { mockCommitEventResponse, @@ -29,6 +28,7 @@ import { GetMetadataBlockQuery, } from "./kamu.graphql.interface"; import { MaybeNullOrUndefined } from "../common/app.types"; +import { TEST_LOGIN } from "./mock/auth.mock"; describe("DatasetApi", () => { let service: DatasetApi; @@ -54,7 +54,7 @@ describe("DatasetApi", () => { it("should query dataset main data", () => { service .getDatasetMainData({ - accountName: TEST_USER_NAME, + accountName: TEST_LOGIN, datasetName: TEST_DATASET_NAME, }) .subscribe((res: GetDatasetMainDataQuery) => { @@ -65,7 +65,7 @@ describe("DatasetApi", () => { }); const op = controller.expectOne(GetDatasetMainDataDocument); - expect(op.operation.variables.accountName).toEqual(TEST_USER_NAME); + expect(op.operation.variables.accountName).toEqual(TEST_LOGIN); expect(op.operation.variables.datasetName).toEqual(TEST_DATASET_NAME); op.flush({ @@ -105,7 +105,7 @@ describe("DatasetApi", () => { it("should extract dataset history", () => { service .getDatasetHistory({ - accountName: TEST_USER_NAME, + accountName: TEST_LOGIN, datasetName: TEST_DATASET_NAME, numRecords: 20, numPage: 1, @@ -117,7 +117,7 @@ describe("DatasetApi", () => { }); const op = controller.expectOne(GetDatasetHistoryDocument); - expect(op.operation.variables.accountName).toEqual(TEST_USER_NAME); + expect(op.operation.variables.accountName).toEqual(TEST_LOGIN); expect(op.operation.variables.datasetName).toEqual(TEST_DATASET_NAME); op.flush({ @@ -126,7 +126,7 @@ describe("DatasetApi", () => { }); it("should extract datasets by account name", () => { - service.fetchDatasetsByAccountName(TEST_USER_NAME).subscribe((res: DatasetsByAccountNameQuery) => { + service.fetchDatasetsByAccountName(TEST_LOGIN).subscribe((res: DatasetsByAccountNameQuery) => { expect(res.datasets.byAccountName.totalCount).toEqual( mockDatasetsByAccountNameQuery.datasets.byAccountName.totalCount, ); @@ -136,7 +136,7 @@ describe("DatasetApi", () => { }); const op = controller.expectOne(DatasetsByAccountNameDocument); - expect(op.operation.variables.accountName).toEqual(TEST_USER_NAME); + expect(op.operation.variables.accountName).toEqual(TEST_LOGIN); op.flush({ data: mockDatasetsByAccountNameQuery, @@ -147,7 +147,7 @@ describe("DatasetApi", () => { const blockByHash = mockGetMetadataBlockQuery.datasets.byOwnerAndName?.metadata.chain.blockByHash; service .getBlockByHash({ - accountName: TEST_USER_NAME, + accountName: TEST_LOGIN, datasetName: TEST_DATASET_NAME, blockHash: TEST_BLOCK_HASH, }) @@ -161,7 +161,7 @@ describe("DatasetApi", () => { }); const op = controller.expectOne(GetMetadataBlockDocument); - expect(op.operation.variables.accountName).toEqual(TEST_USER_NAME); + expect(op.operation.variables.accountName).toEqual(TEST_LOGIN); expect(op.operation.variables.datasetName).toEqual(TEST_DATASET_NAME); expect(op.operation.variables.blockHash).toEqual(TEST_BLOCK_HASH); op.flush({ diff --git a/src/app/api/mock/auth.mock.ts b/src/app/api/mock/auth.mock.ts index 2230733b1..c73cb55fe 100644 --- a/src/app/api/mock/auth.mock.ts +++ b/src/app/api/mock/auth.mock.ts @@ -1,16 +1,29 @@ import { GraphQLError } from "graphql"; import { AccountDetailsFragment, FetchAccountInfoMutation, LoginMutation } from "../kamu.graphql.interface"; import AppValues from "src/app/common/app.values"; +import { AppConfigLoginInstructions } from "src/app/app-config.model"; +import { LOGIN_METHOD_PASSWORD, PasswordLoginCredentials } from "../auth.api.model"; export const TEST_GITHUB_CODE = "12345"; export const TEST_ACCESS_TOKEN_GITHUB = "someTokenViaGithub"; export const TEST_ACCESS_TOKEN_PASSWORD = "someTokenViaPassword"; export const TEST_LOGIN = "foo"; export const TEST_PASSWORD = "bar"; +export const TEST_USER_NAME = "Test User"; + +const mockPasswordLoginCredentials: PasswordLoginCredentials = { + login: TEST_LOGIN, + password: TEST_PASSWORD, +}; + +export const mockLoginInstructions: AppConfigLoginInstructions = { + loginMethod: LOGIN_METHOD_PASSWORD, + loginCredentialsJson: JSON.stringify(mockPasswordLoginCredentials), +}; export const mockAccountDetails: AccountDetailsFragment = { - login: "test-user", - name: "Test User", + login: TEST_LOGIN, + name: TEST_USER_NAME, avatarUrl: AppValues.DEFAULT_AVATAR_URL, }; diff --git a/src/app/api/mock/dataset.mock.ts b/src/app/api/mock/dataset.mock.ts index ede8ee9fc..b9b81f961 100644 --- a/src/app/api/mock/dataset.mock.ts +++ b/src/app/api/mock/dataset.mock.ts @@ -9,7 +9,6 @@ import { import { DataSchemaFormat } from "../kamu.graphql.interface"; import { DatasetsAccountResponse } from "src/app/interface/dataset.interface"; -export const TEST_USER_NAME = "test-user"; export const TEST_DATASET_NAME = "test-dataset"; export const TEST_BLOCK_HASH = "zW1hNbxPz28K1oLNGbddudUzKKLT9LDPh8chjksEo6HcDev"; diff --git a/src/app/app-config.model.ts b/src/app/app-config.model.ts new file mode 100644 index 000000000..80677f673 --- /dev/null +++ b/src/app/app-config.model.ts @@ -0,0 +1,15 @@ +export interface AppConfig { + apiServerGqlUrl: string; + featureFlags: AppConfigFeatureFlags; + loginInstructions?: AppConfigLoginInstructions; +} + +export interface AppConfigLoginInstructions { + loginMethod: string; + loginCredentialsJson: string; +} + +export interface AppConfigFeatureFlags { + enableLogin: boolean; + enableLogout: boolean; +} diff --git a/src/app/app-config.service.ts b/src/app/app-config.service.ts index c711cf757..f28d26f69 100644 --- a/src/app/app-config.service.ts +++ b/src/app/app-config.service.ts @@ -1,21 +1,5 @@ import { Injectable } from "@angular/core"; -import { AccountDetailsFragment } from "./api/kamu.graphql.interface"; -import { FeatureFlags } from "./common/feature-flags.model"; - -interface AppConfig { - apiServerGqlUrl: string; - featureFlags: { - enableLogin: boolean; - enableLogout: boolean; - }; - loggedUser?: AppConfigLoggedUser; -} - -interface AppConfigLoggedUser { - login: string; - name: string; - avatarUrl?: string; -} +import { AppConfig, AppConfigFeatureFlags, AppConfigLoginInstructions } from "./app-config.model"; @Injectable({ providedIn: "root", @@ -31,7 +15,7 @@ export class AppConfigService { return this.appConfig.apiServerGqlUrl; } - get featureFlags(): FeatureFlags { + get featureFlags(): AppConfigFeatureFlags { if (!this.appConfig) { this.appConfig = AppConfigService.loadAppConfig(); } @@ -39,15 +23,13 @@ export class AppConfigService { return this.appConfig.featureFlags; } - get loggedUser(): AccountDetailsFragment | null { + get loginInstructions(): AppConfigLoginInstructions | null { if (!this.appConfig) { this.appConfig = AppConfigService.loadAppConfig(); } - if (this.appConfig.loggedUser) { - return { - ...this.appConfig.loggedUser, - } as AccountDetailsFragment; + if (this.appConfig.loginInstructions) { + return this.appConfig.loginInstructions; } else { return null; } diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 9aa57e389..25a39aa63 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -15,8 +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"; +import { AppConfigFeatureFlags } from "./app-config.model"; export const ALL_URLS_WITHOUT_HEADER: string[] = [ProjectLinks.URL_LOGIN, ProjectLinks.URL_GITHUB_CALLBACK]; @@ -31,7 +31,7 @@ export class AppComponent extends BaseComponent implements OnInit { login: "", name: AppValues.DEFAULT_USERNAME, }; - public static readonly DEFAULT_FEATURE_FLAGS: FeatureFlags = { + public static readonly DEFAULT_FEATURE_FLAGS: AppConfigFeatureFlags = { enableLogin: true, enableLogout: true, }; @@ -41,7 +41,7 @@ export class AppComponent extends BaseComponent implements OnInit { public isMobileView = false; public isHeaderVisible = true; - public featureFlags: FeatureFlags = AppComponent.DEFAULT_FEATURE_FLAGS; + public featureFlags: AppConfigFeatureFlags = AppComponent.DEFAULT_FEATURE_FLAGS; public loggedUserInfo: AccountDetailsFragment = AppComponent.ANONYMOUS_ACCOUNT_INFO; @HostListener("window:resize") diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 419e8e488..5c799bad3 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -65,6 +65,7 @@ import { ToastrModule } from "ngx-toastr"; import { LoggedUserService } from "./auth/logged-user.service"; import { firstValueFrom } from "rxjs"; import { LoginService } from "./auth/login/login.service"; +import { logError } from "./common/app.helpers"; const Services = [ { @@ -93,7 +94,14 @@ const Services = [ provide: APOLLO_OPTIONS, useFactory: (httpLink: HttpLink, appConfig: AppConfigService) => { return { - cache: new InMemoryCache(), + cache: new InMemoryCache({ + typePolicies: { + User: { + // For now we are faking account IDs on the server, so they are a bad caching field + keyFields: ["name"], + }, + }, + }), link: httpLink.create({ uri: appConfig.apiServerGqlUrl, }), @@ -115,7 +123,7 @@ const Services = [ provide: APP_INITIALIZER, useFactory: (loggedUserService: LoggedUserService) => { return (): Promise => { - return firstValueFrom(loggedUserService.initialize()); + return firstValueFrom(loggedUserService.initialize()).catch((e) => logError(e)); }; }, deps: [LoggedUserService], diff --git a/src/app/auth/account/account.component.html b/src/app/auth/account/account.component.html index be7b075ef..48b52ff86 100644 --- a/src/app/auth/account/account.component.html +++ b/src/app/auth/account/account.component.html @@ -24,7 +24,7 @@ - {{ accountName }} + {{ user?.name }}