Fixed bulk unsubscribe counts (#14871)

refs https://ghost.slack.com/archives/C02G9E68C/p1652980792270029

- When bulk unsubscribing members, the number of deleted newsletter relations are returned instead of the number of members with newsletters that were cleared
- Updates members-api to 8.1.0, which uses this new option to delete newsletter relations by member_id instead of the id of the relation (which allows us to fetch the number of successfully/failed member deletes) Changes: https://github.com/TryGhost/Members/pull/400
- Added tests for bulk unsubscribe and bulk delete labels (because they both use the updated bulkDestroy method)
This commit is contained in:
Simon Backx 2022-05-20 13:40:55 +02:00 committed by GitHub
parent b9e520c657
commit 419fa24f27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 535 additions and 6 deletions

View File

@ -85,7 +85,7 @@
"@tryghost/logging": "2.1.8",
"@tryghost/magic-link": "1.0.26",
"@tryghost/member-events": "0.4.6",
"@tryghost/members-api": "8.0.1",
"@tryghost/members-api": "8.1.0",
"@tryghost/members-events-service": "0.4.3",
"@tryghost/members-importer": "0.5.15",
"@tryghost/members-offers": "0.11.6",

View File

@ -30,6 +30,249 @@ Object {
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 2,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members 3: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members 4: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members with filters 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk delete a label from members with filters 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with deprecated subscribed filter (actual) 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 6,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with deprecated subscribed filter (actual) 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with deprecated subscribed filter 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 2,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with deprecated subscribed filter 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with filter 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with filter 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with filter 3: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Can bulk unsubscribe members with filter 4: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations Doesn't delete labels apart from the passed label id 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations Doesn't delete labels apart from the passed label id 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Bulk operations doesn't delete labels apart from the passed label id 1: [body] 1`] = `
Object {
"bulk": Object {
"meta": Object {
"errors": Array [],
"stats": Object {
"successful": 1,
"unsuccessful": 0,
},
"unsuccessfulData": Array [],
},
},
}
`;
exports[`Members API Bulk operations doesn't delete labels apart from the passed label id 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "95",
"content-type": "application/json; charset=utf-8",
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
}
`;
exports[`Members API Can add 1: [body] 1`] = `
Object {
"members": Array [

View File

@ -214,7 +214,7 @@ describe('Members Importer API', function () {
should.exist(bulkUnsubscribeResponse.body.bulk.meta);
should.exist(bulkUnsubscribeResponse.body.bulk.meta.stats);
should.exist(bulkUnsubscribeResponse.body.bulk.meta.stats.successful);
should.equal(bulkUnsubscribeResponse.body.bulk.meta.stats.successful, 8 * filteredNewsletters.length);
should.equal(bulkUnsubscribeResponse.body.bulk.meta.stats.successful, 8);
const postUnsubscribeBrowseResponse = await request
.get(localUtils.API.getApiQuery('members/?filter=label:bulk-unsubscribe-test'))

View File

@ -1991,3 +1991,289 @@ describe('Members API', function () {
});
});
});
describe('Members API Bulk operations', function () {
beforeEach(async function () {
agent = await agentProvider.getAdminAPIAgent();
await fixtureManager.init('newsletters', 'members:newsletters');
await agent.loginAsOwner();
mockManager.mockStripe();
mockManager.mockMail();
});
afterEach(function () {
mockManager.restore();
});
it('Can bulk unsubscribe members with filter', async function () {
// This member has 2 subscriptions
const member = fixtureManager.get('members', 4);
const newsletterCount = 2;
const model = await models.Member.findOne({id: member.id}, {withRelated: 'newsletters'});
should(model.relations.newsletters.models.length).equal(newsletterCount, 'This test requires a member with 2 or more newsletters');
await agent
.put(`/members/bulk/?filter=id:${member.id}`)
.body({bulk: {
action: 'unsubscribe'
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
// Should contain the count of members, not the newsletter count!
successful: 1,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
const updatedModel = await models.Member.findOne({id: member.id}, {withRelated: 'newsletters'});
should(updatedModel.relations.newsletters.models.length).equal(0, 'This member should be unsubscribed from all newsletters');
// When we do it again, we should still receive a count of 1, because we unsubcribed one member (who happens to be already unsubscribed)
await agent
.put(`/members/bulk/?filter=id:${member.id}`)
.body({bulk: {
action: 'unsubscribe'
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
// Should contain the count of members, not the newsletter count!
successful: 1,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
it('Can bulk unsubscribe members with deprecated subscribed filter', async function () {
await agent
.put(`/members/bulk/?filter=subscribed:false`)
.body({bulk: {
action: 'unsubscribe'
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 2, // We have two members who are subscribed to an inactive newsletter
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
it('Can bulk unsubscribe members with deprecated subscribed filter (actual)', async function () {
// This member is subscribed to an inactive newsletter
const ignoredMember = fixtureManager.get('members', 6);
await agent
.put(`/members/bulk/?filter=subscribed:true`)
.body({bulk: {
action: 'unsubscribe'
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 6, // not 7 because members subscribed to an inactive newsletter aren't subscribed (newsletter fixture[2])
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
const allMembers = await models.Member.findAll({withRelated: 'newsletters'});
for (const model of allMembers) {
if (model.id === ignoredMember.id) {
continue;
}
should(model.relations.newsletters.models.length).equal(0, 'This member should be unsubscribed from all newsletters');
}
});
it('Can bulk delete a label from members', async function () {
await agent
.put(`/members/bulk/?all=true`)
.body({bulk: {
action: 'removeLabel',
meta: {
label: {
// Note! this equals DataGenerator.Content.labels[2]
// the index is different in the fixtureManager
id: fixtureManager.get('labels', 1).id
}
}
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 2,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
await agent
.put(`/members/bulk/?all=true`)
.body({bulk: {
action: 'removeLabel',
meta: {
label: {
id: fixtureManager.get('labels', 0).id
}
}
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 1,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
});
it(`Doesn't delete labels apart from the passed label id`, async function () {
const member = fixtureManager.get('members', 1);
// Manually add 2 labels to a member
await models.Member.edit({labels: [{name: 'first-tag'}, {name: 'second-tag'}]}, {id: member.id});
const model = await models.Member.findOne({id: member.id}, {withRelated: 'labels'});
should(model.relations.labels.models.map(m => m.get('name'))).match(['first-tag', 'second-tag']);
const firstId = model.relations.labels.models[0].id;
const secondId = model.relations.labels.models[1].id;
// Delete first label only
await agent
.put(`/members/bulk/?all=true`)
.body({bulk: {
action: 'removeLabel',
meta: {
label: {
id: secondId
}
}
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 1,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
const updatedModel = await models.Member.findOne({id: member.id}, {withRelated: 'labels'});
should(updatedModel.relations.labels.models.map(m => m.id)).match([firstId]);
});
it('Can bulk delete a label from members with filters', async function () {
const member1 = fixtureManager.get('members', 0);
const member2 = fixtureManager.get('members', 1);
// Manually add 2 labels to a member
await models.Member.edit({labels: [{name: 'first-tag'}, {name: 'second-tag'}]}, {id: member1.id});
const model1 = await models.Member.findOne({id: member1.id}, {withRelated: 'labels'});
should(model1.relations.labels.models.map(m => m.get('name'))).match(['first-tag', 'second-tag']);
const firstId = model1.relations.labels.models[0].id;
const secondId = model1.relations.labels.models[1].id;
await models.Member.edit({labels: [{name: 'first-tag'}, {name: 'second-tag'}]}, {id: member2.id});
const model2 = await models.Member.findOne({id: member2.id}, {withRelated: 'labels'});
should(model2.relations.labels.models.map(m => m.id)).match([firstId, secondId]);
await agent
.put(`/members/bulk/?filter=id:${member1.id}`)
.body({bulk: {
action: 'removeLabel',
meta: {
label: {
// Note! this equals DataGenerator.Content.labels[2]
// the index is different in the fixtureManager
id: firstId
}
}
}})
.expectStatus(200)
.matchBodySnapshot({
bulk: {
meta: {
stats: {
successful: 1,
unsuccessful: 0
},
unsuccessfulData: [],
errors: []
}
}
})
.matchHeaderSnapshot({
etag: anyEtag
});
const updatedModel1 = await models.Member.findOne({id: member1.id}, {withRelated: 'labels'});
should(updatedModel1.relations.labels.models.map(m => m.id)).match([secondId]);
const updatedModel2 = await models.Member.findOne({id: member2.id}, {withRelated: 'labels'});
should(updatedModel2.relations.labels.models.map(m => m.id)).match([firstId, secondId]);
});
});

View File

@ -1861,10 +1861,10 @@
"@tryghost/domain-events" "^0.1.14"
"@tryghost/member-events" "^0.4.6"
"@tryghost/members-api@8.0.1":
version "8.0.1"
resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-8.0.1.tgz#f4aa3c74701a6689c816d63437545872f9d949b9"
integrity sha512-9/IGfDSF/ZDFfRJH6t/bjb2ldQ+V4JG3OrT64npZsDx1JFaEWDijbN6Hn6dC2aMqCvirQuPAMQYjC6QAqHNq9g==
"@tryghost/members-api@8.1.0":
version "8.1.0"
resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-8.1.0.tgz#756e41a0cf1bc9680bb4e580348d4aebeb8825bc"
integrity sha512-iYo19Z/+ktonIh95sKlia5PJ1nZ/+r7Eg/ygwfU3X2kminhag9vedcHHb9s3h2PTjsx7Rjuy1BxDZMgrgV0L5g==
dependencies:
"@nexes/nql" "^0.6.0"
"@tryghost/debug" "^0.1.2"