From 3d26c4bb6f76a748e202e73924d1e9bef0ad591d Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Wed, 22 Jun 2022 11:34:25 +0200 Subject: [PATCH] [web] Add new admin tool for surveys (#8356) * Setup survey module and admin page skeleton * Replace survey staff access permission with admin-only * Manage survey config with admin tool * Display configurable survey in project list + add preview in admin * Fix linting errors and unit tests * Add acceptance tests for survey module * Move survey-form to survey components * Add configuration option for Recurly group subscription users on surveys * Change survey pre-link text to a lighter gray for accessibility * Cleanup survey options implementation after review GitOrigin-RevId: 8f621951efeae458d1ab081fe98b8d0d539cca1a --- .../src/Features/Project/ProjectController.js | 13 +++++ .../Subscription/SubscriptionLocator.js | 19 +++++++ .../app/src/Features/Survey/SurveyCache.js | 25 ++++++++++ .../app/src/Features/Survey/SurveyHandler.js | 25 ++++++++++ .../app/src/Features/Survey/SurveyManager.js | 37 ++++++++++++++ .../web/app/src/infrastructure/mongodb.js | 1 + services/web/app/src/models/Survey.js | 49 +++++++++++++++++++ .../web/app/views/layout/navbar-marketing.pug | 5 ++ services/web/app/views/layout/navbar.pug | 4 ++ services/web/app/views/project/list.pug | 21 ++++++++ .../js/main/project-list/project-list.js | 9 ++++ .../stylesheets/app/sidebar-v2-dash-pane.less | 46 +++++++++++++++++ .../frontend/stylesheets/core/variables.less | 1 + .../src/Project/ProjectControllerTests.js | 4 ++ 14 files changed, 259 insertions(+) create mode 100644 services/web/app/src/Features/Survey/SurveyCache.js create mode 100644 services/web/app/src/Features/Survey/SurveyHandler.js create mode 100644 services/web/app/src/Features/Survey/SurveyManager.js create mode 100644 services/web/app/src/models/Survey.js diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 5ef812a56b..8b003854de 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -42,6 +42,7 @@ const UserPrimaryEmailCheckHandler = require('../User/UserPrimaryEmailCheckHandl const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper') const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures') const SubscriptionViewModelBuilder = require('../Subscription/SubscriptionViewModelBuilder') +const SurveyHandler = require('../Survey/SurveyHandler') const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => { if (!affiliation.institution) return false @@ -471,6 +472,17 @@ const ProjectController = { } ) }, + survey(cb) { + SurveyHandler.getSurvey(userId, (err, survey) => { + if (err) { + logger.warn({ err }, 'failed to get survey') + // do not fail loading the project list if we fail to load the survey + cb(null, null) + } else { + cb(null, survey) + } + }) + }, }, (err, results) => { if (err != null) { @@ -630,6 +642,7 @@ const ProjectController = { metadata: { viewport: false }, showThinFooter: true, // don't show the fat footer on the projects dashboard, as there's a fixed space available usersBestSubscription: results.usersBestSubscription, + survey: results.survey, } const paidUser = diff --git a/services/web/app/src/Features/Subscription/SubscriptionLocator.js b/services/web/app/src/Features/Subscription/SubscriptionLocator.js index 2819c5c5e8..f83a4d323c 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionLocator.js +++ b/services/web/app/src/Features/Subscription/SubscriptionLocator.js @@ -40,6 +40,22 @@ const SubscriptionLocator = { .exec(callback) }, + hasRecurlyGroupSubscription(userOrId, callback) { + const userId = SubscriptionLocator._getUserId(userOrId) + Subscription.exists( + { + groupPlan: true, + recurlySubscription_id: { $exists: true }, + $or: [ + { member_ids: userId }, + { manager_ids: userId }, + { admin_id: userId }, + ], + }, + callback + ) + }, + getSubscription(subscriptionId, callback) { Subscription.findOne({ _id: subscriptionId }, callback) }, @@ -110,5 +126,8 @@ SubscriptionLocator.promises = { SubscriptionLocator.getUserDeletedSubscriptions ), getDeletedSubscription: promisify(SubscriptionLocator.getDeletedSubscription), + hasRecurlyGroupSubscription: promisify( + SubscriptionLocator.hasRecurlyGroupSubscription + ), } module.exports = SubscriptionLocator diff --git a/services/web/app/src/Features/Survey/SurveyCache.js b/services/web/app/src/Features/Survey/SurveyCache.js new file mode 100644 index 0000000000..796a188ac0 --- /dev/null +++ b/services/web/app/src/Features/Survey/SurveyCache.js @@ -0,0 +1,25 @@ +const SurveyManager = require('./SurveyManager') +const { Survey } = require('../../models/Survey') +const { CacheLoader } = require('cache-flow') + +class SurveyCache extends CacheLoader { + constructor() { + super('survey', { + expirationTime: 60, // 1min in seconds + }) + } + + async load() { + return await SurveyManager.getSurvey() + } + + serialize(value) { + return value?.toObject() + } + + deserialize(value) { + return new Survey(value) + } +} + +module.exports = new SurveyCache() diff --git a/services/web/app/src/Features/Survey/SurveyHandler.js b/services/web/app/src/Features/Survey/SurveyHandler.js new file mode 100644 index 0000000000..561b211c11 --- /dev/null +++ b/services/web/app/src/Features/Survey/SurveyHandler.js @@ -0,0 +1,25 @@ +const SurveyCache = require('./SurveyCache') +const SubscriptionLocator = require('../Subscription/SubscriptionLocator') +const { callbackify } = require('../../util/promises') + +async function getSurvey(userId) { + const survey = await SurveyCache.get(true) + if (survey) { + if (survey.options?.hasRecurlyGroupSubscription) { + const hasRecurlyGroupSubscription = + await SubscriptionLocator.promises.hasRecurlyGroupSubscription(userId) + if (!hasRecurlyGroupSubscription) { + return + } + } + const { name, preText, linkText, url } = survey?.toObject() || {} + return { name, preText, linkText, url } + } +} + +module.exports = { + getSurvey: callbackify(getSurvey), + promises: { + getSurvey, + }, +} diff --git a/services/web/app/src/Features/Survey/SurveyManager.js b/services/web/app/src/Features/Survey/SurveyManager.js new file mode 100644 index 0000000000..ed8a3a3b20 --- /dev/null +++ b/services/web/app/src/Features/Survey/SurveyManager.js @@ -0,0 +1,37 @@ +const { Survey } = require('../../models/Survey') +const OError = require('@overleaf/o-error') + +async function getSurvey() { + try { + return await Survey.findOne().exec() + } catch (error) { + throw OError.tag(error, 'Failed to get survey') + } +} + +async function updateSurvey({ name, preText, linkText, url, options }) { + let survey = await getSurvey() + if (!survey) { + survey = new Survey() + } + survey.name = name + survey.preText = preText + survey.linkText = linkText + survey.url = url + survey.options = options + await survey.save() + return survey +} + +async function deleteSurvey() { + const survey = await getSurvey() + if (survey) { + await survey.remove() + } +} + +module.exports = { + getSurvey, + updateSurvey, + deleteSurvey, +} diff --git a/services/web/app/src/infrastructure/mongodb.js b/services/web/app/src/infrastructure/mongodb.js index de3e9a9a10..7a4ce4d124 100644 --- a/services/web/app/src/infrastructure/mongodb.js +++ b/services/web/app/src/infrastructure/mongodb.js @@ -64,6 +64,7 @@ async function setupDb() { db.spellingPreferences = internalDb.collection('spellingPreferences') db.splittests = internalDb.collection('splittests') db.subscriptions = internalDb.collection('subscriptions') + db.surveys = internalDb.collection('surveys') db.systemmessages = internalDb.collection('systemmessages') db.tags = internalDb.collection('tags') db.teamInvites = internalDb.collection('teamInvites') diff --git a/services/web/app/src/models/Survey.js b/services/web/app/src/models/Survey.js new file mode 100644 index 0000000000..8a5027f665 --- /dev/null +++ b/services/web/app/src/models/Survey.js @@ -0,0 +1,49 @@ +const mongoose = require('../infrastructure/Mongoose') +const { Schema } = mongoose + +const MIN_NAME_LENGTH = 3 +const MAX_NAME_LENGTH = 200 +const NAME_REGEX = /^[a-z0-9-]+$/ + +const SurveySchema = new Schema( + { + name: { + type: String, + minLength: MIN_NAME_LENGTH, + maxlength: MAX_NAME_LENGTH, + required: true, + validate: { + validator: function (input) { + return input !== null && NAME_REGEX.test(input) + }, + message: `invalid, must match: ${NAME_REGEX}`, + }, + }, + preText: { + type: String, + required: true, + }, + linkText: { + type: String, + required: true, + }, + url: { + type: String, + required: true, + }, + options: { + hasRecurlyGroupSubscription: { + type: Boolean, + default: false, + }, + }, + }, + { + collection: 'surveys', + } +) + +module.exports = { + Survey: mongoose.model('Survey', SurveySchema), + SurveySchema, +} diff --git a/services/web/app/views/layout/navbar-marketing.pug b/services/web/app/views/layout/navbar-marketing.pug index ee632951b7..4f3100127c 100644 --- a/services/web/app/views/layout/navbar-marketing.pug +++ b/services/web/app/views/layout/navbar-marketing.pug @@ -18,6 +18,8 @@ nav.navbar.navbar-default.navbar-main - var canDisplayAdminMenu = hasAdminAccess() - var canDisplayAdminRedirect = canRedirectToAdminDomain() - var canDisplaySplitTestMenu = hasFeature('saas') && (canDisplayAdminMenu || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) + - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu + .navbar-collapse.collapse(data-ol-navbar-main-collapse) ul.nav.navbar-nav.navbar-right if (canDisplayAdminMenu || canDisplayAdminRedirect || canDisplaySplitTestMenu) @@ -45,6 +47,9 @@ nav.navbar.navbar-default.navbar-main if canDisplaySplitTestMenu li a(href="/admin/split-test") Manage Split Tests + if canDisplaySurveyMenu + li + a(href="/admin/survey") Manage Surveys // loop over header_extras each item in ((splitTestVariants && (splitTestVariants['unified-navigation'] === 'show-unified-navigation')) ? nav.header_extras_unified : nav.header_extras) diff --git a/services/web/app/views/layout/navbar.pug b/services/web/app/views/layout/navbar.pug index 98ca507df3..feb995ee70 100644 --- a/services/web/app/views/layout/navbar.pug +++ b/services/web/app/views/layout/navbar.pug @@ -14,6 +14,7 @@ nav.navbar.navbar-default.navbar-main - var canDisplayAdminMenu = hasAdminAccess() - var canDisplayAdminRedirect = canRedirectToAdminDomain() - var canDisplaySplitTestMenu = hasFeature('saas') && (canDisplayAdminMenu || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) + - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu if (typeof(suppressNavbarRight) == "undefined") .navbar-collapse.collapse(collapse="navCollapsed") @@ -37,6 +38,9 @@ nav.navbar.navbar-default.navbar-main if canDisplaySplitTestMenu li a(href="/admin/split-test") Manage Split Tests + if canDisplaySurveyMenu + li + a(href="/admin/survey") Manage Surveys // loop over header_extras each item in ((splitTestVariants && (splitTestVariants['unified-navigation'] === 'show-unified-navigation')) ? nav.header_extras_unified : nav.header_extras) diff --git a/services/web/app/views/project/list.pug b/services/web/app/views/project/list.pug index ea7cbc4b59..e7758a0776 100644 --- a/services/web/app/views/project/list.pug +++ b/services/web/app/views/project/list.pug @@ -13,6 +13,7 @@ block append meta meta(name="ol-userHasNoSubscription" data-type="boolean" content=!!(settings.enableSubscriptions && !hasSubscription)) meta(name="ol-allInReconfirmNotificationPeriods" data-type="json" content=allInReconfirmNotificationPeriods) meta(name="ol-reconfirmedViaSAML" content=reconfirmedViaSAML) + meta(name="ol-survey-name" data-type="string" content=(survey ? survey.name : undefined)) block content @@ -42,6 +43,26 @@ block content .project-list-sidebar-wrapper.col-md-2.col-xs-3 aside.project-list-sidebar include ./list/side-bar + + if (survey && survey.name) + .project-list-sidebar-survey( + ng-if="shouldShowSurveyLink" + ng-cloak + ) + | #{survey.preText} + a.project-list-sidebar-survey-link( + href=survey.url + target="_blank" + rel="noreferrer noopener" + ) #{survey.linkText} + button.project-list-sidebar-survey-dismiss( + type="button" + title="Dismiss Overleaf survey" + ng-click="dismissSurvey()" + ) + span( + aria-hidden="true" + ) × .project-list-main.col-md-10.col-xs-9 include ./list/notifications diff --git a/services/web/frontend/js/main/project-list/project-list.js b/services/web/frontend/js/main/project-list/project-list.js index 1cc25b287b..a152652800 100644 --- a/services/web/frontend/js/main/project-list/project-list.js +++ b/services/web/frontend/js/main/project-list/project-list.js @@ -1,6 +1,7 @@ import _ from 'lodash' import App from '../../base' import './services/project-list' +import getMeta from '../../utils/meta' App.controller( 'ProjectPageController', function ( @@ -29,6 +30,14 @@ App.controller( newValue === 'ownerName' ? ownerNameComparator : defaultComparator }) + const surveyName = getMeta('ol-survey-name') + $scope.shouldShowSurveyLink = + localStorage(`dismissed-${surveyName}`) !== true + $scope.dismissSurvey = () => { + localStorage(`dismissed-${surveyName}`, true) + $scope.shouldShowSurveyLink = false + } + $timeout(() => recalculateProjectListHeight(), 10) $scope.$watch( diff --git a/services/web/frontend/stylesheets/app/sidebar-v2-dash-pane.less b/services/web/frontend/stylesheets/app/sidebar-v2-dash-pane.less index 5262b72058..cb7af915fc 100644 --- a/services/web/frontend/stylesheets/app/sidebar-v2-dash-pane.less +++ b/services/web/frontend/stylesheets/app/sidebar-v2-dash-pane.less @@ -11,6 +11,52 @@ padding-right: 15px; } +.project-list-sidebar-survey { + position: relative; + font-size: @font-size-small; + background-color: @v2-dash-pane-bg; + color: @v2-dash-pane-subdued-color; + padding: @folders-menu-item-v-padding 20px @folders-menu-item-v-padding + @folders-menu-item-h-padding; + &::before { + content: ''; + display: block; + height: 15px; + background-image: linear-gradient(to top, rgba(0, 0, 0, 0.1), transparent); + position: absolute; + bottom: 100%; + width: 100%; + left: 0; + } +} + +.project-list-sidebar-survey-link { + color: @v2-dash-pane-color; + font-weight: bold; + &:hover, + &:active, + &:focus { + text-decoration: none; + color: @v2-dash-pane-color; + } +} + +.project-list-sidebar-survey-dismiss { + .btn-inline-link; + position: absolute; + top: @folders-menu-item-v-padding; + right: @folders-menu-item-v-padding; + font-size: @font-size-base; + line-height: 1; + color: @v2-dash-pane-color; + &:hover, + &:active, + &:focus { + text-decoration: none; + color: @v2-dash-pane-color; + } +} + .project-list-sidebar-v2-pane { flex-grow: 0; flex-shrink: 0; diff --git a/services/web/frontend/stylesheets/core/variables.less b/services/web/frontend/stylesheets/core/variables.less index f91e9b7957..3bdd54b17c 100644 --- a/services/web/frontend/stylesheets/core/variables.less +++ b/services/web/frontend/stylesheets/core/variables.less @@ -933,6 +933,7 @@ @v2-dash-pane-bg: @ol-blue-gray-4; @v2-dash-pane-link-color: #fff; @v2-dash-pane-color: #fff; +@v2-dash-pane-subdued-color: @ol-blue-gray-1; @v2-dash-pane-toggle-color: #fff; @v2-dash-pane-btn-bg: @ol-blue-gray-5; @v2-dash-pane-btn-hover-bg: @ol-blue-gray-6; diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index b45c3b8b5f..7c7d7580b3 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -140,6 +140,9 @@ describe('ProjectController', function () { this.SubscriptionViewModelBuilder = { getBestSubscription: sinon.stub().yields(null, { type: 'free' }), } + this.SurveyHandler = { + getSurvey: sinon.stub().yields(null, {}), + } this.ProjectController = SandboxedModule.require(MODULE_PATH, { requires: { @@ -185,6 +188,7 @@ describe('ProjectController', function () { getUserDictionary: sinon.stub().yields(null, []), }, '../Institutions/InstitutionsFeatures': this.InstitutionsFeatures, + '../Survey/SurveyHandler': this.SurveyHandler, }, })