diff --git a/app/client/models/DocModel.ts b/app/client/models/DocModel.ts index a41bd214..9efd1d18 100644 --- a/app/client/models/DocModel.ts +++ b/app/client/models/DocModel.ts @@ -26,6 +26,7 @@ import MetaTableModel from 'app/client/models/MetaTableModel'; import * as rowset from 'app/client/models/rowset'; import {TableData} from 'app/client/models/TableData'; import {isHiddenTable, isSummaryTable} from 'app/common/isHiddenTable'; +import {canEdit} from 'app/common/roles'; import {RowFilterFunc} from 'app/common/RowFilterFunc'; import {schema, SchemaTypes} from 'app/common/schema'; import {UIRowId} from 'app/common/UIRowId'; @@ -44,7 +45,7 @@ import {createViewSectionRec, ViewSectionRec} from 'app/client/models/entities/V import {CellRec, createCellRec} from 'app/client/models/entities/CellRec'; import {RefListValue} from 'app/common/gristTypes'; import {decodeObject} from 'app/plugin/objtypes'; -import { toKo } from 'grainjs'; +import {toKo} from 'grainjs'; // Re-export all the entity types available. The recommended usage is like this: // import {ColumnRec, ViewFieldRec} from 'app/client/models/DocModel'; @@ -166,7 +167,12 @@ export class DocModel { // is initialized once on page load. If set, then the tour page (if any) will be visible. public showDocTourTable: boolean = (urlState().state.get().docPage === 'GristDocTour'); - public showDocTutorialTable: boolean = !this._docPageModel.isTutorialFork.get(); + // Whether the GristDocTutorial table should be shown. Initialized once on page load. + public showDocTutorialTable: boolean = + // We skip subscribing to the observables below since they normally shouldn't change during + // this object's lifetime. If that changes, this should be made into a computed observable. + !this._docPageModel.isTutorialFork.get() || + canEdit(this._docPageModel.currentDoc.get()?.trunkAccess ?? null); // List of all the metadata tables. private _metaTables: Array>; diff --git a/app/client/models/entities/PageRec.ts b/app/client/models/entities/PageRec.ts index e0817ae7..2aa87cbd 100644 --- a/app/client/models/entities/PageRec.ts +++ b/app/client/models/entities/PageRec.ts @@ -14,7 +14,7 @@ export function createPageRec(this: PageRec, docModel: DocModel): void { // Page is hidden when any of this is true: // - It has an empty name (or no name at all) // - It is GristDocTour (unless user wants to see it) - // - It is GristDocTutorial (and the document is a tutorial fork) + // - It is GristDocTutorial (unless user should see it) // - It is a page generated for a hidden table TODO: Follow up - don't create // pages for hidden tables. // This is used currently only the left panel, to hide pages from the user. diff --git a/app/client/ui/DocTutorial.ts b/app/client/ui/DocTutorial.ts index fca01785..97f68a4f 100644 --- a/app/client/ui/DocTutorial.ts +++ b/app/client/ui/DocTutorial.ts @@ -52,6 +52,11 @@ export class DocTutorial extends FloatingPopup { public async start() { this.showPopup(); await this._loadSlides(); + + const tableData = this._docData.getTable('GristDocTutorial'); + if (tableData) { + this.autoDispose(tableData.tableActionEmitter.addListener(() => this._reloadSlides())); + } } protected _buildTitle() { @@ -214,6 +219,16 @@ export class DocTutorial extends FloatingPopup { this._slides.set(slides); } + private async _reloadSlides() { + await this._loadSlides(); + const slides = this._slides.get(); + if (!slides) { return; } + + if (this._currentSlideIndex.get() > slides.length - 1) { + this._currentSlideIndex.set(slides.length - 1); + } + } + private async _saveCurrentSlidePosition() { const currentOptions = this._currentDoc?.options ?? {}; await this._appModel.api.updateDoc(this._docId, { diff --git a/app/client/ui/ShareMenu.ts b/app/client/ui/ShareMenu.ts index a68d4bda..a9b988ef 100644 --- a/app/client/ui/ShareMenu.ts +++ b/app/client/ui/ShareMenu.ts @@ -41,11 +41,15 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { return shareButton(t("Back to Current"), () => [ menuManageUsers(doc, pageModel), menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel, true), + menuOriginal(doc, appModel, {isSnapshot: true}), menuExports(doc, pageModel), ], {buttonAction: backToCurrent}); } else if (doc.isTutorialFork) { - return null; + return shareButton(t("Save Copy"), () => [ + menuSaveCopy(t("Save Copy"), doc, appModel), + menuOriginal(doc, appModel, {isTutorialFork: true}), + menuExports(doc, pageModel), + ], {buttonAction: saveCopy}); } else if (doc.isPreFork || doc.isBareFork) { // A new unsaved document, or a fiddle, or a public example. const saveActionTitle = doc.isBareFork ? t("Save Document") : t("Save Copy"); @@ -63,14 +67,14 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents { return shareButton(t("Save Copy"), () => [ menuManageUsers(doc, pageModel), menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel, false), + menuOriginal(doc, appModel), menuExports(doc, pageModel), ], {buttonAction: saveCopy}); } else { return shareButton(t("Unsaved"), () => [ menuManageUsers(doc, pageModel), menuSaveCopy(t("Save Copy"), doc, appModel), - menuOriginal(doc, appModel, false), + menuOriginal(doc, appModel), menuExports(doc, pageModel), ]); } @@ -142,9 +146,25 @@ function menuManageUsers(doc: DocInfo, pageModel: DocPageModel) { ]; } -// Renders "Return to Original" and "Replace Original" menu items. When used with snapshots, we -// say "Current Version" in place of the word "Original". -function menuOriginal(doc: Document, appModel: AppModel, isSnapshot: boolean) { +interface MenuOriginalOptions { + /** Defaults to false. */ + isSnapshot?: boolean; + /** Defaults to false. */ + isTutorialFork?: boolean; +} + +/** + * Renders "Return to Original" and "Replace Original" menu items. + * + * When used with snapshots, we say "Current Version" in place of the word "Original". + * + * When used with tutorial forks, the "Return to Original" and "Compare to Original" menu + * items are excluded. Note that it's still possible to return to the original by manually + * 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 = {}) { + const {isSnapshot = false, isTutorialFork = false} = options; const termToUse = isSnapshot ? t("Current Version") : t("Original"); const origUrlId = buildOriginalUrlId(doc.id, isSnapshot); const originalUrl = urlState().makeUrl({doc: origUrlId}); @@ -167,7 +187,7 @@ function menuOriginal(doc: Document, appModel: AppModel, isSnapshot: boolean) { replaceTrunkWithFork(user, doc, appModel, origUrlId).catch(reportError); } return [ - cssMenuSplitLink({href: originalUrl}, + isTutorialFork ? null : cssMenuSplitLink({href: originalUrl}, cssMenuSplitLinkText(t("Return to {{termToUse}}", {termToUse})), testId('return-to-original'), cssMenuIconLink({href: originalUrl, target: '_blank'}, testId('open-original'), cssMenuIcon('FieldLink'), @@ -179,7 +199,7 @@ function menuOriginal(doc: Document, appModel: AppModel, isSnapshot: boolean) { dom.cls('disabled', !roles.canEdit(doc.trunkAccess || null) || comparingSnapshots), testId('replace-original'), ), - menuItemLink(compareHref, {target: '_blank'}, t("Compare to {{termToUse}}", {termToUse}), + isTutorialFork ? null : menuItemLink(compareHref, {target: '_blank'}, t("Compare to {{termToUse}}", {termToUse}), menuAnnotate('Beta'), testId('compare-original'), ), diff --git a/test/nbrowser/DocTutorial.ts b/test/nbrowser/DocTutorial.ts index 8c5a6f7f..806ef994 100644 --- a/test/nbrowser/DocTutorial.ts +++ b/test/nbrowser/DocTutorial.ts @@ -9,9 +9,12 @@ describe('DocTutorial', function () { this.timeout(30000); setupTestSuite(); + gu.bigScreen(); + let doc: DocCreationInfo; let api: UserAPI; - let session: gu.Session; + let ownerSession: gu.Session; + let viewerSession: gu.Session; let oldEnv: EnvironmentSnapshot; const cleanup = setupTestSuite({team: true}); @@ -20,9 +23,9 @@ describe('DocTutorial', function () { oldEnv = new EnvironmentSnapshot(); process.env.GRIST_UI_FEATURES = 'tutorials'; await server.restart(); - session = await gu.session().teamSite.user('support').login(); - doc = await session.tempDoc(cleanup, 'DocTutorial.grist'); - api = session.createHomeApi(); + ownerSession = await gu.session().teamSite.user('support').login(); + doc = await ownerSession.tempDoc(cleanup, 'DocTutorial.grist'); + api = ownerSession.createHomeApi(); await api.updateDoc(doc.id, {type: 'tutorial'}); await api.updateDocPermissions(doc.id, {users: { 'anon@getgrist.com': 'viewers', @@ -37,11 +40,11 @@ describe('DocTutorial', function () { describe('when logged out', function () { before(async () => { - session = await gu.session().anon.login(); + viewerSession = await gu.session().anon.login(); }); it('shows a tutorial card', async function() { - await session.loadRelPath('/'); + await viewerSession.loadRelPath('/'); await gu.waitForDocMenuToLoad(); await gu.skipWelcomeQuestions(); @@ -54,7 +57,7 @@ describe('DocTutorial', function () { }); it('redirects user to log in', async function() { - await session.loadDoc(`/doc/${doc.id}`, false); + await viewerSession.loadDoc(`/doc/${doc.id}`, false); await gu.checkLoginPage(); }); }); @@ -63,13 +66,13 @@ describe('DocTutorial', function () { let forkUrl: string; before(async () => { - session = await gu.session().teamSite.user('user1').login({showTips: true}); + ownerSession = await gu.session().teamSite.user('user1').login({showTips: true}); }); afterEach(() => gu.checkForErrors()); it('shows a tutorial card', async function() { - await session.loadRelPath('/'); + await ownerSession.loadRelPath('/'); await gu.waitForDocMenuToLoad(); await gu.skipWelcomeQuestions(); @@ -97,7 +100,7 @@ describe('DocTutorial', function () { }); it('creates a fork the first time the document is opened', async function() { - await session.loadDoc(`/doc/${doc.id}`); + await ownerSession.loadDoc(`/doc/${doc.id}`); await driver.wait(async () => { forkUrl = await driver.getCurrentUrl(); return /~/.test(forkUrl); @@ -298,7 +301,18 @@ describe('DocTutorial', function () { assert.isTrue(await driver.findWait('.test-doc-tutorial-popup', 2000).isDisplayed()); }); - it('does not show the GristDocTutorial page or table', async function() { + it('shows the GristDocTutorial page and table to editors', async function() { + assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'GristDocTutorial']); + await driver.find('.test-tools-raw').click(); + await driver.findWait('.test-raw-data-list', 1000); + await gu.waitForServer(); + assert.isTrue(await driver.findContent('.test-raw-data-table-id', + /GristDocTutorial/).isPresent()); + }); + + it('does not show the GristDocTutorial page or table to non-editors', async function() { + viewerSession = await gu.session().teamSite.user('user2').login(); + await viewerSession.loadDoc(`/doc/${doc.id}`); assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2']); await driver.find('.test-tools-raw').click(); await driver.findWait('.test-raw-data-list', 1000); @@ -315,16 +329,15 @@ describe('DocTutorial', function () { }); it('only allows users access to their own forks', async function() { - const otherSession = await gu.session().teamSite.user('user2').login(); await driver.navigate().to(forkUrl); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); - await otherSession.loadDoc(`/doc/${doc.id}`); + await viewerSession.loadDoc(`/doc/${doc.id}`); let otherForkUrl: string; await driver.wait(async () => { otherForkUrl = await driver.getCurrentUrl(); return /~/.test(forkUrl); }); - session = await gu.session().teamSite.user('user1').login(); + ownerSession = await gu.session().teamSite.user('user1').login(); await driver.navigate().to(otherForkUrl!); assert.match(await driver.findWait('.test-error-header', 2000).getText(), /Access denied/); await driver.navigate().to(forkUrl); @@ -470,7 +483,7 @@ describe('DocTutorial', function () { await gu.getCell(0, 1).click(); await gu.sendKeys('Redacted', Key.ENTER); await gu.waitForServer(); - await session.loadDoc(`/doc/${doc.id}`); + await ownerSession.loadDoc(`/doc/${doc.id}`); let currentUrl: string; await driver.wait(async () => { currentUrl = await driver.getCurrentUrl(); @@ -481,7 +494,7 @@ describe('DocTutorial', function () { }); it('skips starting or resuming a tutorial if the open mode is set to default', async function() { - await session.loadDoc(`/doc/${doc.id}/m/default`); + await ownerSession.loadDoc(`/doc/${doc.id}/m/default`); assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'GristDocTutorial']); await driver.find('.test-tools-raw').click(); await gu.waitForServer(); @@ -501,7 +514,7 @@ describe('DocTutorial', function () { await driver.findWait('.test-doc-tutorial-popup', 2000); // Check that the new table isn't in the fork. - assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2']); + assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'GristDocTutorial']); assert.deepEqual(await gu.getVisibleGridCells({cols: [0], rowNums: [1]}), ['Redacted']); // Restart the tutorial. @@ -527,13 +540,53 @@ describe('DocTutorial', function () { // Check that changes made to the tutorial since it was last started are included. assert.equal(await driver.find('.test-doc-tutorial-popup-header').getText(), 'DocTutorial V2'); - assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'NewTable']); + assert.deepEqual(await gu.getPageNames(), ['Page 1', 'Page 2', 'GristDocTutorial', 'NewTable']); + }); + + it('allows editors to replace original', async function() { + // Make an edit to one of the tutorial slides. + await gu.openPage('GristDocTutorial'); + await gu.getCell(1, 1).click(); + await gu.sendKeys( + '# Intro', + Key.chord(Key.SHIFT, Key.ENTER), + Key.chord(Key.SHIFT, Key.ENTER), + 'Welcome to the Grist Basics tutorial V2.', + Key.ENTER, + ); + await gu.waitForServer(); + + // Check that the update is immediately reflected in the tutorial popup. + assert.equal( + await driver.find('.test-doc-tutorial-popup p').getText(), + 'Welcome to the Grist Basics tutorial V2.' + ); + + // Replace the original via the Share menu. + await driver.find('.test-tb-share').click(); + await driver.find('.test-replace-original').click(); + await driver.findWait('.test-modal-confirm', 3000).click(); + await gu.waitForServer(); + + // Switch to another user and restart the tutorial. + viewerSession = await gu.session().teamSite.user('user2').login(); + await viewerSession.loadDoc(`/doc/${doc.id}`); + await driver.find('.test-doc-tutorial-popup-restart').click(); + await driver.find('.test-modal-confirm').click(); + await gu.waitForServer(); + await driver.findWait('.test-doc-tutorial-popup', 2000); + + // Check that the changes we made earlier are included. + assert.equal( + await driver.find('.test-doc-tutorial-popup p').getText(), + 'Welcome to the Grist Basics tutorial V2.' + ); }); 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}`); + const otherSiteSession = await gu.session().personalSite.user('user1').addLogin(); + await otherSiteSession.loadDocMenu('/'); + await ownerSession.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 gu.waitForDocMenuToLoad(); @@ -544,8 +597,8 @@ describe('DocTutorial', function () { describe('without tutorial flag set', function () { before(async () => { await api.updateDoc(doc.id, {type: null}); - session = await gu.session().teamSite.user('user1').login(); - await session.loadDoc(`/doc/${doc.id}`); + ownerSession = await gu.session().teamSite.user('user1').login(); + await ownerSession.loadDoc(`/doc/${doc.id}`); }); afterEach(() => gu.checkForErrors()); @@ -557,10 +610,7 @@ describe('DocTutorial', function () { assert.deepEqual( await gu.getVisibleGridCells({cols: [1, 2], rowNums: [1]}), [ - "# Intro\n\nWelcome to the Grist Basics tutorial. We will cover" - + " the most important Grist concepts and features. Let’s get" - + " started.\n\n![Grist Basics Tutorial](\n" - + "https://www.getgrist.com/wp-content/uploads/2023/03/Row-1-Intro.png)", + '# Intro\n\nWelcome to the Grist Basics tutorial V2.', '', ] );