Merge pull request #2399 from omnivore-app/fix/subscription-state

fix/subscription state

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

@@ -115,6 +115,14 @@ export const markEmailAsItemResolver = authorized<
       },
       newsletterEmail
     )
+    if (!success) {
+      log.info('newsletter not created', recentEmail.id)
+
+      return {
+        errorCodes: [MarkEmailAsItemErrorCode.BadRequest],
+      }
+    }
+
     // update received email type
     await updateReceivedEmail(recentEmail.id, 'article')
 

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

@@ -111,9 +111,7 @@ export const unsubscribeResolver = authorized<
     }
 
     if (!subscription.unsubscribeMailTo && !subscription.unsubscribeHttpUrl) {
-      return {
-        errorCodes: [UnsubscribeErrorCode.UnsubscribeMethodNotFound],
-      }
+      log.info('No unsubscribe method found')
     }
 
     await unsubscribe(subscription)

packages/api/src/routers/svc/newsletters.ts

@@ -3,6 +3,7 @@ import {
   createPubSubClient,
   readPushSubscription,
 } from '../../datalayer/pubsub'
+import { SubscriptionStatus } from '../../generated/graphql'
 import {
   getNewsletterEmail,
   updateConfirmationCode,
@@ -13,6 +14,7 @@ import {
   saveNewsletterEmail,
 } from '../../services/save_newsletter_email'
 import { saveUrlFromEmail } from '../../services/save_url'
+import { getSubscriptionByNameAndUserId } from '../../services/subscriptions'
 import { isUrl } from '../../utils/helpers'
 
 interface SetConfirmationCodeMessage {
@@ -128,6 +130,16 @@ export function newsletterServiceRouter() {
           return res.status(500).send('Error saving url from email')
         }
       } else {
+        // do not subscribe if subscription already exists and is unsubscribed
+        const existingSubscription = await getSubscriptionByNameAndUserId(
+          data.author,
+          newsletterEmail.user.id
+        )
+        if (existingSubscription?.status === SubscriptionStatus.Unsubscribed) {
+          console.log('newsletter already unsubscribed:', data.author)
+          return res.status(200).send('newsletter already unsubscribed')
+        }
+
         // save newsletter instead
         const result = await saveNewsletterEmail(data, newsletterEmail, saveCtx)
         if (!result) {

packages/api/src/services/save_newsletter_email.ts

@@ -67,27 +67,24 @@ export const saveNewsletterEmail = async (
     return false
   }
 
-  if (!page.siteIcon || isBase64Image(page.siteIcon)) {
+  let icon = page.siteIcon
+  if (!icon || isBase64Image(icon)) {
     // fetch favicon if not already set or is a base64 image
-    const favicon = await fetchFavicon(page.url)
-    if (favicon) {
-      page.siteIcon = favicon
-      await updatePage(page.id, { siteIcon: favicon }, saveCtx)
+    icon = await fetchFavicon(page.url)
+    if (icon) {
+      await updatePage(page.id, { siteIcon: icon }, saveCtx)
     }
   }
 
-  // creates or updates subscription only if their is a valid unsubscribe link
-  if (data.unsubMailTo || data.unsubHttpUrl) {
-    const subscriptionId = await saveSubscription({
-      userId: newsletterEmail.user.id,
-      name: data.author,
-      newsletterEmail,
-      unsubscribeMailTo: data.unsubMailTo,
-      unsubscribeHttpUrl: data.unsubHttpUrl,
-      icon: page.siteIcon,
-    })
-    console.log('subscription saved', subscriptionId)
-  }
+  const subscriptionId = await saveSubscription({
+    userId: newsletterEmail.user.id,
+    name: data.author,
+    newsletterEmail,
+    unsubscribeMailTo: data.unsubMailTo,
+    unsubscribeHttpUrl: data.unsubHttpUrl,
+    icon,
+  })
+  console.log('subscription saved', subscriptionId)
 
   // adds newsletters label to page
   const result = await addLabelToPage(saveCtx, page.id, {

packages/api/src/services/subscriptions.ts

@@ -37,30 +37,57 @@ export const parseUnsubscribeMailTo = (unsubscribeMailTo: string) => {
 const sendUnsubscribeEmail = async (
   unsubscribeMailTo: string,
   newsletterEmail: string
-): Promise<void> => {
-  // get subject from unsubscribe email address if exists
-  const parsed = parseUnsubscribeMailTo(unsubscribeMailTo)
+): Promise<boolean> => {
+  try {
+    // get subject from unsubscribe email address if exists
+    const parsed = parseUnsubscribeMailTo(unsubscribeMailTo)
 
-  const sent = await sendEmail({
-    to: parsed.to,
-    subject: parsed.subject,
-    text: UNSUBSCRIBE_EMAIL_TEXT,
-    from: newsletterEmail,
-  })
+    const sent = await sendEmail({
+      to: parsed.to,
+      subject: parsed.subject,
+      text: UNSUBSCRIBE_EMAIL_TEXT,
+      from: newsletterEmail,
+    })
 
-  if (!sent) {
-    throw new Error(`Failed to unsubscribe, email: ${unsubscribeMailTo}`)
+    if (!sent) {
+      console.log('Failed to send unsubscribe email', unsubscribeMailTo)
+      return false
+    }
+
+    return true
+  } catch (error) {
+    console.log('Failed to send unsubscribe email', error)
+    return false
   }
 }
 
-const sendUnsubscribeHttpRequest = async (url: string): Promise<void> => {
-  const response = await axios.get(url)
+const sendUnsubscribeHttpRequest = async (url: string): Promise<boolean> => {
+  try {
+    await axios.get(url, {
+      timeout: 5000, // 5 seconds
+    })
 
-  if (response.status !== 200) {
-    throw new Error(`Failed to unsubscribe, response: ${response.statusText}`)
+    return true
+  } catch (error) {
+    if (axios.isAxiosError(error)) {
+      console.log('Failed to send unsubscribe http request', error.message)
+    } else {
+      console.log('Failed to send unsubscribe http request', error)
+    }
+    return false
   }
 }
 
+export const getSubscriptionByNameAndUserId = async (
+  name: string,
+  userId: string
+): Promise<Subscription | null> => {
+  return getRepository(Subscription).findOneBy({
+    name,
+    user: { id: userId },
+  })
+}
+
 export const saveSubscription = async ({
   userId,
   name,
@@ -85,20 +112,26 @@ export const saveSubscription = async ({
 }
 
 export const unsubscribe = async (subscription: Subscription) => {
+  let unsubscribed = false
   if (subscription.unsubscribeMailTo) {
-    // unsubscribe by sending email first
-    await sendUnsubscribeEmail(
+    // unsubscribe by sending email
+    unsubscribed = await sendUnsubscribeEmail(
       subscription.unsubscribeMailTo,
       subscription.newsletterEmail.address
     )
-  } else if (subscription.unsubscribeHttpUrl) {
-    // unsubscribe by sending http request if no unsubscribeMailTo
-    await sendUnsubscribeHttpRequest(subscription.unsubscribeHttpUrl)
-  } else {
-    throw new Error('No unsubscribe method defined')
+  }
+  // TODO: find a good way to unsubscribe by url if email fails or not provided
+  // because it often requires clicking a button on the page to unsubscribe
+
+  if (!unsubscribed) {
+    // update subscription status to unsubscribed if failed to unsubscribe
+    console.log('Failed to unsubscribe', subscription.id)
+    return getRepository(Subscription).update(subscription.id, {
+      status: SubscriptionStatus.Unsubscribed,
+    })
   }
 
-  // delete the subscription
+  // delete the subscription if successfully unsubscribed
   await getRepository(Subscription).delete(subscription.id)
 }
 
@@ -114,7 +147,7 @@ export const unsubscribeAll = async (
       relations: ['newsletterEmail'],
     })
 
-    for (const subscription of subscriptions) {
+    for await (const subscription of subscriptions) {
       try {
         await unsubscribe(subscription)
       } catch (error) {