From d894b60fd4cd1dcdac82e11f592105ab05dfd368 Mon Sep 17 00:00:00 2001 From: Jakub Serafin Date: Tue, 18 Jul 2023 09:24:10 +0200 Subject: [PATCH] (core) deleting queue from single webhook Summary: Using standard tost notification, message about webhook queue being overflown was added. message is permanent as long as queue is full. Message contains linkt to the webhook setings Test Plan: two nbrowser test was added - one to check if message is show when queue is full, and second to check if message is dismiss when queue was cleaned. Reviewers: georgegevoian Reviewed By: georgegevoian Subscribers: jarek Differential Revision: https://phab.getgrist.com/D3929 --- app/client/components/GristDoc.ts | 246 ++++++++++++++++++-------- app/client/models/DocModel.ts | 2 +- app/client/models/DocPageModel.ts | 20 +-- app/client/models/NotifyModel.ts | 25 ++- app/client/ui/WebhookPage.ts | 117 +++++++----- app/common/CommTypes.ts | 8 +- app/common/UserAPI.ts | 13 +- app/gen-server/lib/DocApiForwarder.ts | 2 +- app/server/lib/ActiveDoc.ts | 18 +- app/server/lib/DocApi.ts | 10 ++ app/server/lib/Triggers.ts | 35 +++- buildtools/fly-template.toml | 1 + test/nbrowser/WebhookOverflow.ts | 117 ++++++++++++ test/nbrowser/WebhookPage.ts | 12 +- 14 files changed, 475 insertions(+), 151 deletions(-) create mode 100644 test/nbrowser/WebhookOverflow.ts diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 6ef365bd..c92927a8 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -18,7 +18,6 @@ import {EditorMonitor} from "app/client/components/EditorMonitor"; import * as GridView from 'app/client/components/GridView'; import {Importer} from 'app/client/components/Importer'; import {RawDataPage, RawDataPopup} from 'app/client/components/RawDataPage'; -import {DocSettingsPage} from 'app/client/ui/DocumentSettings'; import {ActionGroupWithCursorPos, UndoStack} from 'app/client/components/UndoStack'; import {ViewLayout} from 'app/client/components/ViewLayout'; import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals'; @@ -43,6 +42,7 @@ import {App} from 'app/client/ui/App'; import {DocHistory} from 'app/client/ui/DocHistory'; import {startDocTour} from "app/client/ui/DocTour"; import {DocTutorial} from 'app/client/ui/DocTutorial'; +import {DocSettingsPage} from 'app/client/ui/DocumentSettings'; import {isTourActive} from "app/client/ui/OnBoardingPopups"; import {IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker'; import {linkFromId, selectBy} from 'app/client/ui/selectBy'; @@ -54,8 +54,8 @@ import {isNarrowScreen, mediaSmall, mediaXSmall, testId, theme} from 'app/client import {IconName} from 'app/client/ui2018/IconList'; import {icon} from 'app/client/ui2018/icons'; import {invokePrompt} from 'app/client/ui2018/modals'; -import {FieldEditor} from "app/client/widgets/FieldEditor"; import {DiscussionPanel} from 'app/client/widgets/DiscussionEditor'; +import {FieldEditor} from "app/client/widgets/FieldEditor"; import {MinimalActionGroup} from 'app/common/ActionGroup'; import {ClientQuery} from "app/common/ActiveDocAPI"; import {CommDocChatter, CommDocUsage, CommDocUserAction} from 'app/common/CommTypes'; @@ -120,8 +120,9 @@ const RightPanelTool = StringUnion("none", "docHistory", "validations", "discuss export interface IExtraTool { icon: IconName; label: DomContents; - content: TabContent[]|IDomComponent; + content: TabContent[] | IDomComponent; } + interface RawSectionOptions { viewSection: ViewSectionRec; hash: HashLink; @@ -137,10 +138,10 @@ export class GristDoc extends DisposableWithEvents { public docInfo: DocInfoRec; public docPluginManager: DocPluginManager; public querySetManager: QuerySetManager; - public rightPanelTool: Observable; + public rightPanelTool: Observable; public isReadonly = this.docPageModel.isReadonly; public isReadonlyKo = toKo(ko, this.isReadonly); - public comparison: DocStateComparison|null; + public comparison: DocStateComparison | null; // component for keeping track of latest cursor position public cursorMonitor: CursorMonitor; // component for keeping track of a cell that is being edited @@ -174,11 +175,11 @@ export class GristDoc extends DisposableWithEvents { // section (or raw data section) that is not part of this view. Maximized section is a section // in the view, so there is no need to render it twice, layout just hides all other sections to make // the space. - public maximizedSectionId: Observable = Observable.create(this, null); + public maximizedSectionId: Observable = Observable.create(this, null); // This is id of the section that is currently shown in the popup. Probably this is an external // section, like raw data view, or a section from another view.. - public externalSectionId: Computed; - public viewLayout: ViewLayout|null = null; + public externalSectionId: Computed; + public viewLayout: ViewLayout | null = null; // Holder for the popped up formula editor. public readonly formulaPopup = Holder.create(this); @@ -191,15 +192,15 @@ export class GristDoc extends DisposableWithEvents { private _actionLog: ActionLog; private _undoStack: UndoStack; - private _lastOwnActionGroup: ActionGroupWithCursorPos|null = null; + private _lastOwnActionGroup: ActionGroupWithCursorPos | null = null; private _rightPanelTabs = new Map(); private _docHistory: DocHistory; private _discussionPanel: DiscussionPanel; private _rightPanelTool = createSessionObs(this, "rightPanelTool", "none", RightPanelTool.guard); private _showGristTour = getUserOrgPrefObs(this.userOrgPrefs, 'showGristTour'); private _seenDocTours = getUserOrgPrefObs(this.userOrgPrefs, 'seenDocTours'); - private _rawSectionOptions: Observable = Observable.create(this, null); - private _activeContent: Computed; + private _rawSectionOptions: Observable = Observable.create(this, null); + private _activeContent: Computed; private _docTutorialHolder = Holder.create(this); private _isRickRowing: Observable = Observable.create(this, false); private _showBackgroundVideoPlayer: Observable = Observable.create(this, false); @@ -285,9 +286,13 @@ export class GristDoc extends DisposableWithEvents { } })); + // Subscribe to URL state, and navigate to anchor or open a popup if necessary. this.autoDispose(subscribe(urlState().state, async (use, state) => { - if (!state.hash) { return; } + if (!state.hash) { + return; + } + try { if (state.hash.popup) { @@ -321,7 +326,9 @@ export class GristDoc extends DisposableWithEvents { this._waitForView() .then(() => { const cursor = document.querySelector('.selected_cursor.active_cursor'); - if (!cursor) { return; } + if (!cursor) { + return; + } this.behavioralPromptsManager.showTip(cursor, 'rickRow', { forceShow: true, @@ -427,8 +434,8 @@ export class GristDoc extends DisposableWithEvents { // Set the available import sources in the DocPageModel. this.docPageModel.importSources = importMenuItems; - this._actionLog = this.autoDispose(ActionLog.create({ gristDoc: this })); - this._undoStack = this.autoDispose(UndoStack.create(openDocResponse.log, { gristDoc: this })); + this._actionLog = this.autoDispose(ActionLog.create({gristDoc: this})); + this._undoStack = this.autoDispose(UndoStack.create(openDocResponse.log, {gristDoc: this})); this._docHistory = DocHistory.create(this, this.docPageModel, this._actionLog); this._discussionPanel = DiscussionPanel.create(this, this); @@ -439,9 +446,15 @@ export class GristDoc extends DisposableWithEvents { /* Command binding */ this.autoDispose(commands.createGroup({ - undo() { this._undoStack.sendUndoAction().catch(reportError); }, - redo() { this._undoStack.sendRedoAction().catch(reportError); }, - reloadPlugins() { void this.docComm.reloadPlugins().then(() => G.window.location.reload(false)); }, + undo() { + this._undoStack.sendUndoAction().catch(reportError); + }, + redo() { + this._undoStack.sendRedoAction().catch(reportError); + }, + reloadPlugins() { + void this.docComm.reloadPlugins().then(() => G.window.location.reload(false)); + }, // Command to be manually triggered on cell selection. Moves the cursor to the selected cell. // This is overridden by the formula editor to insert "$col" variables when clicking cells. @@ -454,6 +467,8 @@ export class GristDoc extends DisposableWithEvents { this.listenTo(app.comm, 'docChatter', this.onDocChatter); + this._handleTriggerQueueOverflowMessage(); + this.autoDispose(DocConfigTab.create({gristDoc: this})); this.rightPanelTool = Computed.create(this, (use) => this._getToolContent(use(this._rightPanelTool))); @@ -478,7 +493,9 @@ export class GristDoc extends DisposableWithEvents { // switch for a moment to knockout to fix this. const viewInstance = fromKo(this.autoDispose(ko.pureComputed(() => { const viewId = toKo(ko, this.activeViewId)(); - if (!isViewDocPage(viewId)) { return null; } + if (!isViewDocPage(viewId)) { + return null; + } const section = this.viewModel.activeSection(); if (section?.isDisposed()) { return null; } const view = section.viewInstance(); @@ -490,7 +507,9 @@ export class GristDoc extends DisposableWithEvents { if (view) { await view.getLoadingDonePromise(); } - if (view?.isDisposed()) { return; } + if (view?.isDisposed()) { + return; + } // finally set the current view as fully loaded this.currentView.set(view); })); @@ -499,12 +518,18 @@ export class GristDoc extends DisposableWithEvents { this.cursorPosition = Computed.create(this, use => { // get the BaseView const view = use(this.currentView); - if (!view) { return undefined; } + if (!view) { + return undefined; + } const viewId = use(this.activeViewId); - if (!isViewDocPage(viewId)) { return undefined; } + if (!isViewDocPage(viewId)) { + return undefined; + } // read latest position const currentPosition = use(view.cursor.currentPosition); - if (currentPosition) { return { ...currentPosition, viewId }; } + if (currentPosition) { + return {...currentPosition, viewId}; + } return undefined; }); @@ -520,7 +545,9 @@ export class GristDoc extends DisposableWithEvents { // When active section is changed to a chart or custom widget, change the tab in the creator // panel to the table. this.autoDispose(this.viewModel.activeSection.subscribe((section) => { - if (section.isDisposed() || section._isDeleted.peek()) { return; } + if (section.isDisposed() || section._isDeleted.peek()) { + return; + } if (['chart', 'custom'].includes(section.parentKey.peek())) { commands.allCommands.viewTabFocus.run(); } @@ -609,12 +636,12 @@ export class GristDoc extends DisposableWithEvents { * fields { sectionId, rowId, fieldIndex }. Fields may be missing if no section is active. */ public getCursorPos(): CursorPos { - const pos = { sectionId: this.viewModel.activeSectionId() }; + const pos = {sectionId: this.viewModel.activeSectionId()}; const viewInstance = this.viewModel.activeSection.peek().viewInstance.peek(); return Object.assign(pos, viewInstance ? viewInstance.cursor.getCursorPos() : {}); } - public async onSetCursorPos(rowModel: BaseRowModel|undefined, fieldModel?: ViewFieldRec) { + public async onSetCursorPos(rowModel: BaseRowModel | undefined, fieldModel?: ViewFieldRec) { return this.setCursorPos({ rowIndex: rowModel?._index() || 0, fieldIndex: fieldModel?._index() || 0, @@ -669,7 +696,7 @@ export class GristDoc extends DisposableWithEvents { } try { await this.setCursorPos(cursorPos); - } catch(e) { + } catch (e) { reportError(e); } } @@ -732,7 +759,9 @@ export class GristDoc extends DisposableWithEvents { * observables. */ public onDocUsageMessage(message: CommDocUsage) { - if (!this.docComm.isActionFromThisDoc(message)) { return; } + if (!this.docComm.isActionFromThisDoc(message)) { + return; + } bundleChanges(() => { this.docPageModel.updateCurrentDocUsage(message.data.docUsage); @@ -741,8 +770,15 @@ export class GristDoc extends DisposableWithEvents { } public onDocChatter(message: CommDocChatter) { - if (!this.docComm.isActionFromThisDoc(message)) { return; } - if (message.data.webhooks) { + if (!this.docComm.isActionFromThisDoc(message) || + !message.data.webhooks) { + return; + } + if (message.data.webhooks.type == 'webhookOverflowError') { + this.trigger('webhookOverflowError', + t('New changes are temporarily suspended. Webhooks queue overflowed.' + + ' Please check webhooks settings, remove invalid webhooks, and clean the queue.'),); + } else { this.trigger('webhooks', message.data.webhooks); } } @@ -755,7 +791,9 @@ export class GristDoc extends DisposableWithEvents { // in effect. public getTableModelMaybeWithDiff(tableId: string): DataTableModel { const tableModel = this.getTableModel(tableId); - if (!this.comparison?.details) { return tableModel; } + if (!this.comparison?.details) { + return tableModel; + } // TODO: cache wrapped models and share between views. return new DataTableModelWithDiff(tableModel, this.comparison.details); } @@ -765,7 +803,9 @@ export class GristDoc extends DisposableWithEvents { */ public async addEmptyTable(): Promise { const name = await this._promptForName(); - if (name === undefined) { return; } + if (name === undefined) { + return; + } const tableInfo = await this.docData.sendAction(['AddEmptyTable', name || null]); await this.openDocPage(this.docModel.tables.getRowModel(tableInfo.id).primaryViewId()); } @@ -776,7 +816,7 @@ export class GristDoc extends DisposableWithEvents { public async addWidgetToPage(val: IPageWidget) { const docData = this.docModel.docData; const viewName = this.viewModel.name.peek(); - let tableId: string|null|undefined; + let tableId: string | null | undefined; if (val.table === 'New Table') { tableId = await this._promptForName(); if (tableId === undefined) { @@ -797,7 +837,7 @@ export class GristDoc extends DisposableWithEvents { /** * The actual implementation of addWidgetToPage */ - public async addWidgetToPageImpl(val: IPageWidget, tableId: string|null = null) { + public async addWidgetToPageImpl(val: IPageWidget, tableId: string | null = null) { const viewRef = this.activeViewId.get(); const tableRef = val.table === 'New Table' ? 0 : val.table; const result = await this.docData.sendAction( @@ -816,7 +856,9 @@ export class GristDoc extends DisposableWithEvents { public async addNewPage(val: IPageWidget) { if (val.table === 'New Table') { const name = await this._promptForName(); - if (name === undefined) { return; } + if (name === undefined) { + return; + } const result = await this.docData.sendAction(['AddEmptyTable', name]); await this.openDocPage(result.views[0].id); } else { @@ -842,8 +884,10 @@ export class GristDoc extends DisposableWithEvents { * primary view. */ public async uploadNewTable(): Promise { - const uploadResult = await selectFiles({docWorkerUrl: this.docComm.docWorkerUrl, - multiple: true}); + const uploadResult = await selectFiles({ + docWorkerUrl: this.docComm.docWorkerUrl, + multiple: true + }); if (uploadResult) { const dataSource = {uploadId: uploadResult.uploadId, transforms: []}; const importResult = await this.docComm.finishImportFiles(dataSource, [], {}); @@ -865,7 +909,7 @@ export class GristDoc extends DisposableWithEvents { } return await this.viewLayout!.freezeUntil(docData.bundleActions( - t("Saved linked section {{title}} in view {{name}}", {title:section.title(), name: viewModel.name()}), + t("Saved linked section {{title}} in view {{name}}", {title: section.title(), name: viewModel.name()}), async () => { // if table changes or a table is made a summary table, let's replace the view section by a @@ -932,7 +976,9 @@ export class GristDoc extends DisposableWithEvents { // adds new view fields; ignore colIds that do not exist in new table. await Promise.all(colIds.map((colId, i) => { - if (!mapColIdToColumn.has(colId)) { return; } + if (!mapColIdToColumn.has(colId)) { + return; + } const colInfo = { parentId: section.id(), colRef: mapColIdToColumn.get(colId).id(), @@ -983,7 +1029,7 @@ export class GristDoc extends DisposableWithEvents { } // Turn the given columns into empty columns, losing any data stored in them. - public async clearColumns(colRefs: number[], {keepType}: {keepType?: boolean} = {}): Promise { + public async clearColumns(colRefs: number[], {keepType}: { keepType?: boolean } = {}): Promise { await this.docModel.columns.sendTableAction( ['BulkUpdateRecord', colRefs, { isFormula: colRefs.map(f => true), @@ -1003,7 +1049,7 @@ export class GristDoc extends DisposableWithEvents { } // Convert the given columns to data, saving the calculated values and unsetting the formulas. - public async convertIsFormula(colRefs: number[], opts: {toFormula: boolean, noRecalc?: boolean}): Promise { + public async convertIsFormula(colRefs: number[], opts: { toFormula: boolean, noRecalc?: boolean }): Promise { return this.docModel.columns.sendTableAction( ['BulkUpdateRecord', colRefs, { isFormula: colRefs.map(f => opts.toFormula), @@ -1076,8 +1122,12 @@ export class GristDoc extends DisposableWithEvents { setAsActiveSection: boolean, silent: boolean = false): Promise { try { - if (!cursorPos.sectionId) { throw new Error('sectionId required'); } - if (!cursorPos.rowId) { throw new Error('rowId required'); } + if (!cursorPos.sectionId) { + throw new Error('sectionId required'); + } + if (!cursorPos.rowId) { + throw new Error('rowId required'); + } const section = this.docModel.viewSections.getRowModel(cursorPos.sectionId); if (!section.id.peek()) { throw new Error(`Section ${cursorPos.sectionId} does not exist`); @@ -1125,7 +1175,9 @@ export class GristDoc extends DisposableWithEvents { } srcRowId = srcTable.getRowIds().find(getFilterFunc(this.docData, query)); } - if (!srcRowId || typeof srcRowId !== 'number') { throw new Error('cannot trace rowId'); } + if (!srcRowId || typeof srcRowId !== 'number') { + throw new Error('cannot trace rowId'); + } await this.recursiveMoveToCursorPos({ rowId: srcRowId, sectionId: srcSection.id.peek(), @@ -1133,14 +1185,20 @@ export class GristDoc extends DisposableWithEvents { } const view: ViewRec = section.view.peek(); const docPage: ViewDocPage = section.isRaw.peek() ? "data" : view.getRowId(); - if (docPage != this.activeViewId.get()) { await this.openDocPage(docPage); } - if (setAsActiveSection) { view.activeSectionId(cursorPos.sectionId); } + if (docPage != this.activeViewId.get()) { + await this.openDocPage(docPage); + } + if (setAsActiveSection) { + view.activeSectionId(cursorPos.sectionId); + } const fieldIndex = cursorPos.fieldIndex; const viewInstance = await waitObs(section.viewInstance); - if (!viewInstance) { throw new Error('view not found'); } + if (!viewInstance) { + throw new Error('view not found'); + } // Give any synchronous initial cursor setting a chance to happen. await delay(0); - viewInstance.setCursorPos({ ...cursorPos, fieldIndex }); + viewInstance.setCursorPos({...cursorPos, fieldIndex}); // TODO: column selection not working on card/detail view, or getting overridden - // look into it (not a high priority for now since feature not easily discoverable // in this view). @@ -1162,7 +1220,7 @@ export class GristDoc extends DisposableWithEvents { * Opens up an editor at cursor position * @param input Optional. Cell's initial value */ - public async activateEditorAtCursor(options?: { init?: string, state?: any}) { + public async activateEditorAtCursor(options?: { init?: string, state?: any }) { const view = await this._waitForView(); view?.activateEditorAtCursor(options); } @@ -1183,7 +1241,9 @@ export class GristDoc extends DisposableWithEvents { */ public async openPopup(hash: HashLink) { // We can only open a popup for a section. - if (!hash.sectionId) { return; } + if (!hash.sectionId) { + return; + } // We might open popup either for a section in this view or some other section (like Raw Data Page). if (this.viewModel.viewSections.peek().peek().some(s => s.id.peek() === hash.sectionId)) { if (this.viewLayout) { @@ -1196,7 +1256,7 @@ export class GristDoc extends DisposableWithEvents { const fieldIndex = activeSection.viewFields.peek().all().findIndex(f => f.colRef.peek() === hash.colRef); if (fieldIndex >= 0) { const view = await this._waitForView(activeSection); - view?.setCursorPos({ sectionId: hash.sectionId, rowId: hash.rowId, fieldIndex }); + view?.setCursorPos({sectionId: hash.sectionId, rowId: hash.rowId, fieldIndex}); } } this.viewLayout?.maximized.set(hash.sectionId); @@ -1219,17 +1279,23 @@ export class GristDoc extends DisposableWithEvents { viewSection: popupSection, close: () => { // In case we are already close, do nothing. - if (!this._rawSectionOptions.get()) { return; } + if (!this._rawSectionOptions.get()) { + return; + } if (popupSection !== prevSection) { // We need to blur raw view section. Otherwise it will automatically be opened // on raw data view. Note: raw data section doesn't have its own view, it uses // empty row model as a parent (which feels like a hack). - if (!popupSection.isDisposed()) { popupSection.hasFocus(false); } + if (!popupSection.isDisposed()) { + popupSection.hasFocus(false); + } // We need to restore active viewSection for a view that we borrowed. // When this popup was opened we tricked active view by setting its activeViewSection // to our viewSection (which might be a completely diffrent section or a raw data section) not // connected to this view. - if (!prevSection.isDisposed()) { prevSection.hasFocus(true); } + if (!prevSection.isDisposed()) { + prevSection.hasFocus(true); + } } // Clearing popup data will close this popup. this._rawSectionOptions.set(null); @@ -1240,7 +1306,7 @@ export class GristDoc extends DisposableWithEvents { const fieldIndex = popupSection.viewFields.peek().all().findIndex(f => f.colRef.peek() === hash.colRef); if (fieldIndex >= 0) { const view = await this._waitForView(popupSection); - view?.setCursorPos({ sectionId: hash.sectionId, rowId: hash.rowId, fieldIndex }); + view?.setCursorPos({sectionId: hash.sectionId, rowId: hash.rowId, fieldIndex}); } } } @@ -1250,7 +1316,9 @@ export class GristDoc extends DisposableWithEvents { */ public async playRickRollVideo() { const backgroundVideoPlayer = this._backgroundVideoPlayerHolder.get(); - if (!backgroundVideoPlayer) { return; } + if (!backgroundVideoPlayer) { + return; + } await backgroundVideoPlayer.isLoaded(); backgroundVideoPlayer.play(); @@ -1272,7 +1340,9 @@ export class GristDoc extends DisposableWithEvents { await setVolume(0, 100, 5); await delay(190 * 1000); - if (!this._isRickRowing.get()) { return; } + if (!this._isRickRowing.get()) { + return; + } await setVolume(100, 0, 5); @@ -1289,6 +1359,7 @@ export class GristDoc extends DisposableWithEvents { if (!sectionToCheck.getRowId()) { return null; } + async function singleWait(s: ViewSectionRec): Promise { const view = await waitObs( sectionToCheck.viewInstance, @@ -1296,6 +1367,7 @@ export class GristDoc extends DisposableWithEvents { ); return view!; } + let view = await singleWait(sectionToCheck); if (view.isDisposed()) { // If the view is disposed (it can happen, as wait is not reliable enough, because it uses @@ -1324,7 +1396,7 @@ export class GristDoc extends DisposableWithEvents { return content ? {icon: 'Validation', label: 'Validation Rules', content} : null; } case 'discussion': { - return {icon: 'Chat', label: this._discussionPanel.buildMenu(), content: this._discussionPanel}; + return {icon: 'Chat', label: this._discussionPanel.buildMenu(), content: this._discussionPanel}; } case 'none': default: { @@ -1350,7 +1422,9 @@ export class GristDoc extends DisposableWithEvents { await commands.allCommands.rightPanelOpen.run(); const editLayoutButton = document.querySelector('.behavioral-prompt-edit-card-layout'); - if (!editLayoutButton) { throw new Error('GristDoc failed to find edit card layout button'); } + if (!editLayoutButton) { + throw new Error('GristDoc failed to find edit card layout button'); + } this.behavioralPromptsManager.showTip(editLayoutButton, 'editCardLayout', { popupOptions: { @@ -1424,7 +1498,7 @@ export class GristDoc extends DisposableWithEvents { * Helper called before an action is sent to the server. It saves cursor position to come back to * in case of Undo. */ - private _onSendActionsStart(ev: {cursorPos: CursorPos}) { + private _onSendActionsStart(ev: { cursorPos: CursorPos }) { this._lastOwnActionGroup = null; ev.cursorPos = this.getCursorPos(); } @@ -1433,7 +1507,7 @@ export class GristDoc extends DisposableWithEvents { * Helper called when server responds to an action. It attaches the saved cursor position to the * received action (if any), and stores also the resulting position. */ - private _onSendActionsEnd(ev: {cursorPos: CursorPos}) { + private _onSendActionsEnd(ev: { cursorPos: CursorPos }) { const a = this._lastOwnActionGroup; if (a) { a.cursorPos = ev.cursorPos; @@ -1445,15 +1519,15 @@ export class GristDoc extends DisposableWithEvents { private _getDocApiDownloadParams() { const filters = this.viewModel.activeSection.peek().activeFilters.get().map(filterInfo => ({ - colRef : filterInfo.fieldOrColumn.origCol().origColRef(), - filter : filterInfo.filter() + colRef: filterInfo.fieldOrColumn.origCol().origColRef(), + filter: filterInfo.filter() })); const params = { viewSection: this.viewModel.activeSectionId(), tableId: this.viewModel.activeSection().table().tableId(), activeSortSpec: JSON.stringify(this.viewModel.activeSection().activeSortSpec()), - filters : JSON.stringify(filters), + filters: JSON.stringify(filters), }; return params; } @@ -1485,10 +1559,14 @@ export class GristDoc extends DisposableWithEvents { private async _getTableData(section: ViewSectionRec): Promise { const viewInstance = await waitObs(section.viewInstance); - if (!viewInstance) { throw new Error('view not found'); } + if (!viewInstance) { + throw new Error('view not found'); + } await viewInstance.getLoadingDonePromise(); const table = this.docData.getTable(section.table.peek().tableId.peek()); - if (!table) { throw new Error('no section table'); } + if (!table) { + throw new Error('no section table'); + } return table; } @@ -1496,13 +1574,15 @@ export class GristDoc extends DisposableWithEvents { * Convert a url hash to a cursor position. */ private _getCursorPosFromHash(hash: HashLink): CursorPos { - const cursorPos: CursorPos = { rowId: hash.rowId, sectionId: hash.sectionId }; - if (cursorPos.sectionId != undefined && hash.colRef !== undefined){ + const cursorPos: CursorPos = {rowId: hash.rowId, sectionId: hash.sectionId}; + if (cursorPos.sectionId != undefined && hash.colRef !== undefined) { // translate colRef to a fieldIndex const section = this.docModel.viewSections.getRowModel(cursorPos.sectionId); const fieldIndex = section.viewFields.peek().all() - .findIndex(x=> x.colRef.peek() == hash.colRef); - if (fieldIndex >= 0) { cursorPos.fieldIndex = fieldIndex; } + .findIndex(x => x.colRef.peek() == hash.colRef); + if (fieldIndex >= 0) { + cursorPos.fieldIndex = fieldIndex; + } } return cursorPos; } @@ -1556,11 +1636,13 @@ export class GristDoc extends DisposableWithEvents { const viewFields = viewSection.viewFields.peek().peek(); // If no y-series, then simply return. - if (viewFields.length === 1) { return; } + if (viewFields.length === 1) { + return; + } const field = viewSection.viewFields.peek().peek()[1]; if (isNumericOnly(viewSection.chartTypeDef.peek()) && - !isNumericLike(field.column.peek())) { + !isNumericLike(field.column.peek())) { const actions: UserAction[] = []; // remove non-numeric field @@ -1580,10 +1662,28 @@ export class GristDoc extends DisposableWithEvents { await this.docModel.viewFields.sendTableActions(actions); } } + + private _handleTriggerQueueOverflowMessage() { + this.listenTo(this, 'webhookOverflowError', (err: any) => { + this.app.topAppModel.notifier.createNotification({ + message: err.toString(), + canUserClose: false, + level: "error", + badgeCounter: true, + expireSec: 5, + key: 'webhookOverflowError', + actions: [{ + label: t('go to webhook settings'), action: async () => { + await urlState().pushUrl({docPage: 'webhook'}); + } + }] + }); + }); + } } async function finalizeAnchor() { - await urlState().pushUrl({ hash: {} }, { replace: true }); + await urlState().pushUrl({hash: {}}, {replace: true}); setTestState({anchorApplied: true}); } diff --git a/app/client/models/DocModel.ts b/app/client/models/DocModel.ts index e3e30510..ee3bf8c5 100644 --- a/app/client/models/DocModel.ts +++ b/app/client/models/DocModel.ts @@ -49,7 +49,7 @@ import {toKo} from 'grainjs'; // Re-export all the entity types available. The recommended usage is like this: // import {ColumnRec, ViewFieldRec} from 'app/client/models/DocModel'; export type {ColumnRec, DocInfoRec, FilterRec, PageRec, TabBarRec, TableRec, ValidationRec, - ViewFieldRec, ViewRec, ViewSectionRec, CellRec}; + ViewFieldRec, ViewRec, ViewSectionRec, CellRec}; /** * Creates the type for a MetaRowModel containing a KoSaveableObservable for each field listed in diff --git a/app/client/models/DocPageModel.ts b/app/client/models/DocPageModel.ts index 3c711e2d..9a3474c3 100644 --- a/app/client/models/DocPageModel.ts +++ b/app/client/models/DocPageModel.ts @@ -117,20 +117,20 @@ export class DocPageModelImpl extends Disposable implements DocPageModel { public readonly currentWorkspace = Computed.create(this, this.currentDoc, (use, doc) => doc && doc.workspace); public readonly currentOrg = Computed.create(this, this.currentWorkspace, (use, ws) => ws && ws.org); public readonly currentOrgName = Computed.create(this, this.currentOrg, - (use, org) => getOrgNameOrGuest(org, this.appModel.currentUser)); + (use, org) => getOrgNameOrGuest(org, this.appModel.currentUser)); public readonly currentDocTitle = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.name : ''); public readonly isReadonly = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isReadonly : false); public readonly isPrefork = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isPreFork : false); public readonly isFork = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isFork : false); public readonly isRecoveryMode = Computed.create(this, this.currentDoc, - (use, doc) => doc ? doc.isRecoveryMode : false); + (use, doc) => doc ? doc.isRecoveryMode : false); public readonly userOverride = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.userOverride : null); public readonly isBareFork = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isBareFork : false); public readonly isSnapshot = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isSnapshot : false); public readonly isTutorialTrunk = Computed.create(this, this.currentDoc, - (use, doc) => doc ? doc.isTutorialTrunk : false); + (use, doc) => doc ? doc.isTutorialTrunk : false); public readonly isTutorialFork = Computed.create(this, this.currentDoc, - (use, doc) => doc ? doc.isTutorialFork : false); + (use, doc) => doc ? doc.isTutorialFork : false); public readonly importSources: ImportSource[] = []; @@ -169,7 +169,7 @@ export class DocPageModelImpl extends Disposable implements DocPageModel { FlowRunner.create(this._openerHolder, (flow: AsyncFlow) => this._openDoc(flow, urlId, urlOpenMode, state.params?.compare, linkParameters) ) - .resultPromise.catch(err => this._onOpenError(err)); + .resultPromise.catch(err => this._onOpenError(err)); } } })); @@ -266,7 +266,7 @@ export class DocPageModelImpl extends Disposable implements DocPageModel { explanation: ( isDocOwner ? t("You can try reloading the document, or using recovery mode. \ -Recovery mode opens the document to be fully accessible to owners, and inaccessible to others. \ + Recovery mode opens the document to be fully accessible to owners, and inaccessible to others. \ It also disables formulas. [{{error}}]", {error: err.message}) : isDenied ? t('Sorry, access to this document has been denied. [{{error}}]', {error: err.message}) @@ -301,7 +301,7 @@ It also disables formulas. [{{error}}]", {error: err.message}) comparisonUrlId: string | undefined, linkParameters: Record | undefined): Promise { console.log(`DocPageModel _openDoc starting for ${urlId} (mode ${urlOpenMode})` + - (comparisonUrlId ? ` (compare ${comparisonUrlId})` : '')); + (comparisonUrlId ? ` (compare ${comparisonUrlId})` : '')); const gristDocModulePromise = loadGristDoc(); const docResponse = await retryOnNetworkError(flow, getDoc.bind(null, this._api, urlId)); @@ -375,7 +375,7 @@ It also disables formulas. [{{error}}]", {error: err.message}) await this._api.getDocAPI(urlId).compareDoc(comparisonUrlId, { detail: true }) : undefined; const gristDoc = gdModule.GristDoc.create(flow, this._appObj, this.appModel, docComm, this, openDocResponse, - this.appModel.topAppModel.plugins, {comparison}); + this.appModel.topAppModel.plugins, {comparison}); // Move ownership of docComm to GristDoc. gristDoc.autoDispose(flow.release(docComm)); @@ -399,13 +399,13 @@ function addMenu(importSources: ImportSource[], gristDoc: GristDoc, isReadonly: return [ menuItem( (elem) => openPageWidgetPicker(elem, gristDoc, (val) => gristDoc.addNewPage(val).catch(reportError), - {isNewPage: true, buttonLabel: 'Add Page'}), + {isNewPage: true, buttonLabel: 'Add Page'}), menuIcon("Page"), t("Add Page"), testId('dp-add-new-page'), dom.cls('disabled', isReadonly) ), menuItem( (elem) => openPageWidgetPicker(elem, gristDoc, (val) => gristDoc.addWidgetToPage(val).catch(reportError), - {isNewPage: false, selectBy}), + {isNewPage: false, selectBy}), menuIcon("Widget"), t("Add Widget to Page"), testId('dp-add-widget-to-page'), // disable for readonly doc and all special views dom.cls('disabled', (use) => typeof use(gristDoc.activeViewId) !== 'number' || isReadonly), diff --git a/app/client/models/NotifyModel.ts b/app/client/models/NotifyModel.ts index 40bcfb66..c3af0b37 100644 --- a/app/client/models/NotifyModel.ts +++ b/app/client/models/NotifyModel.ts @@ -1,14 +1,25 @@ import * as log from 'app/client/lib/log'; import {ConnectState, ConnectStateManager} from 'app/client/models/ConnectState'; +import {isNarrowScreenObs, testId} from 'app/client/ui2018/cssVars'; import {delay} from 'app/common/delay'; import {isLongerThan} from 'app/common/gutil'; import {InactivityTimer} from 'app/common/InactivityTimer'; import {timeFormat} from 'app/common/timeFormat'; -import {bundleChanges, Disposable, Holder, IDisposable, IDisposableOwner } from 'grainjs'; -import {Computed, dom, DomElementArg, MutableObsArray, obsArray, Observable} from 'grainjs'; +import { + bundleChanges, + Computed, + Disposable, + dom, + DomElementArg, + Holder, + IDisposable, + IDisposableOwner, + MutableObsArray, + obsArray, + Observable +} from 'grainjs'; import clamp = require('lodash/clamp'); import defaults = require('lodash/defaults'); -import {isNarrowScreenObs, testId} from 'app/client/ui2018/cssVars'; // When rendering app errors, we'll only show the last few. const maxAppErrors = 5; @@ -79,9 +90,11 @@ export class Expirable extends Disposable { /** * Sets status to 'expiring', then calls dispose after a short delay. */ - public async expire(): Promise { + public async expire(withoutDelay: boolean = false): Promise { this.status.set('expiring'); - await delay(Expirable.fadeDelay); + if(!withoutDelay) { + await delay(Expirable.fadeDelay); + } if (!this.isDisposed()) { this.dispose(); } @@ -339,7 +352,7 @@ export class Notifier extends Disposable implements INotifier { if (key) { const prev = this._keyedItems.get(key); if (prev) { - await prev.expire(); + await prev.expire(true); } this._keyedItems.set(key, n); n.onDispose(() => this.isDisposed() || this._keyedItems.delete(key)); diff --git a/app/client/ui/WebhookPage.ts b/app/client/ui/WebhookPage.ts index 4cd00ba6..f8af4ee5 100644 --- a/app/client/ui/WebhookPage.ts +++ b/app/client/ui/WebhookPage.ts @@ -1,19 +1,26 @@ -import { GristDoc } from 'app/client/components/GristDoc'; -import { ViewSectionHelper } from 'app/client/components/ViewLayout'; -import { makeT } from 'app/client/lib/localization'; -import { reportMessage, reportSuccess } from 'app/client/models/errors'; -import { IEdit, IExternalTable, VirtualTable } from 'app/client/models/VirtualTable'; -import { docListHeader } from 'app/client/ui/DocMenuCss'; -import { bigPrimaryButton } from 'app/client/ui2018/buttons'; -import { mediaSmall, testId } from 'app/client/ui2018/cssVars'; -import { ApiError } from 'app/common/ApiError'; -import { DisposableWithEvents } from 'app/common/DisposableWithEvents'; -import { DocAction, getColIdsFromDocAction, getColValues, - isDataAction, TableDataAction, UserAction } from 'app/common/DocActions'; -import { WebhookSummary } from 'app/common/Triggers'; -import { DocAPI } from 'app/common/UserAPI'; -import { GristObjCode, RowRecord } from 'app/plugin/GristData'; -import { dom, styled } from 'grainjs'; +import {GristDoc} from 'app/client/components/GristDoc'; +import {ViewSectionHelper} from 'app/client/components/ViewLayout'; +import {makeT} from 'app/client/lib/localization'; +import {reportMessage, reportSuccess} from 'app/client/models/errors'; +import {IEdit, IExternalTable, VirtualTable} from 'app/client/models/VirtualTable'; +import {docListHeader} from 'app/client/ui/DocMenuCss'; +import {bigPrimaryButton} from 'app/client/ui2018/buttons'; +import {mediaSmall, testId} from 'app/client/ui2018/cssVars'; +import {ApiError} from 'app/common/ApiError'; +import {DisposableWithEvents} from 'app/common/DisposableWithEvents'; +import { + DocAction, + getColIdsFromDocAction, + getColValues, + isDataAction, + TableDataAction, + UserAction +} from 'app/common/DocActions'; +import {WebhookSummary} from 'app/common/Triggers'; +import {DocAPI} from 'app/common/UserAPI'; +import {GristObjCode, RowRecord} from 'app/plugin/GristData'; +import {dom, styled} from 'grainjs'; +import {observableArray, ObservableArray} from "knockout"; import omit = require('lodash/omit'); import pick = require('lodash/pick'); import range = require('lodash/range'); @@ -122,21 +129,26 @@ class WebhookExternalTable implements IExternalTable { public saveableFields = [ 'tableId', 'url', 'eventTypes', 'enabled', 'name', 'memo', 'isReadyColumn', ]; + public webhooks: ObservableArray = observableArray([]); - public constructor(private _docApi: DocAPI) {} + public constructor(private _docApi: DocAPI) { + } public async fetchAll(): Promise { const webhooks = await this._docApi.getWebhooks(); + this._initalizeWebhookList(webhooks); const indices = range(webhooks.length); return ['TableData', this.name, indices.map(i => i + 1), - getColValues(indices.map(rowId => _mapWebhookValues(webhooks[rowId])))]; + getColValues(indices.map(rowId => _mapWebhookValues(webhooks[rowId])))]; } public async beforeEdit(editor: IEdit) { const results = editor.actions; for (const r of results) { for (const d of r.stored) { - if (!isDataAction(d)) { continue; } + if (!isDataAction(d)) { + continue; + } const colIds = new Set(getColIdsFromDocAction(d) || []); if (colIds.has('webhookId') || colIds.has('status')) { throw new Error(`Sorry, not all fields can be edited.`); @@ -146,7 +158,9 @@ class WebhookExternalTable implements IExternalTable { const delta = editor.delta; for (const recId of delta.removeRows) { const rec = editor.getRecord(recId); - if (!rec) { continue; } + if (!rec) { + continue; + } await this._removeWebhook(rec); reportMessage(`Removed webhook.`); } @@ -159,13 +173,16 @@ class WebhookExternalTable implements IExternalTable { } } } + public async afterEdit(editor: IEdit) { const {delta} = editor; const updates = new Set(delta.updateRows); const addsAndUpdates = new Set([...delta.addRows, ...delta.updateRows]); for (const recId of addsAndUpdates) { const rec = editor.getRecord(recId); - if (!rec) { continue; } + if (!rec) { + continue; + } const notes: string[] = []; const values: Record = {}; if (!rec.webhookId) { @@ -192,6 +209,12 @@ class WebhookExternalTable implements IExternalTable { } } + private _initalizeWebhookList(webhooks: WebhookSummary[]){ + + this.webhooks.removeAll(); + this.webhooks.push(...webhooks); + } + public async sync(editor: IEdit): Promise { // Map from external webhookId to local arbitrary rowId. const rowMap = new Map(editor.getRowIds().map(rowId => [editor.getRecord(rowId)!.webhookId, rowId])); @@ -206,6 +229,7 @@ class WebhookExternalTable implements IExternalTable { // webhooks, or that "updating" something that hasn't actually // changed is not disruptive. const webhooks = await this._docApi.getWebhooks(); + this._initalizeWebhookList(webhooks); for (const webhook of webhooks) { const values = _mapWebhookValues(webhook); const rowId = rowMap.get(webhook.id); @@ -240,12 +264,12 @@ class WebhookExternalTable implements IExternalTable { const choices = editor.gristDoc.docModel.visibleTables.all().map(tableRec => tableRec.tableId()); editor.gristDoc.docData.receiveAction([ 'UpdateRecord', '_grist_Tables_column', 'vt_webhook_fc1' as any, { - widgetOptions: JSON.stringify({ - widget: 'TextBox', - alignment: 'left', - choices, - }) - }]); + widgetOptions: JSON.stringify({ + widget: 'TextBox', + alignment: 'left', + choices, + }) + }]); } private _getErrorString(e: ApiError): string { @@ -296,16 +320,22 @@ export class WebhookPage extends DisposableWithEvents { public docApi = this.gristDoc.docPageModel.appModel.api.getDocAPI(this.gristDoc.docId()); public sharedTable: VirtualTable; + private _webhookExternalTable: WebhookExternalTable; + constructor(public gristDoc: GristDoc) { super(); - - const table = new VirtualTable(this, gristDoc, new WebhookExternalTable(this.docApi)); + //this._webhooks = observableArray(); + this._webhookExternalTable = new WebhookExternalTable(this.docApi); + const table = new VirtualTable(this, gristDoc, this._webhookExternalTable); this.listenTo(gristDoc, 'webhooks', async () => { await table.lazySync(); + }); } + + public buildDom() { const viewSectionModel = this.gristDoc.docModel.viewSections.getRowModel('vt_webhook_fs1' as any); ViewSectionHelper.create(this, this.gristDoc, viewSectionModel); @@ -313,9 +343,9 @@ export class WebhookPage extends DisposableWithEvents { cssHeader(t('Webhook Settings')), cssControlRow( bigPrimaryButton(t("Clear Queue"), - dom.on('click', () => this.reset()), - testId('webhook-reset'), - ), + dom.on('click', () => this.reset()), + testId('webhook-reset'), + ) ), // active_section here is a bit of a hack, to allow tests to run // more easily. @@ -327,6 +357,11 @@ export class WebhookPage extends DisposableWithEvents { await this.docApi.flushWebhooks(); reportSuccess('Cleared webhook queue.'); } + + public async resetSelected(id: string) { + await this.docApi.flushWebhook(id); + reportSuccess(`Cleared webhook ${id} queue.`); + } } const cssHeader = styled(docListHeader, ` @@ -369,15 +404,15 @@ function _prepareWebhookInitialActions(tableId: string): DocAction[] { return [[ // Add the virtual table. 'AddTable', tableId, - WEBHOOK_COLUMNS.map(col => ({ - isFormula: true, - type: 'Any', - formula: '', - id: col.colId - })) + WEBHOOK_COLUMNS.map(col => ({ + isFormula: true, + type: 'Any', + formula: '', + id: col.colId + })) ], [ // Add an entry for the virtual table. - 'AddRecord', '_grist_Tables', 'vt_webhook_ft1' as any, { tableId, primaryViewId: 0 }, + 'AddRecord', '_grist_Tables', 'vt_webhook_ft1' as any, {tableId, primaryViewId: 0}, ], [ // Add entries for the columns of the virtual table. 'BulkAddRecord', '_grist_Tables_column', @@ -391,10 +426,10 @@ function _prepareWebhookInitialActions(tableId: string): DocAction[] { ], [ // Add a view section. 'AddRecord', '_grist_Views_section', 'vt_webhook_fs1' as any, - { tableRef: 'vt_webhook_ft1', parentKey: 'detail', title: '', borderWidth: 1, defaultWidth: 100, theme: 'blocks' } + {tableRef: 'vt_webhook_ft1', parentKey: 'detail', title: '', borderWidth: 1, defaultWidth: 100, theme: 'blocks'} ], [ // List the fields shown in the view section. - 'BulkAddRecord', '_grist_Views_section_field', WEBHOOK_VIEW_FIELDS.map((_, i) => `vt_webhook_ff${i+1}`) as any, { + 'BulkAddRecord', '_grist_Views_section_field', WEBHOOK_VIEW_FIELDS.map((_, i) => `vt_webhook_ff${i + 1}`) as any, { colRef: WEBHOOK_VIEW_FIELDS.map(colId => WEBHOOK_COLUMNS.find(r => r.colId === colId)!.id), parentId: WEBHOOK_VIEW_FIELDS.map(() => 'vt_webhook_fs1'), parentPos: WEBHOOK_VIEW_FIELDS.map((_, i) => i), diff --git a/app/common/CommTypes.ts b/app/common/CommTypes.ts index 32c6cd79..5a2b7244 100644 --- a/app/common/CommTypes.ts +++ b/app/common/CommTypes.ts @@ -2,8 +2,8 @@ import {ActionGroup} from 'app/common/ActionGroup'; import {DocAction} from 'app/common/DocActions'; import {FilteredDocUsageSummary} from 'app/common/DocUsage'; import {Product} from 'app/common/Features'; -import {StringUnion} from 'app/common/StringUnion'; import {UserProfile} from 'app/common/LoginSessionAPI'; +import {StringUnion} from 'app/common/StringUnion'; export const ValidEvent = StringUnion( 'docListAction', 'docUserAction', 'docShutdown', 'docError', @@ -90,11 +90,17 @@ export interface CommDocUserAction extends CommMessageBase { }; } + +export enum WebhookMessageType { + Update = 'webhookUpdate', + Overflow = 'webhookOverflowError' +} export interface CommDocChatter extends CommMessageBase { type: 'docChatter'; docFD: number; data: { webhooks?: { + type: WebhookMessageType, // If present, something happened related to webhooks. // Currently, we give no details, leaving it to client // to call back for details if it cares. diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 7c2304ac..0992fd85 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -4,18 +4,18 @@ import {AssistanceRequest, AssistanceResponse} from 'app/common/AssistancePrompt import {BaseAPI, IOptions} from 'app/common/BaseAPI'; import {BillingAPI, BillingAPIImpl} from 'app/common/BillingAPI'; import {BrowserSettings} from 'app/common/BrowserSettings'; +import {ICustomWidget} from 'app/common/CustomWidget'; import {BulkColValues, TableColValues, TableRecordValue, TableRecordValues, UserAction} from 'app/common/DocActions'; import {DocCreationInfo, OpenDocMode} from 'app/common/DocListAPI'; import {OrgUsageSummary} from 'app/common/DocUsage'; import {Product} from 'app/common/Features'; -import {ICustomWidget} from 'app/common/CustomWidget'; import {isClient} from 'app/common/gristUrls'; +import {encodeQueryParams} from 'app/common/gutil'; import {FullUser, UserProfile} from 'app/common/LoginSessionAPI'; import {OrgPrefs, UserOrgPrefs, UserPrefs} from 'app/common/Prefs'; import * as roles from 'app/common/roles'; -import {addCurrentOrgToPath} from 'app/common/urlUtils'; -import {encodeQueryParams} from 'app/common/gutil'; import {WebhookFields, WebhookSubscribe, WebhookSummary, WebhookUpdate} from 'app/common/Triggers'; +import {addCurrentOrgToPath} from 'app/common/urlUtils'; import omitBy from 'lodash/omitBy'; @@ -463,6 +463,7 @@ export interface DocAPI { // Update webhook updateWebhook(webhook: WebhookUpdate): Promise; flushWebhooks(): Promise; + flushWebhook(webhookId: string): Promise; getAssistance(params: AssistanceRequest): Promise; } @@ -949,6 +950,12 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { }); } + public async flushWebhook(id: string): Promise { + await this.request(`${this._url}/webhooks/queue/${id}`, { + method: 'DELETE' + }); + } + public async forceReload(): Promise { await this.request(`${this._url}/force-reload`, { method: 'POST' diff --git a/app/gen-server/lib/DocApiForwarder.ts b/app/gen-server/lib/DocApiForwarder.ts index fb2013f8..e825a70e 100644 --- a/app/gen-server/lib/DocApiForwarder.ts +++ b/app/gen-server/lib/DocApiForwarder.ts @@ -70,7 +70,7 @@ export class DocApiForwarder { let docId: string|null = null; if (withDocId) { const docAuth = await getOrSetDocAuth(req as RequestWithLogin, this._dbManager, - this._gristServer, req.params.docId); + this._gristServer, req.params.docId); if (role) { assertAccess(role, docAuth, {allowRemoved: true}); } diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index c37f405e..57b32838 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -35,6 +35,7 @@ import { import {ApiError} from 'app/common/ApiError'; import {mapGetOrSet, MapWithTTL} from 'app/common/AsyncCreate'; import {AttachmentColumns, gatherAttachmentIds, getAttachmentColumns} from 'app/common/AttachmentColumns'; +import {WebhookMessageType} from 'app/common/CommTypes'; import { BulkAddRecord, BulkRemoveRecord, @@ -129,8 +130,7 @@ import {createAttachmentsIndex, DocStorage, REMOVE_UNUSED_ATTACHMENTS_DELAY} fro import {expandQuery} from './ExpandedQuery'; import {GranularAccess, GranularAccessForBundle} from './GranularAccess'; import {OnDemandActions} from './OnDemandActions'; -import {getLogMetaFromDocSession, getPubSubPrefix, getTelemetryMetaFromDocSession, - timeoutReached} from './serverUtils'; +import {getLogMetaFromDocSession, getPubSubPrefix, getTelemetryMetaFromDocSession, timeoutReached} from './serverUtils'; import {findOrAddAllEnvelope, Sharing} from './Sharing'; import cloneDeep = require('lodash/cloneDeep'); import flatten = require('lodash/flatten'); @@ -1769,6 +1769,10 @@ export class ActiveDoc extends EventEmitter { await this._triggers.clearWebhookQueue(); } + public async clearSingleWebhookQueue(webhookId: string) { + await this._triggers.clearSingleWebhookQueue(webhookId); + } + /** * Returns the list of outgoing webhook for a table in this document. */ @@ -1778,13 +1782,13 @@ export class ActiveDoc extends EventEmitter { /** * Send a message to clients connected to the document that something - * webhook-related has happened (a change in configuration, or a - * delivery, or an error). There is room to give details in future, - * if that proves useful, but for now no details are needed. + * webhook-related has happened (a change in configuration, a delivery, + * or an error). It passes information about the type of event (currently data being updated in some way + * or an OverflowError, i.e., too many events waiting to be sent). More data may be added when necessary. */ - public async sendWebhookNotification() { + public async sendWebhookNotification(type: WebhookMessageType = WebhookMessageType.Update) { await this.docClients.broadcastDocMessage(null, 'docChatter', { - webhooks: {}, + webhooks: {type}, }); } diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index a415c06a..f49773e5 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -763,6 +763,16 @@ export class DocWorkerApi { + // Clears a single webhook in the queue for this document. + this._app.delete('/api/docs/:docId/webhooks/queue/:webhookId', isOwner, + withDoc(async (activeDoc, req, res) => { + const webhookId = req.params.webhookId; + await activeDoc.clearSingleWebhookQueue(webhookId); + await activeDoc.sendWebhookNotification(); + res.json({success: true}); + }) + ); + // Lists all webhooks and their current status in the document. this._app.get('/api/docs/:docId/webhooks', isOwner, withDoc(async (activeDoc, req, res) => { diff --git a/app/server/lib/Triggers.ts b/app/server/lib/Triggers.ts index 915d24c7..39a6d212 100644 --- a/app/server/lib/Triggers.ts +++ b/app/server/lib/Triggers.ts @@ -3,6 +3,7 @@ import {summarizeAction} from 'app/common/ActionSummarizer'; import {ActionSummary, TableDelta} from 'app/common/ActionSummary'; import {ApiError} from 'app/common/ApiError'; import {MapWithTTL} from 'app/common/AsyncCreate'; +import {WebhookMessageType} from "app/common/CommTypes"; import {fromTableDataAction, RowRecord, TableColValues, TableDataAction} from 'app/common/DocActions'; import {StringUnion} from 'app/common/StringUnion'; import {MetaRowRecord} from 'app/common/TableData'; @@ -234,7 +235,9 @@ export class DocTriggers { // Prevent further document activity while the queue is too full. while (this._drainingQueue && !this._shuttingDown) { - await delayAbort(1000, this._loopAbort?.signal); + const sendNotificationPromise = this._activeDoc.sendWebhookNotification(WebhookMessageType.Overflow); + const delayPromise = delayAbort(5000, this._loopAbort?.signal); + await Promise.all([sendNotificationPromise, delayPromise]); } return summary; @@ -335,6 +338,36 @@ export class DocTriggers { await this._stats.clear(); } + public async clearSingleWebhookQueue(webhookId: string) { + // Make sure we are after start and in sync with redis. + if (this._getRedisQueuePromise) { + await this._getRedisQueuePromise; + } + // Clear in-memory queue for given webhook key. + let removed = 0; + for(let i=0; i< this._webHookEventQueue.length; i++){ + if(this._webHookEventQueue[i].id == webhookId){ + this._webHookEventQueue.splice(i, 1); + removed++; + } + } + // Notify the loop that it should restart. + this._loopAbort?.abort(); + // If we have backup in redis, clear it also. + // NOTE: this is subject to a race condition, currently it is not possible, but any future modification probably + // will require some kind of locking over the queue (or a rewrite) + if (removed && this._redisClient) { + const multi = this._redisClient.multi(); + multi.del(this._redisQueueKey); + + // Re-add all the remaining events to the queue. + const strings = this._webHookEventQueue.map(e => JSON.stringify(e)); + multi.rpush(this._redisQueueKey, ...strings); + await multi.execAsync(); + } + await this._stats.clear(); + } + // Converts a table to tableId by looking it up in _grist_Tables. private _getTableId(rowId: number) { const docData = this._activeDoc.docData; diff --git a/buildtools/fly-template.toml b/buildtools/fly-template.toml index b1fca821..ed8c7fb9 100644 --- a/buildtools/fly-template.toml +++ b/buildtools/fly-template.toml @@ -7,6 +7,7 @@ processes = [] APP_DOC_URL="https://{APP_NAME}.fly.dev" APP_HOME_URL="https://{APP_NAME}.fly.dev" APP_STATIC_URL="https://{APP_NAME}.fly.dev" + ALLOWED_WEBHOOK_DOMAINS="webhook.site" GRIST_SINGLE_ORG="docs" PORT = "8080" FLY_DEPLOY_EXPIRATION = "{FLY_DEPLOY_EXPIRATION}" diff --git a/test/nbrowser/WebhookOverflow.ts b/test/nbrowser/WebhookOverflow.ts new file mode 100644 index 00000000..92c510c7 --- /dev/null +++ b/test/nbrowser/WebhookOverflow.ts @@ -0,0 +1,117 @@ +import {DocCreationInfo} from 'app/common/DocListAPI'; +import {DocAPI} from 'app/common/UserAPI'; +import {assert, driver, Key} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {server, setupTestSuite} from 'test/nbrowser/testUtils'; +import {EnvironmentSnapshot} from 'test/server/testUtils'; +import {WebhookFields} from "../../app/common/Triggers"; + +describe('WebhookOverflow', function () { + this.timeout(30000); + const cleanup = setupTestSuite(); + let session: gu.Session; + let oldEnv: EnvironmentSnapshot; + let doc: DocCreationInfo; + let docApi: DocAPI; + + before(async function () { + oldEnv = new EnvironmentSnapshot(); + //host = new URL(server.getHost()).host; + process.env.ALLOWED_WEBHOOK_DOMAINS = '*'; + process.env.GRIST_MAX_QUEUE_SIZE = '2'; + await server.restart(); + session = await gu.session().teamSite.login(); + const api = session.createHomeApi(); + doc = await session.tempDoc(cleanup, 'Hello.grist'); + docApi = api.getDocAPI(doc.id); + await api.applyUserActions(doc.id, [ + ['AddTable', 'Table2', [{id: 'A'}, {id: 'B'}, {id: 'C'}, {id: 'D'}, {id: 'E'}]], + ]); + const webhookDetails: WebhookFields = { + url: 'https://localhost/WrongWebhook', + eventTypes: ["add", "update"], + enabled: true, + name: 'test webhook', + tableId: 'Table2', + }; + await docApi.addWebhook(webhookDetails); + }); + + after(async function () { + oldEnv.restore(); + await server.restart(); + }); + + async function enterCellWithoutWaitingOnServer(...keys: string[]) { + const lastKey = keys[keys.length - 1]; + if (![Key.ENTER, Key.TAB, Key.DELETE].includes(lastKey)) { + keys.push(Key.ENTER); + } + await driver.sendKeys(...keys); + } + + it('should show a message when overflown', async function () { + await gu.openPage('Table2'); + await gu.getCell('A', 1).click(); + await gu.enterCell('123'); + await gu.getCell('B', 1).click(); + await enterCellWithoutWaitingOnServer('124'); + const toast = await driver.wait(() => gu.getToasts(), 10000); + assert.include(toast, 'New changes are temporarily suspended. Webhooks queue overflowed.' + + ' Please check webhooks settings, remove invalid webhooks, and clean the queue.\ngo to webhook settings'); + }); + + it('message should disappear after clearing queue', async function () { + await openWebhookPageWithoutWaitForServer(); + await driver.findContent('button', /Clear Queue/).click(); + await gu.waitForServer(); + await waitForOverflownMessageToDisappear(); + const toast = await driver.wait(() => gu.getToasts()); + assert.notInclude(toast, 'New changes are temporarily suspended. Webhooks queue overflowed.' + + ' Please check webhooks settings, remove invalid webhooks, and clean the queue.\ngo to webhook settings'); + }); +}); + +async function waitForOverflownMessageToDisappear(maxWait = 12500) { + await driver.wait(async () => { + try { + for (;;) { + const toasts = await gu.getToasts(); + const filteredToasts = toasts.find(t => t=='New changes are temporarily suspended. Webhooks queue overflowed.' + + ' Please check webhooks settings, remove invalid webhooks, and clean the queue.\ngo to webhook settings'); + if (!filteredToasts) { + return true; + } + } + } catch (e) { + return false; + } + }, maxWait, 'Overflown message did not disappear'); +} + +async function openWebhookPageWithoutWaitForServer() { + await openDocumentSettings(); + const button = await driver.findContentWait('a', /Manage Webhooks/, 3000); + await gu.scrollIntoView(button).click(); + await waitForWebhookPage(); +} + +async function waitForWebhookPage() { + await driver.findContentWait('button', /Clear Queue/, 3000); + // No section, so no easy utility for setting focus. Click on a random cell. + await gu.getDetailCell({col: 'Webhook Id', rowNum: 1}).click(); +} + +export async function openAccountMenu() { + await driver.findWait('.test-dm-account', 1000).click(); + // Since the AccountWidget loads orgs and the user data asynchronously, the menu + // can expand itself causing the click to land on a wrong button. + await driver.findWait('.test-site-switcher-org', 1000); + await driver.sleep(250); // There's still some jitter (scroll-bar? other user accounts?) +} + +export async function openDocumentSettings() { + await openAccountMenu(); + await driver.findContent('.grist-floating-menu a', 'Document Settings').click(); + await gu.waitForUrl(/settings/, 5000); +} diff --git a/test/nbrowser/WebhookPage.ts b/test/nbrowser/WebhookPage.ts index a236674b..639e63fd 100644 --- a/test/nbrowser/WebhookPage.ts +++ b/test/nbrowser/WebhookPage.ts @@ -1,11 +1,9 @@ -import { DocCreationInfo } from 'app/common/DocListAPI'; -import { DocAPI } from 'app/common/UserAPI'; -import { assert, driver, Key } from 'mocha-webdriver'; +import {DocCreationInfo} from 'app/common/DocListAPI'; +import {DocAPI} from 'app/common/UserAPI'; +import {assert, driver, Key} from 'mocha-webdriver'; import * as gu from 'test/nbrowser/gristUtils'; -import { setupTestSuite } from 'test/nbrowser/testUtils'; -import { EnvironmentSnapshot } from 'test/server/testUtils'; -import { server } from 'test/nbrowser/testUtils'; -//import { Deps as TriggersDeps } from 'app/server/lib/Triggers'; +import {server, setupTestSuite} from 'test/nbrowser/testUtils'; +import {EnvironmentSnapshot} from 'test/server/testUtils'; describe('WebhookPage', function () { this.timeout(60000);