(core) switch to newer download endpoint in client

Summary:
 * Fix old download endpoint to correctly pass org info in redirect.
 * Switch to use newer download endpoint in client.

Old endpoint not removed. I started doing that, but it is used in copying, and it struck me that I'm not sure what should happen when copying from a site document to "Personal" - should it be the Personal that is associated with docs.getgrist.com currently, of should it be the Personal that is associated with the email of the user on whatever-site-we-are-on.getgrist.com. So leaving that as separate work.

Test Plan: updated tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2639
This commit is contained in:
Paul Fitzpatrick 2020-10-19 12:13:20 -04:00
parent ad7be0fd8d
commit 27fd894fc7
5 changed files with 21 additions and 12 deletions

View File

@ -475,14 +475,6 @@ export class GristDoc extends DisposableWithEvents {
}
}
public getDownloadLink() {
return this.docComm.docUrl('download') + '?' + encodeQueryParams({
doc: this.docPageModel.currentDocId.get(),
title: this.docPageModel.currentDocTitle.get(),
...this.docComm.getUrlParams(),
});
}
public getCsvLink() {
return this.docComm.docUrl('gen_csv') + '?' + encodeQueryParams({
...this.docComm.getUrlParams(),

View File

@ -194,7 +194,10 @@ function menuExports(doc: Document, pageModel: DocPageModel) {
(isElectron ?
menuItem(() => gristDoc.app.comm.showItemInFolder(doc.name),
'Show in folder', testId('tb-share-option')) :
menuItemLink({ href: gristDoc.getDownloadLink(), target: '_blank', download: ''},
menuItemLink({
href: pageModel.appModel.api.getDocAPI(doc.id).getDownloadUrl(),
target: '_blank', download: ''
},
menuIcon('Download'), 'Download', testId('tb-share-option'))
),
menuItemLink({ href: gristDoc.getCsvLink(), target: '_blank', download: ''},

View File

@ -312,6 +312,7 @@ export interface DocAPI {
// Currently, leftHash is expected to be an ancestor of rightHash. If rightHash
// is HEAD, the result will contain a copy of any rows added or updated.
compareVersion(leftHash: string, rightHash: string): Promise<DocStateComparison>;
getDownloadUrl(template?: boolean): string;
}
// Operations that are supported by a doc worker.
@ -712,9 +713,13 @@ export class DocAPIImpl extends BaseAPI implements DocAPI {
}
public async compareVersion(leftHash: string, rightHash: string): Promise<DocStateComparison> {
const url = new URL(`${this._url}/compare`);
const url = new URL(`${this._url}/compare`);
url.searchParams.append('left', leftHash);
url.searchParams.append('right', rightHash);
return this.requestJson(url.href);
}
public getDownloadUrl(template: boolean = false) {
return this._url + `/download?template=${Number(template)}`;
}
}

View File

@ -38,7 +38,7 @@ import {getLoginMiddleware} from 'app/server/lib/logins';
import {getAppPathTo, getAppRoot, getUnpackedAppRoot} from 'app/server/lib/places';
import {addPluginEndpoints, limitToPlugins} from 'app/server/lib/PluginEndpoint';
import {PluginManager} from 'app/server/lib/PluginManager';
import {adaptServerUrl, addOrgToPathIfNeeded, addPermit, getScope, optStringParam, RequestWithGristInfo, stringParam,
import {adaptServerUrl, addOrgToPath, addPermit, getScope, optStringParam, RequestWithGristInfo, stringParam,
TEST_HTTPS_OFFSET, trustOrigin} from 'app/server/lib/requestUtils';
import {ISendAppPageOptions, makeGristConfig, makeSendAppPage} from 'app/server/lib/sendAppPage';
import {getDatabaseUrl} from 'app/server/lib/serverUtils';
@ -1117,7 +1117,7 @@ export class FlexServer implements GristServer {
this.app.get('/download', ...docAccessMiddleware, expressWrap(async (req, res) => {
// Forward this endpoint to regular API. This endpoint is now deprecated.
const docId = String(req.query.doc);
let url = await this.getHomeUrlByDocId(docId, addOrgToPathIfNeeded(req, `/api/docs/${docId}/download`));
let url = await this.getHomeUrlByDocId(docId, addOrgToPath(req, `/api/docs/${docId}/download`));
if (req.query.template === '1') { url += '?template=1'; }
return res.redirect(url);
}));

View File

@ -49,11 +49,20 @@ export function adaptServerUrl(url: URL, req: RequestWithOrg): void {
/**
* If org is not encoded in domain, prefix it to path - otherwise leave path unchanged.
* The domain is extracted from the request, so this method is only useful for constructing
* urls that stay within that domain.
*/
export function addOrgToPathIfNeeded(req: RequestWithOrg, path: string): string {
return (isOrgInPathOnly(req.hostname) && req.org) ? `/o/${req.org}${path}` : path;
}
/**
* If org is known, prefix it to path unconditionally.
*/
export function addOrgToPath(req: RequestWithOrg, path: string): string {
return req.org ? `/o/${req.org}${path}` : path;
}
/**
* Returns true for requests from permitted origins. For such requests, an
* "Access-Control-Allow-Origin" header is added to the response. Vary: Origin