diff --git a/services/web/app/src/Features/Email/EmailBuilder.js b/services/web/app/src/Features/Email/EmailBuilder.js index 2683d71cf6..5077b17da2 100644 --- a/services/web/app/src/Features/Email/EmailBuilder.js +++ b/services/web/app/src/Features/Email/EmailBuilder.js @@ -476,7 +476,7 @@ templates.userOnboardingEmail = NoCTAEmailTemplate({ ) const userSettingsLink = EmailMessageHelper.displayLink( 'here', - `${settings.siteUrl}/user/settings`, + `${settings.siteUrl}/user/email-preferences`, isPlainText ) const onboardingSurveyLink = EmailMessageHelper.displayLink( diff --git a/services/web/app/src/Features/Newsletter/NewsletterManager.js b/services/web/app/src/Features/Newsletter/NewsletterManager.js index 80d5cd24a6..7f05d5b7b4 100644 --- a/services/web/app/src/Features/Newsletter/NewsletterManager.js +++ b/services/web/app/src/Features/Newsletter/NewsletterManager.js @@ -8,6 +8,7 @@ const OError = require('@overleaf/o-error') const provider = getProvider() module.exports = { + subscribed: callbackify(provider.subscribed), subscribe: callbackify(provider.subscribe), unsubscribe: callbackify(provider.unsubscribe), changeEmail: callbackify(provider.changeEmail), @@ -39,11 +40,27 @@ function makeMailchimpProvider() { const MAILCHIMP_LIST_ID = Settings.mailchimp.list_id return { + subscribed, subscribe, unsubscribe, changeEmail, } + async function subscribed(user) { + try { + const path = getSubscriberPath(user.email) + const result = await mailchimp.get(path) + return result?.status === 'subscribed' + } catch (err) { + if (err.status === 404) { + return false + } + throw OError.tag(err, 'error getting newsletter subscriptions status', { + userId: user._id, + }) + } + } + async function subscribe(user) { try { const path = getSubscriberPath(user.email) @@ -194,11 +211,20 @@ function makeMailchimpProvider() { function makeNullProvider() { return { + subscribed, subscribe, unsubscribe, changeEmail, } + async function subscribed(user) { + logger.info( + { user }, + 'Not checking user because no newsletter provider is configured' + ) + return false + } + async function subscribe(user) { logger.info( { user }, diff --git a/services/web/app/src/Features/Subscription/SubscriptionEmailBuilder.js b/services/web/app/src/Features/Subscription/SubscriptionEmailBuilder.js index 75a1fd8a55..bafdee408b 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionEmailBuilder.js +++ b/services/web/app/src/Features/Subscription/SubscriptionEmailBuilder.js @@ -98,7 +98,7 @@ EmailBuilder.templates.trialOnboarding = EmailBuilder.NoCTAEmailTemplate({ const unsubscribe = EmailMessageHelper.displayLink( 'here', - `${settings.siteUrl}/user/settings`, + `${settings.siteUrl}/user/email-preferences`, isPlainText ) diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index 5af414189f..be3b8e7431 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -261,6 +261,24 @@ const UserController = { ) }, + subscribe(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + UserGetter.getUser(userId, (err, user) => { + if (err != null) { + return next(err) + } + NewsletterManager.subscribe(user, err => { + if (err != null) { + OError.tag(err, 'error subscribing to newsletter') + return next(err) + } + return res.json({ + message: req.i18n.translate('thanks_settings_updated'), + }) + }) + }) + }, + unsubscribe(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) UserGetter.getUser(userId, (err, user) => { @@ -269,12 +287,12 @@ const UserController = { } NewsletterManager.unsubscribe(user, err => { if (err != null) { - logger.warn( - { err, user }, - 'Failed to unsubscribe user from newsletter' - ) + OError.tag(err, 'error unsubscribing to newsletter') + return next(err) } - res.sendStatus(200) + return res.json({ + message: req.i18n.translate('thanks_settings_updated'), + }) }) }) }, diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index 4f9433da9a..4a86c0395f 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -5,6 +5,7 @@ const logger = require('@overleaf/logger') const Settings = require('@overleaf/settings') const AuthenticationController = require('../Authentication/AuthenticationController') const SessionManager = require('../Authentication/SessionManager') +const NewsletterManager = require('../Newsletter/NewsletterManager') const _ = require('lodash') const { expressify } = require('../../util/promises') const SplitTestHandler = require('../SplitTests/SplitTestHandler') @@ -211,6 +212,25 @@ const UserPagesController = { ) }, + emailPreferencesPage(req, res, next) { + const userId = SessionManager.getLoggedInUserId(req.session) + UserGetter.getUser(userId, (err, user) => { + if (err != null) { + return next(err) + } + NewsletterManager.subscribed(user, (err, subscribed) => { + if (err != null) { + OError.tag(err, 'error getting newsletter subscription status') + return next(err) + } + res.render('user/email-preferences', { + title: 'newsletter_info_title', + subscribed, + }) + }) + }) + }, + _restructureThirdPartyIds(user) { // 3rd party identifiers are an array of objects // this turn them into a single object, which diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 17467d77b0..2ba11e92a5 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -264,11 +264,31 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { UserController.clearSessions ) + // deprecated webRouter.delete( '/user/newsletter/unsubscribe', AuthenticationController.requireLogin(), UserController.unsubscribe ) + + webRouter.post( + '/user/newsletter/unsubscribe', + AuthenticationController.requireLogin(), + UserController.unsubscribe + ) + + webRouter.post( + '/user/newsletter/subscribe', + AuthenticationController.requireLogin(), + UserController.subscribe + ) + + webRouter.get( + '/user/email-preferences', + AuthenticationController.requireLogin(), + UserPagesController.emailPreferencesPage + ) + webRouter.post( '/user/delete', RateLimiterMiddleware.rateLimit({ diff --git a/services/web/app/views/beta_program/opt_in.pug b/services/web/app/views/beta_program/opt_in.pug index 14b32ab3b5..2cb2b4ddf0 100644 --- a/services/web/app/views/beta_program/opt_in.pug +++ b/services/web/app/views/beta_program/opt_in.pug @@ -6,7 +6,7 @@ block content .row .col-md-10.col-md-offset-1.col-lg-8.col-lg-offset-2 .card - .page-header.text-centered + .page-header h1 | #{translate("sharelatex_beta_program")} .beta-opt-in @@ -49,8 +49,6 @@ block content ) span(data-ol-inflight="idle") #{translate("beta_program_opt_out_action")} span(hidden data-ol-inflight="pending") #{translate("processing")}… - .form-group - a(href="/project").btn.btn-link.btn-sm #{translate("back_to_your_projects")} else form( data-ol-regular-form @@ -65,5 +63,7 @@ block content ) span(data-ol-inflight="idle") #{translate("beta_program_opt_in_action")} span(hidden data-ol-inflight="pending") #{translate("joining")}… - .form-group - a(href="/project").btn.btn-link.btn-sm #{translate("back_to_your_projects")} + .page-separator + a.btn.btn-default.text-capitalize(href='/user/settings') #{translate('back_to_account_settings')} + | + a.btn.btn-default.text-capitalize(href='/project') #{translate('back_to_your_projects')} diff --git a/services/web/app/views/user/email-preferences.pug b/services/web/app/views/user/email-preferences.pug new file mode 100644 index 0000000000..c06fb48fdc --- /dev/null +++ b/services/web/app/views/user/email-preferences.pug @@ -0,0 +1,47 @@ +extends ../layout-marketing + +block content + main.content.content-alt#main-content + .container + .row + .col-md-10.col-md-offset-1.col-lg-8.col-lg-offset-2 + .card + .page-header + h1 #{translate("newsletter_info_title")} + + p #{translate("newsletter_info_summary")} + + - var submitAction + if subscribed + - submitAction = '/user/newsletter/unsubscribe' + p !{translate("newsletter_info_subscribed", {}, ['strong'])} + else + - submitAction = '/user/newsletter/subscribe' + p !{translate("newsletter_info_unsubscribed", {}, ['strong'])} + + form( + data-ol-async-form + data-ol-reload-on-success + name="newsletterForm" + action=submitAction + method="POST" + ) + input(name='_csrf', type='hidden', value=csrfToken) + +formMessages() + p.actions.text-center + if subscribed + button.btn-danger.btn(type='submit', data-ol-disabled-inflight) + span(data-ol-inflight="idle") #{translate("unsubscribe")} + span(hidden data-ol-inflight="pending") #{translate("saving")}… + else + button.btn-primary.btn(type='submit', data-ol-disabled-inflight) + span(data-ol-inflight="idle") #{translate("subscribe")} + span(hidden data-ol-inflight="pending") #{translate("saving")}… + + if subscribed + p #{translate("newsletter_info_note")} + + .page-separator + a.btn.btn-default.text-capitalize(href='/user/settings') #{translate('back_to_account_settings')} + | + a.btn.btn-default.text-capitalize(href='/project') #{translate('back_to_your_projects')} diff --git a/services/web/app/views/user/sessions.pug b/services/web/app/views/user/sessions.pug index 03162aca1e..1aa5d312ce 100644 --- a/services/web/app/views/user/sessions.pug +++ b/services/web/app/views/user/sessions.pug @@ -67,3 +67,7 @@ block content p.text-success.text-center | #{translate('clear_sessions_success')} + .page-separator + a.btn.btn-default.text-capitalize(href='/user/settings') #{translate('back_to_account_settings')} + | + a.btn.btn-default.text-capitalize(href='/project') #{translate('back_to_your_projects')} diff --git a/services/web/frontend/js/features/form-helpers/hydrate-form.js b/services/web/frontend/js/features/form-helpers/hydrate-form.js index 5cea762a81..65ea016ad0 100644 --- a/services/web/frontend/js/features/form-helpers/hydrate-form.js +++ b/services/web/frontend/js/features/form-helpers/hydrate-form.js @@ -57,6 +57,12 @@ function formSubmitHelper(formEl) { }) } + // Handle reloads + if (formEl.hasAttribute('data-ol-reload-on-success')) { + window.setTimeout(window.location.reload.bind(window.location), 1000) + return + } + // Let the user re-submit the form. formEl.dispatchEvent(new Event('idle')) } catch (error) { diff --git a/services/web/frontend/stylesheets/core/type.less b/services/web/frontend/stylesheets/core/type.less index 60b6fd77c9..83534f98ea 100755 --- a/services/web/frontend/stylesheets/core/type.less +++ b/services/web/frontend/stylesheets/core/type.less @@ -211,6 +211,7 @@ cite { // Page header // ------------------------- +.page-separator, .page-header { padding-bottom: ((@line-height-computed / 2) - 1); margin: (@line-height-computed) 0 @line-height-computed; diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 29a6f940a7..59abd473db 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -922,6 +922,11 @@ "newsletter": "Newsletter", "manage_newsletter": "Manage Your Newsletter Preferences", "newsletter_info_and_unsubscribe": "Every few months we send a newsletter out summarizing the new features available. If you would prefer not to receive this email then you can unsubscribe at any time:", + "newsletter_info_title": "Newsletter Preferences", + "newsletter_info_summary": "Every few months we send a newsletter out summarizing the new features available.", + "newsletter_info_subscribed": "You are currently <0>subscribed to the __appName__ newsletter. If you would prefer not to receive this email then you can unsubscribe at any time.", + "newsletter_info_unsubscribed": "You are currently <0>unsubscribed to the __appName__ newsletter.", + "newsletter_info_note": "Please note: you will still receive important emails, such as project invites and security notifications (password resets, account linking, etc).", "unsubscribed": "Unsubscribed", "unsubscribing": "Unsubscribing", "unsubscribe": "Unsubscribe", @@ -1145,6 +1150,7 @@ "add_your_first_group_member_now": "Add your first group members now", "thanks_for_subscribing_you_help_sl": "Thank you for subscribing to the __planName__ plan. It’s support from people like yourself that allows __appName__ to continue to grow and improve.", "back_to_your_projects": "Back to your projects", + "back_to_account_settings": "Back to account settings", "goes_straight_to_our_inboxes": "It goes straight to both our inboxes", "need_anything_contact_us_at": "If there is anything you ever need please feel free to contact us directly at", "regards": "Regards", diff --git a/services/web/test/unit/src/Newsletter/NewsletterManagerTests.js b/services/web/test/unit/src/Newsletter/NewsletterManagerTests.js index 689d29a37e..7d29c8707d 100644 --- a/services/web/test/unit/src/Newsletter/NewsletterManagerTests.js +++ b/services/web/test/unit/src/Newsletter/NewsletterManagerTests.js @@ -13,6 +13,7 @@ describe('NewsletterManager', function () { }, } this.mailchimp = { + get: sinon.stub(), put: sinon.stub(), patch: sinon.stub(), delete: sinon.stub(), @@ -42,6 +43,30 @@ describe('NewsletterManager', function () { this.emailHash = 'c02f60ed0ef51818186274e406c9a48f' }) + describe('subscribed', function () { + it('calls Mailchimp to get the user status', async function () { + await this.NewsletterManager.subscribed(this.user) + expect(this.mailchimp.get).to.have.been.calledWith( + `/lists/list_id/members/${this.emailHash}` + ) + }) + + it('returns true when subscribed', async function () { + this.mailchimp.get.resolves({ status: 'subscribed' }) + + const subscribed = await this.NewsletterManager.subscribed(this.user) + expect(subscribed).to.be.true + }) + + it('returns false on 404', async function () { + const err = new Error() + err.status = 404 + this.mailchimp.get.rejects(err) + const subscribed = await this.NewsletterManager.subscribed(this.user) + expect(subscribed).to.be.false + }) + }) + describe('subscribe', function () { it('calls Mailchimp to subscribe the user', async function () { await this.NewsletterManager.subscribe(this.user) diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index 52a813b906..01652fdce9 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -42,7 +42,10 @@ describe('UserController', function () { promises: { getUser: sinon.stub().resolves(this.user) }, } this.User = { findById: sinon.stub().callsArgWith(1, null, this.user) } - this.NewsLetterManager = { unsubscribe: sinon.stub().callsArgWith(1) } + this.NewsLetterManager = { + subscribe: sinon.stub().yields(), + unsubscribe: sinon.stub().yields(), + } this.AuthenticationController = { establishUserSession: sinon.stub().callsArg(2), } @@ -288,9 +291,23 @@ describe('UserController', function () { }) }) + describe('subscribe', function () { + it('should send the user to subscribe', function (done) { + this.res.json = data => { + expect(data.message).to.equal('thanks_settings_updated') + this.NewsLetterManager.subscribe + .calledWith(this.user) + .should.equal(true) + done() + } + this.UserController.subscribe(this.req, this.res) + }) + }) + describe('unsubscribe', function () { it('should send the user to unsubscribe', function (done) { - this.res.sendStatus = () => { + this.res.json = data => { + expect(data.message).to.equal('thanks_settings_updated') this.NewsLetterManager.unsubscribe .calledWith(this.user) .should.equal(true) diff --git a/services/web/test/unit/src/User/UserPagesControllerTests.js b/services/web/test/unit/src/User/UserPagesControllerTests.js index 099696f6cd..2d961488d2 100644 --- a/services/web/test/unit/src/User/UserPagesControllerTests.js +++ b/services/web/test/unit/src/User/UserPagesControllerTests.js @@ -60,6 +60,9 @@ describe('UserPagesController', function () { getLoggedInUserId: sinon.stub().returns(this.user._id), getSessionUser: sinon.stub().returns(this.user), } + this.NewsletterManager = { + subscribed: sinon.stub().yields(), + } this.AuthenticationController = { _getRedirectFromSession: sinon.stub(), setRedirectInSession: sinon.stub(), @@ -74,6 +77,7 @@ describe('UserPagesController', function () { '@overleaf/settings': this.settings, './UserGetter': this.UserGetter, './UserSessionsManager': this.UserSessionsManager, + '../Newsletter/NewsletterManager': this.NewsletterManager, '../Errors/ErrorController': this.ErrorController, '../Authentication/AuthenticationController': this.AuthenticationController, @@ -225,6 +229,34 @@ describe('UserPagesController', function () { }) }) + describe('emailPreferencesPage', function () { + beforeEach(function () { + this.UserGetter.getUser = sinon.stub().yields(null, this.user) + }) + + it('render page with subscribed status', function (done) { + this.NewsletterManager.subscribed.yields(null, true) + this.res.render = function (page, data) { + page.should.equal('user/email-preferences') + data.title.should.equal('newsletter_info_title') + data.subscribed.should.equal(true) + return done() + } + return this.UserPagesController.emailPreferencesPage(this.req, this.res) + }) + + it('render page with unsubscribed status', function (done) { + this.NewsletterManager.subscribed.yields(null, false) + this.res.render = function (page, data) { + page.should.equal('user/email-preferences') + data.title.should.equal('newsletter_info_title') + data.subscribed.should.equal(false) + return done() + } + return this.UserPagesController.emailPreferencesPage(this.req, this.res) + }) + }) + describe('settingsPage', function () { beforeEach(function () { this.request.get = sinon