Merge pull request #2627 from omnivore-app/fix/opt-in-feature

fix existing opted in users not being able to be granted with the feature after increasing the limit

packages/api/src/entity/feature.ts

@@ -5,11 +5,13 @@ import {
   JoinColumn,
   ManyToOne,
   PrimaryGeneratedColumn,
+  Unique,
   UpdateDateColumn,
 } from 'typeorm'
 import { User } from './user'
 
 @Entity({ name: 'features' })
+@Unique(['user', 'name'])
 export class Feature {
   @PrimaryGeneratedColumn('uuid')
   id!: string

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

@@ -1,4 +1,3 @@
-import { authorized } from '../../utils/helpers'
 import {
   MutationOptInFeatureArgs,
   OptInFeatureError,
@@ -10,6 +9,7 @@ import {
   optInFeature,
   signFeatureToken,
 } from '../../services/features'
+import { authorized } from '../../utils/helpers'
 
 export const optInFeatureResolver = authorized<
   OptInFeatureSuccess,
@@ -33,18 +33,19 @@ export const optInFeatureResolver = authorized<
       }
     }
 
-    const optIn = await optInFeature(featureName, claims.uid)
-    if (!optIn) {
+    const optedInFeature = await optInFeature(featureName, claims.uid)
+    if (!optedInFeature) {
       return {
         errorCodes: [OptInFeatureErrorCode.NotFound],
       }
     }
+    log.info('Opted in to a feature', optedInFeature)
 
-    const token = signFeatureToken(optIn, claims.uid)
+    const token = signFeatureToken(optedInFeature, claims.uid)
 
     return {
       feature: {
-        ...optIn,
+        ...optedInFeature,
         token,
       },
     }

packages/api/src/services/features.ts

@@ -3,6 +3,7 @@ import { IsNull, Not } from 'typeorm'
 import { Feature } from '../entity/feature'
 import { getRepository } from '../entity/utils'
 import { env } from '../env'
+import { AppDataSource } from '../server'
 import { logger } from '../utils/logger'
 
 export enum FeatureName {
@@ -29,6 +30,7 @@ const optInUltraRealisticVoice = async (uid: string): Promise<Feature> => {
     where: {
       user: { id: uid },
       name: FeatureName.UltraRealisticVoice,
+      grantedAt: Not(IsNull()),
     },
     relations: ['user'],
   })
@@ -38,23 +40,44 @@ const optInUltraRealisticVoice = async (uid: string): Promise<Feature> => {
     return feature
   }
 
-  // opt in to feature for the first 1000 users
-  const count = await getRepository(Feature).countBy({
-    name: FeatureName.UltraRealisticVoice,
-    grantedAt: Not(IsNull()),
-  })
+  const MAX_USERS = 1500
+  // opt in to feature for the first 1500 users
+  const optedInFeatures = (await AppDataSource.query(
+    `insert into omnivore.features (user_id, name, granted_at) 
+    select $1, $2, $3 from omnivore.features 
+    where name = $2 and granted_at is not null 
+    having count(*) < $4 
+    on conflict (user_id, name) 
+    do update set granted_at = $3 
+    returning *, granted_at as "grantedAt", created_at as "createdAt", updated_at as "updatedAt";`,
+    [uid, FeatureName.UltraRealisticVoice, new Date(), MAX_USERS]
+  )) as Feature[]
 
-  let grantedAt: Date | null = new Date()
-  if (count >= 1000) {
-    logger.info('feature limit reached')
-    grantedAt = null
+  // if no new features were created then user has exceeded max users
+  if (optedInFeatures.length === 0) {
+    logger.info('exceeded max users')
+
+    // create/update an opt-in record with null grantedAt
+    const optInRecord = {
+      user: { id: uid },
+      name: FeatureName.UltraRealisticVoice,
+      grantedAt: null,
+    }
+    const result = await getRepository(Feature).upsert(optInRecord, [
+      'user',
+      'name',
+    ])
+    if (result.generatedMaps.length === 0) {
+      throw new Error('failed to update opt-in record')
+    }
+
+    logger.info('opt-in record updated', result.generatedMaps)
+    return { ...optInRecord, ...(result.generatedMaps[0] as Feature) }
   }
 
-  return getRepository(Feature).save({
-    user: { id: uid },
-    name: FeatureName.UltraRealisticVoice,
-    grantedAt,
-  })
+  logger.info('opted in', { uid, feature: optedInFeatures[0] })
+
+  return optedInFeatures[0]
 }
 
 export const signFeatureToken = (
@@ -64,6 +87,8 @@ export const signFeatureToken = (
   },
   userId: string
 ): string => {
+  logger.info('signing feature token', feature)
+
   return jwt.sign(
     {
       uid: userId,

packages/api/test/resolvers/features.test.ts

@@ -1,16 +1,15 @@
-import 'mocha'
 import { expect } from 'chai'
+import * as jwt from 'jsonwebtoken'
+import 'mocha'
+import sinon, { SinonFakeTimers } from 'sinon'
+import { Feature } from '../../src/entity/feature'
 import { User } from '../../src/entity/user'
+import { getRepository } from '../../src/entity/utils'
+import { env } from '../../src/env'
 import { createTestUser, deleteTestUser } from '../db'
 import { graphqlRequest, request } from '../util'
-import { getRepository } from '../../src/entity/utils'
-import { Feature } from '../../src/entity/feature'
-import * as jwt from 'jsonwebtoken'
-import sinon, { SinonFakeTimers } from 'sinon'
-import { env } from '../../src/env'
-import { Like } from 'typeorm'
 
-xdescribe('features resolvers', () => {
+describe('features resolvers', () => {
   let loginUser: User
   let authToken: string
 
@@ -25,7 +24,7 @@ xdescribe('features resolvers', () => {
   })
 
   after(async () => {
-    await deleteTestUser(loginUser.name)
+    await deleteTestUser(loginUser.id)
   })
 
   describe('optInFeature API', () => {
@@ -62,7 +61,7 @@ xdescribe('features resolvers', () => {
       clock.restore()
     })
 
-    context('when user is the first 1000 users', () => {
+    context('when user is the first 1500 users', () => {
       after(async () => {
         // reset feature
         await getRepository(Feature).delete({
@@ -95,10 +94,12 @@ xdescribe('features resolvers', () => {
       })
     })
 
-    context('when user is not the first 1000 users', () => {
+    context('when user is not the first 1500 users', () => {
+      let users: User[]
+
       before(async () => {
-        // create 1000 opt-in users
-        const usersToSave = Array.from(Array(1000).keys()).map((i) => {
+        // create 1500 opt-in users
+        const usersToSave = Array.from(Array(1500).keys()).map((i) => {
           return {
             name: `opt-in-user-${i}`,
             source: 'GOOGLE',
@@ -109,7 +110,7 @@ xdescribe('features resolvers', () => {
           }
         })
 
-        const users = await getRepository(User).save(usersToSave)
+        users = await getRepository(User).save(usersToSave)
 
         const features = users.map((user) => {
           return {
@@ -124,9 +125,8 @@ xdescribe('features resolvers', () => {
 
       after(async () => {
         // reset opt-in users
-        await getRepository(User).delete({
-          name: Like(`opt-in-user-%`),
-        })
+        Promise.all(users.map((user) => deleteTestUser(user.id)))
+        // reset feature
         await getRepository(Feature).delete({
           name: featureName,
         })