Skip to content

Commit

Permalink
Pass token, not viewInfo and surface raw errors to make API more flex…
Browse files Browse the repository at this point in the history
…ible. (#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
  • Loading branch information
cnrudd authored Jan 7, 2025
1 parent 67b3abd commit e9fd103
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 40 deletions.
8 changes: 4 additions & 4 deletions cmp/viewmanager/DataAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ export class DataAccess<T> {
}

/** Fetch the latest version of a view. */
async fetchViewAsync(info: ViewInfo): Promise<View<T>> {
async fetchViewAsync(token: string): Promise<View<T>> {
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});
}
}

Expand Down
3 changes: 3 additions & 0 deletions cmp/viewmanager/ViewInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down
53 changes: 27 additions & 26 deletions cmp/viewmanager/ViewManagerModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -337,7 +337,7 @@ export class ViewManagerModel<T = PlainObject> 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) {
Expand All @@ -346,7 +346,12 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
}
}

async selectViewAsync(info: ViewInfo, opts = {alertUnsavedChanges: true}): Promise<void> {
async selectViewAsync(
view: string | ViewInfo,
opts = {alertUnsavedChanges: true}
): Promise<void> {
const token = isObject(view) ? view.token : view;

// ensure any pending auto-save gets completed
if (this.isValueDirty && this.isViewAutoSavable) {
await this.maybeAutoSaveAsync();
Expand All @@ -362,7 +367,7 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
return;
}

await this.loadViewAsync(info).catch(e => this.handleException(e));
return this.loadViewAsync(token);
}

async saveAsAsync(spec: ViewCreateSpec): Promise<void> {
Expand All @@ -380,26 +385,22 @@ export class ViewManagerModel<T = PlainObject> 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<void> {
await this.loadViewAsync(this.view.info).catch(e => this.handleException(e));
return this.loadViewAsync(this.view.token);
}

//--------------------------------
Expand Down Expand Up @@ -483,7 +484,7 @@ export class ViewManagerModel<T = PlainObject> 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;
Expand All @@ -508,8 +509,8 @@ export class ViewManagerModel<T = PlainObject> 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)) {
Expand All @@ -518,7 +519,7 @@ export class ViewManagerModel<T = PlainObject> 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);
Expand Down Expand Up @@ -569,13 +570,13 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {
}

private async loadViewAsync(
info: ViewInfo,
token: string,
pendingValue: PendingValue<T> = null
): Promise<void> {
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();
})
Expand Down Expand Up @@ -649,7 +650,7 @@ export class ViewManagerModel<T = PlainObject> extends HoistModel {

private async maybeConfirmSaveAsync(view: View, pendingValue: PendingValue<T>) {
// 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;
Expand Down
8 changes: 3 additions & 5 deletions desktop/cmp/viewmanager/ViewManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,15 @@ const menuButton = hoistCmp.factory<ViewManagerLocalModel>({
const saveButton = hoistCmp.factory<ViewManagerLocalModel>({
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',
icon: Icon.save(),
tooltip: `Save changes to this ${typeDisplayName}`,
intent: 'primary',
disabled: !isValueDirty || isLoading,
onClick: () => {
parent.isViewSavable ? parent.saveAsync() : saveAsDialogModel.open();
},
onClick: () => model.saveAsync(),
...rest
});
}
Expand All @@ -144,7 +142,7 @@ const revertButton = hoistCmp.factory<ViewManagerLocalModel>({
tooltip: `Revert changes to this ${typeDisplayName}`,
intent: 'danger',
disabled: !isValueDirty || isLoading,
onClick: () => model.parent.resetAsync(),
onClick: () => model.revertAsync(),
...rest
});
}
Expand Down
18 changes: 18 additions & 0 deletions desktop/cmp/viewmanager/ViewManagerLocalModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions desktop/cmp/viewmanager/ViewMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
);
}
Expand All @@ -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(),
Expand All @@ -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({
Expand Down Expand Up @@ -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()
});
}
2 changes: 1 addition & 1 deletion desktop/cmp/viewmanager/dialog/ManageDialogModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class ManageDialogModel extends HoistModel {
}

activateSelectedViewAndClose() {
this.viewManagerModel.selectViewAsync(this.selectedView);
this.viewManagerModel.selectViewAsync(this.selectedView).catchDefault();
this.close();
}

Expand Down

0 comments on commit e9fd103

Please sign in to comment.