From 3df0ffc5d3d4674f1cdc9a6e0dcdbf975fb41687 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 8 Aug 2023 22:08:37 +0800 Subject: [PATCH 1/4] fix existing opted in users not being able to be granted with the feature after increasing the limit --- packages/api/src/resolvers/features/index.ts | 1 + packages/api/src/services/features.ts | 39 +++++++++++++------- packages/api/test/resolvers/features.test.ts | 26 ++++++------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/packages/api/src/resolvers/features/index.ts b/packages/api/src/resolvers/features/index.ts index a5226ef48..88bd99bfb 100644 --- a/packages/api/src/resolvers/features/index.ts +++ b/packages/api/src/resolvers/features/index.ts @@ -39,6 +39,7 @@ export const optInFeatureResolver = authorized< errorCodes: [OptInFeatureErrorCode.NotFound], } } + log.info('Opted in to a feature', optIn) const token = signFeatureToken(optIn, claims.uid) diff --git a/packages/api/src/services/features.ts b/packages/api/src/services/features.ts index ba6081f0e..70157e516 100644 --- a/packages/api/src/services/features.ts +++ b/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 => { where: { user: { id: uid }, name: FeatureName.UltraRealisticVoice, + grantedAt: Not(IsNull()), }, relations: ['user'], }) @@ -39,22 +41,31 @@ const optInUltraRealisticVoice = async (uid: string): Promise => { } // opt in to feature for the first 1000 users - const count = await getRepository(Feature).countBy({ - name: FeatureName.UltraRealisticVoice, - grantedAt: Not(IsNull()), - }) + const newFeatures = (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(*) < 1000 + 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()] + )) 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 (newFeatures.length === 0) { + logger.info('exceeded max users') + + return getRepository(Feature).save({ + user: { id: uid }, + name: FeatureName.UltraRealisticVoice, + grantedAt: null, + }) } - return getRepository(Feature).save({ - user: { id: uid }, - name: FeatureName.UltraRealisticVoice, - grantedAt, - }) + logger.info('opted in', { uid, feature: newFeatures[0] }) + + return newFeatures[0] } export const signFeatureToken = ( @@ -64,6 +75,8 @@ export const signFeatureToken = ( }, userId: string ): string => { + logger.info('signing feature token', { grantedAt: feature.grantedAt }) + return jwt.sign( { uid: userId, diff --git a/packages/api/test/resolvers/features.test.ts b/packages/api/test/resolvers/features.test.ts index 7ed43cfe0..43fd5bd9d 100644 --- a/packages/api/test/resolvers/features.test.ts +++ b/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', () => { @@ -96,6 +95,8 @@ xdescribe('features resolvers', () => { }) context('when user is not the first 1000 users', () => { + let users: User[] + before(async () => { // create 1000 opt-in users const usersToSave = Array.from(Array(1000).keys()).map((i) => { @@ -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, }) From 51e0dfde235371103033e00a1de662b66a03e1ff Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 10 Aug 2023 09:43:40 +0800 Subject: [PATCH 2/4] set new limit = 1500 --- packages/api/src/services/features.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/api/src/services/features.ts b/packages/api/src/services/features.ts index 70157e516..c6ca41c7f 100644 --- a/packages/api/src/services/features.ts +++ b/packages/api/src/services/features.ts @@ -40,16 +40,17 @@ const optInUltraRealisticVoice = async (uid: string): Promise => { return feature } - // opt in to feature for the first 1000 users + const MAX_USERS = 1500 + // opt in to feature for the first 1500 users const newFeatures = (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(*) < 1000 + 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()] + [uid, FeatureName.UltraRealisticVoice, new Date(), MAX_USERS] )) as Feature[] // if no new features were created then user has exceeded max users From 93bd8c1f7a85e59288814fd835aad2db4e278267 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 10 Aug 2023 10:48:03 +0800 Subject: [PATCH 3/4] fix test --- packages/api/test/resolvers/features.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/api/test/resolvers/features.test.ts b/packages/api/test/resolvers/features.test.ts index 43fd5bd9d..d4781612c 100644 --- a/packages/api/test/resolvers/features.test.ts +++ b/packages/api/test/resolvers/features.test.ts @@ -61,7 +61,7 @@ describe('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({ @@ -94,12 +94,12 @@ describe('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', From 7506e27dd031cc9f1dc4252bcf97ec57f06b588a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 10 Aug 2023 13:47:39 +0800 Subject: [PATCH 4/4] create or update an opt-in record with null grantedAt if exceeding max users --- packages/api/src/entity/feature.ts | 2 ++ packages/api/src/resolvers/features/index.ts | 12 +++++----- packages/api/src/services/features.ts | 25 ++++++++++++++------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/packages/api/src/entity/feature.ts b/packages/api/src/entity/feature.ts index 77ca572e7..cf7603478 100644 --- a/packages/api/src/entity/feature.ts +++ b/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 diff --git a/packages/api/src/resolvers/features/index.ts b/packages/api/src/resolvers/features/index.ts index 88bd99bfb..73f9036b0 100644 --- a/packages/api/src/resolvers/features/index.ts +++ b/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,19 +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', optIn) + 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, }, } diff --git a/packages/api/src/services/features.ts b/packages/api/src/services/features.ts index c6ca41c7f..7d3158d70 100644 --- a/packages/api/src/services/features.ts +++ b/packages/api/src/services/features.ts @@ -42,7 +42,7 @@ const optInUltraRealisticVoice = async (uid: string): Promise => { const MAX_USERS = 1500 // opt in to feature for the first 1500 users - const newFeatures = (await AppDataSource.query( + 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 @@ -54,19 +54,30 @@ const optInUltraRealisticVoice = async (uid: string): Promise => { )) as Feature[] // if no new features were created then user has exceeded max users - if (newFeatures.length === 0) { + if (optedInFeatures.length === 0) { logger.info('exceeded max users') - return getRepository(Feature).save({ + // 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) } } - logger.info('opted in', { uid, feature: newFeatures[0] }) + logger.info('opted in', { uid, feature: optedInFeatures[0] }) - return newFeatures[0] + return optedInFeatures[0] } export const signFeatureToken = ( @@ -76,7 +87,7 @@ export const signFeatureToken = ( }, userId: string ): string => { - logger.info('signing feature token', { grantedAt: feature.grantedAt }) + logger.info('signing feature token', feature) return jwt.sign( {