Merge pull request #1670 from omnivore-app/fix/subscriptions

Fix active subscription count

packages/api/src/resolvers/subscriptions/index.ts

@@ -111,7 +111,7 @@ export const unsubscribeResolver = authorized<
       }
     }
 
-    const unsubscribed = await unsubscribe(subscription)
+    await unsubscribe(subscription)
 
     analytics.track({
       userId: uid,
@@ -124,8 +124,8 @@ export const unsubscribeResolver = authorized<
 
     return {
       subscription: {
-        ...unsubscribed,
-        newsletterEmail: unsubscribed.newsletterEmail.address,
+        ...subscription,
+        newsletterEmail: subscription.newsletterEmail.address,
       },
     }
   } catch (error) {

packages/api/src/services/newsletters.ts

@@ -1,7 +1,10 @@
 import { NewsletterEmail } from '../entity/newsletter_email'
 import { nanoid } from 'nanoid'
 import { User } from '../entity/user'
-import { CreateNewsletterEmailErrorCode } from '../generated/graphql'
+import {
+  CreateNewsletterEmailErrorCode,
+  SubscriptionStatus,
+} from '../generated/graphql'
 import { env } from '../env'
 import { getRepository } from '../entity/utils'
 import addressparser = require('nodemailer/lib/addressparser')
@@ -38,11 +41,20 @@ export const createNewsletterEmail = async (
 export const getNewsletterEmails = async (
   userId: string
 ): Promise<NewsletterEmail[]> => {
-  return getRepository(NewsletterEmail).find({
-    where: { user: { id: userId } },
-    order: { createdAt: 'DESC' },
-    relations: ['user', 'subscriptions'],
-  })
+  return getRepository(NewsletterEmail)
+    .createQueryBuilder('newsletter_email')
+    .leftJoinAndSelect('newsletter_email.user', 'user')
+    .leftJoinAndSelect(
+      'newsletter_email.subscriptions',
+      'subscriptions',
+      'subscriptions.status = :status',
+      {
+        status: SubscriptionStatus.Active,
+      }
+    )
+    .where('newsletter_email.user_id = :userId', { userId })
+    .orderBy('newsletter_email.createdAt', 'DESC')
+    .getMany()
 }
 
 export const deleteNewsletterEmail = async (id: string): Promise<boolean> => {

packages/api/src/services/subscriptions.ts

@@ -52,7 +52,6 @@ export const saveSubscription = async ({
       name,
       newsletterEmail: { id: newsletterEmail.id },
       user: { id: userId },
-      status: SubscriptionStatus.Active,
       unsubscribeHttpUrl,
       unsubscribeMailTo,
       icon,
@@ -63,9 +62,7 @@ export const saveSubscription = async ({
   return result.identifiers[0].id as string
 }
 
-export const unsubscribe = async (
-  subscription: Subscription
-): Promise<Subscription> => {
+export const unsubscribe = async (subscription: Subscription) => {
   if (subscription.unsubscribeMailTo) {
     // unsubscribe by sending email first
     await sendUnsubscribeEmail(
@@ -79,9 +76,8 @@ export const unsubscribe = async (
     throw new Error('No unsubscribe method defined')
   }
 
-  // set status to unsubscribed
-  subscription.status = SubscriptionStatus.Unsubscribed
-  return getRepository(Subscription).save(subscription)
+  // delete the subscription
+  await getRepository(Subscription).delete(subscription.id)
 }
 
 export const unsubscribeAll = async (
@@ -91,7 +87,6 @@ export const unsubscribeAll = async (
     const subscriptions = await getRepository(Subscription).find({
       where: {
         user: { id: newsletterEmail.user.id },
-        status: SubscriptionStatus.Active,
         newsletterEmail: { id: newsletterEmail.id },
       },
       relations: ['newsletterEmail'],

packages/api/test/db.ts

@@ -15,7 +15,6 @@ import { SnakeNamingStrategy } from 'typeorm-naming-strategies'
 import { SubscriptionStatus } from '../src/generated/graphql'
 import { Integration } from '../src/entity/integration'
 import { FindOptionsWhere } from 'typeorm'
-import { ContentDisplayReport } from '../src/entity/reports/content_display_report'
 
 const runMigrations = async () => {
   const migrationDirectory = __dirname + '/../../db/migrations'
@@ -198,12 +197,13 @@ export const createTestLabel = async (
 export const createTestSubscription = async (
   user: User,
   name: string,
-  newsletterEmail: NewsletterEmail
+  newsletterEmail: NewsletterEmail,
+  status = SubscriptionStatus.Active
 ): Promise<Subscription> => {
   return getRepository(Subscription).save({
     user,
     name,
-    status: SubscriptionStatus.Active,
+    status,
     newsletterEmail: { id: newsletterEmail.id },
   })
 }

packages/api/test/resolvers/newsletters.test.ts

@@ -9,13 +9,16 @@ import { generateFakeUuid, graphqlRequest, request } from '../util'
 import { NewsletterEmail } from '../../src/entity/newsletter_email'
 import { User } from '../../src/entity/user'
 import { expect } from 'chai'
-import { DeleteNewsletterEmailErrorCode } from '../../src/generated/graphql'
+import {
+  DeleteNewsletterEmailErrorCode,
+  SubscriptionStatus,
+} from '../../src/generated/graphql'
 import 'mocha'
+import { getRepository } from '../../src/entity/utils'
 
 describe('Newsletters API', () => {
   let user: User
   let authToken: string
-  let newsletterEmails: NewsletterEmail[]
 
   before(async () => {
     // create test user and login
@@ -25,20 +28,6 @@ describe('Newsletters API', () => {
       .send({ fakeEmail: user.email })
 
     authToken = res.body.authToken
-
-    //  create test newsletter emails
-    const newsletterEmail1 = await createTestNewsletterEmail(
-      user,
-      'Test_email_address_1@omnivore.app'
-    )
-    const newsletterEmail2 = await createTestNewsletterEmail(
-      user,
-      'Test_email_address_2@omnivore.app'
-    )
-    newsletterEmails = [newsletterEmail1, newsletterEmail2]
-
-    //  create testing subscriptions
-    await createTestSubscription(user, 'sub', newsletterEmail2)
   })
 
   after(async () => {
@@ -67,33 +56,94 @@ describe('Newsletters API', () => {
       }
     `
 
-    it('responds with newsletter emails sort by created_at desc', async () => {
-      const response = await graphqlRequest(query, authToken).expect(200)
-      expect(
-        response.body.data.newsletterEmails.newsletterEmails.map((e: any) => {
-          return {
-            ...e,
-            createdAt: new Date(e.createdAt).toISOString().split('.')[0] + 'Z',
-          }
-        })
-      ).to.eqls([
-        {
-          id: newsletterEmails[1].id,
-          address: newsletterEmails[1].address,
-          confirmationCode: newsletterEmails[1].confirmationCode,
-          createdAt:
-            newsletterEmails[1].createdAt.toISOString().split('.')[0] + 'Z',
-          subscriptionCount: 1,
-        },
-        {
-          id: newsletterEmails[0].id,
-          address: newsletterEmails[0].address,
-          confirmationCode: newsletterEmails[0].confirmationCode,
-          createdAt:
-            newsletterEmails[0].createdAt.toISOString().split('.')[0] + 'Z',
-          subscriptionCount: 0,
-        },
-      ])
+    context('when has active subscriptions', () => {
+      let newsletterEmails: NewsletterEmail[]
+
+      before(async () => {
+        //  create test newsletter emails
+        const newsletterEmail1 = await createTestNewsletterEmail(
+          user,
+          'Test_email_address_1@omnivore.app'
+        )
+        const newsletterEmail2 = await createTestNewsletterEmail(
+          user,
+          'Test_email_address_2@omnivore.app'
+        )
+        newsletterEmails = [newsletterEmail1, newsletterEmail2]
+
+        //  create testing subscriptions
+        await createTestSubscription(user, 'sub', newsletterEmail2)
+      })
+
+      after(async () => {
+        // clean up
+        await getRepository(NewsletterEmail).delete(
+          newsletterEmails.map((e) => e.id)
+        )
+      })
+
+      it('responds with newsletter emails sort by created_at desc', async () => {
+        const response = await graphqlRequest(query, authToken).expect(200)
+        expect(
+          response.body.data.newsletterEmails.newsletterEmails.map((e: any) => {
+            return {
+              ...e,
+              createdAt:
+                new Date(e.createdAt).toISOString().split('.')[0] + 'Z',
+            }
+          })
+        ).to.eqls([
+          {
+            id: newsletterEmails[1].id,
+            address: newsletterEmails[1].address,
+            confirmationCode: newsletterEmails[1].confirmationCode,
+            createdAt:
+              newsletterEmails[1].createdAt.toISOString().split('.')[0] + 'Z',
+            subscriptionCount: 1,
+          },
+          {
+            id: newsletterEmails[0].id,
+            address: newsletterEmails[0].address,
+            confirmationCode: newsletterEmails[0].confirmationCode,
+            createdAt:
+              newsletterEmails[0].createdAt.toISOString().split('.')[0] + 'Z',
+            subscriptionCount: 0,
+          },
+        ])
+      })
+    })
+
+    context('when unsubscribe newsletter email', () => {
+      let newsletterEmail: NewsletterEmail
+
+      before(async () => {
+        //  create test newsletter emails
+        newsletterEmail = await createTestNewsletterEmail(
+          user,
+          'Test_email_address_1@omnivore.app'
+        )
+
+        //  create unsubscribed subscriptions
+        await createTestSubscription(
+          user,
+          'sub',
+          newsletterEmail,
+          SubscriptionStatus.Unsubscribed
+        )
+      })
+
+      after(async () => {
+        // clean up
+        await getRepository(NewsletterEmail).delete(newsletterEmail.id)
+      })
+
+      it('responds with right count of subscriptions', async () => {
+        const response = await graphqlRequest(query, authToken).expect(200)
+        expect(
+          response.body.data.newsletterEmails.newsletterEmails[0]
+            .subscriptionCount
+        ).to.eqls(0)
+      })
     })
 
     it('responds status code 400 when invalid query', async () => {
@@ -175,8 +225,18 @@ describe('Newsletters API', () => {
     })
 
     context('when newsletter email exists', () => {
-      before(() => {
-        newsletterEmailId = newsletterEmails[0].id
+      before(async () => {
+        //  create test newsletter emails
+        const newsletterEmail = await createTestNewsletterEmail(
+          user,
+          'Test_email_address_1@omnivore.app'
+        )
+        newsletterEmailId = newsletterEmail.id
+      })
+
+      after(async () => {
+        // clean up
+        await getRepository(NewsletterEmail).delete(newsletterEmailId)
       })
 
       it('responds with status code 200', async () => {

packages/api/test/resolvers/subscriptions.test.ts

@@ -6,6 +6,7 @@ import 'mocha'
 import { User } from '../../src/entity/user'
 import { getRepository } from '../../src/entity/utils'
 import { NewsletterEmail } from '../../src/entity/newsletter_email'
+import { SubscriptionStatus } from '../../src/generated/graphql'
 
 describe('Subscriptions API', () => {
   let user: User
@@ -31,6 +32,12 @@ describe('Subscriptions API', () => {
     //  create testing subscriptions
     const sub1 = await createTestSubscription(user, 'sub_1', newsletterEmail)
     const sub2 = await createTestSubscription(user, 'sub_2', newsletterEmail)
+    await createTestSubscription(
+      user,
+      'sub_3',
+      newsletterEmail,
+      SubscriptionStatus.Unsubscribed
+    )
     subscriptions = [sub2, sub1]
   })
 

packages/db/migrations/0107.do.grant_delete_permission_on_subscription.sql

@@ -0,0 +1,16 @@
+-- Type: DO
+-- Name: grant_delete_permission_to_subscription
+-- Description: Add delete permission to subscription table
+
+BEGIN;
+
+GRANT DELETE ON omnivore.subscriptions TO omnivore_user;
+
+ALTER TABLE omnivore.subscriptions
+    DROP CONSTRAINT subscriptions_newsletter_email_id_fkey,
+    ADD CONSTRAINT subscriptions_newsletter_email_id_fkey
+        FOREIGN KEY (newsletter_email_id)
+        REFERENCES omnivore.newsletter_emails (id)
+        ON DELETE CASCADE;
+
+COMMIT;

packages/db/migrations/0107.undo.grant_delete_permission_on_subscription.sql

@@ -0,0 +1,15 @@
+-- Type: UNDO
+-- Name: grant_delete_permission_to_subscription
+-- Description: Add delete permission to subscription table
+
+BEGIN;
+
+REVOKE DELETE ON omnivore.subscriptions FROM omnivore_user;
+
+ALTER TABLE omnivore.subscriptions
+    DROP CONSTRAINT subscriptions_newsletter_email_id_fkey,
+    ADD CONSTRAINT subscriptions_newsletter_email_id_fkey
+        FOREIGN KEY (newsletter_email_id)
+            REFERENCES omnivore.newsletter_emails (id);
+
+COMMIT;