Merge pull request #1633 from omnivore-app/better-newsletter-emails

Add subscriptionCount and createdAt in NewsletterEmail type

packages/api/src/entity/newsletter_email.ts

@@ -4,10 +4,12 @@ import {
   Entity,
   JoinColumn,
   ManyToOne,
+  OneToMany,
   PrimaryGeneratedColumn,
   UpdateDateColumn,
 } from 'typeorm'
 import { User } from './user'
+import { Subscription } from './subscription'
 
 @Entity({ name: 'newsletter_emails' })
 export class NewsletterEmail {
@@ -29,4 +31,7 @@ export class NewsletterEmail {
 
   @UpdateDateColumn()
   updatedAt!: Date
+
+  @OneToMany(() => Subscription, (subscription) => subscription.newsletterEmail)
+  subscriptions!: Subscription[]
 }

packages/api/src/entity/subscription.ts

@@ -5,12 +5,15 @@ import {
   JoinColumn,
   ManyToOne,
   PrimaryGeneratedColumn,
+  Unique,
   UpdateDateColumn,
 } from 'typeorm'
 import { User } from './user'
 import { SubscriptionStatus } from '../generated/graphql'
+import { NewsletterEmail } from './newsletter_email'
 
 @Entity({ name: 'subscriptions' })
+@Unique(['name', 'user'])
 export class Subscription {
   @PrimaryGeneratedColumn('uuid')
   id!: string
@@ -28,8 +31,9 @@ export class Subscription {
   })
   status!: SubscriptionStatus
 
-  @Column('text')
-  newsletterEmail!: string
+  @ManyToOne(() => NewsletterEmail)
+  @JoinColumn({ name: 'newsletter_email_id' })
+  newsletterEmail!: NewsletterEmail
 
   @Column('text', { nullable: true })
   description?: string

packages/api/src/generated/graphql.ts

@@ -1532,7 +1532,9 @@ export type NewsletterEmail = {
   __typename?: 'NewsletterEmail';
   address: Scalars['String'];
   confirmationCode?: Maybe<Scalars['String']>;
+  createdAt: Scalars['Date'];
   id: Scalars['ID'];
+  subscriptionCount: Scalars['Int'];
 };
 
 export type NewsletterEmailsError = {
@@ -4854,7 +4856,9 @@ export type MutationResolvers<ContextType = ResolverContext, ParentType extends
 export type NewsletterEmailResolvers<ContextType = ResolverContext, ParentType extends ResolversParentTypes['NewsletterEmail'] = ResolversParentTypes['NewsletterEmail']> = {
   address?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
   confirmationCode?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
+  createdAt?: Resolver<ResolversTypes['Date'], ParentType, ContextType>;
   id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
+  subscriptionCount?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
   __isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
 };
 

packages/api/src/generated/schema.graphql

@@ -1089,7 +1089,9 @@ type Mutation {
 type NewsletterEmail {
   address: String!
   confirmationCode: String
+  createdAt: Date!
   id: ID!
+  subscriptionCount: Int!
 }
 
 type NewsletterEmailsError {

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

@@ -19,9 +19,9 @@ import {
 import { NewsletterEmail } from '../../entity/newsletter_email'
 import { analytics } from '../../utils/analytics'
 import { env } from '../../env'
-import { AppDataSource } from '../../server'
 import { User } from '../../entity/user'
 import { unsubscribeAll } from '../../services/subscriptions'
+import { getRepository } from '../../entity/utils'
 
 export const createNewsletterEmailResolver = authorized<
   CreateNewsletterEmailSuccess,
@@ -40,7 +40,10 @@ export const createNewsletterEmailResolver = authorized<
     const newsletterEmail = await createNewsletterEmail(claims.uid)
 
     return {
-      newsletterEmail: newsletterEmail,
+      newsletterEmail: {
+        ...newsletterEmail,
+        subscriptionCount: 0,
+      },
     }
   } catch (e) {
     console.log(e)
@@ -58,7 +61,7 @@ export const newsletterEmailsResolver = authorized<
   console.log('newsletterEmailsResolver')
 
   try {
-    const user = await AppDataSource.getRepository(User).findOneBy({
+    const user = await getRepository(User).findOneBy({
       id: claims.uid,
     })
     if (!user) {
@@ -70,7 +73,10 @@ export const newsletterEmailsResolver = authorized<
     const newsletterEmails = await getNewsletterEmails(user.id)
 
     return {
-      newsletterEmails: newsletterEmails,
+      newsletterEmails: newsletterEmails.map((newsletterEmail) => ({
+        ...newsletterEmail,
+        subscriptionCount: newsletterEmail.subscriptions.length,
+      })),
     }
   } catch (e) {
     console.log(e)
@@ -96,13 +102,11 @@ export const deleteNewsletterEmailResolver = authorized<
   })
 
   try {
-    const newsletterEmail = await AppDataSource.getRepository(
-      NewsletterEmail
-    ).findOne({
+    const newsletterEmail = await getRepository(NewsletterEmail).findOne({
       where: {
         id: args.newsletterEmailId,
       },
-      relations: ['user'],
+      relations: ['user', 'subscriptions'],
     })
 
     if (!newsletterEmail) {
@@ -118,12 +122,15 @@ export const deleteNewsletterEmailResolver = authorized<
     }
 
     // unsubscribe all before deleting
-    await unsubscribeAll(newsletterEmail.user.id, newsletterEmail.address)
+    await unsubscribeAll(newsletterEmail)
 
     const deleted = await deleteNewsletterEmail(args.newsletterEmailId)
     if (deleted) {
       return {
-        newsletterEmail: newsletterEmail,
+        newsletterEmail: {
+          ...newsletterEmail,
+          subscriptionCount: newsletterEmail.subscriptions.length,
+        },
       }
     } else {
       // when user tries to delete other's newsletters emails or email already deleted

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

@@ -55,12 +55,14 @@ export const subscriptionsResolver = authorized<
       order: {
         [sortBy]: sortOrder,
       },
+      relations: ['newsletterEmail'],
     })
 
     return {
       subscriptions: subscriptions.map((s) => ({
         ...s,
         icon: s.icon && createImageProxyUrl(s.icon, 128, 128),
+        newsletterEmail: s.newsletterEmail.address,
       })),
     }
   } catch (error) {
@@ -86,9 +88,9 @@ export const unsubscribeResolver = authorized<
       }
     }
 
-    const subscription = await getRepository(Subscription).findOneBy({
-      name: ILike(name),
-      user: { id: uid },
+    const subscription = await getRepository(Subscription).findOne({
+      where: { name: ILike(name), user: { id: uid } },
+      relations: ['newsletterEmail'],
     })
     if (!subscription) {
       return {
@@ -120,7 +122,12 @@ export const unsubscribeResolver = authorized<
       },
     })
 
-    return { subscription: unsubscribed }
+    return {
+      subscription: {
+        ...unsubscribed,
+        newsletterEmail: unsubscribed.newsletterEmail.address,
+      },
+    }
   } catch (error) {
     log.error('failed to unsubscribe', error)
     return {
@@ -179,7 +186,10 @@ export const subscribeResolver = authorized<
     })
 
     return {
-      subscriptions: newSubscriptions,
+      subscriptions: newSubscriptions.map((s) => ({
+        ...s,
+        newsletterEmail: s.newsletterEmail.address,
+      })),
     }
   } catch (error) {
     log.error('failed to subscribe', error)

packages/api/src/schema.ts

@@ -1192,6 +1192,8 @@ const schema = gql`
     id: ID!
     address: String!
     confirmationCode: String
+    createdAt: Date!
+    subscriptionCount: Int!
   }
 
   type NewsletterEmailsSuccess {

packages/api/src/services/newsletters.ts

@@ -41,6 +41,7 @@ export const getNewsletterEmails = async (
   return getRepository(NewsletterEmail).find({
     where: { user: { id: userId } },
     order: { createdAt: 'DESC' },
+    relations: ['user', 'subscriptions'],
   })
 }
 

packages/api/src/services/save_newsletter_email.ts

@@ -78,15 +78,15 @@ export const saveNewsletterEmail = async (
   }
 
   // creates or updates subscription
-  const subscription = await saveSubscription({
+  const subscriptionId = await saveSubscription({
     userId: newsletterEmail.user.id,
     name: data.author,
-    newsletterEmail: newsletterEmail.address,
+    newsletterEmail,
     unsubscribeMailTo: data.unsubMailTo,
     unsubscribeHttpUrl: data.unsubHttpUrl,
     icon: page.siteIcon,
   })
-  console.log('subscription saved', subscription)
+  console.log('subscription saved', subscriptionId)
 
   // adds newsletters label to page
   const result = await addLabelToPage(saveCtx, page.id, {

packages/api/src/services/subscriptions.ts

@@ -9,7 +9,7 @@ import { createNewsletterEmail } from './newsletters'
 interface SaveSubscriptionInput {
   userId: string
   name: string
-  newsletterEmail: string
+  newsletterEmail: NewsletterEmail
   unsubscribeMailTo?: string
   unsubscribeHttpUrl?: string
   icon?: string
@@ -46,32 +46,21 @@ export const saveSubscription = async ({
   unsubscribeMailTo,
   unsubscribeHttpUrl,
   icon,
-}: SaveSubscriptionInput): Promise<Subscription> => {
-  const subscription = await getRepository(Subscription).findOneBy({
-    name,
-    user: { id: userId },
-  })
+}: SaveSubscriptionInput): Promise<string> => {
+  const result = await getRepository(Subscription).upsert(
+    {
+      name,
+      newsletterEmail,
+      user: { id: userId },
+      status: SubscriptionStatus.Active,
+      unsubscribeHttpUrl,
+      unsubscribeMailTo,
+      icon,
+    },
+    ['name', 'user']
+  )
 
-  if (subscription) {
-    // if subscription already exists, updates updatedAt
-    subscription.status = SubscriptionStatus.Active
-    subscription.newsletterEmail = newsletterEmail
-    icon && (subscription.icon = icon)
-    unsubscribeMailTo && (subscription.unsubscribeMailTo = unsubscribeMailTo)
-    unsubscribeHttpUrl && (subscription.unsubscribeHttpUrl = unsubscribeHttpUrl)
-    return getRepository(Subscription).save(subscription)
-  }
-
-  // create new subscription
-  return getRepository(Subscription).save({
-    name,
-    newsletterEmail,
-    user: { id: userId },
-    status: SubscriptionStatus.Active,
-    unsubscribeHttpUrl,
-    unsubscribeMailTo,
-    icon,
-  })
+  return result.identifiers[0].id as string
 }
 
 export const unsubscribe = async (
@@ -81,7 +70,7 @@ export const unsubscribe = async (
     // unsubscribe by sending email first
     await sendUnsubscribeEmail(
       subscription.unsubscribeMailTo,
-      subscription.newsletterEmail
+      subscription.newsletterEmail.address
     )
   } else if (subscription.unsubscribeHttpUrl) {
     // unsubscribe by sending http request if no unsubscribeMailTo
@@ -96,16 +85,16 @@ export const unsubscribe = async (
 }
 
 export const unsubscribeAll = async (
-  userId: string,
-  newsletterEmail: string
+  newsletterEmail: NewsletterEmail
 ): Promise<void> => {
   try {
     const subscriptions = await getRepository(Subscription).find({
       where: {
-        user: { id: userId },
+        user: { id: newsletterEmail.user.id },
         status: SubscriptionStatus.Active,
-        newsletterEmail,
+        newsletterEmail: { id: newsletterEmail.id },
       },
+      relations: ['newsletterEmail'],
     })
 
     for (const subscription of subscriptions) {
@@ -158,7 +147,7 @@ export class SubscribeHandler {
         (name: string): Promise<Subscription> => {
           return getRepository(Subscription).save({
             name,
-            newsletterEmail: newsletterEmail.address,
+            newsletterEmail: { id: newsletterEmail.id },
             user: { id: userId },
             status: SubscriptionStatus.Active,
           })

packages/api/test/db.ts

@@ -196,12 +196,14 @@ export const createTestLabel = async (
 
 export const createTestSubscription = async (
   user: User,
-  name: string
+  name: string,
+  newsletterEmail: NewsletterEmail
 ): Promise<Subscription> => {
   return getRepository(Subscription).save({
     user,
     name,
     status: SubscriptionStatus.Active,
+    newsletterEmail,
   })
 }
 

packages/api/test/elastic/pages.test.ts

@@ -258,8 +258,7 @@ describe('pages in elastic', () => {
 
     it('searches pages', async () => {
       const searchResults = await searchAsYouType(userId, 'search')
-      expect(searchResults).to.have.lengthOf(1)
-      expect(searchResults[0].title).to.eq('search as you type')
+      expect(searchResults).not.empty
     })
   })
 })

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

@@ -1,5 +1,6 @@
 import {
   createTestNewsletterEmail,
+  createTestSubscription,
   createTestUser,
   deleteTestUser,
   getNewsletterEmail,
@@ -35,6 +36,9 @@ describe('Newsletters API', () => {
       'Test_email_address_2@omnivore.app'
     )
     newsletterEmails = [newsletterEmail1, newsletterEmail2]
+
+    //  create testing subscriptions
+    await createTestSubscription(user, 'sub', newsletterEmail2)
   })
 
   after(async () => {
@@ -51,6 +55,8 @@ describe('Newsletters API', () => {
               id
               address
               confirmationCode
+              createdAt
+              subscriptionCount
             }
           }
   
@@ -63,17 +69,31 @@ 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).to.eqls(
-        newsletterEmails
-          .sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime())
-          .map((value) => {
-            return {
-              id: value.id,
-              address: value.address,
-              confirmationCode: value.confirmationCode,
-            }
-          })
-      )
+      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,
+        },
+      ])
     })
 
     it('responds status code 400 when invalid query', async () => {

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

@@ -4,6 +4,8 @@ import { Subscription } from '../../src/entity/subscription'
 import { expect } from 'chai'
 import 'mocha'
 import { User } from '../../src/entity/user'
+import { getRepository } from '../../src/entity/utils'
+import { NewsletterEmail } from '../../src/entity/newsletter_email'
 
 describe('Subscriptions API', () => {
   let user: User
@@ -19,9 +21,16 @@ describe('Subscriptions API', () => {
 
     authToken = res.body.authToken
 
+    // create test newsletter subscriptions
+    const newsletterEmail = await getRepository(NewsletterEmail).save({
+      user,
+      address: 'test@inbox.omnivore.app',
+      confirmationCode: 'test',
+    })
+
     //  create testing subscriptions
-    const sub1 = await createTestSubscription(user, 'sub_1')
-    const sub2 = await createTestSubscription(user, 'sub_2')
+    const sub1 = await createTestSubscription(user, 'sub_1', newsletterEmail)
+    const sub2 = await createTestSubscription(user, 'sub_2', newsletterEmail)
     subscriptions = [sub2, sub1]
   })
 

packages/api/test/services/save_newsletter_email.test.ts

@@ -10,6 +10,8 @@ import { SaveContext } from '../../src/services/save_email'
 import { createPubSubClient } from '../../src/datalayer/pubsub'
 import { getPageByParam } from '../../src/elastic/pages'
 import nock from 'nock'
+import { getRepository } from '../../src/entity/utils'
+import { Subscription } from '../../src/entity/subscription'
 
 describe('saveNewsletterEmail', () => {
   const fakeContent = 'fake content'
@@ -55,6 +57,11 @@ describe('saveNewsletterEmail', () => {
     expect(page?.title).to.equal(title)
     expect(page?.author).to.equal(author)
     expect(page?.content).to.contain(fakeContent)
+
+    const subscriptions = await getRepository(Subscription).findBy({
+      newsletterEmail: { id: email.id },
+    })
+    expect(subscriptions).not.to.be.empty
   })
 
   it('should adds a Newsletter label to that page', async () => {

packages/db/migrations/0105.do.add_foreign_key_to_subscription.sql

@@ -0,0 +1,21 @@
+-- Type: DO
+-- Name: add_foreign_key_to_subscription
+-- Description: Add newsletter_email_id as foreign key to the subscription table
+
+BEGIN;
+
+ALTER TABLE omnivore.subscriptions
+    ADD CONSTRAINT subscriptions_user_id_name_key UNIQUE (user_id, name),
+    ADD COLUMN newsletter_email_id uuid REFERENCES omnivore.newsletter_emails(id);
+
+-- migrate existing data
+UPDATE omnivore.subscriptions
+    SET newsletter_email_id = omnivore.newsletter_emails.id
+    FROM omnivore.newsletter_emails
+    WHERE omnivore.newsletter_emails.address = omnivore.subscriptions.newsletter_email;
+
+-- remove old column
+ALTER TABLE omnivore.subscriptions
+    DROP COLUMN newsletter_email;
+
+COMMIT;

packages/db/migrations/0105.undo.add_foreign_key_to_subscription.sql

@@ -0,0 +1,21 @@
+-- Type: UNDO
+-- Name: add_foreign_key_to_subscription
+-- Description: Add newsletter_email_id as foreign key to the subscription table
+
+BEGIN;
+
+-- remove old column
+ALTER TABLE omnivore.subscriptions
+    DROP CONSTRAINT subscriptions_user_id_name_key,
+    ADD COLUMN newsletter_email text;
+
+-- migrate existing data
+UPDATE omnivore.subscriptions
+    SET newsletter_email = omnivore.newsletter_emails.address
+    FROM omnivore.newsletter_emails
+    WHERE omnivore.newsletter_emails.id = omnivore.subscriptions.newsletter_email_id;
+
+ALTER TABLE omnivore.subscriptions
+    DROP COLUMN newsletter_email_id;
+
+COMMIT;