Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-06 22:40:14 -05:00
Code Issues Activity

Refactor validating specified newsletters in custom sign-up form (#18032)

Browse source 
refs https://github.com/TryGhost/Product/issues/3837

Moved the logic for validating specified newsletters to controller so
that the request can be failed
This commit is contained in:
Michael Barrett 2023-09-08 13:55:02 +01:00 committed by GitHub
parent 0e81eb9f1f
commit 72cc285184
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 186 additions and 169 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
ghost/members-api/lib/controllers/RouterController.js
View file

@ -16,7 +16,8 @@ const messages = {
memberNotFound: 'No member exists with this e-mail address.',
memberNotFoundSignUp: 'No member exists with this e-mail address. Please sign up first.',
invalidType: 'Invalid checkout type.',
notConfigured: 'This site is not accepting payments at the moment.'
notConfigured: 'This site is not accepting payments at the moment.',
invalidNewsletterId: 'Cannot subscribe to invalid newsletter {ids}'
};
module.exports = class RouterController {
@ -35,6 +36,7 @@ module.exports = class RouterController {
* @param {any} deps.tokenService
* @param {any} deps.sendEmailWithMagicLink
* @param {{isSet(name: string): boolean}} deps.labsService
* @param {any} deps.newslettersService
*/
constructor({
offersAPI,
@ -48,7 +50,8 @@ module.exports = class RouterController {
tokenService,
memberAttributionService,
sendEmailWithMagicLink,
labsService
labsService,
newslettersService
}) {
this._offersAPI = offersAPI;
this._paymentsService = paymentsService;
@ -62,6 +65,7 @@ module.exports = class RouterController {
this._sendEmailWithMagicLink = sendEmailWithMagicLink;
this._memberAttributionService = memberAttributionService;
this.labsService = labsService;
this._newslettersService = newslettersService;
}
async ensureStripe(_req, res, next) {
@ -476,12 +480,37 @@ module.exports = class RouterController {
});
}
// Validate requested newsletters
let {newsletters: requestedNewsletters} = req.body;
if (requestedNewsletters && requestedNewsletters.length > 0) {
const newsletterIds = requestedNewsletters.map(newsletter => newsletter.id);
const newsletters = await this._newslettersService.browse({
filter: `id:[${newsletterIds}]`,
columns: ['id','status']
});
if (newsletters.length !== newsletterIds.length) {
const validNewsletterIds = newsletters.map(newsletter => newsletter.id);
const invalidNewsletterIds = newsletterIds.filter(id => !validNewsletterIds.includes(id));
throw new errors.BadRequestError({
message: tpl(messages.invalidNewsletterId, {ids: invalidNewsletterIds})
});
}
requestedNewsletters = newsletters
.filter(newsletter => newsletter.status === 'active')
.map(newsletter => ({id: newsletter.id}));
}
// Someone tries to signup with a user that already exists
// -> doesn't really matter: we'll send a login link
const tokenData = _.pick(req.body, ['labels', 'name', 'newsletters']);
const tokenData = _.pick(req.body, ['labels', 'name']);
if (req.ip) {
tokenData.reqIp = req.ip;
}
tokenData.newsletters = requestedNewsletters;
// Save attribution data in the tokenData
tokenData.attribution = await this._memberAttributionService.getAttribution(req.body.urlHistory);

3
ghost/members-api/lib/members-api.js
View file

@ -188,7 +188,8 @@ module.exports = function MembersAPI({
tokenService,
sendEmailWithMagicLink,
memberAttributionService,
labsService
labsService,
newslettersService
});
const wellKnownController = new WellKnownController({

20
ghost/members-api/lib/repositories/MemberRepository.js
View file

@ -18,8 +18,7 @@ const messages = {
productNotFound: 'Could not find Product {id}',
bulkActionRequiresFilter: 'Cannot perform {action} without a filter or all=true',
tierArchived: 'Cannot use archived Tiers',
invalidEmail: 'Invalid Email',
invalidNewsletterId: 'Cannot subscribe to invalid newsletter {ids}'
invalidEmail: 'Invalid Email'
};
/**
@ -282,23 +281,6 @@ module.exports = class MemberRepository {
memberStatusData.status = 'comped';
}
//checks for custom signUp forms
if (memberData.newsletters && memberData.newsletters.length > 0) {
const newsletterIds = memberData.newsletters.map(newsletter => newsletter.id);
const newsletters = await this._newslettersService.browse({
filter: `id:[${newsletterIds}]`,
columns: ['id','status']
});
if (newsletters.length !== newsletterIds.length) {
const validNewsletterIds = newsletters.map(newsletter => newsletter.id);
const invalidIds = newsletterIds.filter(id => !validNewsletterIds.includes(id));
throw new errors.BadRequestError({message: tpl(messages.invalidNewsletterId, {ids: invalidIds})});
}
memberData.newsletters = newsletters
.filter(newsletter => newsletter.status === 'active')
.map(newsletter => ({id: newsletter.id}));
}
// Subscribe members to default newsletters
if (memberData.subscribed !== false && !memberData.newsletters) {
const browseOptions = _.pick(options, 'transacting');

151
ghost/members-api/test/unit/lib/controllers/router.test.js
View file

@ -90,4 +90,155 @@ describe('RouterController', function () {
sinon.restore();
});
});
describe('sendMagicLink', function () {
describe('newsletters', function () {
let req, res, sendEmailWithMagicLinkStub;
const createRouterController = (deps = {}) => {
return new RouterController({
allowSelfSignup: sinon.stub().returns(true),
memberAttributionService: {
getAttribution: sinon.stub().resolves({})
},
sendEmailWithMagicLink: sendEmailWithMagicLinkStub,
...deps
});
};
beforeEach(function () {
req = {
body: {
email: 'jamie@example.com',
emailType: 'signup'
},
get: sinon.stub()
};
res = {
writeHead: sinon.stub(),
end: sinon.stub()
};
sendEmailWithMagicLinkStub = sinon.stub().resolves();
});
it('adds specified newsletters to the tokenData', async function () {
const newsletters = [
{
id: 'abc123',
status: 'active'
},
{
id: 'def456',
status: 'active'
},
{
id: 'ghi789',
status: 'active'
}
];
req.body.newsletters = newsletters.map(newsletter => ({id: newsletter.id}));
const newsletterIds = newsletters.map(newsletter => newsletter.id);
const newslettersServiceStub = {
browse: sinon.stub()
};
newslettersServiceStub.browse
.withArgs({
filter: `id:[${newsletterIds}]`,
columns: ['id', 'status']
})
.resolves(newsletters);
const controller = createRouterController({
newslettersService: newslettersServiceStub
});
await controller.sendMagicLink(req, res);
res.writeHead.calledOnceWith(201).should.be.true();
res.end.calledOnceWith('Created.').should.be.true();
sendEmailWithMagicLinkStub.calledOnce.should.be.true();
sendEmailWithMagicLinkStub.args[0][0].tokenData.newsletters.should.eql([
{id: newsletters[0].id},
{id: newsletters[1].id},
{id: newsletters[2].id}
]);
});
it('validates specified newsletters', async function () {
const INVALID_NEWSLETTER_ID = 'abc123';
req.body.newsletters = [
{id: INVALID_NEWSLETTER_ID}
];
const newslettersServiceStub = {
browse: sinon.stub()
};
newslettersServiceStub.browse
.withArgs({
filter: `id:[${INVALID_NEWSLETTER_ID}]`,
columns: ['id', 'status']
})
.resolves([]);
const controller = createRouterController({
newslettersService: newslettersServiceStub
});
await controller.sendMagicLink(req, res).should.be.rejectedWith(`Cannot subscribe to invalid newsletter ${INVALID_NEWSLETTER_ID}`);
});
it('does not add specified newsletters to the tokenData if they are archived', async function () {
const newsletters = [
{
id: 'abc123',
status: 'active'
},
{
id: 'def456',
status: 'archived'
},
{
id: 'ghi789',
status: 'active'
}
];
req.body.newsletters = newsletters.map(newsletter => ({id: newsletter.id}));
const newsletterIds = newsletters.map(newsletter => newsletter.id);
const newslettersServiceStub = {
browse: sinon.stub()
};
newslettersServiceStub.browse
.withArgs({
filter: `id:[${newsletterIds}]`,
columns: ['id', 'status']
})
.resolves(newsletters);
const controller = createRouterController({
newslettersService: newslettersServiceStub
});
await controller.sendMagicLink(req, res);
res.writeHead.calledOnceWith(201).should.be.true();
res.end.calledOnceWith('Created.').should.be.true();
sendEmailWithMagicLinkStub.calledOnce.should.be.true();
sendEmailWithMagicLinkStub.args[0][0].tokenData.newsletters.should.eql([
{id: newsletters[0].id},
{id: newsletters[2].id}
]);
});
});
});
});

146
ghost/members-api/test/unit/lib/repositories/member.test.js
View file

@ -310,150 +310,4 @@ describe('MemberRepository', function () {
})).should.be.true();
});
});
describe('create', function () {
let memberStub, memberModelStub, newslettersServiceStub;
beforeEach(function () {
memberStub = {
get: sinon.stub(),
related: sinon.stub()
};
memberStub.related
.withArgs('products').returns({
models: []
})
.withArgs('newsletters').returns({
models: []
});
memberModelStub = {
add: sinon.stub().resolves(memberStub)
};
newslettersServiceStub = {
browse: sinon.stub()
};
});
it('subscribes a member to the specified newsletters', async function () {
const newsletters = [{
id: 'abc123',
status: 'active'
},
{
id: 'def456',
status: 'active'
},
{
id: 'ghi789',
status: 'active'
}];
const newsletterIds = newsletters.map(newsletter => newsletter.id);
newslettersServiceStub.browse
.withArgs({
filter: `id:[${newsletterIds}]`,
columns: ['id', 'status']
})
.resolves(newsletters);
const repo = new MemberRepository({
Member: memberModelStub,
MemberStatusEvent: {
add: sinon.stub().resolves()
},
MemberSubscribeEvent: {
add: sinon.stub().resolves()
},
newslettersService: newslettersServiceStub
});
await repo.create({
email: 'jamie@example.com',
email_disabled: false,
newsletters: [
{id: newsletters[0].id},
{id: newsletters[1].id},
{id: newsletters[2].id}
]
});
newslettersServiceStub.browse.calledOnce.should.be.true();
memberModelStub.add.calledOnce.should.be.true();
memberModelStub.add.args[0][0].newsletters.should.eql([
{id: newsletters[0].id},
{id: newsletters[1].id},
{id: newsletters[2].id}
]);
});
it('does not allow a member to be subscribed to an invalid newsletter', async function () {
const INVALID_NEWSLETTER_ID = 'abc123';
newslettersServiceStub.browse
.withArgs({
filter: `id:[${INVALID_NEWSLETTER_ID}]`,
columns: ['id', 'status']
})
.resolves([]);
const repo = new MemberRepository({
Member: memberModelStub,
MemberStatusEvent: {
add: sinon.stub().resolves()
},
MemberSubscribeEvent: {
add: sinon.stub().resolves()
},
newslettersService: newslettersServiceStub
});
await repo.create({
email: 'jamie@example.com',
email_disabled: false,
newsletters: [
{id: INVALID_NEWSLETTER_ID}
]
}).should.be.rejectedWith(`Cannot subscribe to invalid newsletter ${INVALID_NEWSLETTER_ID}`);
});
it('does not subscribe a member to an archived newsletter', async function () {
const newsletter = {
id: 'abc123',
status: 'archived'
};
newslettersServiceStub.browse
.withArgs({
filter: `id:[${newsletter.id}]`,
columns: ['id', 'status']
})
.resolves([newsletter]);
const repo = new MemberRepository({
Member: memberModelStub,
MemberStatusEvent: {
add: sinon.stub().resolves()
},
MemberSubscribeEvent: {
add: sinon.stub().resolves()
},
newslettersService: newslettersServiceStub
});
await repo.create({
email: 'jamie@example.com',
email_disabled: false,
newsletters: [
{id: newsletter.id}
]
});
newslettersServiceStub.browse.calledOnce.should.be.true();
memberModelStub.add.calledOnce.should.be.true();
memberModelStub.add.args[0][0].newsletters.should.eql([]);
});
});
});