From 959f8a45c6d27099ebb0abd0d45ee893ae26f1b5 Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 1 May 2023 11:24:23 -0700 Subject: [PATCH] (core) Direct users to last visited site when possible Summary: When clicking the logo in the top-left corner, or finishing a tutorial, we now direct users to the site they last visited, if possible. If unknown, a new redirect endpoint, /welcome/home, is used instead, which directs users to a sensible location based on the number of sites they have. Test Plan: Browser tests. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3878 --- app/client/lib/localStorageObs.ts | 18 +++++++--- app/client/models/AppModel.ts | 12 +++++++ app/client/models/gristUrlState.ts | 15 ++++++-- app/client/ui/AppHeader.ts | 13 +++++-- app/client/ui/DocTutorial.ts | 9 +++-- app/server/lib/FlexServer.ts | 58 +++++++++++++++++++++++------- test/nbrowser/DocTutorial.ts | 10 ++++-- 7 files changed, 108 insertions(+), 27 deletions(-) diff --git a/app/client/lib/localStorageObs.ts b/app/client/lib/localStorageObs.ts index 05a36c20..2caab21d 100644 --- a/app/client/lib/localStorageObs.ts +++ b/app/client/lib/localStorageObs.ts @@ -26,14 +26,24 @@ export function sessionStorageBoolObs(key: string, defValue = false): Observable return getStorageBoolObs(getSessionStorage(), key, defValue); } +function getStorageObs(store: Storage, key: string, defaultValue?: string) { + const obs = Observable.create(null, store.getItem(key) ?? defaultValue ?? null); + obs.addListener((val) => (val === null) ? store.removeItem(key) : store.setItem(key, val)); + return obs; +} + /** * Helper to create a string observable whose state is stored in localStorage. */ export function localStorageObs(key: string, defaultValue?: string): Observable { - const store = getStorage(); - const obs = Observable.create(null, store.getItem(key) ?? defaultValue ?? null); - obs.addListener((val) => (val === null) ? store.removeItem(key) : store.setItem(key, val)); - return obs; + return getStorageObs(getStorage(), key, defaultValue); +} + +/** + * Similar to `localStorageObs`, but always uses sessionStorage (or an in-memory equivalent). + */ +export function sessionStorageObs(key: string, defaultValue?: string): Observable { + return getStorageObs(getSessionStorage(), key, defaultValue); } /** diff --git a/app/client/models/AppModel.ts b/app/client/models/AppModel.ts index 0182f833..c5860db4 100644 --- a/app/client/models/AppModel.ts +++ b/app/client/models/AppModel.ts @@ -1,6 +1,7 @@ import {BehavioralPromptsManager} from 'app/client/components/BehavioralPromptsManager'; import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals'; import {makeT} from 'app/client/lib/localization'; +import {sessionStorageObs} from 'app/client/lib/localStorageObs'; import {error} from 'app/client/lib/log'; import {reportError, setErrorNotifier} from 'app/client/models/errors'; import {urlState} from 'app/client/models/gristUrlState'; @@ -83,6 +84,7 @@ export interface AppModel { isTeamSite: boolean; // Is it a team site? isLegacySite: boolean; // Is it a legacy site? orgError?: OrgError; // If currentOrg is null, the error that caused it. + lastVisitedOrgDomain: Observable; currentProduct: Product|null; // The current org's product. currentFeatures: Features; // Features of the current org's product. @@ -227,6 +229,8 @@ export class AppModelImpl extends Disposable implements AppModel { public readonly currentOrgUsage: Observable = Observable.create(this, null); + public readonly lastVisitedOrgDomain = this.autoDispose(sessionStorageObs('grist-last-visited-org-domain')); + public readonly currentProduct = this.currentOrg?.billingAccount?.product ?? null; public readonly currentFeatures = this.currentProduct?.features ?? {}; @@ -284,6 +288,14 @@ export class AppModelImpl extends Disposable implements AppModel { this.dismissedPopups.set(seen ? DismissedPopup.values : []); this.behavioralPromptsManager.reset(); }; + + this.autoDispose(subscribe(urlState().state, async (_use, {doc, org}) => { + // Keep track of the last valid org domain the user visited, ignoring those + // with a document id in the URL. + if (!this.currentOrg || doc) { return; } + + this.lastVisitedOrgDomain.set(org ?? null); + })); } public get planName() { diff --git a/app/client/models/gristUrlState.ts b/app/client/models/gristUrlState.ts index 7faea7d4..a95f2776 100644 --- a/app/client/models/gristUrlState.ts +++ b/app/client/models/gristUrlState.ts @@ -85,6 +85,10 @@ export function getLoginOrSignupUrl(nextUrl: string = _getCurrentUrl()): string return _getLoginLogoutUrl('signin', nextUrl); } +export function getWelcomeHomeUrl() { + return _buildUrl('welcome/home').href; +} + // Returns the relative URL (i.e. path) of the current page, except when it's the // "/signed-out" page, in which case it returns the home page ("/"). // This is a good URL to use for a post-login redirect. @@ -97,12 +101,17 @@ function _getCurrentUrl(): string { // Returns the URL for the given login page, with 'next' param optionally set. function _getLoginLogoutUrl(page: 'login'|'logout'|'signin'|'signup', nextUrl?: string | null): string { + const startUrl = _buildUrl(page); + if (nextUrl) { startUrl.searchParams.set('next', nextUrl); } + return startUrl.href; +} + +function _buildUrl(page?: string): URL { const startUrl = new URL(window.location.href); - startUrl.pathname = addOrgToPath('', window.location.href, true) + '/' + page; + startUrl.pathname = addOrgToPath('', window.location.href, true) + '/' + (page ?? ''); startUrl.search = ''; startUrl.hash = ''; - if (nextUrl) { startUrl.searchParams.set('next', nextUrl); } - return startUrl.href; + return startUrl; } /** diff --git a/app/client/ui/AppHeader.ts b/app/client/ui/AppHeader.ts index 6fe7c0ce..d8dadfee 100644 --- a/app/client/ui/AppHeader.ts +++ b/app/client/ui/AppHeader.ts @@ -1,4 +1,4 @@ -import {urlState} from 'app/client/models/gristUrlState'; +import {getWelcomeHomeUrl, urlState} from 'app/client/models/gristUrlState'; import {buildAppMenuBillingItem} from 'app/client/ui/BillingButtons'; import {getTheme} from 'app/client/ui/CustomThemes'; import {cssLeftPane} from 'app/client/ui/PagePanels'; @@ -48,7 +48,7 @@ export class AppHeader extends Disposable { cssAppLogo( {title: `Version ${version.version}` + ((version.gitcommit as string) !== 'unknown' ? ` (${version.gitcommit})` : '')}, - urlState().setLinkUrl({}), + this._setHomePageUrl(), testId('dm-logo') ), cssOrg( @@ -79,6 +79,15 @@ export class AppHeader extends Disposable { ), ); } + + private _setHomePageUrl() { + const lastVisitedOrg = this._appModel.lastVisitedOrgDomain.get(); + if (lastVisitedOrg) { + return urlState().setLinkUrl({org: lastVisitedOrg}); + } else { + return {href: getWelcomeHomeUrl()}; + } + } } export function productPill(org: Organization|null, options: {large?: boolean} = {}): DomContents { diff --git a/app/client/ui/DocTutorial.ts b/app/client/ui/DocTutorial.ts index f24b8426..fca01785 100644 --- a/app/client/ui/DocTutorial.ts +++ b/app/client/ui/DocTutorial.ts @@ -1,5 +1,5 @@ import {GristDoc} from 'app/client/components/GristDoc'; -import {urlState} from 'app/client/models/gristUrlState'; +import {getWelcomeHomeUrl, urlState} from 'app/client/models/gristUrlState'; import {renderer} from 'app/client/ui/DocTutorialRenderer'; import {cssPopupBody, FloatingPopup} from 'app/client/ui/FloatingPopup'; import {sanitizeHTML} from 'app/client/ui/sanitizeHTML'; @@ -242,7 +242,12 @@ export class DocTutorial extends FloatingPopup { private async _finishTutorial() { this._saveCurrentSlidePositionDebounced.cancel(); await this._saveCurrentSlidePosition(); - await urlState().pushUrl({}); + const lastVisitedOrg = this._appModel.lastVisitedOrgDomain.get(); + if (lastVisitedOrg) { + await urlState().pushUrl({org: lastVisitedOrg}); + } else { + window.location.assign(getWelcomeHomeUrl()); + } } private async _restartTutorial() { diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index e6f82bda..5f613b0d 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -1239,20 +1239,32 @@ export class FlexServer implements GristServer { return this._redirectToLoginOrSignup({ nextUrl: new URL(getOrgUrl(req, '/welcome/start')), }, req, resp); - } else { - const userId = getUserId(req); - const domain = getOrgFromRequest(req); - const orgs = this._dbManager.unwrapQueryResult( - await this._dbManager.getOrgs(userId, domain, { - ignoreEveryoneShares: true, - }) - ); - if (orgs.length > 1) { - resp.redirect(getOrgUrl(req, '/welcome/teams')); - } else { - resp.redirect(getOrgUrl(req)); - } } + + await this._redirectToHomeOrWelcomePage(req as RequestWithLogin, resp); + })); + + /** + * Like /welcome/start, but doesn't redirect anonymous users to sign in. + * + * Used by the client when the last site the user visited is unknown, and + * a suitable site is needed for the home page. + * + * For example, on templates.getgrist.com it does: + * 1) If logged in and no team site -> https://docs.getgrist.com/ + * 2) If logged in and has team sites -> https://docs.getgrist.com/welcome/teams + * 3) If logged out -> https://docs.getgrist.com/ + */ + this.app.get('/welcome/home', [ + this._redirectToHostMiddleware, + this._userIdMiddleware, + ], expressWrap(async (req, resp) => { + const mreq = req as RequestWithLogin; + if (isAnonymousUser(req)) { + return resp.redirect(this.getMergedOrgUrl(mreq)); + } + + await this._redirectToHomeOrWelcomePage(mreq, resp, {redirectToMergedOrg: true}); })); this.app.post('/welcome/info', ...middleware, expressWrap(async (req, resp, next) => { @@ -1803,6 +1815,26 @@ export class FlexServer implements GristServer { const getRedirectUrl = signUp ? this._getSignUpRedirectUrl : this._getLoginRedirectUrl; resp.redirect(await getRedirectUrl(req, nextUrl)); } + + private async _redirectToHomeOrWelcomePage( + mreq: RequestWithLogin, + resp: express.Response, + options: {redirectToMergedOrg?: boolean} = {} + ) { + const {redirectToMergedOrg} = options; + const userId = getUserId(mreq); + const domain = getOrgFromRequest(mreq); + const orgs = this._dbManager.unwrapQueryResult( + await this._dbManager.getOrgs(userId, domain, { + ignoreEveryoneShares: true, + }) + ); + if (orgs.length > 1) { + resp.redirect(getOrgUrl(mreq, '/welcome/teams')); + } else { + resp.redirect(redirectToMergedOrg ? this.getMergedOrgUrl(mreq) : getOrgUrl(mreq)); + } + } } /** diff --git a/test/nbrowser/DocTutorial.ts b/test/nbrowser/DocTutorial.ts index ebdbadfc..07a52ff7 100644 --- a/test/nbrowser/DocTutorial.ts +++ b/test/nbrowser/DocTutorial.ts @@ -520,10 +520,14 @@ describe('DocTutorial', function () { assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'NewTable']); }); - it('redirects to the doc menu when finished', async function() { - await driver.find('.test-doc-tutorial-popup-slide-13').click(); + it('redirects to the last visited site when finished', async function() { + const otherSession = await gu.session().personalSite.user('user1').addLogin(); + await otherSession.loadDocMenu('/'); + await session.loadDoc(`/doc/${doc.id}`); + await driver.findWait('.test-doc-tutorial-popup-slide-13', 2000).click(); await driver.find('.test-doc-tutorial-popup-next').click(); - await driver.findWait('.test-dm-doclist', 2000); + await gu.waitForDocMenuToLoad(); + assert.match(await driver.getCurrentUrl(), /o\/docs\/$/); }); });