From 3dadf93c98a52e4bd167897005e17ad370079883 Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Wed, 6 Sep 2023 14:35:46 -0400 Subject: [PATCH] (core) Add support for auto-copying docs on signup Summary: The new "copyDoc" query parameter on the login page sets a short-lived cookie, which is then read when welcoming a new user to copy that document to their Home workspace, and redirect to it. Currently, only templates and bare forks set this parameter. A new API endpoint for copying a document to a workspace was also added. Test Plan: Browser tests. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3992 --- app/client/models/DocPageModel.ts | 23 ++++ app/client/models/gristUrlState.ts | 35 +++-- app/client/ui/AccountWidget.ts | 8 +- app/client/ui/App.ts | 5 +- app/client/ui/MakeCopyMenu.ts | 79 +++++------ app/client/ui/ShareMenu.ts | 65 ++++++--- app/client/ui/WelcomePage.ts | 4 +- app/common/UserAPI.ts | 29 ++++ app/common/gristUrls.ts | 5 +- app/server/lib/AppEndpoint.ts | 150 +------------------- app/server/lib/DocApi.ts | 124 +++++++++++++---- app/server/lib/DocWorkerUtils.ts | 158 +++++++++++++++++++++ app/server/lib/FlexServer.ts | 71 +++++++++- app/server/lib/requestUtils.ts | 11 ++ app/server/lib/uploads.ts | 2 +- test/nbrowser/CellColor.ts | 2 +- test/nbrowser/DocTutorial.ts | 2 +- test/nbrowser/Fork.ts | 38 ++++-- test/nbrowser/GridView.ts | 44 ++++++ test/nbrowser/HomeIntro.ts | 1 + test/nbrowser/ImportReferences.ts | 212 +++++++++++++++++++++++++++++ test/nbrowser/Smoke.ts | 3 +- test/nbrowser/UploadLimits.ts | 143 +++++++++++++++++++ test/nbrowser/gristUtils.ts | 34 ++++- test/nbrowser/homeUtil.ts | 11 ++ test/nbrowser/importerTestUtils.ts | 74 ++++++++++ 26 files changed, 1057 insertions(+), 276 deletions(-) create mode 100644 app/server/lib/DocWorkerUtils.ts create mode 100644 test/nbrowser/GridView.ts create mode 100644 test/nbrowser/ImportReferences.ts create mode 100644 test/nbrowser/UploadLimits.ts create mode 100644 test/nbrowser/importerTestUtils.ts diff --git a/app/client/models/DocPageModel.ts b/app/client/models/DocPageModel.ts index 727f2e70..8b45c58e 100644 --- a/app/client/models/DocPageModel.ts +++ b/app/client/models/DocPageModel.ts @@ -1,5 +1,6 @@ import {GristDoc} from 'app/client/components/GristDoc'; import {IUndoState} from 'app/client/components/UndoStack'; +import {UnsavedChange} from 'app/client/components/UnsavedChanges'; import {loadGristDoc} from 'app/client/lib/imports'; import {AppModel, getOrgNameOrGuest, reportError} from 'app/client/models/AppModel'; import {getDoc} from 'app/client/models/gristConfigCache'; @@ -94,6 +95,7 @@ export interface DocPageModel { // the error that prompted the offer. If user is not owner, just flag that // document needs attention of an owner. offerRecovery(err: Error): void; + clearUnsavedChanges(): void; } export interface ImportSource { @@ -154,6 +156,15 @@ export class DocPageModelImpl extends Disposable implements DocPageModel { // (with the previous promise cancelled) when _openerDocKey changes. private _openerHolder = Holder.create(this); + private readonly _unsavedChangeHolder = Holder.create(this); + + private readonly _isUnsavedFork = Computed.create(this, + this.isFork, + this.isSnapshot, + this.isTutorialFork, + (use, isFork, isSnapshot, isTutorialFork) => isFork && !isSnapshot && !isTutorialFork + ); + constructor(private _appObj: App, public readonly appModel: AppModel, private _api: UserAPI = appModel.api) { super(); @@ -185,6 +196,14 @@ export class DocPageModelImpl extends Disposable implements DocPageModel { this.currentProduct.set(org?.billingAccount?.product ?? null); } })); + + this.autoDispose(this._isUnsavedFork.addListener((isUnsavedFork) => { + if (isUnsavedFork) { + UnsavedChange.create(this._unsavedChangeHolder); + } else { + this._unsavedChangeHolder.clear(); + } + })); } public createLeftPane(leftPanelOpen: Observable) { @@ -289,6 +308,10 @@ It also disables formulas. [{{error}}]", {error: err.message}) ); } + public clearUnsavedChanges(): void { + this._unsavedChangeHolder.clear(); + } + private _onOpenError(err: Error) { if (err instanceof CancelledError) { // This means that we started loading a new doc before the previous one finished loading. diff --git a/app/client/models/gristUrlState.ts b/app/client/models/gristUrlState.ts index 4e1523fe..0ff3c79d 100644 --- a/app/client/models/gristUrlState.ts +++ b/app/client/models/gristUrlState.ts @@ -66,24 +66,30 @@ export function getMainOrgUrl(): string { return urlState().makeUrl({}); } // When on a document URL, returns the URL with just the doc ID, omitting other bits (like page). export function getCurrentDocUrl(): string { return urlState().makeUrl({docPage: undefined}); } -// Get url for the login page, which will then redirect to `nextUrl` (current page by default). -export function getLoginUrl(nextUrl: string | null = _getCurrentUrl()): string { - return _getLoginLogoutUrl('login', nextUrl); +export interface GetLoginOrSignupUrlOptions { + srcDocId?: string | null; + /** Defaults to the current URL. */ + nextUrl?: string | null; } -// Get url for the signup page, which will then redirect to `nextUrl` (current page by default). -export function getSignupUrl(nextUrl: string = _getCurrentUrl()): string { - return _getLoginLogoutUrl('signup', nextUrl); +// Get URL for the login page. +export function getLoginUrl(options: GetLoginOrSignupUrlOptions = {}): string { + return _getLoginLogoutUrl('login', options); } -// Get url for the logout page. +// Get URL for the signup page. +export function getSignupUrl(options: GetLoginOrSignupUrlOptions = {}): string { + return _getLoginLogoutUrl('signup', options); +} + +// Get URL for the logout page. export function getLogoutUrl(): string { return _getLoginLogoutUrl('logout'); } -// Get url for the signin page, which will then redirect to `nextUrl` (current page by default). -export function getLoginOrSignupUrl(nextUrl: string = _getCurrentUrl()): string { - return _getLoginLogoutUrl('signin', nextUrl); +// Get URL for the signin page. +export function getLoginOrSignupUrl(options: GetLoginOrSignupUrlOptions = {}): string { + return _getLoginLogoutUrl('signin', options); } export function getWelcomeHomeUrl() { @@ -100,9 +106,14 @@ function _getCurrentUrl(): string { return parseFirstUrlPart('o', pathname).path + search + hash; } -// Returns the URL for the given login page, with 'next' param optionally set. -function _getLoginLogoutUrl(page: 'login'|'logout'|'signin'|'signup', nextUrl?: string | null): string { +// Returns the URL for the given login page. +function _getLoginLogoutUrl( + page: 'login'|'logout'|'signin'|'signup', + options: GetLoginOrSignupUrlOptions = {} +): string { + const {srcDocId, nextUrl = _getCurrentUrl()} = options; const startUrl = _buildUrl(page); + if (srcDocId) { startUrl.searchParams.set('srcDocId', srcDocId); } if (nextUrl) { startUrl.searchParams.set('next', nextUrl); } return startUrl.href; } diff --git a/app/client/ui/AccountWidget.ts b/app/client/ui/AccountWidget.ts index 955864d1..071c036f 100644 --- a/app/client/ui/AccountWidget.ts +++ b/app/client/ui/AccountWidget.ts @@ -81,10 +81,10 @@ export class AccountWidget extends Disposable { private _buildUseThisTemplateButton() { return cssUseThisTemplateButton(t('Use This Template'), dom.attr('href', use => { - // Keep the redirect param of the login/signup URL fresh. - use(urlState().state); - return getLoginOrSignupUrl(); + const {doc: srcDocId} = use(urlState().state); + return getLoginOrSignupUrl({srcDocId}); }), + dom.on('click', () => { this._docPageModel?.clearUnsavedChanges(); }), testId('dm-account-use-this-template'), ); } @@ -109,7 +109,7 @@ export class AccountWidget extends Disposable { t("Toggle Mobile Mode"), cssCheckmark('Tick', dom.show(viewport.viewportEnabled)), testId('usermenu-toggle-mobile'), - ); + ); if (!user) { return [ diff --git a/app/client/ui/App.ts b/app/client/ui/App.ts index 5fe1f246..322c2fd6 100644 --- a/app/client/ui/App.ts +++ b/app/client/ui/App.ts @@ -172,10 +172,9 @@ export class App extends DisposableWithEvents { G.window.addEventListener('beforeunload', (ev: BeforeUnloadEvent) => { if (unsavedChanges.haveUnsavedChanges()) { // Following https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event - const msg = 'You have some unsaved changes'; - ev.returnValue = msg; + ev.returnValue = true; ev.preventDefault(); - return msg; + return true; } this.dispose(); }); diff --git a/app/client/ui/MakeCopyMenu.ts b/app/client/ui/MakeCopyMenu.ts index a23a4636..6a723f98 100644 --- a/app/client/ui/MakeCopyMenu.ts +++ b/app/client/ui/MakeCopyMenu.ts @@ -5,8 +5,8 @@ import {makeT} from 'app/client/lib/localization'; import {AppModel, reportError} from 'app/client/models/AppModel'; -import {DocPageModel} from "app/client/models/DocPageModel"; -import {getLoginOrSignupUrl, urlState} from 'app/client/models/gristUrlState'; +import {DocPageModel} from 'app/client/models/DocPageModel'; +import {urlState} from 'app/client/models/gristUrlState'; import {getWorkspaceInfo, ownerName, workspaceName} from 'app/client/models/WorkspaceInfo'; import {cssInput} from 'app/client/ui/cssInput'; import {bigBasicButton, bigPrimaryButtonLink} from 'app/client/ui2018/buttons'; @@ -14,16 +14,7 @@ import {cssRadioCheckboxOptions, labeledSquareCheckbox, radioCheckboxOption} fro import {testId, theme, vars} from 'app/client/ui2018/cssVars'; import {loadingSpinner} from 'app/client/ui2018/loaders'; import {select} from 'app/client/ui2018/menus'; -import { - confirmModal, - cssModalBody, - cssModalButtons, - cssModalTitle, - cssModalWidth, - modal, - saveModal -} from 'app/client/ui2018/modals'; -import {FullUser} from 'app/common/LoginSessionAPI'; +import {confirmModal, cssModalBody, cssModalButtons, cssModalTitle, modal, saveModal} from 'app/client/ui2018/modals'; import * as roles from 'app/common/roles'; import {Document, isTemplatesOrg, Organization, Workspace} from 'app/common/UserAPI'; import {Computed, Disposable, dom, input, Observable, styled, subscribe} from 'grainjs'; @@ -31,8 +22,9 @@ import sortBy = require('lodash/sortBy'); const t = makeT('MakeCopyMenu'); -export async function replaceTrunkWithFork(user: FullUser|null, doc: Document, app: AppModel, origUrlId: string) { - const trunkAccess = (await app.api.getDoc(origUrlId)).access; +export async function replaceTrunkWithFork(doc: Document, pageModel: DocPageModel, origUrlId: string) { + const {appModel} = pageModel; + const trunkAccess = (await appModel.api.getDoc(origUrlId)).access; if (!roles.canEdit(trunkAccess)) { modal((ctl) => [ cssModalBody(t("Replacing the original requires editing rights on the original document.")), @@ -42,7 +34,7 @@ export async function replaceTrunkWithFork(user: FullUser|null, doc: Document, a ]); return; } - const docApi = app.api.getDocAPI(origUrlId); + const docApi = appModel.api.getDocAPI(origUrlId); const cmp = await docApi.compareDoc(doc.id); let titleText = t("Update Original"); let buttonText = t("Update"); @@ -64,6 +56,7 @@ not in this document. Those changes will be overwritten.")}`; async () => { try { await docApi.replace({sourceDocId: doc.id}); + pageModel.clearUnsavedChanges(); await urlState().pushUrl({doc: origUrlId}); } catch (e) { reportError(e); // For example: no write access on trunk. @@ -71,18 +64,6 @@ not in this document. Those changes will be overwritten.")}`; }, {explanation: warningText}); } -// Show message in a modal with a `Sign up` button that redirects to the login page. -function signupModal(message: string) { - return modal((ctl) => [ - cssModalBody(message), - cssModalButtons( - bigPrimaryButtonLink(t("Sign up"), {href: getLoginOrSignupUrl(), target: '_blank'}, testId('modal-signup')), - bigBasicButton(t("Cancel"), dom.on('click', () => ctl.close())), - ), - cssModalWidth('normal'), - ]); -} - /** * Whether we should offer user the option to copy this doc to other orgs. * We allow copying out of source org when the source org is a personal org, or user has owner @@ -104,12 +85,14 @@ function allowOtherOrgs(doc: Document, app: AppModel): boolean { /** * Ask user for the destination and new name, and make a copy of the doc using those. */ -export async function makeCopy(doc: Document, app: AppModel, modalTitle: string): Promise { - if (!app.currentValidUser) { - signupModal(t("To save your changes, please sign up, then reload this page.")); - return; - } - let orgs = allowOtherOrgs(doc, app) ? await app.api.getOrgs(true) : null; +export async function makeCopy(options: { + pageModel: DocPageModel, + doc: Document, + modalTitle: string, +}): Promise { + const {pageModel, doc, modalTitle} = options; + const {appModel} = pageModel; + let orgs = allowOtherOrgs(doc, appModel) ? await appModel.api.getOrgs(true) : null; if (orgs) { // Don't show the templates org since it's selected by default, and // is not writable to. @@ -118,7 +101,7 @@ export async function makeCopy(doc: Document, app: AppModel, modalTitle: string) // Show a dialog with a form to select destination. saveModal((ctl, owner) => { - const saveCopyModal = SaveCopyModal.create(owner, doc, app, orgs); + const saveCopyModal = SaveCopyModal.create(owner, {pageModel, doc, orgs}); return { title: modalTitle, body: saveCopyModal.buildDom(), @@ -129,7 +112,17 @@ export async function makeCopy(doc: Document, app: AppModel, modalTitle: string) }); } +interface SaveCopyModalParams { + pageModel: DocPageModel; + doc: Document; + orgs: Organization[]|null; +} + class SaveCopyModal extends Disposable { + private _pageModel = this._params.pageModel; + private _app = this._pageModel.appModel; + private _doc = this._params.doc; + private _orgs = this._params.orgs; private _workspaces = Observable.create(this, null); private _destName = Observable.create(this, ''); private _destOrg = Observable.create(this, this._app.currentOrg); @@ -142,10 +135,10 @@ class SaveCopyModal extends Disposable { private _showWorkspaces = Computed.create(this, this._destOrg, (use, org) => Boolean(org && !org.owner)); // If orgs is non-null, then we show a selector for orgs. - constructor(private _doc: Document, private _app: AppModel, private _orgs: Organization[]|null) { + constructor(private _params: SaveCopyModalParams) { super(); - if (_doc.name !== 'Untitled') { - this._destName.set(_doc.name + ' (copy)'); + if (this._doc.name !== 'Untitled') { + this._destName.set(this._doc.name + ' (copy)'); } if (this._orgs && this._app.currentOrg) { // Set _destOrg to an Organization object from _orgs array; there should be one equivalent @@ -163,12 +156,14 @@ class SaveCopyModal extends Disposable { if (!ws) { throw new Error(t("No destination workspace")); } const api = this._app.api; const org = this._destOrg.get(); - const docWorker = await api.getWorkerAPI('import'); - const destName = this._destName.get() + '.grist'; + const destName = this._destName.get(); try { - const uploadId = await docWorker.copyDoc(this._doc.id, this._asTemplate.get(), destName); - const {id} = await docWorker.importDocToWorkspace(uploadId, ws.id); - await urlState().pushUrl({org: org?.domain || undefined, doc: id, docPage: urlState().state.get().docPage}); + const doc = await api.copyDoc(this._doc.id, ws.id, { + documentName: destName, + asTemplate: this._asTemplate.get(), + }); + this._pageModel.clearUnsavedChanges(); + await urlState().pushUrl({org: org?.domain || undefined, doc, docPage: urlState().state.get().docPage}); } catch(err) { // Convert access denied errors to normal Error to make it consistent with other endpoints. // TODO: Should not allow to click this button when user doesn't have permissions. diff --git a/app/client/ui/ShareMenu.ts b/app/client/ui/ShareMenu.ts index 97a24491..63e282b0 100644 --- a/app/client/ui/ShareMenu.ts +++ b/app/client/ui/ShareMenu.ts @@ -1,7 +1,7 @@ import {loadUserManager} from 'app/client/lib/imports'; import {AppModel, reportError} from 'app/client/models/AppModel'; import {DocInfo, DocPageModel} from 'app/client/models/DocPageModel'; -import {docUrl, urlState} from 'app/client/models/gristUrlState'; +import {docUrl, getLoginOrSignupUrl, urlState} from 'app/client/models/gristUrlState'; import {GristTooltips} from 'app/client/ui/GristTooltips'; import {downloadDocModal, makeCopy, replaceTrunkWithFork} from 'app/client/ui/MakeCopyMenu'; import {sendToDrive} from 'app/client/ui/sendToDrive'; @@ -34,20 +34,19 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { // to render its contents, but we handle by merely skipping such content if gristDoc is not yet // available (a user quick enough to open the menu in this state would have to re-open it). return dom.maybe(pageModel.currentDoc, (doc) => { - const appModel = pageModel.appModel; - const saveCopy = () => makeCopy(doc, appModel, t("Save Document")).catch(reportError); + const saveCopy = () => handleSaveCopy({pageModel, doc, modalTitle: t("Save Document")}); if (doc.isSnapshot) { const backToCurrent = () => urlState().pushUrl({doc: buildOriginalUrlId(doc.id, true)}); return shareButton(t("Back to Current"), () => [ menuManageUsers(doc, pageModel), - menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel, {isSnapshot: true}), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Save Copy")}), + menuOriginal(doc, pageModel, {isSnapshot: true}), menuExports(doc, pageModel), ], {buttonAction: backToCurrent}); } else if (doc.isTutorialFork) { return shareButton(t("Save Copy"), () => [ - menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel, {isTutorialFork: true}), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Save Copy")}), + menuOriginal(doc, pageModel, {isTutorialFork: true}), menuExports(doc, pageModel), ], {buttonAction: saveCopy}); } else if (doc.isPreFork || doc.isBareFork) { @@ -55,7 +54,7 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { const saveActionTitle = doc.isBareFork ? t("Save Document") : t("Save Copy"); return shareButton(saveActionTitle, () => [ menuManageUsers(doc, pageModel), - menuSaveCopy(saveActionTitle, doc, appModel), + menuSaveCopy({pageModel, doc, saveActionTitle}), menuExports(doc, pageModel), ], {buttonAction: saveCopy}); } else if (doc.isFork) { @@ -66,22 +65,22 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { if (!roles.canEdit(doc.trunkAccess || null)) { return shareButton(t("Save Copy"), () => [ menuManageUsers(doc, pageModel), - menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Save Copy")}), + menuOriginal(doc, pageModel), menuExports(doc, pageModel), ], {buttonAction: saveCopy}); } else { return shareButton(t("Unsaved"), () => [ menuManageUsers(doc, pageModel), - menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Save Copy")}), + menuOriginal(doc, pageModel), menuExports(doc, pageModel), ]); } } else { return shareButton(null, () => [ menuManageUsers(doc, pageModel), - menuSaveCopy(t("Duplicate Document"), doc, appModel), + menuSaveCopy({pageModel, doc, saveActionTitle: t("Duplicate Document")}), menuWorkOnCopy(pageModel), menuExports(doc, pageModel), ]); @@ -134,6 +133,22 @@ function shareButton(buttonText: string|null, menuCreateFunc: MenuCreateFunc, } } +async function handleSaveCopy(options: { + pageModel: DocPageModel, + doc: Document, + modalTitle: string, +}) { + const {pageModel} = options; + const {appModel} = pageModel; + if (!appModel.currentValidUser) { + pageModel.clearUnsavedChanges(); + window.location.href = getLoginOrSignupUrl({srcDocId: urlState().state.get().doc}); + return; + } + + return makeCopy(options); +} + // Renders "Manage Users" menu item. function menuManageUsers(doc: DocInfo, pageModel: DocPageModel) { return [ @@ -163,7 +178,7 @@ interface MenuOriginalOptions { * setting the open mode in the URL to "/m/default" - if the menu item were to ever be included * again, it should likely be a shortcut to setting the open mode back to default. */ -function menuOriginal(doc: Document, appModel: AppModel, options: MenuOriginalOptions = {}) { +function menuOriginal(doc: Document, pageModel: DocPageModel, options: MenuOriginalOptions = {}) { const {isSnapshot = false, isTutorialFork = false} = options; const termToUse = isSnapshot ? t("Current Version") : t("Original"); const origUrlId = buildOriginalUrlId(doc.id, isSnapshot); @@ -183,15 +198,17 @@ function menuOriginal(doc: Document, appModel: AppModel, options: MenuOriginalOp const comparingSnapshots: boolean = isSnapshot && Boolean(compareUrlId && parseUrlId(compareUrlId).snapshotId); function replaceOriginal() { - const user = appModel.currentValidUser; - replaceTrunkWithFork(user, doc, appModel, origUrlId).catch(reportError); + replaceTrunkWithFork(doc, pageModel, origUrlId).catch(reportError); } return [ isTutorialFork ? null : cssMenuSplitLink({href: originalUrl}, - cssMenuSplitLinkText(t("Return to {{termToUse}}", {termToUse})), testId('return-to-original'), - cssMenuIconLink({href: originalUrl, target: '_blank'}, testId('open-original'), + cssMenuSplitLinkText(t("Return to {{termToUse}}", {termToUse})), + cssMenuIconLink({href: originalUrl, target: '_blank'}, cssMenuIcon('FieldLink'), - ) + testId('open-original'), + ), + dom.on('click', () => { pageModel.clearUnsavedChanges(); }), + testId('return-to-original'), ), menuItem(replaceOriginal, t("Replace {{termToUse}}...", {termToUse}), // Disable if original is not writable, and also when comparing snapshots (since it's @@ -201,6 +218,7 @@ function menuOriginal(doc: Document, appModel: AppModel, options: MenuOriginalOp ), isTutorialFork ? null : menuItemLink(compareHref, {target: '_blank'}, t("Compare to {{termToUse}}", {termToUse}), menuAnnotate('Beta'), + dom.on('click', () => { pageModel.clearUnsavedChanges(); }), testId('compare-original'), ), ]; @@ -208,8 +226,13 @@ function menuOriginal(doc: Document, appModel: AppModel, options: MenuOriginalOp // Renders "Save Copy..." and "Copy as Template..." menu items. The name of the first action is // specified in saveActionTitle. -function menuSaveCopy(saveActionTitle: string, doc: Document, appModel: AppModel) { - const saveCopy = () => makeCopy(doc, appModel, saveActionTitle).catch(reportError); +function menuSaveCopy(options: { + pageModel: DocPageModel, + doc: Document, + saveActionTitle: string, +}) { + const {pageModel, doc, saveActionTitle} = options; + const saveCopy = () => handleSaveCopy({pageModel, doc, modalTitle: saveActionTitle}); return [ // TODO Disable these when user has no accessible destinations. menuItem(saveCopy, `${saveActionTitle}...`, testId('save-copy')), diff --git a/app/client/ui/WelcomePage.ts b/app/client/ui/WelcomePage.ts index 87dada3f..dda7d865 100644 --- a/app/client/ui/WelcomePage.ts +++ b/app/client/ui/WelcomePage.ts @@ -99,7 +99,7 @@ export class WelcomePage extends Disposable { `If you already have a Grist account as `, dom('b', email.get()), ` you can just `, - cssLink({href: getLoginUrl('')}, 'log in'), + cssLink({href: getLoginUrl({nextUrl: null})}, 'log in'), ` now. Otherwise, please pick a password.` ), cssSeparatedLabel('The email address you activated Grist with:'), @@ -184,7 +184,7 @@ export class WelcomePage extends Disposable { 'Apply verification code' : 'Resend verification email') ), bigBasicButtonLink('More sign-up options', - {href: getSignupUrl('')}) + {href: getSignupUrl({nextUrl: null})}) ) ); } diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 5a9b7159..4617c055 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -340,6 +340,11 @@ export interface DocStateComparisonDetails { rightChanges: ActionSummary; } +export interface CopyDocOptions { + documentName: string; + asTemplate?: boolean; +} + export interface UserAPI { getSessionActive(): Promise; setSessionActive(email: string, org?: string): Promise; @@ -355,6 +360,7 @@ export interface UserAPI { newWorkspace(props: Partial, orgId: number|string): Promise; newDoc(props: Partial, workspaceId: number): Promise; newUnsavedDoc(options?: {timezone?: string}): Promise; + copyDoc(sourceDocumentId: string, workspaceId: number, options: CopyDocOptions): Promise; renameOrg(orgId: number|string, name: string): Promise; renameWorkspace(workspaceId: number, name: string): Promise; renameDoc(docId: string, name: string): Promise; @@ -569,6 +575,21 @@ export class UserAPIImpl extends BaseAPI implements UserAPI { }); } + public async copyDoc( + sourceDocumentId: string, + workspaceId: number, + options: CopyDocOptions + ): Promise { + return this.requestJson(`${this._url}/api/docs`, { + method: 'POST', + body: JSON.stringify({ + sourceDocumentId, + workspaceId, + ...options, + }), + }); + } + public async renameOrg(orgId: number|string, name: string): Promise { await this.request(`${this._url}/api/orgs/${orgId}`, { method: 'PATCH', @@ -984,6 +1005,14 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { return this.requestJson(`${this._url}/compare/${remoteDocId}${q}`); } + public async copyDoc(workspaceId: number, options: CopyDocOptions): Promise { + const {documentName, asTemplate} = options; + return this.requestJson(`${this._url}/copy`, { + body: JSON.stringify({workspaceId, documentName, asTemplate}), + method: 'POST' + }); + } + public async compareVersion(leftHash: string, rightHash: string): Promise { const url = new URL(`${this._url}/compare`); url.searchParams.append('left', leftHash); diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index dc0ca1c0..91897d04 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -124,6 +124,7 @@ export interface IGristUrlState { billingTask?: BillingTask; embed?: boolean; state?: string; + srcDocId?: string; style?: InterfaceStyle; compare?: string; linkParameters?: Record; // Parameters to pass as 'user.Link' in granular ACLs. @@ -395,7 +396,9 @@ export function decodeUrl(gristConfig: Partial, location: Locat if (sp.has('state')) { state.params!.state = sp.get('state')!; } - + if (sp.has('srcDocId')) { + state.params!.srcDocId = sp.get('srcDocId')!; + } if (sp.has('style')) { let style = sp.get('style'); if (style === 'light') { diff --git a/app/server/lib/AppEndpoint.ts b/app/server/lib/AppEndpoint.ts index 41968271..5af03097 100644 --- a/app/server/lib/AppEndpoint.ts +++ b/app/server/lib/AppEndpoint.ts @@ -4,11 +4,9 @@ * of the client-side code. */ import * as express from 'express'; -import fetch, {Response as FetchResponse, RequestInit} from 'node-fetch'; import {ApiError} from 'app/common/ApiError'; -import {getSlugIfNeeded, parseSubdomainStrictly, parseUrlId} from 'app/common/gristUrls'; -import {removeTrailingSlash} from 'app/common/gutil'; +import {getSlugIfNeeded, parseUrlId} from 'app/common/gristUrls'; import {LocalPlugin} from "app/common/plugin"; import {TELEMETRY_TEMPLATE_SIGNUP_COOKIE_NAME} from 'app/common/Telemetry'; import {Document as APIDocument} from 'app/common/UserAPI'; @@ -17,13 +15,14 @@ import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager'; import {assertAccess, getTransitiveHeaders, getUserId, isAnonymousUser, RequestWithLogin} from 'app/server/lib/Authorizer'; import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; +import {customizeDocWorkerUrl, getWorker, useWorkerPool} from 'app/server/lib/DocWorkerUtils'; import {expressWrap} from 'app/server/lib/expressWrap'; import {DocTemplate, GristServer} from 'app/server/lib/GristServer'; import {getCookieDomain} from 'app/server/lib/gristSessions'; import {getTemplateOrg} from 'app/server/lib/gristSettings'; import {getAssignmentId} from 'app/server/lib/idUtils'; import log from 'app/server/lib/log'; -import {adaptServerUrl, addOrgToPathIfNeeded, pruneAPIResult, trustOrigin} from 'app/server/lib/requestUtils'; +import {addOrgToPathIfNeeded, pruneAPIResult, trustOrigin} from 'app/server/lib/requestUtils'; import {ISendAppPageOptions} from 'app/server/lib/sendAppPage'; export interface AttachOptions { @@ -38,144 +37,6 @@ export interface AttachOptions { gristServer: GristServer; } -/** - * This method transforms a doc worker's public url as needed based on the request. - * - * For historic reasons, doc workers are assigned a public url at the time - * of creation. In production/staging, this is of the form: - * https://doc-worker-NNN-NNN-NNN-NNN.getgrist.com/v/VVVV/ - * and in dev: - * http://localhost:NNNN/v/VVVV/ - * - * Prior to support for different base domains, this was fine. Now that different - * base domains are supported, a wrinkle arises. When a web client communicates - * with a doc worker, it is important that it accesses the doc worker via a url - * containing the same base domain as the web page the client is on (for cookie - * purposes). Hence this method. - * - * If both the request and docWorkerUrl contain identifiable base domains (not localhost), - * then the base domain of docWorkerUrl is replaced with that of the request. - * - * But wait, there's another wrinkle: custom domains. In this case, we have a single - * domain available to serve a particular org from. This method will use the origin of req - * and include a /dw/doc-worker-NNN-NNN-NNN-NNN/ - * (or /dw/local-NNNN/) prefix in all doc worker paths. Once this is in place, it - * will allow doc worker routing to be changed so it can be overlaid on a custom - * domain. - * - * TODO: doc worker registration could be redesigned to remove the assumption - * of a fixed base domain. - */ -function customizeDocWorkerUrl(docWorkerUrlSeed: string|undefined, req: express.Request): string|null { - if (!docWorkerUrlSeed) { - // When no doc worker seed, we're in single server mode. - // Return null, to signify that the URL prefix serving the - // current endpoint is the only one available. - return null; - } - const docWorkerUrl = new URL(docWorkerUrlSeed); - const workerSubdomain = parseSubdomainStrictly(docWorkerUrl.hostname).org; - adaptServerUrl(docWorkerUrl, req); - - // We wish to migrate to routing doc workers by path, so insert a doc worker identifier - // in the path (if not already present). - if (!docWorkerUrl.pathname.startsWith('/dw/')) { - // When doc worker is localhost, the port number is necessary and sufficient for routing. - // Let's add a /dw/... prefix just for consistency. - const workerIdent = workerSubdomain || `local-${docWorkerUrl.port}`; - docWorkerUrl.pathname = `/dw/${workerIdent}${docWorkerUrl.pathname}`; - } - return docWorkerUrl.href; -} - -/** - * - * Gets the worker responsible for a given assignment, and fetches a url - * from the worker. - * - * If the fetch fails, we throw an exception, unless we see enough evidence - * to unassign the worker and try again. - * - * - If GRIST_MANAGED_WORKERS is set, we assume that we've arranged - * for unhealthy workers to be removed automatically, and that if a - * fetch returns a 404 with specific content, it is proof that the - * worker is no longer in existence. So if we see a 404 with that - * specific content, we can safely de-list the worker from redis, - * and repeat. - * - If GRIST_MANAGED_WORKERS is not set, we accept a broader set - * of failures as evidence of a missing worker. - * - * The specific content of a 404 that will be treated as evidence of - * a doc worker not being present is: - * - A json format body - * - With a key called "message" - * - With the value of "message" being "document worker not present" - * In production, this is provided by a special doc-worker-* load balancer - * rule. - * - */ -async function getWorker(docWorkerMap: IDocWorkerMap, assignmentId: string, - urlPath: string, config: RequestInit = {}) { - if (!useWorkerPool()) { - // This should never happen. We are careful to not use getWorker - // when everything is on a single server, since it is burdensome - // for self-hosted users to figure out the correct settings for - // the server to be able to contact itself, and there are cases - // of the defaults not working. - throw new Error("AppEndpoint.getWorker was called unnecessarily"); - } - let docStatus: DocStatus|undefined; - const workersAreManaged = Boolean(process.env.GRIST_MANAGED_WORKERS); - for (;;) { - docStatus = await docWorkerMap.assignDocWorker(assignmentId); - const configWithTimeout = {timeout: 10000, ...config}; - const fullUrl = removeTrailingSlash(docStatus.docWorker.internalUrl) + urlPath; - try { - const resp: FetchResponse = await fetch(fullUrl, configWithTimeout); - if (resp.ok) { - return { - resp, - docStatus, - }; - } - if (resp.status === 403) { - throw new ApiError("You do not have access to this document.", resp.status); - } - if (resp.status !== 404) { - throw new ApiError(resp.statusText, resp.status); - } - let body: any; - try { - body = await resp.json(); - } catch (e) { - throw new ApiError(resp.statusText, resp.status); - } - if (!(body && body.message && body.message === 'document worker not present')) { - throw new ApiError(resp.statusText, resp.status); - } - // This is a 404 with the expected content for a missing worker. - } catch (e) { - log.rawDebug(`AppEndpoint.getWorker failure`, { - url: fullUrl, - docId: assignmentId, - status: e.status, - message: String(e), - workerId: docStatus.docWorker.id, - }); - // If workers are managed, no errors merit continuing except a 404. - // Otherwise, we continue if we see a system error (e.g. ECONNREFUSED). - // We don't accept timeouts since there is too much potential to - // bring down a single-worker deployment that has a hiccup. - if (workersAreManaged || !(e.type === 'system')) { - throw e; - } - } - log.warn(`fetch from ${fullUrl} failed convincingly, removing that worker`); - await docWorkerMap.removeWorker(docStatus.docWorker.id); - docStatus = undefined; - } -} - export function attachAppEndpoint(options: AttachOptions): void { const {app, middleware, docMiddleware, docWorkerMap, forceLogin, sendAppPage, dbManager, plugins, gristServer} = options; @@ -358,8 +219,3 @@ export function attachAppEndpoint(options: AttachOptions): void { app.get('/:urlId([^/]{12,})/:slug([^/]+):remainder(*)', ...docMiddleware, docHandler); } - -// Return true if document related endpoints are served by separate workers. -function useWorkerPool() { - return process.env.GRIST_SINGLE_PORT !== 'true'; -} diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 1b707d7c..b883c5b0 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -36,6 +36,7 @@ import {appSettings} from "app/server/lib/AppSettings"; import {sendForCompletion} from 'app/server/lib/Assistance'; import { assertAccess, + getAuthorizedUserId, getOrSetDocAuth, getTransitiveHeaders, getUserId, @@ -64,6 +65,7 @@ import { getScope, integerParam, isParameterOn, + optBooleanParam, optIntegerParam, optStringParam, sendOkReply, @@ -73,7 +75,8 @@ import { import {ServerColumnGetters} from 'app/server/lib/ServerColumnGetters'; import {localeFromRequest} from "app/server/lib/ServerLocale"; import {isUrlAllowed, WebhookAction, WebHookSecret} from "app/server/lib/Triggers"; -import {handleOptionalUpload, handleUpload} from "app/server/lib/uploads"; +import {fetchDoc, globalUploadSet, handleOptionalUpload, handleUpload, + makeAccessId} from "app/server/lib/uploads"; import * as assert from 'assert'; import contentDisposition from 'content-disposition'; import {Application, NextFunction, Request, RequestHandler, Response} from "express"; @@ -1225,7 +1228,11 @@ export class DocWorkerApi { * Create a document. * * When an upload is included, it is imported as the initial state of the document. - * Otherwise, the document is left empty. + * + * When a source document id is included, its structure and (optionally) data is + * included in the new document. + * + * In all other cases, the document is left empty. * * If a workspace id is included, the document will be saved there instead of * being left "unsaved". @@ -1249,54 +1256,117 @@ export class DocWorkerApi { parameters = req.body; } - const documentName = optStringParam(parameters.documentName, 'documentName', { - allowEmpty: false, - }); + const sourceDocumentId = optStringParam(parameters.sourceDocumentId, 'sourceDocumentId'); const workspaceId = optIntegerParam(parameters.workspaceId, 'workspaceId'); const browserSettings: BrowserSettings = {}; if (parameters.timezone) { browserSettings.timezone = parameters.timezone; } browserSettings.locale = localeFromRequest(req); let docId: string; - if (uploadId !== undefined) { + if (sourceDocumentId !== undefined) { + docId = await this._copyDocToWorkspace(req, { + userId, + sourceDocumentId, + workspaceId: integerParam(parameters.workspaceId, 'workspaceId'), + documentName: stringParam(parameters.documentName, 'documentName'), + asTemplate: optBooleanParam(parameters.asTemplate, 'asTemplate'), + }); + } else if (uploadId !== undefined) { const result = await this._docManager.importDocToWorkspace({ userId, uploadId, - documentName, + documentName: optStringParam(parameters.documentName, 'documentName'), workspaceId, browserSettings, }); docId = result.id; } else if (workspaceId !== undefined) { - const {status, data, errMessage} = await this._dbManager.addDocument(getScope(req), workspaceId, { - name: documentName ?? 'Untitled document', + docId = await this._createNewSavedDoc(req, { + workspaceId: workspaceId, + documentName: optStringParam(parameters.documentName, 'documentName'), }); - if (status !== 200) { - throw new ApiError(errMessage || 'unable to create document', status); - } - - docId = data!; } else { - const isAnonymous = isAnonymousUser(req); - const result = makeForkIds({ + docId = await this._createNewUnsavedDoc(req, { userId, - isAnonymous, - trunkDocId: NEW_DOCUMENT_CODE, - trunkUrlId: NEW_DOCUMENT_CODE, + browserSettings, }); - docId = result.docId; - await this._docManager.createNamedDoc( - makeExceptionalDocSession('nascent', { - req: req as RequestWithLogin, - browserSettings, - }), - docId - ); } return res.status(200).json(docId); })); } + + private async _copyDocToWorkspace(req: Request, options: { + userId: number, + sourceDocumentId: string, + workspaceId: number, + documentName: string, + asTemplate?: boolean, + }): Promise { + const {userId, sourceDocumentId, workspaceId, documentName, asTemplate = false} = options; + + // First, upload a copy of the document. + let uploadResult; + try { + const accessId = makeAccessId(req, getAuthorizedUserId(req)); + uploadResult = await fetchDoc(this._grist, sourceDocumentId, req, accessId, asTemplate); + globalUploadSet.changeUploadName(uploadResult.uploadId, accessId, `${documentName}.grist`); + } catch (err) { + if ((err as ApiError).status === 403) { + throw new ApiError('Insufficient access to document to copy it entirely', 403); + } + + throw err; + } + + // Then, import the copy to the workspace. + const result = await this._docManager.importDocToWorkspace({ + userId, + uploadId: uploadResult.uploadId, + documentName, + workspaceId, + }); + return result.id; + } + + private async _createNewSavedDoc(req: Request, options: { + workspaceId: number, + documentName?: string, + }): Promise { + const {documentName, workspaceId} = options; + const {status, data, errMessage} = await this._dbManager.addDocument(getScope(req), workspaceId, { + name: documentName ?? 'Untitled document', + }); + if (status !== 200) { + throw new ApiError(errMessage || 'unable to create document', status); + } + + return data!; + } + + private async _createNewUnsavedDoc(req: Request, options: { + userId: number, + browserSettings?: BrowserSettings, + }): Promise { + const {userId, browserSettings} = options; + const isAnonymous = isAnonymousUser(req); + const result = makeForkIds({ + userId, + isAnonymous, + trunkDocId: NEW_DOCUMENT_CODE, + trunkUrlId: NEW_DOCUMENT_CODE, + }); + const docId = result.docId; + await this._docManager.createNamedDoc( + makeExceptionalDocSession('nascent', { + req: req as RequestWithLogin, + browserSettings, + }), + docId + ); + return docId; + } + /** * Check for read access to the given document, and return its * canonical docId. Throws error if read access not available. diff --git a/app/server/lib/DocWorkerUtils.ts b/app/server/lib/DocWorkerUtils.ts new file mode 100644 index 00000000..1396b876 --- /dev/null +++ b/app/server/lib/DocWorkerUtils.ts @@ -0,0 +1,158 @@ +import {ApiError} from 'app/common/ApiError'; +import {parseSubdomainStrictly} from 'app/common/gristUrls'; +import {removeTrailingSlash} from 'app/common/gutil'; +import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap'; +import log from 'app/server/lib/log'; +import {adaptServerUrl} from 'app/server/lib/requestUtils'; +import * as express from 'express'; +import fetch, {Response as FetchResponse, RequestInit} from 'node-fetch'; + +/** + * This method transforms a doc worker's public url as needed based on the request. + * + * For historic reasons, doc workers are assigned a public url at the time + * of creation. In production/staging, this is of the form: + * https://doc-worker-NNN-NNN-NNN-NNN.getgrist.com/v/VVVV/ + * and in dev: + * http://localhost:NNNN/v/VVVV/ + * + * Prior to support for different base domains, this was fine. Now that different + * base domains are supported, a wrinkle arises. When a web client communicates + * with a doc worker, it is important that it accesses the doc worker via a url + * containing the same base domain as the web page the client is on (for cookie + * purposes). Hence this method. + * + * If both the request and docWorkerUrl contain identifiable base domains (not localhost), + * then the base domain of docWorkerUrl is replaced with that of the request. + * + * But wait, there's another wrinkle: custom domains. In this case, we have a single + * domain available to serve a particular org from. This method will use the origin of req + * and include a /dw/doc-worker-NNN-NNN-NNN-NNN/ + * (or /dw/local-NNNN/) prefix in all doc worker paths. Once this is in place, it + * will allow doc worker routing to be changed so it can be overlaid on a custom + * domain. + * + * TODO: doc worker registration could be redesigned to remove the assumption + * of a fixed base domain. + */ +export function customizeDocWorkerUrl( + docWorkerUrlSeed: string|undefined, + req: express.Request +): string|null { + if (!docWorkerUrlSeed) { + // When no doc worker seed, we're in single server mode. + // Return null, to signify that the URL prefix serving the + // current endpoint is the only one available. + return null; + } + const docWorkerUrl = new URL(docWorkerUrlSeed); + const workerSubdomain = parseSubdomainStrictly(docWorkerUrl.hostname).org; + adaptServerUrl(docWorkerUrl, req); + + // We wish to migrate to routing doc workers by path, so insert a doc worker identifier + // in the path (if not already present). + if (!docWorkerUrl.pathname.startsWith('/dw/')) { + // When doc worker is localhost, the port number is necessary and sufficient for routing. + // Let's add a /dw/... prefix just for consistency. + const workerIdent = workerSubdomain || `local-${docWorkerUrl.port}`; + docWorkerUrl.pathname = `/dw/${workerIdent}${docWorkerUrl.pathname}`; + } + return docWorkerUrl.href; +} + +/** + * + * Gets the worker responsible for a given assignment, and fetches a url + * from the worker. + * + * If the fetch fails, we throw an exception, unless we see enough evidence + * to unassign the worker and try again. + * + * - If GRIST_MANAGED_WORKERS is set, we assume that we've arranged + * for unhealthy workers to be removed automatically, and that if a + * fetch returns a 404 with specific content, it is proof that the + * worker is no longer in existence. So if we see a 404 with that + * specific content, we can safely de-list the worker from redis, + * and repeat. + * - If GRIST_MANAGED_WORKERS is not set, we accept a broader set + * of failures as evidence of a missing worker. + * + * The specific content of a 404 that will be treated as evidence of + * a doc worker not being present is: + * - A json format body + * - With a key called "message" + * - With the value of "message" being "document worker not present" + * In production, this is provided by a special doc-worker-* load balancer + * rule. + * + */ +export async function getWorker( + docWorkerMap: IDocWorkerMap, + assignmentId: string, + urlPath: string, + config: RequestInit = {} +) { + if (!useWorkerPool()) { + // This should never happen. We are careful to not use getWorker + // when everything is on a single server, since it is burdensome + // for self-hosted users to figure out the correct settings for + // the server to be able to contact itself, and there are cases + // of the defaults not working. + throw new Error("AppEndpoint.getWorker was called unnecessarily"); + } + let docStatus: DocStatus|undefined; + const workersAreManaged = Boolean(process.env.GRIST_MANAGED_WORKERS); + for (;;) { + docStatus = await docWorkerMap.assignDocWorker(assignmentId); + const configWithTimeout = {timeout: 10000, ...config}; + const fullUrl = removeTrailingSlash(docStatus.docWorker.internalUrl) + urlPath; + try { + const resp: FetchResponse = await fetch(fullUrl, configWithTimeout); + if (resp.ok) { + return { + resp, + docStatus, + }; + } + if (resp.status === 403) { + throw new ApiError("You do not have access to this document.", resp.status); + } + if (resp.status !== 404) { + throw new ApiError(resp.statusText, resp.status); + } + let body: any; + try { + body = await resp.json(); + } catch (e) { + throw new ApiError(resp.statusText, resp.status); + } + if (!(body && body.message && body.message === 'document worker not present')) { + throw new ApiError(resp.statusText, resp.status); + } + // This is a 404 with the expected content for a missing worker. + } catch (e) { + log.rawDebug(`AppEndpoint.getWorker failure`, { + url: fullUrl, + docId: assignmentId, + status: e.status, + message: String(e), + workerId: docStatus.docWorker.id, + }); + // If workers are managed, no errors merit continuing except a 404. + // Otherwise, we continue if we see a system error (e.g. ECONNREFUSED). + // We don't accept timeouts since there is too much potential to + // bring down a single-worker deployment that has a hiccup. + if (workersAreManaged || !(e.type === 'system')) { + throw e; + } + } + log.warn(`fetch from ${fullUrl} failed convincingly, removing that worker`); + await docWorkerMap.removeWorker(docStatus.docWorker.id); + docStatus = undefined; + } +} + +// Return true if document related endpoints are served by separate workers. +export function useWorkerPool() { + return process.env.GRIST_SINGLE_PORT !== 'true'; +} diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index cb60b1d2..feadfa3f 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -5,6 +5,7 @@ import {encodeUrl, getSlugIfNeeded, GristDeploymentType, GristDeploymentTypes, GristLoadConfig, IGristUrlState, isOrgInPathOnly, parseSubdomain, sanitizePathTail} from 'app/common/gristUrls'; import {getOrgUrlInfo} from 'app/common/gristUrls'; +import {safeJsonParse} from 'app/common/gutil'; import {InstallProperties} from 'app/common/InstallAPI'; import {UserProfile} from 'app/common/LoginSessionAPI'; import {tbind} from 'app/common/tbind'; @@ -22,7 +23,7 @@ import {Usage} from 'app/gen-server/lib/Usage'; import {AccessTokens, IAccessTokens} from 'app/server/lib/AccessTokens'; import {attachAppEndpoint} from 'app/server/lib/AppEndpoint'; import {appSettings} from 'app/server/lib/AppSettings'; -import {addRequestUser, getUser, getUserId, isAnonymousUser, +import {addRequestUser, getTransitiveHeaders, getUser, getUserId, isAnonymousUser, isSingleUserMode, redirectToLoginUnconditionally} from 'app/server/lib/Authorizer'; import {redirectToLogin, RequestWithLogin, signInStatusMiddleware} from 'app/server/lib/Authorizer'; import {forceSessionChange} from 'app/server/lib/BrowserSession'; @@ -67,6 +68,7 @@ import {buildWidgetRepository, IWidgetRepository} from 'app/server/lib/WidgetRep import {setupLocale} from 'app/server/localization'; import axios from 'axios'; import * as bodyParser from 'body-parser'; +import * as cookie from 'cookie'; import express from 'express'; import * as fse from 'fs-extra'; import * as http from 'http'; @@ -877,6 +879,13 @@ export class FlexServer implements GristServer { // Give a chance to the login system to react to the first visit after signup. this._loginMiddleware.onFirstVisit?.(req); + // If we need to copy an unsaved document or template as part of sign-up, do so now + // and redirect to it. + const docId = await this._maybeCopyDocToHomeWorkspace(mreq, res); + if (docId) { + return res.redirect(this.getMergedOrgUrl(mreq, `/doc/${docId}`)); + } + const domain = mreq.org ?? null; if (!process.env.GRIST_SINGLE_ORG && this._dbManager.isMergedOrg(domain)) { // We're logging in for the first time on the merged org; if the user has @@ -1915,6 +1924,66 @@ export class FlexServer implements GristServer { resp.redirect(redirectToMergedOrg ? this.getMergedOrgUrl(mreq) : getOrgUrl(mreq)); } } + + /** + * If a valid cookie was set during sign-up to copy a document to the + * user's Home workspace, copy it and return the id of the new document. + * + * If a valid cookie wasn't set or copying failed, return `null`. + */ + private async _maybeCopyDocToHomeWorkspace( + req: RequestWithLogin, + resp: express.Response + ): Promise { + const cookies = cookie.parse(req.headers.cookie || ''); + if (!cookies) { return null; } + + const stateCookie = cookies['gr_signup_state']; + if (!stateCookie) { return null; } + + const state = safeJsonParse(stateCookie, {}); + const {srcDocId} = state; + if (!srcDocId) { return null; } + + let newDocId: string | null = null; + try { + newDocId = await this._copyDocToHomeWorkspace(req, srcDocId); + } catch (e) { + log.error(`FlexServer failed to copy doc ${srcDocId} to Home workspace`, e); + } finally { + resp.clearCookie('gr_signup_state'); + } + return newDocId; + } + + private async _copyDocToHomeWorkspace( + req: express.Request, + docId: string, + ): Promise { + const userId = getUserId(req); + const doc = await this._dbManager.getDoc({userId, urlId: docId}); + if (!doc) { throw new Error(`Doc ${docId} not found`); } + + const workspacesQueryResult = await this._dbManager.getOrgWorkspaces(getScope(req), 0); + const workspaces = this._dbManager.unwrapQueryResult(workspacesQueryResult); + const workspace = workspaces.find(w => w.name === 'Home'); + if (!workspace) { throw new Error('Home workspace not found'); } + + const copyDocUrl = this.getHomeUrl(req, '/api/docs'); + const response = await fetch(copyDocUrl, { + headers: { + ...getTransitiveHeaders(req), + 'Content-Type': 'application/json', + }, + method: 'POST', + body: JSON.stringify({ + sourceDocumentId: doc.id, + workspaceId: workspace.id, + documentName: doc.name, + }), + }); + return await response.json(); + } } /** diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 3bf6d88e..a093da8d 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -300,6 +300,17 @@ export function integerParam(p: any, name: string): number { throw new ApiError(`${name} parameter should be an integer: ${p}`, 400); } +export function optBooleanParam(p: any, name: string): boolean|undefined { + if (p === undefined) { return p; } + + return booleanParam(p, name); +} + +export function booleanParam(p: any, name: string): boolean { + if (typeof p === 'boolean') { return p; } + throw new ApiError(`${name} parameter should be a boolean: ${p}`, 400); +} + export function optJsonParam(p: any, defaultValue: any): any { if (typeof p !== 'string') { return defaultValue; } return gutil.safeJsonParse(p, defaultValue); diff --git a/app/server/lib/uploads.ts b/app/server/lib/uploads.ts index e67a083b..a3eb4582 100644 --- a/app/server/lib/uploads.ts +++ b/app/server/lib/uploads.ts @@ -404,7 +404,7 @@ async function _fetchURL(url: string, accessId: string|null, options?: FetchUrlO * Fetches a Grist doc potentially managed by a different doc worker. Passes on credentials * supplied in the current request. */ -async function fetchDoc(server: GristServer, docId: string, req: Request, accessId: string|null, +export async function fetchDoc(server: GristServer, docId: string, req: Request, accessId: string|null, template: boolean): Promise { // Prepare headers that preserve credentials of current user. const headers = getTransitiveHeaders(req); diff --git a/test/nbrowser/CellColor.ts b/test/nbrowser/CellColor.ts index 3bec5dc0..4026447e 100644 --- a/test/nbrowser/CellColor.ts +++ b/test/nbrowser/CellColor.ts @@ -152,7 +152,7 @@ describe('CellColor', function() { const forkId = await gu.getCurrentUrlId(); // Compare with the original - await mainSession.loadDoc(`/doc/${doc}?compare=${forkId}`); + await mainSession.loadDoc(`/doc/${doc}?compare=${forkId}`, {skipAlert: true}); // check that colors for diffing cells are ok cell = gu.getCell('A', 1).find('.field_clip'); diff --git a/test/nbrowser/DocTutorial.ts b/test/nbrowser/DocTutorial.ts index 8a7f004f..f52e34ba 100644 --- a/test/nbrowser/DocTutorial.ts +++ b/test/nbrowser/DocTutorial.ts @@ -57,7 +57,7 @@ describe('DocTutorial', function () { }); it('redirects user to log in', async function() { - await viewerSession.loadDoc(`/doc/${doc.id}`, false); + await viewerSession.loadDoc(`/doc/${doc.id}`, {wait: false}); await gu.checkLoginPage(); }); }); diff --git a/test/nbrowser/Fork.ts b/test/nbrowser/Fork.ts index 1a5d1ed4..ae884060 100644 --- a/test/nbrowser/Fork.ts +++ b/test/nbrowser/Fork.ts @@ -8,7 +8,7 @@ import uuidv4 from "uuid/v4"; describe("Fork", function() { // this is a relatively slow test in staging. - this.timeout(40000); + this.timeout(60000); const cleanup = setupTestSuite(); let doc: DocCreationInfo; @@ -45,6 +45,7 @@ describe("Fork", function() { assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1'); const forkUrl = await driver.getCurrentUrl(); assert.match(forkUrl, isLoggedIn ? /^[^~]*~[^~]+~\d+$/ : /^[^~]*~[^~]*$/); + await gu.refreshDismiss(); await session.loadDoc((new URL(forkUrl)).pathname + '/m/fork'); await gu.getCell({rowNum: 1, col: 0}).click(); await gu.enterCell('2'); @@ -57,6 +58,7 @@ describe("Fork", function() { assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1'); assert.equal(await driver.getCurrentUrl(), `${forkUrl}/m/fork`); await gu.wipeToasts(); + await gu.refreshDismiss(); } // Run tests with both regular docId and a custom urlId in URL, to make sure @@ -120,6 +122,7 @@ describe("Fork", function() { await gu.enterCell('123'); await gu.waitForServer(); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '123'); + await gu.refreshDismiss(); // edits should persist across reloads await personal.loadDoc(`/doc/${id}`); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '123'); @@ -128,16 +131,17 @@ describe("Fork", function() { await gu.enterCell('234'); await gu.waitForServer(); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '234'); + await gu.refreshDismiss(); if (mode !== 'anonymous') { // if we log out, access should now be denied const anonSession = await personal.anon.login(); - await anonSession.loadDoc(`/doc/${id}`, false); + await anonSession.loadDoc(`/doc/${id}`, {wait: false}); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); // if we log in as a different user, access should be denied const altSession = await personal.user('user2').login(); - await altSession.loadDoc(`/doc/${id}`, false); + await altSession.loadDoc(`/doc/${id}`, {wait: false}); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); } }); @@ -179,6 +183,7 @@ describe("Fork", function() { assert.match(forkUrl, /~/); // check that the tag is there assert.equal(await driver.find('.test-unsaved-tag').isPresent(), true); + await gu.refreshDismiss(); // Open the original url and make sure the change we made is not there await anonSession.loadDoc(`/doc/${doc.id}`); @@ -202,6 +207,7 @@ describe("Fork", function() { await gu.waitForServer(); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1'); const fork1 = await driver.getCurrentUrl(); + await gu.refreshDismiss(); await anonSession.loadDoc(`/doc/${doc.id}/m/fork`); await gu.getCell({rowNum: 1, col: 0}).click(); @@ -209,6 +215,7 @@ describe("Fork", function() { await gu.waitForServer(); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '2'); const fork2 = await driver.getCurrentUrl(); + await gu.refreshDismiss(); await anonSession.loadDoc((new URL(fork1)).pathname); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1'); @@ -229,6 +236,7 @@ describe("Fork", function() { const urlId1 = await gu.getCurrentUrlId(); assert.match(urlId1!, /~/); assert.match(await driver.find('.test-treeview-itemHeader.selected').getText(), /Table2/); + await gu.refreshDismiss(); }); for (const user of [{access: 'viewers', name: 'user3'}, @@ -258,6 +266,7 @@ describe("Fork", function() { const fork = forks.find(f => f.id === forkId); assert.isDefined(fork); assert.equal(fork!.trunkId, doc.id); + await gu.refreshDismiss(); // Open the original url and make sure the change we made is not there await userSession.loadDoc(`/doc/${doc.id}`); @@ -274,6 +283,7 @@ describe("Fork", function() { await gu.getCell({rowNum: 1, col: 0}).click(); await gu.enterCell('1234'); await gu.waitForServer(); + await gu.refreshDismiss(); await userSession.loadDoc((new URL(forkUrl)).pathname); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1234'); @@ -286,6 +296,7 @@ describe("Fork", function() { await gu.getCell({rowNum: 1, col: 0}).click(); await gu.enterCell('12345'); await gu.waitForServer(); + await gu.refreshDismiss(); await team.loadDoc((new URL(forkUrl)).pathname); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1234'); @@ -296,6 +307,7 @@ describe("Fork", function() { await gu.getCell({rowNum: 1, col: 0}).click(); await gu.enterCell('12345'); await gu.waitForServer(); + await gu.refreshDismiss(); await anonSession.loadDoc((new URL(forkUrl)).pathname); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1234'); }); @@ -314,6 +326,7 @@ describe("Fork", function() { // The url of the doc should now be that of a fork const forkUrl = await driver.getCurrentUrl(); assert.match(forkUrl, /~/); + await gu.refreshDismiss(); // Open the original url and make sure the change we made is not there await team.loadDoc(`/doc/${doc2.id}`); @@ -327,6 +340,7 @@ describe("Fork", function() { await gu.getCell({rowNum: 1, col: 0}).click(); await gu.enterCell('1234'); await gu.waitForServer(); + await gu.refreshDismiss(); await team.loadDoc((new URL(forkUrl)).pathname); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '1234'); @@ -350,7 +364,7 @@ describe("Fork", function() { await team.login(); const userId = await team.getUserId(); const altSession = await team.user('user2').login(); - await altSession.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, false); + await altSession.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, {wait: false}); assert.equal(await driver.findWait('.test-modal-dialog', 2000).isDisplayed(), true); assert.match(await driver.find('.test-modal-dialog').getText(), /Document fork not found/); @@ -365,30 +379,30 @@ describe("Fork", function() { // would have some access to fork via acls on trunk, but for a // new doc user2 has no access granted via the doc, or // workspace, or org). - await altSession.loadDoc(`/doc/new~${forkId}~${userId}`, false); + await altSession.loadDoc(`/doc/new~${forkId}~${userId}`, {wait: false}); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); assert.equal(await driver.find('.test-dm-logo').isDisplayed(), true); // Same, but as an anonymous user. const anonSession = await altSession.anon.login(); - await anonSession.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, false); + await anonSession.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, {wait: false}); assert.equal(await driver.findWait('.test-modal-dialog', 2000).isDisplayed(), true); assert.match(await driver.find('.test-modal-dialog').getText(), /Document fork not found/); // A new doc cannot be created either (because of access mismatch). - await altSession.loadDoc(`/doc/new~${forkId}~${userId}`, false); + await altSession.loadDoc(`/doc/new~${forkId}~${userId}`, {wait: false}); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); assert.equal(await driver.find('.test-dm-logo').isDisplayed(), true); // Now as a user who *is* allowed to create the fork. // But doc forks cannot be casually created this way anymore, so it still doesn't work. await team.login(); - await team.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, false); + await team.loadDoc(`/doc/${doc.id}~${forkId}~${userId}`, {wait: false}); assert.equal(await driver.findWait('.test-modal-dialog', 2000).isDisplayed(), true); assert.match(await driver.find('.test-modal-dialog').getText(), /Document fork not found/); // New document can no longer be casually created this way anymore either. - await team.loadDoc(`/doc/new~${forkId}~${userId}`, false); + await team.loadDoc(`/doc/new~${forkId}~${userId}`, {wait: false}); assert.equal(await driver.findWait('.test-modal-dialog', 2000).isDisplayed(), true); assert.match(await driver.find('.test-modal-dialog').getText(), /Document fork not found/); await gu.wipeToasts(); @@ -406,6 +420,7 @@ describe("Fork", function() { await gu.waitForServer(); const forkUrl = await driver.getCurrentUrl(); assert.match(forkUrl, /~/); + await gu.refreshDismiss(); // check that there is no tag on trunk await team.loadDoc(`/doc/${trunk.id}`); @@ -444,6 +459,7 @@ describe("Fork", function() { await gu.waitForDocToLoad(); assert.equal(await gu.getCell({rowNum: 1, col: 0}).getText(), '2'); assert.equal(await driver.find('.test-unsaved-tag').isPresent(), true); + await gu.refreshDismiss(); }); it('disables document renaming for forks', async function() { @@ -456,6 +472,7 @@ describe("Fork", function() { await gu.waitToPass(async () => { assert.equal(await driver.find('.test-bc-doc').getAttribute('disabled'), 'true'); }); + await gu.refreshDismiss(); }); it('navigating browser history play well with the add new menu', async function() { @@ -506,6 +523,7 @@ describe("Fork", function() { // check we're on a fork await gu.waitForUrl(/~/); const urlId = await gu.getCurrentUrlId(); + await gu.refreshDismiss(); // open trunk again, to test that page is reloaded after replacement await team.loadDoc(`/doc/${doc.id}`); @@ -606,6 +624,7 @@ describe("Fork", function() { assert.equal(await confirmButton.getText(), 'Update'); assert.match(await driver.find('.test-modal-dialog').getText(), /already identical/); + await gu.refreshDismiss(); }); it('gives an error when replacing without write access via UI', async function() { @@ -638,6 +657,7 @@ describe("Fork", function() { await driver.find('.test-replace-original').click(); assert.equal(await driver.find('.grist-floating-menu').isDisplayed(), true); await assert.isRejected(driver.findWait('.test-modal-dialog', 500), /Waiting for element/); + await gu.refreshDismiss(); } finally { await api.updateDocPermissions(doc.id, {users: {[altSession.email]: null}}); } diff --git a/test/nbrowser/GridView.ts b/test/nbrowser/GridView.ts new file mode 100644 index 00000000..9357835e --- /dev/null +++ b/test/nbrowser/GridView.ts @@ -0,0 +1,44 @@ +import {assert, driver} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; +import {DocCreationInfo} from "app/common/DocListAPI"; + +describe('GridView', function() { + this.timeout(20000); + const cleanup = setupTestSuite(); + let session: gu.Session, doc: DocCreationInfo, api; + + it('should show tables with no columns without errors', async function() { + session = await gu.session().login(); + doc = await session.tempDoc(cleanup, 'Hello.grist'); + api = session.createHomeApi(); + + // Create and open a new table with no columns + await api.applyUserActions(doc.id, [ + ['AddTable', 'Empty', []], + ]); + await gu.getPageItem(/Empty/).click(); + + // The only 'column' should be the button to add a column + const columnNames = await driver.findAll('.column_name', e => e.getText()); + assert.deepEqual(columnNames, ['+']); + + // There should be no errors + assert.lengthOf(await driver.findAll('.test-notifier-toast-wrapper'), 0); + }); + + // When a grid is scrolled, and then data is changed (due to click in a linked section), some + // records are not rendered or the position of the scroll container is corrupted. + it('should render list with wrapped choices correctly', async function() { + await session.tempDoc(cleanup, 'Teams.grist'); + await gu.selectSectionByTitle("PROJECTS"); + await gu.getCell(0, 1).click(); + await gu.selectSectionByTitle("TODO"); + await gu.scrollActiveView(0, 300); + await gu.selectSectionByTitle("PROJECTS"); + await gu.getCell(0, 2).click(); + await gu.selectSectionByTitle("TODO"); + // This throws an error, as the cell is not rendered. + assert.equal(await gu.getCell(0, 2).getText(), "2021-09-27 Mo\n2021-10-04 Mo"); + }); +}); diff --git a/test/nbrowser/HomeIntro.ts b/test/nbrowser/HomeIntro.ts index eb62c642..3d3b1863 100644 --- a/test/nbrowser/HomeIntro.ts +++ b/test/nbrowser/HomeIntro.ts @@ -323,6 +323,7 @@ async function checkDocAndRestore( await docChecker(); for (let i = 0; i < stepsBackToDocMenu; i++) { await driver.navigate().back(); + if (await gu.isAlertShown()) { await gu.acceptAlert(); } } await gu.waitForDocMenuToLoad(); // If not logged in, we create docs "unsaved" and don't see them in doc-menu. diff --git a/test/nbrowser/ImportReferences.ts b/test/nbrowser/ImportReferences.ts new file mode 100644 index 00000000..172d9e43 --- /dev/null +++ b/test/nbrowser/ImportReferences.ts @@ -0,0 +1,212 @@ +/** + * Parsing strings as references when importing into an existing table + */ +import {assert, driver, Key, WebElement} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; +import {openSource as openSourceMenu, waitForColumnMapping} from 'test/nbrowser/importerTestUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; + +describe('ImportReferences', function() { + this.timeout(30000); + const cleanup = setupTestSuite(); + + before(async function() { + // Log in and import a sample document. + const session = await gu.session().teamSite.user('user1').login(); + await session.tempDoc(cleanup, 'ImportReferences.grist'); + }); + + afterEach(() => gu.checkForErrors()); + + it('should convert strings to references', async function() { + // Import a CSV file containing strings representing references + await gu.importFileDialog('./uploads/name_references.csv'); + assert.equal(await driver.findWait('.test-importer-preview', 2000).isPresent(), true); + + // Change the destination to the existing table + await driver.findContent('.test-importer-target-existing-table', /Table1/).click(); + await gu.waitForServer(); + + // Finish import, and verify the import succeeded. + await driver.find('.test-modal-confirm').click(); + await gu.waitForServer(); + + // Verify data was imported to Names correctly. + assert.deepEqual( + await gu.getVisibleGridCells({rowNums: [1, 2, 3, 4, 5], cols: [0, 1, 2]}), + [ + // Previously existing data in the fixture document + 'Alice', '', '', + 'Bob', '', '', + + // Imported data from the CSV file + // The second column is references which have been successfully parsed from strings + // The third column is a formula equal to the second column to demonstrate the references + 'Charlie', 'Alice', 'Table1[1]', + 'Dennis', 'Bob', 'Table1[2]', + + // 'add new' row + '', '', '', + ] + ); + + // TODO this test relies on the imported data referring to names (Alice,Bob) + // already existing in the table before the import, and not being changed by the import + }); + + it('should support importing into any reference columns and show preview', async function() { + // Switch to page showing Projects and Tasks. + await gu.getPageItem('Projects').click(); + await gu.waitForServer(); // wait for table load + + // Load up a CSV file that matches the structure of the Tasks table. + await gu.importFileDialog('./uploads/ImportReferences-Tasks.csv'); + + // The default import into "New Table" just shows the content of the file. + assert.equal(await driver.findWait('.test-importer-preview', 2000).isPresent(), true); + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4, 5, 6], [1, 2, 3, 4], mapper), [ + 'Foo2', 'Clean', '1000', '1,000', '27 Mar 2023', '', '0', + 'Bar2', 'Wash', '3000', '2,000', '', 'Projects[2]', '2', + 'Baz2', 'Build2', '', '2', '20 Mar 2023', 'Projects[1]', '1', + 'Zoo2', 'Clean', '2000', '4,000', '24 Apr 2023', 'Projects[3]', '3', + ]); + + await driver.findContent('.test-importer-target-existing-table', /Tasks/).click(); + await gu.waitForServer(); + + // See that preview works, and cells that should be valid are valid. + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4], [1, 2, 3, 4], mapper), [ + // Label, PName, PIndex, PDate, PRowID + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '3,000', '', '!Projects[2]', + 'Baz2', '!Build2', '', '!2023-03-20', '!Projects[1]', + 'Zoo2', 'Clean', '2,000', '24 Apr 2023', '!Projects[3]', + ]); + + await driver.find('.test-modal-confirm').click(); + await gu.waitForServer(); + + // Verify data was imported to Tasks correctly. + assert.deepEqual( + await gu.getVisibleGridCells({section: 'TASKS', cols: [0, 1, 2, 3, 4], rowNums: [4, 5, 6, 7, 8, 9], mapper}), [ + // Label, PName, PIndex, PDate, PRowID + // Previous data in the fixture, in row 4 + 'Zoo', 'Clean', '2,000', '27 Mar 2023', 'Projects[3]', + // New rows (values like "!Project[2]" are invalid, which may be fixed in the future). + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '3,000', '', '!Projects[2]', + 'Baz2', '!Build2', '', '!2023-03-20', '!Projects[1]', + 'Zoo2', 'Clean', '2,000', '24 Apr 2023', '!Projects[3]', + // 'Add New' row + '', '', '', '', '', + ]); + + await gu.undo(); + }); + + it('should support importing numeric columns as lookups or rowIDs', async function() { + // Load up the same CSV file again, with Tasks as the destination. + await gu.importFileDialog('./uploads/ImportReferences-Tasks.csv'); + await driver.findContent('.test-importer-target-existing-table', /Tasks/).click(); + await gu.waitForServer(); + await waitForColumnMapping(); + + // Check that preview works, and cells are valid. + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4], [1, 2, 3, 4], mapper), [ + // Label, PName, PIndex, PDate, PRowID + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '3,000', '', '!Projects[2]', + 'Baz2', '!Build2', '', '!2023-03-20', '!Projects[1]', + 'Zoo2', 'Clean', '2,000', '24 Apr 2023', '!Projects[3]', + ]); + + // Check that dropdown for Label does not include "(as row ID)" entries, but the dropdown for + // PName (a reference column) does. + await openSourceMenu('Label'); + assert.equal(await findColumnMenuItem('PIndex').isPresent(), true); + assert.equal(await findColumnMenuItem(/as row ID/).isPresent(), false); + await driver.sendKeys(Key.ESCAPE); + + await openSourceMenu('PName'); + assert.equal(await findColumnMenuItem('PIndex').isPresent(), true); + assert.equal(await findColumnMenuItem('PIndex (as row ID)').isPresent(), true); + await driver.sendKeys(Key.ESCAPE); + + // Change PIndex column from lookup to row ID. + await openSourceMenu('PIndex'); + await findColumnMenuItem('PIndex (as row ID)').click(); + await gu.waitForServer(); + + // The values become invalid because there are no such rowIDs. + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4], [1, 2, 3, 4], mapper), [ + // Label, PName, PIndex, PDate, PRowID + 'Foo2', 'Clean', '!1000', '27 Mar 2023', '', + 'Bar2', 'Wash', '!3000', '', '!Projects[2]', + 'Baz2', '!Build2', '', '!2023-03-20', '!Projects[1]', + 'Zoo2', 'Clean', '!2000', '24 Apr 2023', '!Projects[3]', + ]); + + // Try a lookup using PIndex2. It is differently formatted, one value is invalid, and one is a + // valid row ID (but shouldn't be seen as a rowID for a lookup) + await openSourceMenu('PIndex'); + await findColumnMenuItem('PIndex2').click(); + await gu.waitForServer(); + + // Note: two PIndex values are different, and two are invalid. + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4], [1, 2, 3, 4], mapper), [ + // Label, PName, PIndex, PDate, PRowID + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '2,000', '', '!Projects[2]', + 'Baz2', '!Build2', '!2.0', '!2023-03-20', '!Projects[1]', + 'Zoo2', 'Clean', '!4000.0', '24 Apr 2023', '!Projects[3]', + ]); + + // Change PRowID column to use "PID (as row ID)". It has 3 valid rowIDs. + await openSourceMenu('PRowID'); + await findColumnMenuItem('PID (as row ID)').click(); + await gu.waitForServer(); + + // Note: PRowID values are now valid. + assert.deepEqual(await gu.getPreviewContents([0, 1, 2, 3, 4], [1, 2, 3, 4], mapper), [ + // Label, PName, PIndex, PDate, PRowID + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '2,000', '', 'Projects[2]', + 'Baz2', '!Build2', '!2.0', '!2023-03-20', 'Projects[1]', + 'Zoo2', 'Clean', '!4000.0', '24 Apr 2023', 'Projects[3]', + ]); + + await driver.find('.test-modal-confirm').click(); + await gu.waitForServer(); + + // Verify data was imported to Tasks correctly. + assert.deepEqual( + await gu.getVisibleGridCells({section: 'TASKS', cols: [0, 1, 2, 3, 4], rowNums: [4, 5, 6, 7, 8, 9], mapper}), [ + // Label, PName, PIndex, PDate, PRowID + // Previous data in the fixture, in row 4 + 'Zoo', 'Clean', '2,000', '27 Mar 2023', 'Projects[3]', + // New rows; PRowID values are valid. + 'Foo2', 'Clean', '1,000', '27 Mar 2023', '', + 'Bar2', 'Wash', '2,000', '', 'Projects[2]', + 'Baz2', '!Build2', '!2.0', '!2023-03-20', 'Projects[1]', + 'Zoo2', 'Clean', '!4000.0', '24 Apr 2023', 'Projects[3]', + // 'Add New' row + '', '', '', '', '', + ]); + + await gu.undo(); + }); +}); + +// mapper for getVisibleGridCells and getPreviewContents to get both text and whether the cell is +// invalid (pink). Invalid cells prefixed with "!". +async function mapper(el: WebElement) { + let text = await el.getText(); + if (await el.find(".field_clip").matches(".invalid")) { + text = "!" + text; + } + return text; +} + +function findColumnMenuItem(label: RegExp|string) { + return driver.findContent('.test-importer-column-match-menu-item', label); +} diff --git a/test/nbrowser/Smoke.ts b/test/nbrowser/Smoke.ts index c4c27310..67257479 100644 --- a/test/nbrowser/Smoke.ts +++ b/test/nbrowser/Smoke.ts @@ -36,8 +36,7 @@ describe("Smoke", function() { await gu.dismissWelcomeTourIfNeeded(); await gu.getCell('A', 1).click(); await gu.enterCell('123'); - await driver.navigate().refresh(); - await gu.waitForDocToLoad(); + await gu.refreshDismiss(); assert.equal(await gu.getCell('A', 1).getText(), '123'); }); }); diff --git a/test/nbrowser/UploadLimits.ts b/test/nbrowser/UploadLimits.ts new file mode 100644 index 00000000..67efb6e4 --- /dev/null +++ b/test/nbrowser/UploadLimits.ts @@ -0,0 +1,143 @@ +/** + * Test of the importing logic in the DocMenu page. + */ +import * as fs from 'fs'; +import {assert, driver, Key} from 'mocha-webdriver'; +import * as tmp from 'tmp-promise'; +import * as util from 'util'; + +import { SQLiteDB } from 'app/server/lib/SQLiteDB'; +import * as gu from 'test/nbrowser/gristUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; +import { copyFixtureDoc } from 'test/server/testUtils'; + +const write = util.promisify(fs.write); + +describe('UploadLimits', function() { + this.timeout(20000); + const cleanup = setupTestSuite(); + + const cleanupCbs: Array<() => void> = []; + + async function generateFile(postfix: string, size: number): Promise { + const obj = await tmp.file({postfix, mode: 0o644}); + await write(obj.fd, Buffer.alloc(size, 't')); + cleanupCbs.push(obj.cleanup); + return obj.path; + } + + // Create a valid Grist file of at least the desired length. The file may be + // slightly larger than requested. + async function generateGristFile(minSize: number): Promise { + const obj = await tmp.file({postfix: '.grist', mode: 0o644}); + await copyFixtureDoc('Hello.grist', obj.path); + const size = fs.statSync(obj.path).size; + const db = await SQLiteDB.openDBRaw(obj.path); + // Make a string that is long enough to push the doc over the required size. + const longString = 'x'.repeat(Math.max(1, minSize - size)); + // Add the string somewhere in the doc. For now we place it in a separate + // table - this may eventually become invalid, but it works for now. + // There'll be a little overhead so we'll overshoot the target length a bit, + // but that's fine. + await db.exec('CREATE TABLE _gristsys_extra(txt)'); + await db.run('INSERT INTO _gristsys_extra(txt) VALUES(?)', [longString]); + await db.close(); + const size2 = fs.statSync(obj.path).size; + if (size2 < minSize || size2 > minSize * 1.2) { + throw new Error(`generateGristFile size is off, wanted ${minSize}, got ${size2}`); + } + cleanupCbs.push(obj.cleanup); + return obj.path; + } + + after(function() { + for (const cleanup of cleanupCbs) { + cleanup(); + } + }); + + afterEach(async function() { + await gu.checkForErrors(); + }); + + const maxImport = 1024 * 1024; // See GRIST_MAX_UPLOAD_IMPORT_MB = 1 in testServer.ts + const maxAttachment = 2 * 1024 * 1024; // See GRIST_MAX_UPLOAD_ATTACHMENT_MB = 2 in testServer.ts + + it('should prevent large uploads for imports', async function() { + const session = await gu.session().teamSite.login(); + await session.loadDocMenu('/'); + + // Generate and upload a large csv file. It should by blocked on the client side. + const largeFilePath = await generateFile(".csv", maxImport + 1000); + await gu.docMenuImport(largeFilePath); + + // Ensure an error is shown. + assert.match(await driver.findWait('.test-notifier-toast-message', 1000).getText(), + /Imported files may not exceed 1.0MB/); + + // Now try to import directly to server, and verify that the server enforces this limit too. + const p = gu.importFixturesDoc('Chimpy', 'nasa', 'Horizon', largeFilePath, {load: false}); + await assert.isRejected(p, /Payload Too Large/); + const err = await p.catch((e) => e); + assert.equal(err.status, 413); + assert.isObject(err.details); + assert.match(err.details.userError, /Imported files must not exceed 1.0MB/); + }); + + it('should allow large uploads of .grist docs', async function() { + const session = await gu.session().teamSite.login(); + await session.loadDocMenu('/'); + + // Generate and upload a large .grist file. It should not be subject to limits. + const largeFilePath = await generateGristFile(maxImport * 2 + 1000); + await gu.docMenuImport(largeFilePath); + + await gu.waitForDocToLoad(); + assert.equal(await gu.getCell(0, 1).getText(), 'hello'); + }); + + it('should prevent large uploads for attachments', async function() { + const session = await gu.session().teamSite.login(); + await session.tempDoc(cleanup, 'Hello.grist'); + + // Clear the first cell. + await gu.getCell(0, 1).click(); + await driver.sendKeys(Key.DELETE); + await gu.waitForServer(); + + // Change column to Attachments. + await gu.toggleSidePanel('right', 'open'); + await driver.find('.test-right-tab-field').click(); + await gu.setType(/Attachment/); + await driver.findWait('.test-type-transform-apply', 1000).click(); + await gu.waitForServer(); + + // We can upload multiple smaller files (the limit is per-file here). + const largeFilePath1 = await generateFile(".png", maxAttachment - 1000); + const largeFilePath2 = await generateFile(".jpg", maxAttachment - 1000); + await gu.fileDialogUpload([largeFilePath1, largeFilePath2].join(","), + () => gu.getCell(0, 1).find('.test-attachment-icon').click()); + await gu.getCell(0, 1).findWait('.test-attachment-widget > [class*=test-pw-]', 1000); + + // We don't expect any errors here. + assert.lengthOf(await driver.findAll('.test-notifier-toast-wrapper'), 0); + + // Expect to find two attachments in the cell. + assert.lengthOf(await gu.getCell(0, 1).findAll('.test-attachment-widget > [class*=test-pw-]'), 2); + + // But we can't upload larger files, even one at a time. + const largeFilePath3 = await generateFile(".jpg", maxAttachment + 1000); + await gu.fileDialogUpload(largeFilePath3, + () => gu.getCell(0, 2).find('.test-attachment-icon').click()); + await driver.sleep(200); + await gu.waitForServer(); + + // Check that there is a warning and the cell hasn't changed. + assert.match(await driver.findWait('.test-notifier-toast-message', 1000).getText(), + /Attachments may not exceed 2.0MB/); + assert.lengthOf(await gu.getCell(0, 2).findAll('.test-attachment-widget > [class*=test-pw-]'), 0); + + // TODO We should try to add attachment via API and verify that the server enforces the limit + // too, but at the moment we don't have an endpoint to add attachments via the API. + }); +}); diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index e584be61..ae1552e1 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -1959,8 +1959,16 @@ export class Session { } // Load a document on a site. - public async loadDoc(relPath: string, wait: boolean = true) { + public async loadDoc( + relPath: string, + options: { + wait?: boolean, + skipAlert?: boolean, + } = {} + ) { + const {wait = true, skipAlert = false} = options; await this.loadRelPath(relPath); + if (skipAlert && await isAlertShown()) { await acceptAlert(); } if (wait) { await waitForDocToLoad(); } } @@ -2395,6 +2403,7 @@ async function addSamples() { await templatesApi.updateDoc( exampleDocId, { + type: 'template', isPinned: true, options: { description: 'CRM template and example for linking data, and creating productive layouts.', @@ -2411,6 +2420,7 @@ async function addSamples() { await templatesApi.updateDoc( investmentDocId, { + type: 'template', isPinned: true, options: { description: 'Example for analyzing and visualizing with summary tables and linked charts.', @@ -2425,6 +2435,7 @@ async function addSamples() { await templatesApi.updateDoc( afterschoolDocId, { + type: 'template', isPinned: true, options: { description: 'Example for how to model business data, use formulas, and manage complexity.', @@ -2870,10 +2881,29 @@ export async function getFilterMenuState(): Promise { */ export async function refreshDismiss() { await driver.navigate().refresh(); - await (await driver.switchTo().alert()).accept(); + await acceptAlert(); await waitForDocToLoad(); } +/** + * Accepts an alert. + */ +export async function acceptAlert() { + await (await driver.switchTo().alert()).accept(); +} + +/** + * Returns whether an alert is shown. + */ +export async function isAlertShown() { + try { + await driver.switchTo().alert(); + return true; + } catch { + return false; + } +} + /** * Dismisses any tutorial card that might be active. */ diff --git a/test/nbrowser/homeUtil.ts b/test/nbrowser/homeUtil.ts index 92ead6ea..31690a8c 100644 --- a/test/nbrowser/homeUtil.ts +++ b/test/nbrowser/homeUtil.ts @@ -112,6 +112,7 @@ export class HomeUtil { } // Make sure we revisit page in case login is changing. await this.driver.get('about:blank'); + await this._acceptAlertIfPresent(); // When running against an external server, we log in through the Grist login page. await this.driver.get(this.server.getUrl(org, "")); if (!await this.isOnLoginPage()) { @@ -157,6 +158,7 @@ export class HomeUtil { if (sid) { await testingHooks.setLoginSessionProfile(sid, null, org); } } else { await this.driver.get(`${this.server.getHost()}/logout`); + await this._acceptAlertIfPresent(); } } @@ -251,6 +253,7 @@ export class HomeUtil { public async getGristSid(): Promise { // Load a cheap page on our server to get the session-id cookie from browser. await this.driver.get(`${this.server.getHost()}/test/session`); + await this._acceptAlertIfPresent(); const cookie = await this.driver.manage().getCookie(process.env.GRIST_SESSION_COOKIE || 'grist_sid'); if (!cookie) { return null; } return decodeURIComponent(cookie.value); @@ -475,4 +478,12 @@ export class HomeUtil { `); } } + + private async _acceptAlertIfPresent() { + try { + await (await this.driver.switchTo().alert()).accept(); + } catch { + /* There was no alert to accept. */ + } + } } diff --git a/test/nbrowser/importerTestUtils.ts b/test/nbrowser/importerTestUtils.ts new file mode 100644 index 00000000..81040d41 --- /dev/null +++ b/test/nbrowser/importerTestUtils.ts @@ -0,0 +1,74 @@ +/** + * Testing utilities used in Importer test suites. + */ + +import {driver, stackWrapFunc, WebElementPromise} from 'mocha-webdriver'; +import * as gu from 'test/nbrowser/gristUtils'; + +// Helper to get the input of a matching parse option in the ParseOptions dialog. +export const getParseOptionInput = stackWrapFunc((labelRE: RegExp): WebElementPromise => + driver.findContent('.test-parseopts-opt', labelRE).find('input')); + +type CellDiff = string|[string|undefined, string|undefined, string|undefined]; + +/** + * Returns preview diff cell values when the importer is updating existing records. + * + * If a cell has no diff, just the cell value is returned. Otherwise, a 3-tuple + * containing the parent, remote, and common values (in that order) is returned. + */ +export const getPreviewDiffCellValues = stackWrapFunc(async (cols: number[], rowNums: number[]) => { + return gu.getPreviewContents(cols, rowNums, async (cell) => { + const hasParentDiff = await cell.find('.diff-parent').isPresent(); + const hasRemoteDiff = await cell.find('.diff-remote').isPresent(); + const hasCommonDiff = await cell.find('.diff-common').isPresent(); + + const isDiff = hasParentDiff || hasRemoteDiff || hasCommonDiff; + return !isDiff ? await cell.getText() : + [ + hasParentDiff ? await cell.find('.diff-parent').getText() : undefined, + hasRemoteDiff ? await cell.find('.diff-remote').getText() : undefined, + hasCommonDiff ? await cell.find('.diff-common').getText() : undefined, + ]; + }); +}); + +// Helper that waits for the diff preview to finish loading. +export const waitForDiffPreviewToLoad = stackWrapFunc(async (): Promise => { + await driver.wait(() => driver.find('.test-importer-preview').isPresent(), 5000); +}); + +// Helper that gets the list of visible column matching rows to the left of the preview. +export const getColumnMatchingRows = stackWrapFunc(async (): Promise<{source: string, destination: string}[]> => { + return await driver.findAll('.test-importer-column-match-source-destination', async (el) => { + const source = await el.find('.test-importer-column-match-formula').getText(); + const destination = await el.find('.test-importer-column-match-destination').getText(); + return {source, destination}; + }); +}); + +export async function waitForColumnMapping() { + await driver.wait(() => driver.find(".test-importer-column-match-options").isDisplayed(), 300); +} + +export async function openColumnMapping() { + const selected = driver.find('.test-importer-target-selected'); + await selected.find('.test-importer-target-column-mapping').click(); + await driver.sleep(200); // animation + await waitForColumnMapping(); +} + +export async function openTableMapping() { + await driver.find('.test-importer-table-mapping').click(); + await driver.sleep(200); // animation + await driver.wait(() => driver.find(".test-importer-target").isDisplayed(), 300); +} + +/** + * Opens the menu for the destination column, by clicking the source. + */ +export async function openSource(text: string|RegExp) { + await driver.findContent('.test-importer-column-match-destination', text) + .findClosest('.test-importer-column-match-source-destination') + .find('.test-importer-column-match-source').click(); +}