Added newsletters to subscribe events (#14579)

refs https://github.com/TryGhost/Team/issues/1478

- Moved all admin API members tests to enable the multiple newsletters flag
- Checks if the susbcribe events are added correctly when adding or removing newsletters
- Checks if susbcribe events are added for default newsletters

core/server/api/canary/members.js

@@ -366,6 +366,9 @@ module.exports = {
             if (labsService.isSet('multipleProducts')) {
                 frame.options.withRelated.push('products');
             }
+            if (labsService.isSet('multipleNewsletters')) {
+                frame.options.withRelated.push('newsletters');
+            }
             const page = await membersService.api.members.list(frame.options);
 
             return page;

core/server/models/newsletter.js

@@ -72,6 +72,10 @@ const Newsletter = ghostBookshelf.Model.extend({
         return query;
     }
 }, {
+    orderDefaultRaw: function () {
+        return 'sort_order ASC, created_at ASC, id ASC';
+    },
+
     orderDefaultOptions: function orderDefaultOptions() {
         return {
             sort_order: 'ASC',

package.json

@@ -84,7 +84,7 @@
     "@tryghost/logging": "2.1.5",
     "@tryghost/magic-link": "1.0.21",
     "@tryghost/member-events": "0.4.1",
-    "@tryghost/members-api": "5.12.0",
+    "@tryghost/members-api": "6.0.0-alpha.0",
     "@tryghost/members-events-service": "0.3.3",
     "@tryghost/members-importer": "0.5.8",
     "@tryghost/members-offers": "0.11.1",

test/e2e-api/admin/members.test.js

@@ -9,7 +9,6 @@ const testUtils = require('../../utils');
 const Papa = require('papaparse');
 
 const models = require('../../../core/server/models');
-const {Product} = require('../../../core/server/models/product');
 
 async function assertMemberEvents({eventType, memberId, asserts}) {
     const events = await models[eventType].where('member_id', memberId).fetchAll();
@@ -26,7 +25,11 @@ async function assertSubscription(subscriptionId, asserts) {
 }
 
 async function getPaidProduct() {
-    return await Product.findOne({type: 'paid'});
+    return await models.Product.findOne({type: 'paid'});
+}
+
+async function getNewsletters() {
+    return (await models.Newsletter.findAll({filter: 'status:active'})).models;
 }
 
 const newsletterSnapshot = {
@@ -35,23 +38,17 @@ const newsletterSnapshot = {
     updated_at: anyISODateTime
 };
 
-const memberMatcherNoIncludes = {
+function buildMemberWithoutIncludesSnapshot(options) {
-    id: anyObjectId,
+    return {
-    uuid: anyUuid,
+        id: anyObjectId,
-    created_at: anyISODateTime,
+        uuid: anyUuid,
-    updated_at: anyISODateTime
+        created_at: anyISODateTime,
-};
+        updated_at: anyISODateTime,
+        newsletters: new Array(options.newsletters).fill(newsletterSnapshot)
+    };
+}
 
 const memberMatcherShallowIncludes = {
-    id: anyObjectId,
-    uuid: anyUuid,
-    created_at: anyISODateTime,
-    updated_at: anyISODateTime,
-    subscriptions: anyArray,
-    labels: anyArray
-};
-
-const memberMatcherShallowIncludesForNewsletters = {
     id: anyObjectId,
     uuid: anyUuid,
     created_at: anyISODateTime,
@@ -106,14 +103,19 @@ describe('Members API without Stripe', function () {
 });
 
 describe('Members API', function () {
+    let newsletters;
+
     before(async function () {
         agent = await agentProvider.getAdminAPIAgent();
-        await fixtureManager.init('members');
+        await fixtureManager.init('newsletters', 'members:newsletters');
         await agent.loginAsOwner();
+
+        newsletters = await getNewsletters();
     });
 
     beforeEach(function () {
         mockManager.mockLabsEnabled('multipleProducts');
+        mockManager.mockLabsEnabled('multipleNewsletters');
         mockManager.mockStripe();
         mockManager.mockMail();
     });
@@ -309,7 +311,7 @@ describe('Members API', function () {
             name: 'test',
             email: 'memberTestAdd@test.com',
             note: 'test note',
-            subscribed: false,
+            newsletters: [],
             labels: ['test-label']
         };
 
@@ -343,7 +345,7 @@ describe('Members API', function () {
         });
     });
 
-    it('Can add and send a signup confirmation email', async function () {
+    /*it('Can add and send a signup confirmation email (old)', async function () {
         const member = {
             name: 'Send Me Confirmation',
             email: 'member_getting_confirmation@test.com',
@@ -360,7 +362,7 @@ describe('Members API', function () {
             .body({members: [member]})
             .expectStatus(201)
             .matchBodySnapshot({
-                members: [memberMatcherNoIncludes]
+                members: [news]
             })
             .matchHeaderSnapshot({
                 etag: anyEtag,
@@ -396,6 +398,84 @@ describe('Members API', function () {
             ]
         });
 
+        // @TODO: do we really need to delete this member here?
+        await agent
+            .delete(`members/${body.members[0].id}/`)
+            .matchHeaderSnapshot({
+                etag: anyEtag
+            })
+            .expectStatus(204);
+
+        const events = await models.MemberSubscribeEvent.findAll();
+        assert.equal(events.models.length, 0, 'There should be no MemberSubscribeEvent remaining.');
+    });*/
+
+    it('Can add and send a signup confirmation email', async function () {
+        const member = {
+            name: 'Send Me Confirmation',
+            email: 'member_getting_confirmation@test.com',
+            newsletters: [
+                newsletters[0],
+                newsletters[1]
+            ]
+        };
+
+        const queryParams = {
+            send_email: true,
+            email_type: 'signup'
+        };
+
+        const {body} = await agent
+            .post('/members/?send_email=true&email_type=signup')
+            .body({members: [member]})
+            .expectStatus(201)
+            .matchBodySnapshot({
+                members: [
+                    buildMemberWithoutIncludesSnapshot({
+                        newsletters: 2
+                    })
+                ]
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                location: anyString
+            });
+
+        const newMember = body.members[0];
+
+        mockManager.assert.sentEmail({
+            subject: '🙌 Complete your sign up to Ghost!',
+            to: 'member_getting_confirmation@test.com'
+        });
+
+        await assertMemberEvents({
+            eventType: 'MemberStatusEvent',
+            memberId: newMember.id,
+            asserts: [
+                {
+                    from_status: null,
+                    to_status: 'free'
+                }
+            ]
+        });
+
+        await assertMemberEvents({
+            eventType: 'MemberSubscribeEvent',
+            memberId: newMember.id,
+            asserts: [
+                {
+                    subscribed: true,
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
+                },
+                {
+                    subscribed: true,
+                    source: 'admin',
+                    newsletter_id: newsletters[1].id
+                }
+            ]
+        });
+
         // @TODO: do we really need to delete this member here?
         await agent
             .delete(`members/${body.members[0].id}/`)
@@ -474,7 +554,7 @@ describe('Members API', function () {
         const initialMember = {
             name: 'Name',
             email: 'compedtest@test.com',
-            subscribed: true
+            newsletters: [newsletters[0]]
         };
 
         const compedPayload = {
@@ -520,7 +600,8 @@ describe('Members API', function () {
             memberId: newMember.id,
             asserts: [{
                 subscribed: true,
-                source: 'admin'
+                source: 'admin',
+                newsletter_id: newsletters[0].id
             }]
         });
     });
@@ -529,7 +610,7 @@ describe('Members API', function () {
         const initialMember = {
             name: 'Name',
             email: 'compedtest2@test.com',
-            subscribed: true
+            newsletters: [newsletters[0]]
         };
 
         const {body} = await agent
@@ -581,7 +662,8 @@ describe('Members API', function () {
             memberId: newMember.id,
             asserts: [{
                 subscribed: true,
-                source: 'admin'
+                source: 'admin',
+                newsletter_id: newsletters[0].id
             }]
         });
 
@@ -597,7 +679,7 @@ describe('Members API', function () {
         const initialMember = {
             name: 'Name',
             email: 'compedtest3@test.com',
-            subscribed: true,
+            newsletters: [newsletters[0]],
             products: [
                 {
                     id: product.id
@@ -651,7 +733,8 @@ describe('Members API', function () {
             asserts: [
                 {
                     subscribed: true,
-                    source: 'admin'
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
                 }
             ]
         });
@@ -669,6 +752,7 @@ describe('Members API', function () {
             name: 'Name',
             email: 'compedtest4@test.com',
             subscribed: true,
+            newsletters: [newsletters[0]],
             products: [
                 {
                     id: product.id
@@ -694,7 +778,8 @@ describe('Members API', function () {
                         yearly_price_id: anyObjectId,
                         created_at: anyISODateTime,
                         updated_at: anyISODateTime
-                    })
+                    }),
+                    newsletters: new Array(1).fill(newsletterSnapshot)
                 })
             })
             .matchHeaderSnapshot({
@@ -792,6 +877,7 @@ describe('Members API', function () {
             name: fakeCustomer.name,
             email: fakeCustomer.email,
             subscribed: true,
+            newsletters: [newsletters[0]],
             stripe_customer_id: fakeCustomer.id
         };
 
@@ -807,7 +893,8 @@ describe('Members API', function () {
                     updated_at: anyISODateTime,
                     labels: anyArray,
                     subscriptions: anyArray,
-                    products: anyArray
+                    products: anyArray,
+                    newsletters: new Array(1).fill(newsletterSnapshot)
                 })
             })
             .matchHeaderSnapshot({
@@ -918,6 +1005,7 @@ describe('Members API', function () {
             name: fakeCustomer.name,
             email: fakeCustomer.email,
             subscribed: true,
+            newsletters: [newsletters[0]],
             stripe_customer_id: fakeCustomer.id
         };
 
@@ -933,7 +1021,8 @@ describe('Members API', function () {
                     updated_at: anyISODateTime,
                     labels: anyArray,
                     subscriptions: anyArray,
-                    products: anyArray
+                    products: anyArray,
+                    newsletters: new Array(1).fill(newsletterSnapshot)
                 })
             })
             .matchHeaderSnapshot({
@@ -1020,14 +1109,16 @@ describe('Members API', function () {
             name: 'change me',
             email: 'member2Change@test.com',
             note: 'initial note',
-            subscribed: true
+            newsletters: [
+                newsletters[0]
+            ]
         };
 
         const memberChanged = {
             name: 'changed',
             email: 'cantChangeMe@test.com',
             note: 'edited note',
-            subscribed: false
+            newsletters: []
         };
 
         const {body} = await agent
@@ -1048,7 +1139,8 @@ describe('Members API', function () {
             memberId: newMember.id,
             asserts: [{
                 subscribed: true,
-                source: 'admin'
+                source: 'admin',
+                newsletter_id: newsletters[0].id
             }]
         });
         await assertMemberEvents({
@@ -1085,15 +1177,131 @@ describe('Members API', function () {
             asserts: [
                 {
                     subscribed: true,
-                    source: 'admin'
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
                 }, {
                     subscribed: false,
-                    source: 'admin'
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
                 }
             ]
         });
     });
 
+    it('Can subscribe to a newsletter', async function () {
+        const memberToChange = {
+            name: 'change me',
+            email: 'member3change@test.com',
+            newsletters: [
+                newsletters[0]
+            ]
+        };
+
+        const memberChanged = {
+            newsletters: [
+                newsletters[1]
+            ]
+        };
+
+        const {body} = await agent
+            .post(`/members/`)
+            .body({members: [memberToChange]})
+            .expectStatus(201)
+            .matchBodySnapshot({
+                members: new Array(1).fill(memberMatcherShallowIncludes)
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                location: anyLocationFor('members')
+            });
+        const newMember = body.members[0];
+
+        await assertMemberEvents({
+            eventType: 'MemberSubscribeEvent',
+            memberId: newMember.id,
+            asserts: [{
+                subscribed: true,
+                source: 'admin',
+                newsletter_id: newsletters[0].id
+            }]
+        });
+       
+        await agent
+            .put(`/members/${newMember.id}/`)
+            .body({members: [memberChanged]})
+            .expectStatus(200)
+            .matchBodySnapshot({
+                members: new Array(1).fill(memberMatcherShallowIncludes)
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag
+            });
+
+        await assertMemberEvents({
+            eventType: 'MemberSubscribeEvent',
+            memberId: newMember.id,
+            asserts: [
+                {
+                    subscribed: true,
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
+                }, {
+                    subscribed: true,
+                    source: 'admin',
+                    newsletter_id: newsletters[1].id
+                }, {
+                    subscribed: false,
+                    source: 'admin',
+                    newsletter_id: newsletters[0].id
+                }
+            ]
+        });
+    });
+
+    it('Subscribes to default newsletters', async function () {
+        const filtered = newsletters.filter(n => n.get('subscribe_on_signup'));
+        filtered.length.should.be.greaterThan(0, 'There should be at least one newsletter with subscribe on signup for this test to work');
+
+        const memberToCreate = {
+            name: 'create me',
+            email: 'member2create@test.com'
+        };
+
+        const {body} = await agent
+            .post(`/members/`)
+            .body({members: [memberToCreate]})
+            .expectStatus(201)
+            .matchBodySnapshot({
+                members: new Array(1).fill(memberMatcherShallowIncludes)
+            })
+            .matchHeaderSnapshot({
+                etag: anyEtag,
+                location: anyLocationFor('members')
+            });
+
+        const newMember = body.members[0];
+        newMember.newsletters.should.match([
+            {
+                id: filtered[0].id
+            },
+            {
+                id: filtered[1].id
+            }
+        ]);
+
+        await assertMemberEvents({
+            eventType: 'MemberSubscribeEvent',
+            memberId: newMember.id,
+            asserts: filtered.map((n) => {
+                return {
+                    subscribed: true,
+                    source: 'admin',
+                    newsletter_id: n.id
+                };
+            })
+        });
+    });
+
     it('Can add a subscription', async function () {
         const memberId = testUtils.DataGenerator.Content.members[0].id;
         const price = testUtils.DataGenerator.Content.stripe_prices[0];
@@ -1156,7 +1364,8 @@ describe('Members API', function () {
                                 product_id: anyObjectId
                             }
                         }
-                    }]
+                    }],
+                    newsletters: anyArray
                 })
             })
             .matchHeaderSnapshot({
@@ -1183,7 +1392,8 @@ describe('Members API', function () {
                                 product_id: anyObjectId
                             }
                         }
-                    }]
+                    }],
+                    newsletters: anyArray
                 })
             })
             .matchHeaderSnapshot({
@@ -1339,90 +1549,13 @@ describe('Members API', function () {
                 }]
             });
     });
-});
-
-describe('Members API: with multiple newsletters', function () {
-    before(async function () {
-        agent = await agentProvider.getAdminAPIAgent();
-        await fixtureManager.init('newsletters', 'members:newsletters');
-        await agent.loginAsOwner();
-    });
-
-    beforeEach(function () {
-        mockManager.mockLabsEnabled('multipleNewsletters');
-        mockManager.mockStripe();
-        mockManager.mockMail();
-    });
-
-    afterEach(function () {
-        mockManager.restore();
-    });
-
-    // List Members
-
-    it('Can browse', async function () {
-        await agent
-            .get('/members/')
-            .expectStatus(200)
-            .matchBodySnapshot({
-                members: new Array(8).fill(memberMatcherShallowIncludesForNewsletters)
-            })
-            .matchHeaderSnapshot({
-                etag: anyEtag
-            });
-    });
-
-    // Read a member
-
-    it('Can read', async function () {
-        await agent
-            .get(`/members/${testUtils.DataGenerator.Content.members[0].id}/`)
-            .expectStatus(200)
-            .matchBodySnapshot({
-                members: new Array(1).fill(memberMatcherShallowIncludesForNewsletters)
-            })
-            .matchHeaderSnapshot({
-                etag: anyEtag
-            });
-    });
-
-    // Create a member
-    it('Can add with default newsletters', async function () {
-        const member = {
-            name: 'test',
-            email: 'memberTestNewsletterAdd@test.com',
-            note: 'test note',
-            subscribed: false,
-            labels: ['test-label']
-        };
-
-        await agent
-            .post(`/members/`)
-            .body({members: [member]})
-            .expectStatus(201)
-            .matchBodySnapshot({
-                members: [{
-                    id: anyObjectId,
-                    uuid: anyUuid,
-                    created_at: anyISODateTime,
-                    updated_at: anyISODateTime,
-                    subscriptions: anyArray,
-                    labels: anyArray,
-                    newsletters: Array(2).fill(newsletterSnapshot)
-                }]
-            })
-            .matchHeaderSnapshot({
-                etag: anyEtag,
-                location: anyLocationFor('members')
-            });
-    });
 
     it('Can filter on newsletter slug', async function () {
         await agent
             .get('/members/?filter=newsletters:weekly-newsletter')
             .expectStatus(200)
             .matchBodySnapshot({
-                members: new Array(4).fill(memberMatcherShallowIncludesForNewsletters)
+                members: new Array(4).fill(memberMatcherShallowIncludes)
             })
             .matchHeaderSnapshot({
                 etag: anyEtag

yarn.lock

@@ -2046,10 +2046,10 @@
     "@tryghost/domain-events" "^0.1.9"
     "@tryghost/member-events" "^0.4.1"
 
-"@tryghost/members-api@5.12.0":
+"@tryghost/members-api@6.0.0-alpha.0":
-  version "5.12.0"
+  version "6.0.0-alpha.0"
-  resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-5.12.0.tgz#b26389fef5e66602768e953cdc37da6714bd693f"
+  resolved "https://registry.yarnpkg.com/@tryghost/members-api/-/members-api-6.0.0-alpha.0.tgz#91f99cc7076c612ccb84d8b8dd6135246968b842"
-  integrity sha512-lczUcc5Vd3WKUaBu4x2aP5y9CqFqqHEFOS5Rh/KvYD6Nu8UDdjxtb6mok9IngH48UG2RS8y3WoWkJFNtSNBfpQ==
+  integrity sha512-qV7WEK7GZ3ejenIThj8+bSUa7ubKSN5iqB0LZXXFwihEWaySheZhNXhNJda6e1MiZwQYqQBH1TrVuv4hKfRwCA==
   dependencies:
     "@nexes/nql" "^0.6.0"
     "@tryghost/debug" "^0.1.2"