From e9fd10308dcf13f7fd61742c71731f6970487d76 Mon Sep 17 00:00:00 2001 From: Colin Rudd Date: Tue, 7 Jan 2025 14:26:06 -0500 Subject: [PATCH] Pass token, not viewInfo and surface raw errors to make API more flexible. (#3889) * Move exception handling to end of promise chain, as per doc suggestion: https://github.com/xh/hoist-react/blob/develop/promise/Promise.ts#L48 * fetchViewAsync takes token, not viewInfo, so that selectViewAsync can take token, and not ViewInfo * CR with Lee. * Add some TS + fix loadViewAsync arg --- cmp/viewmanager/DataAccess.ts | 8 +-- cmp/viewmanager/ViewInfo.ts | 3 ++ cmp/viewmanager/ViewManagerModel.ts | 53 ++++++++++--------- desktop/cmp/viewmanager/ViewManager.ts | 8 ++- .../cmp/viewmanager/ViewManagerLocalModel.ts | 18 +++++++ desktop/cmp/viewmanager/ViewMenu.ts | 8 +-- .../viewmanager/dialog/ManageDialogModel.ts | 2 +- 7 files changed, 60 insertions(+), 40 deletions(-) diff --git a/cmp/viewmanager/DataAccess.ts b/cmp/viewmanager/DataAccess.ts index a27276df6..3e8a2d361 100644 --- a/cmp/viewmanager/DataAccess.ts +++ b/cmp/viewmanager/DataAccess.ts @@ -48,14 +48,14 @@ export class DataAccess { } /** Fetch the latest version of a view. */ - async fetchViewAsync(info: ViewInfo): Promise> { + async fetchViewAsync(token: string): Promise> { const {model} = this; - if (!info) return View.createDefault(model); + if (!token) return View.createDefault(model); try { - const raw = await XH.fetchJson({url: 'xhView/get', params: {token: info.token}}); + const raw = await XH.fetchJson({url: 'xhView/get', params: {token}}); return View.fromBlob(raw, model); } catch (e) { - throw XH.exception({message: `Unable to fetch ${info.typedName}`, cause: e}); + throw XH.exception({message: `Unable to fetch view with token ${token}`, cause: e}); } } diff --git a/cmp/viewmanager/ViewInfo.ts b/cmp/viewmanager/ViewInfo.ts index 92b593e49..357948cca 100644 --- a/cmp/viewmanager/ViewInfo.ts +++ b/cmp/viewmanager/ViewInfo.ts @@ -5,6 +5,7 @@ * Copyright © 2025 Extremely Heavy Industries Inc. */ import {SECONDS} from '@xh/hoist/utils/datetime'; +import {throwIf} from '@xh/hoist/utils/js'; import {ViewManagerModel} from './ViewManagerModel'; import {JsonBlob} from '@xh/hoist/svc'; import {PlainObject, XH} from '@xh/hoist/core'; @@ -57,6 +58,8 @@ export class ViewInfo { readonly model: ViewManagerModel; constructor(blob: JsonBlob, model: ViewManagerModel) { + throwIf(blob.type !== model.type, 'View type does not match ViewManager type.'); + this.token = blob.token; this.type = blob.type; this.name = blob.name; diff --git a/cmp/viewmanager/ViewManagerModel.ts b/cmp/viewmanager/ViewManagerModel.ts index d11c1a61e..e8df53f97 100644 --- a/cmp/viewmanager/ViewManagerModel.ts +++ b/cmp/viewmanager/ViewManagerModel.ts @@ -21,7 +21,7 @@ import {fmtDateTime} from '@xh/hoist/format'; import {action, bindable, makeObservable, observable, comparer, runInAction} from '@xh/hoist/mobx'; import {olderThan, SECONDS} from '@xh/hoist/utils/datetime'; import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js'; -import {find, isEqual, isNil, isNull, isUndefined, lowerCase, uniqBy} from 'lodash'; +import {find, isEqual, isNil, isNull, isObject, isUndefined, lowerCase, uniqBy} from 'lodash'; import {ReactNode} from 'react'; import {ViewInfo} from './ViewInfo'; import {View} from './View'; @@ -337,7 +337,7 @@ export class ViewManagerModel extends HoistModel { if (!view.isDefault) { const latestInfo = find(views, {token: view.token}); if (latestInfo && latestInfo.lastUpdated > view.lastUpdated) { - this.loadViewAsync(latestInfo, this.pendingValue); + this.loadViewAsync(view.token, this.pendingValue); } } } catch (e) { @@ -346,7 +346,12 @@ export class ViewManagerModel extends HoistModel { } } - async selectViewAsync(info: ViewInfo, opts = {alertUnsavedChanges: true}): Promise { + async selectViewAsync( + view: string | ViewInfo, + opts = {alertUnsavedChanges: true} + ): Promise { + const token = isObject(view) ? view.token : view; + // ensure any pending auto-save gets completed if (this.isValueDirty && this.isViewAutoSavable) { await this.maybeAutoSaveAsync(); @@ -362,7 +367,7 @@ export class ViewManagerModel extends HoistModel { return; } - await this.loadViewAsync(info).catch(e => this.handleException(e)); + return this.loadViewAsync(token); } async saveAsAsync(spec: ViewCreateSpec): Promise { @@ -380,26 +385,22 @@ export class ViewManagerModel extends HoistModel { return; } const {pendingValue, view, dataAccess} = this; - try { - if (!(await this.maybeConfirmSaveAsync(view, pendingValue))) { - return; - } - const updated = await dataAccess - .updateViewValueAsync(view, pendingValue.value) - .linkTo(this.saveTask); - this.setAsView(updated); - this.noteSuccess(`Saved ${view.typedName}`); - } catch (e) { - this.handleException(e, { - message: `Failed to save ${view.typedName}. If this persists consider \`Save As...\`.` - }); + if (!(await this.maybeConfirmSaveAsync(view, pendingValue))) { + return; } + const updated = await dataAccess + .updateViewValueAsync(view, pendingValue.value) + .linkTo(this.saveTask); + + this.setAsView(updated); + this.noteSuccess(`Saved ${view.typedName}`); + this.refreshAsync(); } async resetAsync(): Promise { - await this.loadViewAsync(this.view.info).catch(e => this.handleException(e)); + return this.loadViewAsync(this.view.token); } //-------------------------------- @@ -483,7 +484,7 @@ export class ViewManagerModel extends HoistModel { const {views} = this; if (toDelete.some(view => view.isCurrentView) && !views.some(view => view.isCurrentView)) { - await this.loadViewAsync(this.initialViewSpec?.(views)); + await this.loadViewAsync(this.initialViewSpec?.(views)?.token); } if (exception) throw exception; @@ -508,8 +509,8 @@ export class ViewManagerModel extends HoistModel { }); // 2) Initialize/choose initial view. Null is ok, and will yield default. - let initialView, - initialTkn = initialState.currentView; + let initialView: ViewInfo, + initialTkn: string = initialState.currentView; if (isUndefined(initialTkn)) { initialView = this.initialViewSpec?.(views); } else if (!isNull(initialTkn)) { @@ -518,7 +519,7 @@ export class ViewManagerModel extends HoistModel { initialView = null; } - await this.loadViewAsync(initialView, this.pendingValue); + await this.loadViewAsync(initialView?.token, this.pendingValue); } catch (e) { // Always ensure at least default view is installed (other state defaults are fine) this.loadViewAsync(null, this.pendingValue); @@ -569,13 +570,13 @@ export class ViewManagerModel extends HoistModel { } private async loadViewAsync( - info: ViewInfo, + token: string, pendingValue: PendingValue = null ): Promise { return this.dataAccess - .fetchViewAsync(info) + .fetchViewAsync(token) .thenAction(latest => { - this.setAsView(latest, pendingValue?.token == info?.token ? pendingValue : null); + this.setAsView(latest, pendingValue?.token == token ? pendingValue : null); this.providers.forEach(it => it.pushStateToTarget()); this.lastPushed = Date.now(); }) @@ -649,7 +650,7 @@ export class ViewManagerModel extends HoistModel { private async maybeConfirmSaveAsync(view: View, pendingValue: PendingValue) { // Get latest from server for reference - const latest = await this.dataAccess.fetchViewAsync(view.info), + const latest = await this.dataAccess.fetchViewAsync(view.token), isGlobal = latest.isGlobal, isStale = latest.lastUpdated > pendingValue.baseUpdated; if (!isStale && !isGlobal) return true; diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index 9280e1e0c..753c1e00e 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -118,7 +118,7 @@ const menuButton = hoistCmp.factory({ const saveButton = hoistCmp.factory({ render({model, mode, ...rest}) { if (hideStateButton(model, mode)) return null; - const {parent, saveAsDialogModel} = model, + const {parent} = model, {typeDisplayName, isLoading, isValueDirty} = parent; return button({ className: 'xh-view-manager__save-button', @@ -126,9 +126,7 @@ const saveButton = hoistCmp.factory({ tooltip: `Save changes to this ${typeDisplayName}`, intent: 'primary', disabled: !isValueDirty || isLoading, - onClick: () => { - parent.isViewSavable ? parent.saveAsync() : saveAsDialogModel.open(); - }, + onClick: () => model.saveAsync(), ...rest }); } @@ -144,7 +142,7 @@ const revertButton = hoistCmp.factory({ tooltip: `Revert changes to this ${typeDisplayName}`, intent: 'danger', disabled: !isValueDirty || isLoading, - onClick: () => model.parent.resetAsync(), + onClick: () => model.revertAsync(), ...rest }); } diff --git a/desktop/cmp/viewmanager/ViewManagerLocalModel.ts b/desktop/cmp/viewmanager/ViewManagerLocalModel.ts index 1917c8922..d111608f7 100644 --- a/desktop/cmp/viewmanager/ViewManagerLocalModel.ts +++ b/desktop/cmp/viewmanager/ViewManagerLocalModel.ts @@ -23,6 +23,24 @@ export class ViewManagerLocalModel extends HoistModel { @bindable isVisible = true; + async saveAsync() { + const {parent} = this, + {view} = parent; + + if (!parent.isViewSavable) { + this.saveAsDialogModel.open(); + return; + } + + return parent.saveAsync().catchDefault({ + message: `Failed to save ${view.typedName}. If this persists consider \`Save As...\`.` + }); + } + + async revertAsync() { + return this.parent.resetAsync().catchDefault(); + } + constructor(parent: ViewManagerModel) { super(); makeObservable(this); diff --git a/desktop/cmp/viewmanager/ViewMenu.ts b/desktop/cmp/viewmanager/ViewMenu.ts index 3156c2962..fd1b2c8c5 100644 --- a/desktop/cmp/viewmanager/ViewMenu.ts +++ b/desktop/cmp/viewmanager/ViewMenu.ts @@ -63,7 +63,7 @@ function getNavMenuItems(model: ViewManagerModel): ReactNode[] { className: 'xh-view-manager__menu-item', icon: view.isDefault ? Icon.check() : Icon.placeholder(), text: `Default ${startCase(typeDisplayName)}`, - onClick: () => model.selectViewAsync(null) + onClick: () => model.selectViewAsync(null).catchDefault() }) ); } @@ -89,7 +89,7 @@ function getOtherMenuItems(model: ViewManagerLocalModel): ReactNode[] { icon: Icon.save(), text: 'Save', disabled: !isViewSavable || !isValueDirty, - onClick: () => parent.saveAsync() + onClick: () => model.saveAsync() }), menuItem({ icon: Icon.placeholder(), @@ -100,7 +100,7 @@ function getOtherMenuItems(model: ViewManagerLocalModel): ReactNode[] { icon: Icon.reset(), text: `Revert`, disabled: !isValueDirty, - onClick: () => parent.resetAsync() + onClick: () => model.revertAsync() }), menuDivider({omit: !enableAutoSave}), menuItem({ @@ -168,6 +168,6 @@ function viewMenuItem(view: ViewInfo, model: ViewManagerModel): ReactNode { text: view.name, title: title.join(' | '), icon, - onClick: () => model.selectViewAsync(view) + onClick: () => model.selectViewAsync(view).catchDefault() }); } diff --git a/desktop/cmp/viewmanager/dialog/ManageDialogModel.ts b/desktop/cmp/viewmanager/dialog/ManageDialogModel.ts index 52ba96ce2..8787c04e5 100644 --- a/desktop/cmp/viewmanager/dialog/ManageDialogModel.ts +++ b/desktop/cmp/viewmanager/dialog/ManageDialogModel.ts @@ -86,7 +86,7 @@ export class ManageDialogModel extends HoistModel { } activateSelectedViewAndClose() { - this.viewManagerModel.selectViewAsync(this.selectedView); + this.viewManagerModel.selectViewAsync(this.selectedView).catchDefault(); this.close(); }