From 732611c356456c0596829c1b227529204a99e499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 8 Aug 2023 11:36:02 +0200 Subject: [PATCH] (core) Removing GRIST_FORMULA_ASSISTANT flag Summary: A floating formula editor is available by default and in the basic setup allows just formula modification. AI assistant is now an optional component of the floating editor and it is controlled by OPENAPI_KEY presence. Env variable GRIST_FORMULA_ASSISTANT was removed, new feature flag HAS_FORMULA_ASSISTANT is derived from the presence of OPENAPI_KEY. Also updated anonymous signup nudge. By default it displays only info that this feature is only for logged in users. Test Plan: updated Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3987 --- README.md | 2 +- app/client/models/features.ts | 15 +++-- app/client/widgets/FormulaAssistant.ts | 88 ++++++++++++++++---------- app/client/widgets/FormulaEditor.ts | 65 ++++++++++--------- app/server/lib/sendAppPage.ts | 2 +- test/nbrowser/gristUtils.ts | 8 ++- 6 files changed, 103 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index b5630df4..7bbe7ff0 100644 --- a/README.md +++ b/README.md @@ -302,7 +302,7 @@ PORT | port number to listen on for Grist server REDIS_URL | optional redis server for browser sessions and db query caching GRIST_SNAPSHOT_TIME_CAP | optional. Define the caps for tracking buckets. Usage: {"hour": 25, "day": 32, "isoWeek": 12, "month": 96, "year": 1000} GRIST_SNAPSHOT_KEEP | optional. Number of recent snapshots to retain unconditionally for a document, regardless of when they were made -OPENAI_API_KEY | optional. Used for the AI formula assistant. Sign up for an account on OpenAI and then generate a secret key [here](https://platform.openai.com/account/api-keys). You also need to set `GRIST_FORMULA_ASSISTANT=1`. +OPENAI_API_KEY | optional. Used for the AI formula assistant. Sign up for an account on OpenAI and then generate a secret key [here](https://platform.openai.com/account/api-keys). Sandbox related variables: diff --git a/app/client/models/features.ts b/app/client/models/features.ts index ce5095c3..aa883e52 100644 --- a/app/client/models/features.ts +++ b/app/client/models/features.ts @@ -3,6 +3,9 @@ import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals'; import {localStorageBoolObs} from 'app/client/lib/localStorageObs'; import {Observable} from 'grainjs'; +/** + * Are comments enabled by feature flag. + */ export function COMMENTS(): Observable { const G = getBrowserGlobals('document', 'window'); if (!G.window.COMMENTS) { @@ -11,11 +14,9 @@ export function COMMENTS(): Observable { return G.window.COMMENTS; } -export function GRIST_FORMULA_ASSISTANT(): Observable { - const G = getBrowserGlobals('document', 'window'); - if (!G.window.GRIST_FORMULA_ASSISTANT) { - G.window.GRIST_FORMULA_ASSISTANT = - localStorageBoolObs('GRIST_FORMULA_ASSISTANT', Boolean(getGristConfig().featureFormulaAssistant)); - } - return G.window.GRIST_FORMULA_ASSISTANT; +/** + * Does backend supports AI assistant. + */ +export function HAS_FORMULA_ASSISTANT() { + return Boolean(getGristConfig().featureFormulaAssistant); } diff --git a/app/client/widgets/FormulaAssistant.ts b/app/client/widgets/FormulaAssistant.ts index df8defa5..aaeae3be 100644 --- a/app/client/widgets/FormulaAssistant.ts +++ b/app/client/widgets/FormulaAssistant.ts @@ -1,29 +1,30 @@ import * as commands from 'app/client/components/commands'; import {GristDoc} from 'app/client/components/GristDoc'; -import {logTelemetryEvent} from 'app/client/lib/telemetry'; import {makeT} from 'app/client/lib/localization'; import {localStorageBoolObs} from 'app/client/lib/localStorageObs'; +import {movable} from 'app/client/lib/popupUtils'; +import {logTelemetryEvent} from 'app/client/lib/telemetry'; import {ColumnRec, ViewFieldRec} from 'app/client/models/DocModel'; import {ChatMessage} from 'app/client/models/entities/ColumnRec'; -import {GRIST_FORMULA_ASSISTANT} from 'app/client/models/features'; +import {HAS_FORMULA_ASSISTANT} from 'app/client/models/features'; import {getLoginOrSignupUrl, urlState} from 'app/client/models/gristUrlState'; import {buildHighlightedCode} from 'app/client/ui/CodeHighlight'; +import {autoGrow} from 'app/client/ui/forms'; import {sanitizeHTML} from 'app/client/ui/sanitizeHTML'; import {createUserImage} from 'app/client/ui/UserImage'; -import {FormulaEditor} from 'app/client/widgets/FormulaEditor'; -import {AssistanceResponse, AssistanceState, FormulaAssistanceContext} from 'app/common/AssistancePrompts'; import {basicButton, bigPrimaryButtonLink, primaryButton} from 'app/client/ui2018/buttons'; import {theme, vars} from 'app/client/ui2018/cssVars'; -import {autoGrow} from 'app/client/ui/forms'; import {icon} from 'app/client/ui2018/icons'; import {cssLink} from 'app/client/ui2018/links'; -import {commonUrls} from 'app/common/gristUrls'; -import {movable} from 'app/client/lib/popupUtils'; import {loadingDots} from 'app/client/ui2018/loaders'; import {menu, menuCssClass, menuItem} from 'app/client/ui2018/menus'; +import {FormulaEditor} from 'app/client/widgets/FormulaEditor'; +import {AssistanceResponse, AssistanceState, FormulaAssistanceContext} from 'app/common/AssistancePrompts'; +import {commonUrls} from 'app/common/gristUrls'; import {TelemetryEvent, TelemetryMetadata} from 'app/common/Telemetry'; -import {Computed, Disposable, dom, DomElementArg, makeTestId, - MutableObsArray, obsArray, Observable, styled, subscribeElem} from 'grainjs'; +import {getGristConfig} from 'app/common/urlUtils'; +import {Computed, Disposable, dom, DomElementArg, makeTestId, MutableObsArray, + obsArray, Observable, styled, subscribeElem} from 'grainjs'; import debounce from 'lodash/debounce'; import noop from 'lodash/noop'; import {marked} from 'marked'; @@ -54,8 +55,8 @@ export class FormulaAssistant extends Disposable { `u:${this._options.gristDoc.appModel.currentUser?.id ?? 0};formulaAssistantExpanded`, true)); /** Is the request pending */ private _waiting = Observable.create(this, false); - /** Is this feature enabled at all */ - private _assistantEnabled: Computed; + /** Is assistant features are enabled */ + private _assistantEnabled: boolean; /** Preview column ref */ private _transformColRef: string; /** Preview column id */ @@ -101,9 +102,7 @@ export class FormulaAssistant extends Disposable { }) { super(); - this._assistantEnabled = Computed.create(this, use => { - return use(GRIST_FORMULA_ASSISTANT()); - }); + this._assistantEnabled = HAS_FORMULA_ASSISTANT(); if (!this._options.field) { // TODO: field is not passed only for rules (as there is no preview there available to the user yet) @@ -192,16 +191,18 @@ export class FormulaAssistant extends Disposable { this._buildChatPanel(), ); - if (!this._assistantExpanded.get()) { - this._chatPanelBody.style.setProperty('height', '0px'); - } else { - // The actual height doesn't matter too much here, so we just pick - // a value that guarantees the assistant will fill as much of the - // available space as possible. - this._chatPanelBody.style.setProperty('height', '999px'); + if (this._assistantEnabled) { + if (!this._assistantExpanded.get()) { + this._chatPanelBody.style.setProperty('height', '0px'); + } else { + // The actual height doesn't matter too much here, so we just pick + // a value that guarantees the assistant will fill as much of the + // available space as possible. + this._chatPanelBody.style.setProperty('height', '999px'); + } } - if (this._assistantEnabled.get() && this._assistantExpanded.get()) { + if (this._assistantEnabled && this._assistantExpanded.get()) { this._logTelemetryEvent('assistantOpen', true); this._hasExpanded = true; } @@ -590,18 +591,8 @@ export class FormulaAssistant extends Disposable { * Builds the signup nudge shown to anonymous users at the bottom of the chat. */ private _buildSignupNudge() { - return cssSignupNudgeWrapper( - cssSignupNudgeParagraph( - t('Sign up for a free Grist account to start using the Formula AI Assistant.'), - ), - cssSignupNudgeButtonsRow( - bigPrimaryButtonLink( - t('Sign Up for Free'), - {href: getLoginOrSignupUrl()}, - testId('ai-assistant-sign-up'), - ), - ), - ); + const {deploymentType} = getGristConfig(); + return deploymentType === 'saas' ? buildSignupNudge() : buildAnonNudge(); } private async _handleChatEnterKeyDown(ev: KeyboardEvent) { @@ -979,6 +970,30 @@ function buildAvatar(grist: GristDoc) { } } +function buildSignupNudge() { + return cssSignupNudgeWrapper( + cssSignupNudgeParagraph( + t('Sign up for a free Grist account to start using the Formula AI Assistant.'), + ), + cssSignupNudgeButtonsRow( + bigPrimaryButtonLink( + t('Sign Up for Free'), + {href: getLoginOrSignupUrl()}, + testId('ai-assistant-sign-up'), + ), + ), + ); +} + +function buildAnonNudge() { + return cssSignupNudgeWrapper( + cssSignupNudgeWrapper.cls('-center'), + cssSignupNudgeParagraph( + t('Formula AI Assistant is only available for logged in users.'), + ), + ); +} + const MIN_FORMULA_EDITOR_HEIGHT_PX = 100; const FORMULA_EDITOR_BUTTONS_HEIGHT_PX = 42; @@ -1275,6 +1290,11 @@ const cssSignupNudgeWrapper = styled('div', ` display: flex; flex-shrink: 0; flex-direction: column; + &-center { + display: flex; + justify-content: center; + align-items: center; + } `); const cssSignupNudgeParagraph = styled('div', ` diff --git a/app/client/widgets/FormulaEditor.ts b/app/client/widgets/FormulaEditor.ts index 91b041d3..31c4d143 100644 --- a/app/client/widgets/FormulaEditor.ts +++ b/app/client/widgets/FormulaEditor.ts @@ -7,7 +7,7 @@ import {DataRowModel} from 'app/client/models/DataRowModel'; import {ColumnRec} from 'app/client/models/DocModel'; import {ViewFieldRec} from 'app/client/models/entities/ViewFieldRec'; import {reportError} from 'app/client/models/errors'; -import {GRIST_FORMULA_ASSISTANT} from 'app/client/models/features'; +import {HAS_FORMULA_ASSISTANT} from 'app/client/models/features'; import {hoverTooltip} from 'app/client/ui/tooltips'; import {textButton} from 'app/client/ui2018/buttons'; import {colors, testId, theme, vars} from 'app/client/ui2018/cssVars'; @@ -53,7 +53,7 @@ export class FormulaEditor extends NewBaseEditor { public isDetached = Observable.create(this, false); protected options: IFormulaEditorOptions; - private _formulaEditor: any; + private _aceEditor: any; private _dom: HTMLElement; private _editorPlacement!: EditorPlacement; private _placementHolder = Holder.create(this); @@ -71,7 +71,7 @@ export class FormulaEditor extends NewBaseEditor { this._isEmpty = Computed.create(this, this.editorState, (_use, state) => state === ''); - this._formulaEditor = AceEditor.create({ + this._aceEditor = AceEditor.create({ // A bit awkward, but we need to assume calcSize is not used until attach() has been called // and _editorPlacement created. column: options.column, @@ -145,14 +145,14 @@ export class FormulaEditor extends NewBaseEditor { // the DOM to update before resizing. this.autoDispose(errorDetails.addListener(() => setTimeout(this.resize.bind(this), 0))); - this._canDetach = Boolean(GRIST_FORMULA_ASSISTANT().get() && options.canDetach && !options.readonly); + this._canDetach = Boolean(options.canDetach && !options.readonly); - this.autoDispose(this._formulaEditor); + this.autoDispose(this._aceEditor); // Show placeholder text when the formula is blank. this._isEmpty.addListener(() => this._updateEditorPlaceholder()); - // Update the placeholder text when expanding or collapsing the editor. + // Disable undo/redo while the editor is detached. this.isDetached.addListener((isDetached) => { // TODO: look into whether we can support undo/redo while the editor is detached. if (isDetached) { @@ -160,8 +160,6 @@ export class FormulaEditor extends NewBaseEditor { } else { options.gristDoc.getUndoStack().enable(); } - - this._updateEditorPlaceholder(); }); this.onDispose(() => { @@ -203,22 +201,22 @@ export class FormulaEditor extends NewBaseEditor { ), cssFormulaEditor.cls('-detached', this.isDetached), dom('div.formula_editor.formula_field_edit', testId('formula-editor'), - this._formulaEditor.buildDom((aceObj: any) => { + this._aceEditor.buildDom((aceObj: any) => { aceObj.setFontSize(11); aceObj.setHighlightActiveLine(false); aceObj.getSession().setUseWrapMode(false); aceObj.renderer.setPadding(0); const val = initialValue; const pos = Math.min(options.cursorPos, val.length); - this._formulaEditor.setValue(val, pos); - this._formulaEditor.attachCommandGroup(aceCommands); + this._aceEditor.setValue(val, pos); + this._aceEditor.attachCommandGroup(aceCommands); // enable formula editing if state was passed if (options.state || options.readonly) { editingFormula(true); } if (options.readonly) { - this._formulaEditor.enable(false); + this._aceEditor.enable(false); aceObj.gotoLine(0, 0); // By moving, ace editor won't highlight anything } // This catches any change to the value including e.g. via backspace or paste. @@ -238,7 +236,7 @@ export class FormulaEditor extends NewBaseEditor { if (this.isDetached.get()) { return; } if (errorDetails.get()){ hideErrDetails.set(!hideErrDetails.get()); - this._formulaEditor.resize(); + this._aceEditor.resize(); } }), dom.maybe(errorDetails, () => @@ -249,7 +247,7 @@ export class FormulaEditor extends NewBaseEditor { if (!this.isDetached.get()) { return; } if (errorDetails.get()){ hideErrDetails.set(!hideErrDetails.get()); - this._formulaEditor.resize(); + this._aceEditor.resize(); } }) )) @@ -281,9 +279,10 @@ export class FormulaEditor extends NewBaseEditor { this._editorPlacement = EditorPlacement.create( this._placementHolder, this._dom, cellElem, {margins: getButtonMargins()}); // Reposition the editor if needed for external reasons (in practice, window resize). - this.autoDispose(this._editorPlacement.onReposition.addListener(this._formulaEditor.resize, this._formulaEditor)); - this._formulaEditor.onAttach(); - this._formulaEditor.resize(); + this.autoDispose(this._editorPlacement.onReposition.addListener(this._aceEditor.resize, this._aceEditor)); + this._aceEditor.onAttach(); + this._updateEditorPlaceholder(); + this._aceEditor.resize(); this.focus(); } @@ -292,33 +291,33 @@ export class FormulaEditor extends NewBaseEditor { } public setFormula(formula: string) { - this._formulaEditor.setValue(formula); + this._aceEditor.setValue(formula); } public getCellValue() { - const value = this._formulaEditor.getValue(); + const value = this._aceEditor.getValue(); // Strip the leading "=" sign, if any, in case users think it should start the formula body (as // it does in Excel, and because the equal sign is also used for formulas in Grist UI). return (value[0] === '=') ? value.slice(1) : value; } public getTextValue() { - return this._formulaEditor.getValue(); + return this._aceEditor.getValue(); } public getCursorPos() { - const aceObj = this._formulaEditor.getEditor(); + const aceObj = this._aceEditor.getEditor(); return aceObj.getSession().getDocument().positionToIndex(aceObj.getCursorPosition()); } public focus() { if (this.isDisposed()) { return; } - this._formulaEditor.getEditor().focus(); + this._aceEditor.getEditor().focus(); } public resize() { if (this.isDisposed()) { return; } - this._formulaEditor.resize(); + this._aceEditor.resize(); } public detach() { @@ -331,25 +330,26 @@ export class FormulaEditor extends NewBaseEditor { this._placementHolder.clear(); // We are going in the full formula edit mode right away. this.options.editingFormula(true); + this._updateEditorPlaceholder(); // Set the focus in timeout, as the dom is added after this function. - setTimeout(() => !this.isDisposed() && this._formulaEditor.resize(), 0); + setTimeout(() => !this.isDisposed() && this._aceEditor.resize(), 0); // Return the dom, it will be moved to the floating editor. return this._dom; } private _updateEditorPlaceholder() { - const editor = this._formulaEditor.getEditor(); + const editor = this._aceEditor.getEditor(); const shouldShowPlaceholder = editor.session.getValue().length === 0; - const placeholderNode = editor.renderer.emptyMessageNode; - if (placeholderNode) { + if (editor.renderer.emptyMessageNode) { // Remove the current placeholder if one is present. - editor.renderer.scroller.removeChild(placeholderNode); + editor.renderer.scroller.removeChild(editor.renderer.emptyMessageNode); } if (!shouldShowPlaceholder) { editor.renderer.emptyMessageNode = null; } else { + const withAiButton = this._canDetach && !this.isDetached.get() && HAS_FORMULA_ASSISTANT(); editor.renderer.emptyMessageNode = cssFormulaPlaceholder( - !this._canDetach || this.isDetached.get() + !withAiButton ? t('Enter formula.') : t('Enter formula or {{button}}.', { button: cssUseAssistantButton( @@ -361,7 +361,6 @@ export class FormulaEditor extends NewBaseEditor { ); editor.renderer.scroller.appendChild(editor.renderer.emptyMessageNode); } - this._formulaEditor.resize(); } private _handleUseAssistantButtonClick(ev: MouseEvent) { @@ -380,7 +379,7 @@ export class FormulaEditor extends NewBaseEditor { }; } - const placeholder: HTMLElement | undefined = this._formulaEditor.getEditor().renderer.emptyMessageNode; + const placeholder: HTMLElement | undefined = this._aceEditor.getEditor().renderer.emptyMessageNode; if (placeholder) { // If we are showing the placeholder, fit it all on the same line. return this._editorPlacement.calcSizeWithPadding(elem, { @@ -422,7 +421,7 @@ export class FormulaEditor extends NewBaseEditor { const colId = col.origCol.peek().colId.peek(); - const aceObj = this._formulaEditor.getEditor(); + const aceObj = this._aceEditor.getEditor(); // Rect only to columns in the same table. if (col.tableId.peek() !== this.options.column.table.peek().tableId.peek()) { @@ -451,7 +450,7 @@ export class FormulaEditor extends NewBaseEditor { // Else touching a normal identifier, don't mangle it } // Resize editor in case it is needed. - this._formulaEditor.resize(); + this._aceEditor.resize(); // This focus method will try to focus a textarea immediately and again on setTimeout. But // other things may happen by the setTimeout time, messing up focus. The reason the immediate diff --git a/app/server/lib/sendAppPage.ts b/app/server/lib/sendAppPage.ts index 8aaf48be..03cbf7b0 100644 --- a/app/server/lib/sendAppPage.ts +++ b/app/server/lib/sendAppPage.ts @@ -74,7 +74,7 @@ export function makeGristConfig(options: MakeGristConfigOptons): GristLoadConfig supportedLngs: readLoadedLngs(req?.i18n), namespaces: readLoadedNamespaces(req?.i18n), featureComments: isAffirmative(process.env.COMMENTS), - featureFormulaAssistant: isAffirmative(process.env.GRIST_FORMULA_ASSISTANT), + featureFormulaAssistant: Boolean(process.env.OPENAI_API_KEY), supportEmail: SUPPORT_EMAIL, userLocale: (req as RequestWithLogin | undefined)?.user?.options?.locale, telemetry: server?.getTelemetry().getTelemetryConfig(), diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 924668ea..a4f43702 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -3014,7 +3014,13 @@ export function withEnvironmentSnapshot(vars: Record) { // Test if the vars are already set, and if so, skip. if (Object.keys(vars).every(k => process.env[k] === vars[k])) { return; } oldEnv = new testUtils.EnvironmentSnapshot(); - Object.assign(process.env, vars); + for(const key of Object.keys(vars)) { + if (vars[key] === undefined || vars[key] === null) { + delete process.env[key]; + } else { + process.env[key] = vars[key]; + } + } await server.restart(); }); after(async () => {