Skip to content

Commit

Permalink
Fixed default feedback enabled when flag is disabled (#15660)
Browse files Browse the repository at this point in the history
fixes https://github.com/TryGhost/Team/issues/2114
fixes https://github.com/TryGhost/Team/issues/2115

When a new newsletter is created, the frontend will send feedback_enabled to true. We'll catch this in the backend and don't allow setting feedback_enabled to true when audience_feedback flag is disabled. This is also handled for editing newsletters.

To fix this in existing sites, I added a migration that disables feedback for all sites (since this is an alpha feature). Once we'll release the feature later, it will be disabled for existing newsletters, just like expected.
  • Loading branch information
SimonBackx authored Oct 24, 2022
1 parent 0f03b5e commit a650ae2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const logging = require('@tryghost/logging');
const {createTransactionalMigration} = require('../../utils');

module.exports = createTransactionalMigration(
async function up(connection) {
const affectedRows = await connection('newsletters')
.update({
feedback_enabled: false
})
.where('feedback_enabled', true);

if (affectedRows > 0) {
// Only log if this site was affected by the issue.
logging.info(`Disabled feedback for ${affectedRows} newsletter(s)`);
}
},
async function down() {
// no-op: we don't need to change it back
}
);
4 changes: 3 additions & 1 deletion ghost/core/core/server/services/newsletters/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const mail = require('../mail');
const models = require('../../models');
const urlUtils = require('../../../shared/url-utils');
const limitService = require('../limits');
const labs = require('../../../shared/labs');

const MAGIC_LINK_TOKEN_VALIDITY = 24 * 60 * 60 * 1000;

Expand All @@ -13,5 +14,6 @@ module.exports = new NewslettersService({
mail,
singleUseTokenProvider: new SingleUseTokenProvider(models.SingleUseToken, MAGIC_LINK_TOKEN_VALIDITY),
urlUtils,
limitService
limitService,
labs
});
12 changes: 11 additions & 1 deletion ghost/core/core/server/services/newsletters/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ class NewslettersService {
* @param {Object} options.singleUseTokenProvider
* @param {Object} options.urlUtils
* @param {ILimitService} options.limitService
* @param {Object} options.labs
*/
constructor({NewsletterModel, MemberModel, mail, singleUseTokenProvider, urlUtils, limitService}) {
constructor({NewsletterModel, MemberModel, mail, singleUseTokenProvider, urlUtils, limitService, labs}) {
this.NewsletterModel = NewsletterModel;
this.MemberModel = MemberModel;
this.urlUtils = urlUtils;
/** @private */
this.limitService = limitService;
/** @private */
this.labs = labs;

/* email verification setup */

Expand Down Expand Up @@ -251,6 +254,13 @@ class NewslettersService {
}
}

if (cleanedAttrs.feedback_enabled) {
if (!this.labs.isSet('audienceFeedback')) {
// Not allowed to set to true
cleanedAttrs.feedback_enabled = false;
}
}

return {cleanedAttrs, emailsToVerify};
}

Expand Down

0 comments on commit a650ae2

Please sign in to comment.