From cf0cbb404eb2434b0be76079d2e6193e8f492cb7 Mon Sep 17 00:00:00 2001 From: Florent Date: Wed, 29 Nov 2023 21:13:29 +0100 Subject: [PATCH] Allow URLs with only a docID #768 (#771) Co-authored-by: Florent FAYOLLE --- app/client/models/AppModel.ts | 4 +-- app/client/ui/AccountWidget.ts | 2 +- app/client/ui/AppUI.ts | 2 +- app/client/ui/SupportGristNudge.ts | 8 ++--- app/client/ui/SupportGristPage.ts | 2 +- app/common/gristUrls.ts | 6 ++-- app/server/lib/AppEndpoint.ts | 2 +- app/server/lib/Telemetry.ts | 2 +- test/gen-server/seed.ts | 2 +- test/server/lib/Authorizer.ts | 58 ++++++++++++++++-------------- test/server/lib/DocApi.ts | 8 ++--- test/server/lib/Webhooks-Proxy.ts | 8 ++--- 12 files changed, 55 insertions(+), 49 deletions(-) diff --git a/app/client/models/AppModel.ts b/app/client/models/AppModel.ts index e83e50aa..21882ca5 100644 --- a/app/client/models/AppModel.ts +++ b/app/client/models/AppModel.ts @@ -41,7 +41,7 @@ export type PageType = | "billing" | "welcome" | "account" - | "support-grist" + | "support" | "activation"; const G = getBrowserGlobals('document', 'window'); @@ -316,7 +316,7 @@ export class AppModelImpl extends Disposable implements AppModel { } else if (state.account) { return 'account'; } else if (state.supportGrist) { - return 'support-grist'; + return 'support'; } else if (state.activation) { return 'activation'; } else { diff --git a/app/client/ui/AccountWidget.ts b/app/client/ui/AccountWidget.ts index 86150d61..56c8d11d 100644 --- a/app/client/ui/AccountWidget.ts +++ b/app/client/ui/AccountWidget.ts @@ -226,7 +226,7 @@ export class AccountWidget extends Disposable { return menuItemLink( t('Support Grist'), cssHeartIcon('💛'), - urlState().setLinkUrl({supportGrist: 'support-grist'}), + urlState().setLinkUrl({supportGrist: 'support'}), testId('usermenu-support-grist'), ); } diff --git a/app/client/ui/AppUI.ts b/app/client/ui/AppUI.ts index 25ce8a62..13a1bab3 100644 --- a/app/client/ui/AppUI.ts +++ b/app/client/ui/AppUI.ts @@ -77,7 +77,7 @@ function createMainPage(appModel: AppModel, appObj: App) { return dom.create(WelcomePage, appModel); } else if (pageType === 'account') { return domAsync(loadAccountPage().then(ap => dom.create(ap.AccountPage, appModel))); - } else if (pageType === 'support-grist') { + } else if (pageType === 'support') { return domAsync(loadSupportGristPage().then(sgp => dom.create(sgp.SupportGristPage, appModel))); } else if (pageType === 'activation') { return domAsync(loadActivationPage().then(ap => dom.create(ap.ActivationPage, appModel))); diff --git a/app/client/ui/SupportGristNudge.ts b/app/client/ui/SupportGristNudge.ts index 77de6c74..3a5de64d 100644 --- a/app/client/ui/SupportGristNudge.ts +++ b/app/client/ui/SupportGristNudge.ts @@ -22,7 +22,7 @@ type ButtonState = | 'expanded'; type CardPage = - | 'support-grist' + | 'support' | 'opted-in'; /** @@ -45,7 +45,7 @@ export class SupportGristNudge extends Disposable { this._buttonState = localStorageObs( `u=${this._appModel.currentValidUser?.id ?? 0};supportGristNudge`, 'expanded' ) as Observable; - this._currentPage = Observable.create(null, 'support-grist'); + this._currentPage = Observable.create(null, 'support'); this._isClosed = Observable.create(this, false); } @@ -122,7 +122,7 @@ export class SupportGristNudge extends Disposable { private _buildCard() { return cssCard( dom.domComputed(this._currentPage, page => { - if (page === 'support-grist') { + if (page === 'support') { return this._buildSupportGristCardContent(); } else { return this._buildOptedInCardContent(); @@ -205,7 +205,7 @@ function helpCenterLink() { function supportGristLink() { return cssLink( t('Support Grist page'), - {href: urlState().makeUrl({supportGrist: 'support-grist'}), target: '_blank'}, + {href: urlState().makeUrl({supportGrist: 'support'}), target: '_blank'}, ); } diff --git a/app/client/ui/SupportGristPage.ts b/app/client/ui/SupportGristPage.ts index f5a50586..72153cd6 100644 --- a/app/client/ui/SupportGristPage.ts +++ b/app/client/ui/SupportGristPage.ts @@ -194,7 +194,7 @@ export class SupportGristPage extends Disposable { const suffix = getPageTitleSuffix(getGristConfig()); switch (page) { case undefined: - case 'support-grist': { + case 'support': { return document.title = `Support Grist${suffix}`; } } diff --git a/app/common/gristUrls.ts b/app/common/gristUrls.ts index 16bd12f4..1138d077 100644 --- a/app/common/gristUrls.ts +++ b/app/common/gristUrls.ts @@ -43,7 +43,7 @@ export type ActivationPage = typeof ActivationPage.type; export const LoginPage = StringUnion('signup', 'login', 'verified', 'forgot-password'); export type LoginPage = typeof LoginPage.type; -export const SupportGristPage = StringUnion('support-grist'); +export const SupportGristPage = StringUnion('support'); export type SupportGristPage = typeof SupportGristPage.type; // Overall UI style. "full" is normal, "singlePage" is a single page focused, panels hidden experience. @@ -408,8 +408,8 @@ export function decodeUrl(gristConfig: Partial, location: Locat state.activation = ActivationPage.parse(map.get('activation')) || 'activation'; } if (map.has('welcome')) { state.welcome = WelcomePage.parse(map.get('welcome')); } - if (map.has('support-grist')) { - state.supportGrist = SupportGristPage.parse(map.get('support-grist')) || 'support-grist'; + if (map.has('support')) { + state.supportGrist = SupportGristPage.parse(map.get('support')) || 'support'; } if (sp.has('planType')) { state.params!.planType = sp.get('planType')!; } if (sp.has('billingPlan')) { state.params!.billingPlan = sp.get('billingPlan')!; } diff --git a/app/server/lib/AppEndpoint.ts b/app/server/lib/AppEndpoint.ts index eb6cc661..937303c7 100644 --- a/app/server/lib/AppEndpoint.ts +++ b/app/server/lib/AppEndpoint.ts @@ -212,6 +212,6 @@ export function attachAppEndpoint(options: AttachOptions): void { // The * is a wildcard in express 4, rather than a regex symbol. // See https://expressjs.com/en/guide/routing.html app.get('/doc/:urlId([^/]+):remainder(*)', ...docMiddleware, docHandler); - app.get('/:urlId([^/]{12,})/:slug([^/]+):remainder(*)', + app.get('/:urlId([^/]{12,})(/:slug([^/]+):remainder(*))?', ...docMiddleware, docHandler); } diff --git a/app/server/lib/Telemetry.ts b/app/server/lib/Telemetry.ts index 5ef44844..5bb48a50 100644 --- a/app/server/lib/Telemetry.ts +++ b/app/server/lib/Telemetry.ts @@ -186,7 +186,7 @@ export class Telemetry implements ITelemetry { public addPages(app: express.Application, middleware: express.RequestHandler[]) { if (this._deploymentType === 'core') { - app.get('/support-grist', ...middleware, expressWrap(async (req, resp) => { + app.get('/support', ...middleware, expressWrap(async (req, resp) => { return this._gristServer.sendAppPage(req, resp, {path: 'app.html', status: 200, config: {}}); })); diff --git a/test/gen-server/seed.ts b/test/gen-server/seed.ts index 7ef2d06b..b450e933 100644 --- a/test/gen-server/seed.ts +++ b/test/gen-server/seed.ts @@ -449,7 +449,7 @@ class Seed { const d = new Document(); d.name = doc; d.workspace = w; - d.id = `sample_${docId}`; + d.id = `sampledocid_${docId}`; docId++; await d.save(); const dgrps = await this.createGroups(w); diff --git a/test/server/lib/Authorizer.ts b/test/server/lib/Authorizer.ts index 12625239..d8d6389d 100644 --- a/test/server/lib/Authorizer.ts +++ b/test/server/lib/Authorizer.ts @@ -101,40 +101,46 @@ describe('Authorizer', function() { it.skip("viewer gets redirect by title", async function() { const resp = await axios.get(`${serverUrl}/o/pr/doc/Bananas`, chimpy); assert.equal(resp.status, 200); - assert.equal(getGristConfig(resp.data).assignmentId, 'sample_6'); - assert.match(resp.request.res.responseUrl, /\/doc\/sample_6$/); + assert.equal(getGristConfig(resp.data).assignmentId, 'sampledocid_6'); + assert.match(resp.request.res.responseUrl, /\/doc\/sampledocid_6$/); const resp2 = await axios.get(`${serverUrl}/o/nasa/doc/Pluto`, chimpy); assert.equal(resp2.status, 200); - assert.equal(getGristConfig(resp2.data).assignmentId, 'sample_2'); - assert.match(resp2.request.res.responseUrl, /\/doc\/sample_2$/); + assert.equal(getGristConfig(resp2.data).assignmentId, 'sampledocid_2'); + assert.match(resp2.request.res.responseUrl, /\/doc\/sampledocid_2$/); + }); + + it('viewer loads document without slug in the URL', async function () { + const docId = docs.Bananas.id; + const resp = await axios.get(`${serverUrl}/o/pr/${docId}`, chimpy); + assert.equal(resp.status, 200); }); it("stranger gets consistent refusal regardless of title", async function() { const resp = await axios.get(`${serverUrl}/o/pr/doc/Bananas`, charon); assert.equal(resp.status, 404); - assert.notMatch(resp.data, /sample_6/); + assert.notMatch(resp.data, /sampledocid_6/); const resp2 = await axios.get(`${serverUrl}/o/pr/doc/Bananas2`, charon); assert.equal(resp2.status, 404); - assert.notMatch(resp.data, /sample_6/); + assert.notMatch(resp.data, /sampledocid_6/); assert.deepEqual(withoutTimestamp(resp.data), withoutTimestamp(resp2.data)); }); it("viewer can access title", async function() { - const resp = await axios.get(`${serverUrl}/o/pr/doc/sample_6`, chimpy); + const resp = await axios.get(`${serverUrl}/o/pr/doc/sampledocid_6`, chimpy); assert.equal(resp.status, 200); const config = getGristConfig(resp.data); assert.equal(config.getDoc![config.assignmentId!].name, 'Bananas'); }); it("stranger cannot access title", async function() { - const resp = await axios.get(`${serverUrl}/o/pr/doc/sample_6`, charon); + const resp = await axios.get(`${serverUrl}/o/pr/doc/sampledocid_6`, charon); assert.equal(resp.status, 403); assert.notMatch(resp.data, /Bananas/); }); it("viewer cannot access document from wrong org", async function() { - const resp = await axios.get(`${serverUrl}/o/nasa/doc/sample_6`, chimpy); + const resp = await axios.get(`${serverUrl}/o/nasa/doc/sampledocid_6`, chimpy); assert.equal(resp.status, 404); }); @@ -142,7 +148,7 @@ describe('Authorizer', function() { const cli = await openClient(server, 'chimpy@getgrist.com', 'pr'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.send("openDoc", "sample_6"); + const openDoc = await cli.send("openDoc", "sampledocid_6"); assert.equal(openDoc.error, undefined); assert.match(JSON.stringify(openDoc.data), /Table1/); await cli.close(); @@ -152,7 +158,7 @@ describe('Authorizer', function() { const cli = await openClient(server, 'charon@getgrist.com', 'pr'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.send("openDoc", "sample_6"); + const openDoc = await cli.send("openDoc", "sampledocid_6"); assert.match(openDoc.error!, /No view access/); assert.equal(openDoc.data, undefined); assert.match(openDoc.errorCode!, /AUTH_NO_VIEW/); @@ -163,7 +169,7 @@ describe('Authorizer', function() { const cli = await openClient(server, 'charon@getgrist.com', 'nasa'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.openDocOnConnect("sample_2"); + const openDoc = await cli.openDocOnConnect("sampledocid_2"); assert.equal(openDoc.error, undefined); const nonce = uuidv4(); const applyUserActions = await cli.send("applyUserActions", @@ -182,7 +188,7 @@ describe('Authorizer', function() { const cli = await openClient(server, 'chimpy@getgrist.com', 'nasa'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.openDocOnConnect("sample_2"); + const openDoc = await cli.openDocOnConnect("sampledocid_2"); assert.equal(openDoc.error, undefined); const nonce = uuidv4(); const applyUserActions = await cli.send("applyUserActions", @@ -209,9 +215,9 @@ describe('Authorizer', function() { editor.ignoreTrivialActions(); viewer.ignoreTrivialActions(); stranger.ignoreTrivialActions(); - assert.equal((await editor.send("openDoc", "sample_2")).error, undefined); - assert.equal((await viewer.send("openDoc", "sample_2")).error, undefined); - assert.match((await stranger.send("openDoc", "sample_2")).error!, /No view access/); + assert.equal((await editor.send("openDoc", "sampledocid_2")).error, undefined); + assert.equal((await viewer.send("openDoc", "sampledocid_2")).error, undefined); + assert.match((await stranger.send("openDoc", "sampledocid_2")).error!, /No view access/); const action = [0, [["UpdateRecord", "Table1", 1, {A: "foo"}]]]; assert.equal((await editor.send("applyUserActions", ...action)).error, undefined); @@ -224,7 +230,7 @@ describe('Authorizer', function() { const cli = await openClient(server, 'thumbnail@getgrist.com', 'nasa'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.send("openDoc", "sample_2"); + const openDoc = await cli.send("openDoc", "sampledocid_2"); assert.equal(openDoc.error, undefined); const nonce = uuidv4(); const applyUserActions = await cli.send("applyUserActions", @@ -243,12 +249,12 @@ describe('Authorizer', function() { const cli = await openClient(server, 'charon@getgrist.com', 'nasa'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - const openDoc = await cli.send("openDoc", "sample_2"); + const openDoc = await cli.send("openDoc", "sampledocid_2"); assert.equal(openDoc.error, undefined); const result = await cli.send("fork", 0); assert.equal(result.data.docId, result.data.urlId); const parts = parseUrlId(result.data.docId); - assert.equal(parts.trunkId, "sample_2"); + assert.equal(parts.trunkId, "sampledocid_2"); assert.isAbove(parts.forkId!.length, 4); assert.equal(parts.forkUserId, await dbManager.testGetId('Charon') as number); }); @@ -258,31 +264,31 @@ describe('Authorizer', function() { const cli = await openClient(server, 'anon@getgrist.com', 'nasa'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - let openDoc = await cli.send("openDoc", "sample_2"); + let openDoc = await cli.send("openDoc", "sampledocid_2"); assert.match(openDoc.error!, /No view access/); // grant anon access to doc and retry await dbManager.updateDocPermissions({ userId: await dbManager.testGetId('Chimpy') as number, - urlId: 'sample_2', + urlId: 'sampledocid_2', org: 'nasa' }, {users: {"anon@getgrist.com": "viewers"}}); dbManager.flushDocAuthCache(); - openDoc = await cli.send("openDoc", "sample_2"); + openDoc = await cli.send("openDoc", "sampledocid_2"); assert.equal(openDoc.error, undefined); // make a fork const result = await cli.send("fork", 0); assert.equal(result.data.docId, result.data.urlId); const parts = parseUrlId(result.data.docId); - assert.equal(parts.trunkId, "sample_2"); + assert.equal(parts.trunkId, "sampledocid_2"); assert.isAbove(parts.forkId!.length, 4); assert.equal(parts.forkUserId, undefined); }); it("can set user via GRIST_PROXY_AUTH_HEADER", async function() { // User can access a doc by setting header. - const docUrl = `${serverUrl}/o/pr/api/docs/sample_6`; + const docUrl = `${serverUrl}/o/pr/api/docs/sampledocid_6`; const resp = await axios.get(docUrl, { headers: {'X-email': 'chimpy@getgrist.com'} }); @@ -297,7 +303,7 @@ describe('Authorizer', function() { let cli = await openClient(server, 'chimpy@getgrist.com', 'pr', 'X-email'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - let openDoc = await cli.send("openDoc", "sample_6"); + let openDoc = await cli.send("openDoc", "sampledocid_6"); assert.equal(openDoc.error, undefined); assert.match(JSON.stringify(openDoc.data), /Table1/); await cli.close(); @@ -306,7 +312,7 @@ describe('Authorizer', function() { cli = await openClient(server, 'notchimpy@getgrist.com', 'pr', 'X-email'); cli.ignoreTrivialActions(); assert.equal((await cli.readMessage()).type, 'clientConnect'); - openDoc = await cli.send("openDoc", "sample_6"); + openDoc = await cli.send("openDoc", "sampledocid_6"); assert.match(openDoc.error!, /No view access/); assert.equal(openDoc.data, undefined); assert.match(openDoc.errorCode!, /AUTH_NO_VIEW/); diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 102b579a..68133754 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -49,10 +49,10 @@ const support = configForUser('support'); // some doc ids const docIds: { [name: string]: string } = { - ApiDataRecordsTest: 'sample_7', - Timesheets: 'sample_13', - Bananas: 'sample_6', - Antartic: 'sample_11' + ApiDataRecordsTest: 'sampledocid_7', + Timesheets: 'sampledocid_13', + Bananas: 'sampledocid_6', + Antartic: 'sampledocid_11' }; // A testDir of the form grist_test_{USER}_{SERVER_NAME} diff --git a/test/server/lib/Webhooks-Proxy.ts b/test/server/lib/Webhooks-Proxy.ts index d8c45cc8..dfd8e11d 100644 --- a/test/server/lib/Webhooks-Proxy.ts +++ b/test/server/lib/Webhooks-Proxy.ts @@ -21,10 +21,10 @@ const chimpy = configForUser('Chimpy'); // some doc ids const docIds: { [name: string]: string } = { - ApiDataRecordsTest: 'sample_7', - Timesheets: 'sample_13', - Bananas: 'sample_6', - Antartic: 'sample_11' + ApiDataRecordsTest: 'sampledocid_7', + Timesheets: 'sampledocid_13', + Bananas: 'sampledocid_6', + Antartic: 'sampledocid_11', }; let dataDir: string;