From 94b9d1fa48f47e1a0513961d3aee4f1c3f34ec41 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Tue, 5 Dec 2023 07:59:55 -0500 Subject: [PATCH] Merge pull request #16073 from overleaf/em-postpone-tutorials Support postponing tutorials GitOrigin-RevId: fe662086c87cc1909d6d9eeac07f85e306d64418 --- .../src/Features/Project/ProjectController.js | 3 +- .../Features/Tutorial/TutorialController.js | 18 +++++- .../src/Features/Tutorial/TutorialHandler.js | 40 +++++++++++-- services/web/app/src/router.js | 6 ++ .../web/app/views/project/editor/meta.pug | 2 +- .../change-list/all-history-list.tsx | 15 ++--- .../table-generator/promotion/popover.tsx | 26 +++------ .../js/shared/context/editor-context.jsx | 21 +++---- .../source-editor/source-editor.stories.tsx | 4 +- .../history/components/change-list.spec.tsx | 6 +- ...codemirror-editor-table-generator.spec.tsx | 6 +- .../src/Project/ProjectControllerTests.js | 4 ++ .../unit/src/Tutorial/TutorialHandlerTests.js | 56 +++++++++++++++++++ 13 files changed, 152 insertions(+), 55 deletions(-) create mode 100644 services/web/test/unit/src/Tutorial/TutorialHandlerTests.js diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 00aa2d61cf..51e898d2c8 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -38,6 +38,7 @@ const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures') const ProjectAuditLogHandler = require('./ProjectAuditLogHandler') const PublicAccessLevels = require('../Authorization/PublicAccessLevels') const TagsHandler = require('../Tags/TagsHandler') +const TutorialHandler = require('../Tutorial/TutorialHandler') /** * @typedef {import("./types").GetProjectsRequest} GetProjectsRequest @@ -847,7 +848,7 @@ const ProjectController = { alphaProgram: user.alphaProgram, betaProgram: user.betaProgram, labsProgram: user.labsProgram, - completedTutorials: user.completedTutorials, + inactiveTutorials: TutorialHandler.getInactiveTutorials(user), isAdmin: hasAdminAccess(user), }, userSettings: { diff --git a/services/web/app/src/Features/Tutorial/TutorialController.js b/services/web/app/src/Features/Tutorial/TutorialController.js index a550898aaf..42058b00a9 100644 --- a/services/web/app/src/Features/Tutorial/TutorialController.js +++ b/services/web/app/src/Features/Tutorial/TutorialController.js @@ -5,6 +5,7 @@ const { expressify } = require('@overleaf/promise-utils') const VALID_KEYS = [ 'react-history-buttons-tutorial', 'table-generator-promotion', + 'writefull-integration', ] async function completeTutorial(req, res, next) { @@ -12,13 +13,26 @@ async function completeTutorial(req, res, next) { const tutorialKey = req.params.tutorialKey if (!VALID_KEYS.includes(tutorialKey)) { - return res.sendStatus(400) + return res.sendStatus(404) } - await TutorialHandler.saveCompletion(userId, tutorialKey) + await TutorialHandler.setTutorialState(userId, tutorialKey, 'completed') + res.sendStatus(204) +} + +async function postponeTutorial(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + const tutorialKey = req.params.tutorialKey + + if (!VALID_KEYS.includes(tutorialKey)) { + return res.sendStatus(404) + } + + await TutorialHandler.setTutorialState(userId, tutorialKey, 'postponed') res.sendStatus(204) } module.exports = { completeTutorial: expressify(completeTutorial), + postponeTutorial: expressify(postponeTutorial), } diff --git a/services/web/app/src/Features/Tutorial/TutorialHandler.js b/services/web/app/src/Features/Tutorial/TutorialHandler.js index ceab01dada..d5819cf354 100644 --- a/services/web/app/src/Features/Tutorial/TutorialHandler.js +++ b/services/web/app/src/Features/Tutorial/TutorialHandler.js @@ -1,13 +1,45 @@ const UserUpdater = require('../User/UserUpdater') -async function saveCompletion(userId, tutorialKey) { - const completionDate = new Date() +const POSTPONE_DURATION_MS = 24 * 60 * 60 * 1000 // 1 day +/** + * Change the tutorial state + * + * @param {string} userId + * @param {string} tutorialKey + * @param {'completed' | 'postponed'} state + */ +async function setTutorialState(userId, tutorialKey, state) { await UserUpdater.promises.updateUser(userId, { $set: { - [`completedTutorials.${tutorialKey}`]: completionDate, + [`completedTutorials.${tutorialKey}`]: { state, updatedAt: new Date() }, }, }) } -module.exports = { saveCompletion } +/** + * Returns a list of inactive tutorials for a given user + * + * The user must be loaded with the completedTutorials property. + */ +function getInactiveTutorials(user, tutorialKey) { + const inactiveTutorials = [] + for (const [key, record] of Object.entries(user.completedTutorials ?? {})) { + if (record instanceof Date) { + // Legacy format: single date means the tutorial was completed + inactiveTutorials.push(key) + } else if (record.state === 'postponed') { + const postponedUntil = new Date( + record.updatedAt.getTime() + POSTPONE_DURATION_MS + ) + if (new Date() < postponedUntil) { + inactiveTutorials.push(key) + } + } else { + inactiveTutorials.push(key) + } + } + return inactiveTutorials +} + +module.exports = { setTutorialState, getInactiveTutorials } diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index d817f225b5..75f311af3b 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -426,6 +426,12 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { TutorialController.completeTutorial ) + webRouter.post( + '/tutorial/:tutorialKey/postpone', + AuthenticationController.requireLogin(), + TutorialController.postponeTutorial + ) + webRouter.get( '/user/projects', AuthenticationController.requireLogin(), diff --git a/services/web/app/views/project/editor/meta.pug b/services/web/app/views/project/editor/meta.pug index 894fa3297b..c34ffcd635 100644 --- a/services/web/app/views/project/editor/meta.pug +++ b/services/web/app/views/project/editor/meta.pug @@ -33,7 +33,7 @@ meta(name="ol-showPersonalAccessToken", data-type="boolean" content=showPersonal meta(name="ol-optionalPersonalAccessToken", data-type="boolean" content=optionalPersonalAccessToken) meta(name="ol-hasTrackChangesFeature", data-type="boolean" content=hasTrackChangesFeature) meta(name="ol-mathJax3Path" content=mathJax3Path) -meta(name="ol-completedTutorials", data-type="json" content=user.completedTutorials) +meta(name="ol-inactiveTutorials", data-type="json" content=user.inactiveTutorials) meta(name="ol-projectTags" data-type="json" content=projectTags) meta(name="ol-idePageReact", data-type="boolean" content=idePageReact) meta(name="ol-loadingText", data-type="string" content=translate("loading")) diff --git a/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx b/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx index 489e6e43dc..6be472a30e 100644 --- a/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx +++ b/services/web/frontend/js/features/history/components/change-list/all-history-list.tsx @@ -16,12 +16,9 @@ import useAsync from '@/shared/hooks/use-async' import { completeHistoryTutorial } from '../../services/api' import { debugConsole } from '@/utils/debugging' -type CompletedTutorials = { - 'react-history-buttons-tutorial': Date -} type EditorTutorials = { - completedTutorials: CompletedTutorials - setCompletedTutorial: (key: string) => void + inactiveTutorials: [string] + deactivateTutorial: (key: string) => void } function AllHistoryList() { @@ -100,18 +97,18 @@ function AllHistoryList() { } }, [updatesLoadingState]) - const { completedTutorials, setCompletedTutorial }: EditorTutorials = + const { inactiveTutorials, deactivateTutorial }: EditorTutorials = useEditorContext() const [showPopover, setShowPopover] = useState(() => { // only show tutorial popover if they haven't dismissed ("completed") it yet - return !completedTutorials?.['react-history-buttons-tutorial'] + return !inactiveTutorials.includes('react-history-buttons-tutorial') }) const completeTutorial = useCallback(() => { setShowPopover(false) - setCompletedTutorial('react-history-buttons-tutorial') - }, [setCompletedTutorial]) + deactivateTutorial('react-history-buttons-tutorial') + }, [deactivateTutorial]) const { runAsync } = useAsync() diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/promotion/popover.tsx b/services/web/frontend/js/features/source-editor/components/table-generator/promotion/popover.tsx index 049223d82d..95a9ef673a 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/promotion/popover.tsx +++ b/services/web/frontend/js/features/source-editor/components/table-generator/promotion/popover.tsx @@ -26,28 +26,20 @@ const NEW_USER_CUTOFF_TIME = new Date(2023, 8, 20).getTime() const NOW_TIME = new Date().getTime() const GRAMMARLY_CUTOFF_TIME = new Date(2023, 9, 10).getTime() -type CompletedTutorials = { - 'table-generator-promotion'?: Date | string -} type EditorTutorials = { - completedTutorials?: CompletedTutorials - setCompletedTutorial: (key: string) => void + inactiveTutorials: [string] + deactivateTutorial: (key: string) => void } const editorContextPropTypes = { - completedTutorials: PropTypes.shape({ - 'table-generator-promotion': PropTypes.oneOfType([ - PropTypes.instanceOf(Date), - PropTypes.string, - ]), - }), - setCompletedTutorial: PropTypes.func.isRequired, + inactiveTutorials: PropTypes.arrayOf(PropTypes.string).isRequired, + deactivateTutorial: PropTypes.func.isRequired, } export const PromotionOverlay: FC = ({ children }) => { const ref = useRef(null) - const { completedTutorials }: EditorTutorials = useEditorContext( + const { inactiveTutorials }: EditorTutorials = useEditorContext( editorContextPropTypes ) const { @@ -68,7 +60,7 @@ export const PromotionOverlay: FC = ({ children }) => { const showPromotion = splitTestVariants['table-generator-promotion'] === 'enabled' && - !completedTutorials?.['table-generator-promotion'] && + !inactiveTutorials.includes('table-generator-promotion') && !hideBecauseNewUser if (!showPromotion) { @@ -88,17 +80,17 @@ const PromotionOverlayContent = memo( _props, ref: Ref ) { - const { setCompletedTutorial }: EditorTutorials = useEditorContext( + const { deactivateTutorial }: EditorTutorials = useEditorContext( editorContextPropTypes ) const [timeoutExpired, setTimeoutExpired] = useState(false) const onClose = useCallback(() => { - setCompletedTutorial('table-generator-promotion') + deactivateTutorial('table-generator-promotion') postJSON('/tutorial/table-generator-promotion/complete').catch( debugConsole.error ) - }, [setCompletedTutorial]) + }, [deactivateTutorial]) const onDismiss = useCallback(() => { onClose() diff --git a/services/web/frontend/js/shared/context/editor-context.jsx b/services/web/frontend/js/shared/context/editor-context.jsx index 8da1462540..57f4892ca0 100644 --- a/services/web/frontend/js/shared/context/editor-context.jsx +++ b/services/web/frontend/js/shared/context/editor-context.jsx @@ -86,18 +86,15 @@ export function EditorProvider({ children }) { const [showSymbolPalette] = useScopeValue('editor.showSymbolPalette') const [toggleSymbolPalette] = useScopeValue('editor.toggleSymbolPalette') - const [completedTutorials, setCompletedTutorials] = useState(() => - getMeta('ol-completedTutorials') + const [inactiveTutorials, setInactiveTutorials] = useState(() => + getMeta('ol-inactiveTutorials', []) ) - const setCompletedTutorial = useCallback( + const deactivateTutorial = useCallback( tutorialKey => { - setCompletedTutorials({ - ...completedTutorials, - [tutorialKey]: new Date(), - }) + setInactiveTutorials([...inactiveTutorials, tutorialKey]) }, - [completedTutorials] + [inactiveTutorials] ) useEffect(() => { @@ -172,8 +169,8 @@ export function EditorProvider({ children }) { showSymbolPalette, toggleSymbolPalette, insertSymbol, - completedTutorials, - setCompletedTutorial, + inactiveTutorials, + deactivateTutorial, }), [ cobranding, @@ -186,8 +183,8 @@ export function EditorProvider({ children }) { showSymbolPalette, toggleSymbolPalette, insertSymbol, - completedTutorials, - setCompletedTutorial, + inactiveTutorials, + deactivateTutorial, ] ) diff --git a/services/web/frontend/stories/source-editor/source-editor.stories.tsx b/services/web/frontend/stories/source-editor/source-editor.stories.tsx index 07346a5a5e..749a1aaae9 100644 --- a/services/web/frontend/stories/source-editor/source-editor.stories.tsx +++ b/services/web/frontend/stories/source-editor/source-editor.stories.tsx @@ -126,9 +126,7 @@ export const Visual = (args: any, { globals: { theme } }: any) => { useMeta({ 'ol-showSymbolPalette': true, 'ol-mathJax3Path': 'https://unpkg.com/mathjax@3.2.2/es5/tex-svg-full.js', - 'ol-completedTutorials': { - 'table-generator-promotion': '2023-09-01T00:00:00.000Z', - }, + 'ol-inactiveTutorials': ['table-generator-promotion'], 'ol-project_id': '63e21c07946dd8c76505f85a', }) diff --git a/services/web/test/frontend/features/history/components/change-list.spec.tsx b/services/web/test/frontend/features/history/components/change-list.spec.tsx index 80874ada1a..207b3e723f 100644 --- a/services/web/test/frontend/features/history/components/change-list.spec.tsx +++ b/services/web/test/frontend/features/history/components/change-list.spec.tsx @@ -51,9 +51,9 @@ describe('change list', function () { cy.intercept('GET', '/project/*/filetree/diff*', { body: { diff: [{ pathname: 'main.tex' }, { pathname: 'name.tex' }] }, }).as('diff') - window.metaAttributesCache.set('ol-completedTutorials', { - 'react-history-buttons-tutorial': Date.now(), - }) + window.metaAttributesCache.set('ol-inactiveTutorials', [ + 'react-history-buttons-tutorial', + ]) }) describe('toggle switch', function () { diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx index 69ab9ca501..29174704fe 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx +++ b/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx @@ -111,9 +111,9 @@ describe(' Table editor', function () { cy.interceptMathJax() cy.interceptCompile('compile', Number.MAX_SAFE_INTEGER) window.metaAttributesCache.set('ol-preventCompileOnLoad', true) - window.metaAttributesCache.set('ol-completedTutorials', { - 'table-generator-promotion': '2023-09-01T00:00:00.000Z', - }) + window.metaAttributesCache.set('ol-inactiveTutorials', [ + 'table-generator-promotion', + ]) }) describe('Table rendering', function () { diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index e6fb75fba4..fe179b2cfb 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -150,6 +150,9 @@ describe('ProjectController', function () { this.ProjectAuditLogHandler = { addEntry: sinon.stub().yields(), } + this.TutorialHandler = { + getInactiveTutorials: sinon.stub().returns([]), + } this.ProjectController = SandboxedModule.require(MODULE_PATH, { requires: { @@ -192,6 +195,7 @@ describe('ProjectController', function () { '../Institutions/InstitutionsFeatures': this.InstitutionsFeatures, '../Survey/SurveyHandler': this.SurveyHandler, './ProjectAuditLogHandler': this.ProjectAuditLogHandler, + '../Tutorial/TutorialHandler': this.TutorialHandler, }, }) diff --git a/services/web/test/unit/src/Tutorial/TutorialHandlerTests.js b/services/web/test/unit/src/Tutorial/TutorialHandlerTests.js new file mode 100644 index 0000000000..42f3f6c9a2 --- /dev/null +++ b/services/web/test/unit/src/Tutorial/TutorialHandlerTests.js @@ -0,0 +1,56 @@ +const SandboxedModule = require('sandboxed-module') +const { expect } = require('chai') +const sinon = require('sinon') +const { ObjectId } = require('mongodb') + +const MODULE_PATH = '../../../../app/src/Features/Tutorial/TutorialHandler' + +describe('TutorialHandler', function () { + beforeEach(function () { + this.clock = sinon.useFakeTimers() + + this.user = { + _id: new ObjectId(), + completedTutorials: { + 'legacy-format': new Date(Date.now() - 1000), + completed: { + state: 'completed', + updatedAt: new Date(Date.now() - 1000), + }, + 'postponed-recently': { + state: 'postponed', + updatedAt: new Date(Date.now() - 1000), + }, + 'postponed-long-ago': { + state: 'postponed', + updatedAt: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000), + }, + }, + } + + this.UserUpdater = { + promises: { + updateUser: sinon.stub().resolves(), + }, + } + + this.TutorialHandler = SandboxedModule.require(MODULE_PATH, { + requires: { + '../User/UserUpdater': this.UserUpdater, + }, + }) + }) + + describe('getInactiveTutorials', function () { + it('returns all recorded tutorials except when they were posponed long ago', function () { + const hiddenTutorials = this.TutorialHandler.getInactiveTutorials( + this.user + ) + expect(hiddenTutorials).to.have.members([ + 'legacy-format', + 'completed', + 'postponed-recently', + ]) + }) + }) +})