Fix loading settings. (#1017)

* Fix loading settings.

* Fix tests

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();
 

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<Router>;
     let appsState: IMock<AppsState>;
     let appGuard: AppMustExistGuard;
+    let uiState: IMock<UIState>;
 
     beforeEach(() => {
+        uiState = Mock.ofType<UIState>();
         router = Mock.ofType<Router>();
         appsState = Mock.ofType<AppsState>();
-        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(<any>{}));
+
         appsState.setup(x => x.select('my-app'))
             .returns(() => of(<any>{}));
 

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;

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

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<TableField>) {
+    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<FieldRuleAction> = [

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({

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<Snapshot> {
             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<Snapshot> {
 
         return false;
     }
+
+    private get appName() {
+        return this.snapshot.appName!;
+    }
 }
 
 function getValue<T>(setting: Settings | undefined | null, path: string, defaultValue: T) {