From dba3a594860a72f406c67560ea1c6501514bb2cd Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Tue, 23 Jan 2024 16:24:57 -0500 Subject: [PATCH] (core) fix form URL when team is encoded in domain Summary: This moves the `formUrl` logic to `encodeUrl`, which is more aware of how the URL is constructed than UserAPI. UserAPI can only reliably construct API URLs. Test Plan: extended tests Reviewers: georgegevoian Reviewed By: georgegevoian Subscribers: georgegevoian Differential Revision: https://phab.getgrist.com/D4171 --- app/client/components/Forms/FormView.ts | 19 ++++++++---- app/common/UserAPI.ts | 40 ------------------------- app/common/gristUrls.ts | 27 +++++++++++++++++ test/client/models/gristUrlState.ts | 20 +++++++++++++ 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/app/client/components/Forms/FormView.ts b/app/client/components/Forms/FormView.ts index ca5b0e34..5331c697 100644 --- a/app/client/components/Forms/FormView.ts +++ b/app/client/components/Forms/FormView.ts @@ -15,6 +15,7 @@ import DataTableModel from 'app/client/models/DataTableModel'; import {ViewFieldRec, ViewSectionRec} from 'app/client/models/DocModel'; import {ShareRec} from 'app/client/models/entities/ShareRec'; import {InsertColOptions} from 'app/client/models/entities/ViewSectionRec'; +import {urlState} from 'app/client/models/gristUrlState'; import {SortedRowSet} from 'app/client/models/rowset'; import {showTransientTooltip} from 'app/client/ui/tooltips'; import {cssButton} from 'app/client/ui2018/buttons'; @@ -299,9 +300,12 @@ export class FormView extends Disposable { this._url = Computed.create(this, use => { const doc = use(this.gristDoc.docPageModel.currentDoc); if (!doc) { return ''; } - const url = this.gristDoc.app.topAppModel.api.formUrl({ - urlId: doc.id, - vsId: use(this.viewSection.id), + const url = urlState().makeUrl({ + api: true, + doc: doc.id, + form: { + vsId: use(this.viewSection.id), + }, }); return url; }); @@ -580,9 +584,12 @@ export class FormView extends Disposable { throw new Error('Unable to copy link: form is not published'); } - const url = this.gristDoc.app.topAppModel.api.formUrl({ - shareKey:remoteShare.key, - vsId: this.viewSection.id(), + const url = urlState().makeUrl({ + doc: undefined, + form: { + shareKey: remoteShare.key, + vsId: this.viewSection.id(), + }, }); await copyToClipboard(url); showTransientTooltip(element, 'Link copied to clipboard', {key: 'copy-form-link'}); diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 29befe82..057361b7 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -419,33 +419,6 @@ export interface UserAPI { * is specific to Grist installation, and might not be supported. */ closeOrg(): Promise; - /** - * Creates publicly shared URL for a rendered form. - */ - formUrl(options: FormUrlOptions): string; -} - -interface FormUrlOptions { - vsId: number; - /** - * The canonical URL or document ID. - * - * If set, the returned form URL will only be accessible by users with access to the - * document. This is currently only used for the preview functionality in the widget, - * where document access is a pre-requisite. - * - * Only one of `urlId` or `shareKey` should be set. - */ - urlId?: string; - /** - * The key of the Share granting access to the form. - * - * If set, the returned form URL will be accessible by anyone, so long as the form - * is published. - * - * Only one of `urlId` or `shareKey` should be set. - */ - shareKey?: string; } /** @@ -537,19 +510,6 @@ export class UserAPIImpl extends BaseAPI implements UserAPI { super(_options); } - public formUrl(options: FormUrlOptions): string { - const {urlId, shareKey, vsId} = options; - if (!urlId && !shareKey) { - throw new Error('Invalid form URL: missing urlId or shareKey'); - } - - if (urlId) { - return `${this._url}/api/docs/${urlId}/forms/${vsId}`; - } else { - return `${this._url}/forms/${shareKey}/${vsId}`; - } - } - public forRemoved(): UserAPI { const extraParameters = new Map([['showRemoved', '1']]); return new UserAPIImpl(this._homeUrl, {...this._options, extraParameters}); diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index daf46603..929848d4 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -145,6 +145,12 @@ export interface IGristUrlState { // But this barely works, and is suitable only for documents. For decoding it // indicates that the URL probably points to an API endpoint. viaShare?: boolean; // Accessing document via a special share. + + // Form URLs can currently be encoded but not decoded. + form?: { + vsId: number; // a view section id of a form. + shareKey?: string; // only one of shareKey or doc should be set. + }, } // Subset of GristLoadConfig used by getOrgUrlInfo(), which affects the interpretation of the @@ -280,6 +286,27 @@ export function encodeUrl(gristConfig: Partial, parts.push(`p/${state.homePage}`); } + /** + * Form URLS can take two forms. If a docId/urlId is set, rather than + * a share key, the returned form URL will only be accessible by users + * with access to the document. This is currently only used for the + * preview functionality in the widget, where document access is a + * pre-requisite. + * + * When a share key is set, the returned form URL will be accessible + * by anyone, so long as the form is published. + * + * Only one of `doc` (docId/urlId) or `shareKey` should be set. + */ + if (state.form) { + if (state.doc) { parts.push('/'); } + parts.push('forms/'); + if (state.form.shareKey) { + parts.push(state.form.shareKey + '/'); + } + parts.push(String(state.form.vsId)); + } + if (state.account) { parts.push(state.account === 'account' ? 'account' : `account/${state.account}`); } diff --git a/test/client/models/gristUrlState.ts b/test/client/models/gristUrlState.ts index e5a2d031..868fa9fc 100644 --- a/test/client/models/gristUrlState.ts +++ b/test/client/models/gristUrlState.ts @@ -236,6 +236,16 @@ describe('gristUrlState', function() { assert.equal(mockWindow.location.href, 'https://foo.example.com/ws/12/'); assert.deepEqual(state.state.get(), {org: 'foo', ws: 12}); assert.equal(state.makeUrl({ws: 4}), 'https://foo.example.com/ws/4/'); + + // Check form URLs in prod setup. They are produced on document pages. + await state.pushUrl({org: 'foo', doc: 'abc'}); + state.loadState(); + assert.equal(state.makeUrl({doc: undefined, form: { vsId: 4, shareKey: 'key' }}), + 'https://foo.example.com/forms/key/4'); + assert.equal(state.makeUrl({api: true, doc: 'abc', form: { vsId: 4 }}), + 'https://foo.example.com/api/docs/abc/forms/4'); + assert.equal(state.makeUrl({api: true, form: { vsId: 4 }}), + 'https://foo.example.com/api/docs/abc/forms/4'); }); it('should produce correct results with single-org config', async function() { @@ -265,6 +275,16 @@ describe('gristUrlState', function() { assert.equal(mockWindow.location.href, 'https://example.com/o/foo/'); assert.deepEqual(state.state.get(), {org: 'foo'}); assert.equal(link.getAttribute('href'), 'https://example.com/o/foo/ws/4/'); + + // Check form URLs in single org setup from document pages. + await state.pushUrl({org: 'foo', doc: 'abc'}); + state.loadState(); + assert.equal(state.makeUrl({doc: undefined, form: { vsId: 4, shareKey: 'key' }}), + 'https://example.com/o/foo/forms/key/4'); + assert.equal(state.makeUrl({api: true, doc: 'abc', form: { vsId: 4 }}), + 'https://example.com/o/foo/api/docs/abc/forms/4'); + assert.equal(state.makeUrl({api: true, form: { vsId: 4 }}), + 'https://example.com/o/foo/api/docs/abc/forms/4'); }); it('should produce correct results with custom config', async function() {