From cd59f96c449b2dd1bc147704050f8d463e98bde4 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Thu, 17 Aug 2023 17:59:40 +0200 Subject: [PATCH] Fix loading settings. (#1017) * Fix loading settings. * Fix tests --- .../pages/contents/contents-page.component.ts | 15 ++--- .../guards/app-must-exist.guard.spec.ts | 9 ++- .../app/shared/guards/app-must-exist.guard.ts | 13 ++++- .../app/shared/services/contents.service.ts | 4 +- .../app/shared/services/schemas.service.ts | 14 ++++- .../src/app/shared/state/ui.state.spec.ts | 13 +---- frontend/src/app/shared/state/ui.state.ts | 58 +++++++++---------- 7 files changed, 67 insertions(+), 59 deletions(-) diff --git a/frontend/src/app/features/content/pages/contents/contents-page.component.ts b/frontend/src/app/features/content/pages/contents/contents-page.component.ts index fb6997a51..4d5e3385a 100644 --- a/frontend/src/app/features/content/pages/contents/contents-page.component.ts +++ b/frontend/src/app/features/content/pages/contents/contents-page.component.ts @@ -9,9 +9,9 @@ import { Component, OnInit, ViewChild } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { combineLatest, Observable } from 'rxjs'; -import { distinctUntilChanged, map, shareReplay, switchMap, take, tap } from 'rxjs/operators'; -import { AppLanguageDto, AppsState, ContentDto, ContentsService, ContentsState, contentsTranslationStatus, ContributorsState, defined, isDataField, LanguagesState, LocalStoreService, ModalModel, Queries, Query, QuerySynchronizer, ResourceOwner, Router2State, SchemaDto, SchemasService, SchemasState, Settings, switchSafe, TableSettings, TempService, TranslationStatus, Types, UIState } from '@app/shared'; +import { Observable } from 'rxjs'; +import { distinctUntilChanged, map, shareReplay, switchMap, take, tap, withLatestFrom } from 'rxjs/operators'; +import { AppLanguageDto, AppsState, ContentDto, ContentsService, ContentsState, contentsTranslationStatus, ContributorsState, defined, getTableFields, LanguagesState, LocalStoreService, ModalModel, Queries, Query, QuerySynchronizer, ResourceOwner, Router2State, SchemaDto, SchemasService, SchemasState, Settings, switchSafe, TableSettings, TempService, TranslationStatus, Types, UIState } from '@app/shared'; import { DueTimeSelectorComponent } from './../../shared/due-time-selector.component'; @Component({ @@ -97,17 +97,14 @@ export class ContentsPageComponent extends ResourceOwner implements OnInit { const tableSetting$ = schema$.pipe(map(s => new TableSettings(this.uiState, s)), shareReplay(1)); - const tableField$ = - tableSetting$.pipe(switchMap(s => s.listFields)); - const tableName$ = - tableField$.pipe(map(t => t.map(x => x.name).filter(isDataField).sorted()), distinctUntilChanged(Types.equals)); + tableSetting$.pipe(switchMap(s => s.listFields), map(s => getTableFields(s)), distinctUntilChanged(Types.equals)); this.tableSettings = tableSetting$; this.own( - combineLatest([schema$, tableName$]) - .subscribe(([schema, fieldNames]) => { + tableName$.pipe(withLatestFrom(schema$)) + .subscribe(([fieldNames, schema]) => { if (this.schema?.id !== schema.id) { this.resetSelection(); diff --git a/frontend/src/app/shared/guards/app-must-exist.guard.spec.ts b/frontend/src/app/shared/guards/app-must-exist.guard.spec.ts index 18054d24c..c060d33ba 100644 --- a/frontend/src/app/shared/guards/app-must-exist.guard.spec.ts +++ b/frontend/src/app/shared/guards/app-must-exist.guard.spec.ts @@ -8,7 +8,7 @@ import { Router } from '@angular/router'; import { firstValueFrom, of } from 'rxjs'; import { IMock, Mock, Times } from 'typemoq'; -import { AppsState } from '@app/shared/internal'; +import { AppsState, UIState } from '@app/shared/internal'; import { AppMustExistGuard } from './app-must-exist.guard'; describe('AppMustExistGuard', () => { @@ -21,11 +21,13 @@ describe('AppMustExistGuard', () => { let router: IMock; let appsState: IMock; let appGuard: AppMustExistGuard; + let uiState: IMock; beforeEach(() => { + uiState = Mock.ofType(); router = Mock.ofType(); appsState = Mock.ofType(); - appGuard = new AppMustExistGuard(appsState.object, router.object); + appGuard = new AppMustExistGuard(appsState.object, router.object, uiState.object); }); it('should navigate to 404 page if app is not found', async () => { @@ -40,6 +42,9 @@ describe('AppMustExistGuard', () => { }); it('should return true if app is found', async () => { + uiState.setup(x => x.loadApp('my-app')) + .returns(() => of({})); + appsState.setup(x => x.select('my-app')) .returns(() => of({})); diff --git a/frontend/src/app/shared/guards/app-must-exist.guard.ts b/frontend/src/app/shared/guards/app-must-exist.guard.ts index a4d09a285..2a8fb6f72 100644 --- a/frontend/src/app/shared/guards/app-must-exist.guard.ts +++ b/frontend/src/app/shared/guards/app-must-exist.guard.ts @@ -7,15 +7,17 @@ import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Router } from '@angular/router'; -import { Observable } from 'rxjs'; -import { map, tap } from 'rxjs/operators'; +import { Observable, of } from 'rxjs'; +import { map, switchMap, tap } from 'rxjs/operators'; import { AppsState } from './../state/apps.state'; +import { UIState } from '../internal'; @Injectable() export class AppMustExistGuard { constructor( private readonly appsState: AppsState, private readonly router: Router, + private readonly uiState: UIState, ) { } @@ -29,6 +31,13 @@ export class AppMustExistGuard { this.router.navigate(['/404']); } }), + switchMap(app => { + if (app) { + return this.uiState.loadApp(appName).pipe(map(() => app)); + } else { + return of(app); + } + }), map(app => !!app)); return result; diff --git a/frontend/src/app/shared/services/contents.service.ts b/frontend/src/app/shared/services/contents.service.ts index 4eae670b0..41b54c2e1 100644 --- a/frontend/src/app/shared/services/contents.service.ts +++ b/frontend/src/app/shared/services/contents.service.ts @@ -12,7 +12,7 @@ import { map } from 'rxjs/operators'; import { ApiUrlConfig, DateTime, ErrorDto, hasAnyLink, HTTP, mapVersioned, pretifyError, Resource, ResourceLinks, Version, Versioned } from '@app/framework'; import { StatusInfo } from './../state/contents.state'; import { Query, sanitize } from './query'; -import { isDataField, parseField, RootFieldDto } from './schemas.service'; +import { parseField, RootFieldDto } from './schemas.service'; export class ScheduleDto { constructor( @@ -374,7 +374,7 @@ function buildHeaders(q: ContentsQuery | undefined) { }; if (q?.fieldNames) { - options.headers['X-Fields'] = q.fieldNames.filter(isDataField).join(','); + options.headers['X-Fields'] = q.fieldNames.join(','); } if (q?.noTotal) { diff --git a/frontend/src/app/shared/services/schemas.service.ts b/frontend/src/app/shared/services/schemas.service.ts index c4ac39bd0..597e3a3ca 100644 --- a/frontend/src/app/shared/services/schemas.service.ts +++ b/frontend/src/app/shared/services/schemas.service.ts @@ -91,8 +91,18 @@ export const META_FIELDS = { }, }; -export function isDataField(name: string) { - return name.indexOf('meta.') < 0; +export function getTableFields(fields: ReadonlyArray) { + const result: string[] = []; + + for (const field of fields) { + if (field.name && field.name.indexOf('meta') < 0) { + result.push(field.name); + } + } + + result.sort(); + + return result; } export const FIELD_RULE_ACTIONS: ReadonlyArray = [ diff --git a/frontend/src/app/shared/state/ui.state.spec.ts b/frontend/src/app/shared/state/ui.state.spec.ts index c95c73cf4..ac23d94c2 100644 --- a/frontend/src/app/shared/state/ui.state.spec.ts +++ b/frontend/src/app/shared/state/ui.state.spec.ts @@ -13,7 +13,6 @@ import { TestValues } from './_test-helpers'; describe('UIState', () => { const { app, - appsState, } = TestValues; const common = { @@ -69,7 +68,9 @@ describe('UIState', () => { usersService.setup(x => x.getResources()) .returns(() => of({ _links: resources })); - uiState = new UIState(appsState.object, uiService.object, usersService.object); + uiState = new UIState(uiService.object, usersService.object); + uiState.load(); + uiState.loadApp(app); }); it('should load settings', () => { @@ -79,8 +80,6 @@ describe('UIState', () => { settings = value; }); - uiState.load().subscribe(); - expect(settings).toEqual({ key: 'xx', map: { @@ -109,7 +108,6 @@ describe('UIState', () => { uiService.setup(x => x.putCommonSetting('root.nested', 123)) .returns(() => of({})).verifiable(); - uiState.load(); uiState.setCommon('root.nested', 123); expect(settings).toEqual({ @@ -153,7 +151,6 @@ describe('UIState', () => { uiService.setup(x => x.putAppSharedSetting(app, 'root.nested', 123)) .returns(() => of({})).verifiable(); - uiState.load(); uiState.setAppShared('root.nested', 123); expect(settings).toEqual({ @@ -197,7 +194,6 @@ describe('UIState', () => { uiService.setup(x => x.putAppUserSetting(app, 'root.nested', 123)) .returns(() => of({})).verifiable(); - uiState.load(); uiState.setAppUser('root.nested', 123); expect(settings).toEqual({ @@ -241,7 +237,6 @@ describe('UIState', () => { uiService.setup(x => x.deleteCommonSetting('map.onlyCommon')) .returns(() => of({})).verifiable(); - uiState.load(); uiState.remove('map.onlyCommon'); expect(settings).toEqual({ @@ -269,7 +264,6 @@ describe('UIState', () => { uiService.setup(x => x.deleteAppSharedSetting(app, 'map.onlyShared')) .returns(() => of({})).verifiable(); - uiState.load(); uiState.remove('map.onlyShared'); expect(settings).toEqual({ @@ -297,7 +291,6 @@ describe('UIState', () => { uiService.setup(x => x.deleteAppUserSetting(app, 'map.onlyUser')) .returns(() => of({})).verifiable(); - uiState.load(); uiState.remove('map.onlyUser'); expect(settings).toEqual({ diff --git a/frontend/src/app/shared/state/ui.state.ts b/frontend/src/app/shared/state/ui.state.ts index a9c664af0..f04b956a5 100644 --- a/frontend/src/app/shared/state/ui.state.ts +++ b/frontend/src/app/shared/state/ui.state.ts @@ -8,10 +8,9 @@ import { Injectable } from '@angular/core'; import { combineLatest } from 'rxjs'; import { distinctUntilChanged, filter, map, tap } from 'rxjs/operators'; -import { debug, defined, hasAnyLink, shareSubscribed, State, Types } from '@app/framework'; +import { debug, hasAnyLink, shareSubscribed, State, Types } from '@app/framework'; import { UIService } from './../services/ui.service'; import { UsersService } from './../services/users.service'; -import { AppsState } from './apps.state'; type Settings = { canCreateApps?: boolean; canCreateTeams?: boolean; [key: string]: any }; @@ -36,6 +35,9 @@ interface Snapshot { // Indicates if the user can use at least one admin resource. canUseAdminResource?: boolean; + + // The app name. + appName?: string; } @Injectable() @@ -84,49 +86,37 @@ export class UIState extends State { distinctUntilChanged()); } - public get appId() { - return this.appsState.appId; - } - - public get appName() { - return this.appsState.appName; - } - constructor( - private readonly appsState: AppsState, private readonly uiService: UIService, private readonly usersService: UsersService, ) { super({}); debug(this, 'settings'); - - appsState.selectedApp.pipe(defined()) - .subscribe(app => { - this.loadForApp(app.name); - }); } public load() { return combineLatest([this.loadCommon(), this.loadResources()]); } - private loadForApp(app: string) { - this.next(s => ({ - ...s, - settingsAppShared: null, - settingsAppUser: null, - }), 'Loading Started'); - - this.uiService.getAppSharedSettings(app) - .subscribe(payload => { - this.next({ settingsAppShared: payload }, 'Loading App Shared Done'); - }); - - this.uiService.getAppUserSettings(app) - .subscribe(payload => { - this.next({ settingsAppUser: payload }, 'Loading App User Done'); - }); + public loadApp(appName: string) { + return combineLatest([this.loadAppShared(appName), this.loadAppUser(appName)]); + } + + private loadAppUser(appName: string) { + return this.uiService.getAppUserSettings(appName).pipe( + tap(settingsAppUser => { + this.next({ settingsAppUser, appName }, 'Loading App User Done'); + }), + shareSubscribed(undefined, { throw: true })); + } + + private loadAppShared(appName: string) { + return this.uiService.getAppSharedSettings(appName).pipe( + tap(settingsAppShared => { + this.next({ settingsAppShared, appName }, 'Loading App Shared Done'); + }), + shareSubscribed(undefined, { throw: true })); } private loadCommon() { @@ -233,6 +223,10 @@ export class UIState extends State { return false; } + + private get appName() { + return this.snapshot.appName!; + } } function getValue(setting: Settings | undefined | null, path: string, defaultValue: T) {