fix a bug of using article as table name in the reminders query (#34)

* fix a bug of using article as table name in the reminders query

* fix test

* increase timeout value

* increase timeout value to 50000

* add import mocha

* add import mocha for label test

* add import mocha for every test

* fix test in deleting labels

* add timeout for each label test

* one more test

* fix reminders router by allowing sendrid template id to be null

* do not retry if reminders not found

packages/api/src/datalayer/reminders/index.ts

@@ -15,10 +15,10 @@ import { UserArticleData } from '../links/model'
 const JOIN_COLS = [
   'links2.id',
   'links2.slug',
-  'article.title',
-  'article.description',
-  'article.author',
-  'article.image',
+  'pages.title',
+  'pages.description',
+  'pages.author',
+  'pages.image',
   'reminders.send_notification',
 ]
 

packages/api/src/entity/link.ts

@@ -39,7 +39,7 @@ export class Link extends BaseEntity {
 
   @OneToOne(() => Page)
   @JoinColumn({ name: 'article_id' })
-  page!: string
+  page!: Page
 
   @Column('timestamp')
   savedAt!: Date

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

@@ -87,7 +87,6 @@ export const createReminderResolver = authorized<
   try {
     if (articleId) {
       // saving from web
-      // this linkId is actually an article.id
       const link = await models.userArticle.getByArticleId(uid, articleId)
       if (!link) {
         log.error('link not found', articleId)

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

@@ -19,12 +19,11 @@ import { env } from '../../env'
 import { sendMulticastPushNotifications } from '../../utils/sendNotification'
 import { getDeviceTokensByUserId } from '../../services/user_device_tokens'
 import { messaging } from 'firebase-admin'
-import MulticastMessage = messaging.MulticastMessage
 import { ContentReader } from '../../generated/graphql'
-import UserArticleModel from '../../datalayer/links'
 import { UserDeviceToken } from '../../entity/user_device_tokens'
 import { UserArticleData } from '../../datalayer/links/model'
 import { ArticleData } from '../../datalayer/article/model'
+import MulticastMessage = messaging.MulticastMessage
 
 interface SetConfirmationCodeMessage {
   emailAddress: string

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

@@ -13,6 +13,7 @@ import { getDeviceTokensByUserId } from '../../services/user_device_tokens'
 import { MulticastMessage } from 'firebase-admin/messaging'
 import { UserDeviceToken } from '../../entity/user_device_tokens'
 import { ContentReader } from '../../generated/graphql'
+import { DataModels } from '../../resolvers/types'
 
 type Article = {
   title: string
@@ -56,7 +57,7 @@ export function remindersServiceRouter() {
       const user = await models.user.get(userId)
       if (!user || !user.email) {
         console.log('user not found', userId)
-        res.status(404).send('Not Found')
+        res.status(400).send('User Not Found')
         return
       }
 
@@ -67,7 +68,7 @@ export function remindersServiceRouter() {
 
       if (!reminders) {
         console.log('reminders not found', userId, scheduleTime)
-        res.status(404).send('Not Found')
+        res.status(200).send('Reminders Not Found')
         return
       }
 
@@ -79,9 +80,17 @@ export function remindersServiceRouter() {
       // If none of the fetch reminders have sendNotification
       // set to true, then we should not send an email or notification
       if (articlesToNotify.length > 0) {
+        // we have configured Sendgrid to send a template
         if (!process.env.SENDGRID_REMINDER_TEMPLATE_ID) {
           console.log('Sendgrid reminder email template_id not set')
-          res.status(400).send('Template Id Not Found')
+
+          await updateRemindersStatus(
+            models,
+            userId,
+            linkIdsToUnarchive,
+            remindAt
+          )
+          res.status(200).send('Template Id Not Found')
           return
         }
 
@@ -109,28 +118,13 @@ export function remindersServiceRouter() {
 
         if (!deviceTokens) {
           console.log('Device tokens not set:', userId)
-          res.status(200).send('Device token Not Found')
+
+          res.status(400).send('Device token Not Found')
           return
         }
       }
 
-      // db update
-      await kx.transaction(async (tx) => {
-        await setClaims(tx, userId)
-        // Unarchive all the links and updated saved_at to now, so they
-        // appear at the top of the user's list.
-        await models.userArticle.updateByIds(
-          linkIdsToUnarchive,
-          {
-            savedAt: new Date(),
-            archivedAt: null,
-          },
-          tx
-        )
-
-        await models.reminder.setRemindersComplete(userId, remindAt, tx)
-      })
-
+      await updateRemindersStatus(models, userId, linkIdsToUnarchive, remindAt)
       res.status(200).send('Reminders triggered')
     } catch (e) {
       console.log(e)
@@ -225,3 +219,27 @@ const messageForLinks = (
     tokens: deviceTokens.map((token) => token.token),
   }
 }
+
+const updateRemindersStatus = async (
+  models: DataModels,
+  userId: string,
+  linkIdsToUnarchive: string[],
+  remindAt: Date
+): Promise<void> => {
+  // db update
+  await kx.transaction(async (tx) => {
+    await setClaims(tx, userId)
+    // Unarchive all the links and updated saved_at to now, so they
+    // appear at the top of the user's list.
+    await models.userArticle.updateByIds(
+      linkIdsToUnarchive,
+      {
+        savedAt: new Date(),
+        archivedAt: null,
+      },
+      tx
+    )
+
+    await models.reminder.setRemindersComplete(userId, remindAt, tx)
+  })
+}

packages/api/src/server.ts

@@ -54,7 +54,7 @@ export const initModels = (kx: Knex, cache = true): DataModels => ({
 })
 
 const initEntities = async (): Promise<Connection> => {
-  const connection = await createConnection({
+  return createConnection({
     type: 'postgres',
     host: env.pg.host,
     port: env.pg.port,
@@ -67,7 +67,6 @@ const initEntities = async (): Promise<Connection> => {
     subscribers: [__dirname + '/events/**/*{.js,.ts}'],
     namingStrategy: new SnakeNamingStrategy(),
   })
-  return connection
 }
 
 export const createApp = (): {

packages/api/test/db.ts

@@ -125,10 +125,7 @@ export const createTestPage = async (): Promise<Page> => {
     .save()
 }
 
-export const createTestLink = async (
-  user: User,
-  page?: string
-): Promise<Link> => {
+export const createTestLink = async (user: User, page: Page): Promise<Link> => {
   return getRepository(Link)
     .create({
       user: user,

packages/api/test/global-setup.ts

@@ -1,6 +1,6 @@
 import { createTestConnection } from './db'
 
-export async function mochaGlobalSetup() {
+export const mochaGlobalSetup = async () => {
   await createTestConnection()
-  console.log('db connection create')
+  console.log('db connection created')
 }

packages/api/test/global-teardown.ts

@@ -1,6 +1,6 @@
 import { getConnection } from 'typeorm'
 
-export async function mochaGlobalTeardown() {
+export const mochaGlobalTeardown = async () => {
   await getConnection().close()
-  console.log('db connection close')
+  console.log('db connection closed')
 }

packages/api/test/resolvers/article.test.ts

@@ -4,12 +4,10 @@ import {
   createTestUser,
   deleteTestUser,
 } from '../db'
-import { generateFakeUuid, graphqlRequest, request } from '../util'
-import { Link } from '../../src/entity/link'
-import { Label } from '../../src/entity/label'
+import { graphqlRequest, request } from '../util'
 import { expect } from 'chai'
 import { Page } from '../../src/entity/page'
-import { getRepository } from 'typeorm'
+import 'mocha'
 
 describe('Article API', () => {
   const username = 'fakeUser'
@@ -28,7 +26,7 @@ describe('Article API', () => {
 
     for (let i = 0; i < 15; i++) {
       const page = await createTestPage()
-      await createTestLink(user, page.id)
+      await createTestLink(user, page)
       links.push(page)
     }
   })
@@ -95,13 +93,18 @@ describe('Article API', () => {
         const res = await graphqlRequest(query, authToken).expect(200)
         expect(res.body.data.articles.pageInfo.endCursor).to.eql('5')
         expect(res.body.data.articles.pageInfo.startCursor).to.eql('')
-        expect(res.body.data.articles.pageInfo.totalCount, 'totalCount').to.eql(15)
-        expect(res.body.data.articles.pageInfo.hasNextPage, 'hasNextPage').to.eql(true)
+        expect(res.body.data.articles.pageInfo.totalCount, 'totalCount').to.eql(
+          15
+        )
+        expect(
+          res.body.data.articles.pageInfo.hasNextPage,
+          'hasNextPage'
+        ).to.eql(true)
       })
     })
 
     context('when we fetch the second page', () => {
-      before(async () => {
+      before(() => {
         after = '5'
       })
 
@@ -118,10 +121,20 @@ describe('Article API', () => {
 
       it('should set the pageInfo', async () => {
         const res = await graphqlRequest(query, authToken).expect(200)
-        expect(res.body.data.articles.pageInfo.totalCount, 'totalCount').to.eql(15)
-        expect(res.body.data.articles.pageInfo.startCursor, 'startCursor').to.eql('5')
-        expect(res.body.data.articles.pageInfo.endCursor, 'endCursor').to.eql('10')
-        expect(res.body.data.articles.pageInfo.hasNextPage, 'hasNextPage').to.eql(true)
+        expect(res.body.data.articles.pageInfo.totalCount, 'totalCount').to.eql(
+          15
+        )
+        expect(
+          res.body.data.articles.pageInfo.startCursor,
+          'startCursor'
+        ).to.eql('5')
+        expect(res.body.data.articles.pageInfo.endCursor, 'endCursor').to.eql(
+          '10'
+        )
+        expect(
+          res.body.data.articles.pageInfo.hasNextPage,
+          'hasNextPage'
+        ).to.eql(true)
         // We don't implement hasPreviousPage in the API and should probably remove it
         // expect(res.body.data.articles.pageInfo.hasPreviousPage).to.eql(true)
       })

packages/api/test/resolvers/labels.test.ts

@@ -10,6 +10,7 @@ import { Label } from '../../src/entity/label'
 import { expect } from 'chai'
 import { Page } from '../../src/entity/page'
 import { getRepository } from 'typeorm'
+import 'mocha'
 
 describe('Labels API', () => {
   const username = 'fakeUser'
@@ -30,7 +31,7 @@ describe('Labels API', () => {
 
     //  create test label
     page = await createTestPage()
-    link = await createTestLink(user, page.id)
+    link = await createTestLink(user, page)
     const label1 = await getRepository(Label)
       .create({
         name: 'label1',
@@ -191,7 +192,7 @@ describe('Labels API', () => {
     let query: string
     let labelId: string
 
-    beforeEach(async () => {
+    beforeEach(() => {
       query = `
         mutation {
           deleteLabel(id: "${labelId}") {

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

@@ -8,6 +8,7 @@ import { generateFakeUuid, graphqlRequest, request } from '../util'
 import { NewsletterEmail } from '../../src/entity/newsletter_email'
 import { expect } from 'chai'
 import { DeleteNewsletterEmailErrorCode } from '../../src/generated/graphql'
+import 'mocha'
 
 describe('Newsletters API', () => {
   const username = 'fakeUser'

packages/api/test/resolvers/reminders.test.ts

@@ -17,6 +17,7 @@ import {
 import { Page } from '../../src/entity/page'
 import { Link } from '../../src/entity/link'
 import { DateTime } from 'luxon'
+import 'mocha'
 
 describe('Reminders API', () => {
   const username = 'fakeUser'
@@ -37,7 +38,7 @@ describe('Reminders API', () => {
 
     // create page, link and reminders test data
     page = await createTestPage()
-    link = await createTestLink(user, page.id)
+    link = await createTestLink(user, page)
     reminder = await createTestReminder(user, link.id)
   })
 

packages/api/test/resolvers/server.test.ts

@@ -1,4 +1,5 @@
 import { request } from '../util'
+import 'mocha'
 
 describe('Server', () => {
   it('should respond for health check', async () => {

packages/api/test/resolvers/user.test.ts

@@ -9,6 +9,7 @@ import {
 } from '../../src/generated/graphql'
 import { User } from '../../src/entity/user'
 import { hashPassword } from '../../src/utils/auth'
+import 'mocha'
 
 describe('User API', () => {
   const username = 'fake_user'

packages/api/test/resolvers/user_device_tokens.test.ts

@@ -8,6 +8,7 @@ import { generateFakeUuid, graphqlRequest, request } from '../util'
 import { expect } from 'chai'
 import { UserDeviceToken } from '../../src/entity/user_device_tokens'
 import { SetDeviceTokenErrorCode } from '../../src/generated/graphql'
+import 'mocha'
 
 describe('Device tokens API', () => {
   const username = 'fakeUser'

packages/api/test/resolvers/user_feed_article.test.ts

@@ -11,6 +11,7 @@ import { Page } from '../../src/entity/page'
 import { Link } from '../../src/entity/link'
 import { Highlight } from '../../src/entity/highlight'
 import { getRepository } from 'typeorm'
+import 'mocha'
 
 describe('User feed article API', () => {
   const existingUsername = 'fakeUser'
@@ -30,7 +31,7 @@ describe('User feed article API', () => {
     authToken = res.body.authToken
 
     page = await createTestPage()
-    link = await createTestLink(user, page.id)
+    link = await createTestLink(user, page)
     highlight = await getRepository(Highlight).save({
       page: page,
       text: 'test',

packages/api/test/routers/article_router.test.ts

@@ -1,22 +1,18 @@
-import {
-  createTestUser,
-  deleteTestUser,
-} from '../db'
+import { createTestUser, deleteTestUser } from '../db'
 import { request } from '../util'
 import { expect } from 'chai'
-import nock from 'nock';
+import nock from 'nock'
+import 'mocha'
 
 describe('/article/save API', () => {
   const username = 'fakeUser'
 
   let authToken: string
 
-  // We need to mock the pupeeteer-parse 
+  // We need to mock the pupeeteer-parse
   // service here because in dev mode the task gets
   // called immediately.
-  nock('http://localhost:8080/')
-    .post('/')
-    .reply(200);
+  nock('http://localhost:8080/').post('/').reply(200)
 
   before(async () => {
     // create test user and login

packages/api/test/routers/reminders_router.test.ts

@@ -0,0 +1,56 @@
+import {
+  createTestLink,
+  createTestPage,
+  createTestReminder,
+  createTestUser,
+  deleteTestUser,
+  getReminder,
+} from '../db'
+import { request } from '../util'
+import { User } from '../../src/entity/user'
+import { Reminder } from '../../src/entity/reminder'
+import { expect } from 'chai'
+import 'mocha'
+
+describe('Reminders Router', () => {
+  const username = 'fakeUser'
+
+  let authToken: string
+  let user: User
+  let reminder: Reminder
+
+  before(async () => {
+    // create test user and login
+    user = await createTestUser(username)
+    const res = await request
+      .post('/local/debug/fake-user-login')
+      .send({ fakeEmail: user.email })
+
+    authToken = res.body.authToken
+
+    const page = await createTestPage()
+    const link = await createTestLink(user, page)
+    reminder = await createTestReminder(user, link.id)
+  })
+
+  after(async () => {
+    // clean up
+    await deleteTestUser(username)
+  })
+
+  describe('trigger reminders', () => {
+    it('should trigger reminders and update status to Complete', async () => {
+      await request
+        .post('/svc/reminders/trigger')
+        .send({
+          userId: user.id,
+          scheduleTime: reminder.remindAt,
+        })
+        .set('Authorization', `${authToken}`)
+        .expect(200)
+
+      const completed = await getReminder(reminder.id)
+      expect(completed?.status).to.eql('COMPLETED')
+    })
+  })
+})