diff --git a/src/backend/models/user.ts b/src/backend/models/user.ts index 97114e1d..8470be4a 100644 --- a/src/backend/models/user.ts +++ b/src/backend/models/user.ts @@ -90,7 +90,7 @@ const UserSchema = new Schema<MongoUser>( type: Schema.Types.String, }, /** - * Station of the user to get the notifications. The value is an universalId, + * Station of the user to get the notifications. The value is a stationId, * e.g. `openaq|FR1012` or `aqicn|1047`. For privacy reasons, we do not store * the user's exact lat/lng. */ @@ -100,32 +100,60 @@ const UserSchema = new Schema<MongoUser>( type: Schema.Types.String, validate: { message: ({ value }: { value: string }): string => - `${value} is not a valid universalId`, - validator: (universalId: string): boolean => { - const [provider, station] = universalId.split('|'); + `${value} is not a valid stationId`, + validator: (stationId: string): boolean => { + const [provider, station] = stationId.split('|'); return !!station && AllProviders.includes(provider); }, }, }, /** - * User's Expo repots preferences. + * User's Expo reports preferences. */ expoReport: { type: ExpoReportSchema, + required: function () { + return !((this as unknown) as MongoUser).emailReport; + }, + validate: { + message: + 'either email or expo report must be set, but not both', + validator: function () { + if (((this as unknown) as MongoUser).emailReport) { + return false; + } + + return true; + }, + }, }, /** * User's email repots preferences. */ emailReport: { type: EmailReportSchema, + required: function () { + return !((this as unknown) as MongoUser).expoReport; + }, + validate: { + message: + 'either email or expo report must be set, but not both', + validator: function () { + if (((this as unknown) as MongoUser).expoReport) { + return false; + } + + return true; + }, + }, }, }, { strict: 'throw', timestamps: true } ); // eslint-disable-next-line @typescript-eslint/no-misused-promises -UserSchema.pre('remove', async () => { +UserSchema.pre('remove', async function () { const userId = ((this as unknown) as MongoUser)._id; // Cascade delete all related PushTickets. diff --git a/src/frontend/util/sentry.ts b/src/frontend/util/sentry.ts index 4599a5e7..15a5fb8a 100644 --- a/src/frontend/util/sentry.ts +++ b/src/frontend/util/sentry.ts @@ -23,6 +23,14 @@ if (typeof window !== 'undefined') { }); } +// We don't send the following errors to Sentry to not pollute it. +const UNTRACKED_ERRORS = [ + // Weird amplitude error. + '0 No request sent', + // First time fetching a user + 'No user with', +]; + /** * Capture an error, and send it to Sentry. * @@ -35,5 +43,7 @@ export function sentryException(err: Error): void { return; } console.error(err); - captureException(err); + if (!UNTRACKED_ERRORS.some((msg) => err.message.includes(msg))) { + captureException(err); + } } diff --git a/test/e2e/backend/createUser.spec.ts b/test/e2e/backend/createUser.spec.ts index 998268e4..a3214186 100644 --- a/test/e2e/backend/createUser.spec.ts +++ b/test/e2e/backend/createUser.spec.ts @@ -21,7 +21,7 @@ import { connection } from 'mongoose'; import { User } from '../../../src/backend/models'; import { connectToDatabase } from '../../../src/backend/util'; -import { alice, axiosConfig, BACKEND_URL } from './util/testdata'; +import { alice, axiosConfig, BACKEND_URL, bob } from './util/testdata'; function testBadInput<T>(name: string, input: T, expErr: string) { it(`should require correct input: ${name}`, async (done) => { @@ -56,7 +56,7 @@ describe('users::createUser', () => { testBadInput( 'invalid lastStationId', { ...alice, lastStationId: 'foo' }, - 'lastStationId: foo is not a valid universalId' + 'lastStationId: foo is not a valid stationId' ); testBadInput( 'no timezone', @@ -86,36 +86,60 @@ describe('users::createUser', () => { testBadInput( 'no expoPushToken', { - ...alice, - expoReport: { ...alice.expoReport, expoPushToken: undefined }, + ...bob, + expoReport: { ...bob.expoReport, expoPushToken: undefined }, }, 'expoReport.expoPushToken: Path `expoPushToken` is required' ); testBadInput( 'wrong expo frequency', - { ...alice, expoReport: { ...alice.expoReport, frequency: 'foo' } }, + { ...bob, expoReport: { ...bob.expoReport, frequency: 'foo' } }, 'expoReport.frequency: `foo` is not a valid enum value for path `frequency`' ); - it('should successfully create a user', async () => { - const { data } = await axios.post<MongoUser>( - `${BACKEND_URL}/api/users`, - alice, - axiosConfig - ); - expect(data._id).toBeTruthy(); - expect(data).toMatchObject(alice); + testBadInput( + 'must set either email and expo notifications', + { ...alice, emailReport: undefined }, + 'emailReport: Path `emailReport` is required., expoReport: Path `expoReport` is required.' + ); + + testBadInput( + 'cannot set both email and expo notifications', + { ...alice, expoReport: { expoPushToken: 'foo', frequency: 'daily' } }, + 'either email or expo report must be set, but not both' + ); + + it('should successfully create two users', async () => { + const aliceData = ( + await axios.post<MongoUser>( + `${BACKEND_URL}/api/users`, + alice, + axiosConfig + ) + ).data; + expect(aliceData._id).toBeTruthy(); + expect(aliceData).toMatchObject(alice); + + const bobData = ( + await axios.post<MongoUser>( + `${BACKEND_URL}/api/users`, + bob, + axiosConfig + ) + ).data; + expect(bobData._id).toBeTruthy(); + expect(bobData).toMatchObject(bob); }); testBadInput( 'duplicate expoPushToken', - alice, + bob, 'E11000 duplicate key error collection: shootismoke.users index: expoReport.expoPushToken_1 dup key' ); testBadInput( 'duplicate email', - { ...alice, expoReport: undefined }, + alice, 'E11000 duplicate key error collection: shootismoke.users index: emailReport.email_1 dup key' ); diff --git a/test/e2e/backend/findUsersForReport.spec.ts b/test/e2e/backend/findUsersForReport.spec.ts index f2dd9b06..4f0f8026 100644 --- a/test/e2e/backend/findUsersForReport.spec.ts +++ b/test/e2e/backend/findUsersForReport.spec.ts @@ -45,7 +45,7 @@ describe('findUsersForReport', () => { const now = new Date('2021-01-15T08:27:21.771Z'); const users = await findUsersForReport('email', now); - expect(users.length).toBe(2); + expect(users.length).toBe(1); // Bob }); it('should work for at UTC 21:00 on a Sunday', async () => { @@ -55,7 +55,7 @@ describe('findUsersForReport', () => { const now = new Date('2021-01-17T20:27:21.771Z'); const users = await findUsersForReport('expo', now); - expect(users.length).toBe(2); + expect(users.length).toBe(1); // Alice }); it('should work for at UTC 21:00 on the 1st of the month', async () => { @@ -65,7 +65,16 @@ describe('findUsersForReport', () => { const now = new Date('2021-01-01T20:27:21.771Z'); const users = await findUsersForReport('expo', now); - expect(users.length).toBe(2); + expect(users.length).toBe(2); // Bob and Charlie + }); + + it('should work at a random time', async () => { + const now = new Date('2021-01-01T10:27:21.771Z'); + + let users = await findUsersForReport('expo', now); + expect(users.length).toBe(0); + users = await findUsersForReport('email', now); + expect(users.length).toBe(0); }); afterAll(() => connection.close()); diff --git a/test/e2e/backend/getUserByExpoPushToken.spec.ts b/test/e2e/backend/getUserByExpoPushToken.spec.ts index 451da256..8cf4de64 100644 --- a/test/e2e/backend/getUserByExpoPushToken.spec.ts +++ b/test/e2e/backend/getUserByExpoPushToken.spec.ts @@ -21,9 +21,9 @@ import { connection } from 'mongoose'; import { User } from '../../../src/backend/models'; import { connectToDatabase } from '../../../src/backend/util'; -import { alice, axiosConfig, BACKEND_URL } from './util/testdata'; +import { axiosConfig, BACKEND_URL, bob } from './util/testdata'; -let dbAlice: MongoUser; +let dbBob: MongoUser; describe('users::getUserByExpoPushToken', () => { beforeAll(async (done) => { @@ -34,11 +34,11 @@ describe('users::getUserByExpoPushToken', () => { const { data } = await axios.post<MongoUser>( `${BACKEND_URL}/api/users`, - alice, + bob, axiosConfig ); - dbAlice = data; + dbBob = data; done(); }); @@ -77,14 +77,12 @@ describe('users::getUserByExpoPushToken', () => { it('should fetch correct user', async (done) => { const { data } = await axios.get<MongoUser>( - `${BACKEND_URL}/api/users/expoPushToken/${ - dbAlice?.expoReport?.expoPushToken as string - }`, + `${BACKEND_URL}/api/users/expoPushToken/${bob.expoReport.expoPushToken}`, axiosConfig ); - expect(data._id).toBe(dbAlice._id); - expect(data).toMatchObject(dbAlice); + expect(data._id).toBe(dbBob._id); + expect(data).toMatchObject(dbBob); done(); }); diff --git a/test/e2e/backend/updateUser.spec.ts b/test/e2e/backend/updateUser.spec.ts index 75c58acf..28b286be 100644 --- a/test/e2e/backend/updateUser.spec.ts +++ b/test/e2e/backend/updateUser.spec.ts @@ -24,12 +24,19 @@ import { connectToDatabase } from '../../../src/backend/util'; import { alice, axiosConfig, BACKEND_URL, bob } from './util/testdata'; let dbAlice: MongoUser; - -function testBadInput<T>(name: string, input: T, expErr: string) { +let dbBob: MongoUser; + +function testBadInput<T>( + name: string, + user: 'alice' | 'bob', + input: T, + expErr: string +) { it(`should require correct input: ${name}`, async (done) => { + const dbUser = user === 'alice' ? dbAlice : dbBob; try { await axios.patch( - `${BACKEND_URL}/api/users/${dbAlice._id}`, + `${BACKEND_URL}/api/users/${dbUser._id}`, input, axiosConfig ); @@ -43,10 +50,11 @@ function testBadInput<T>(name: string, input: T, expErr: string) { }); } -function testGoodInput<T>(name: string, input: T) { +function testGoodInput<T>(name: string, user: 'alice' | 'bob', input: T) { it(`should be successful: ${name}`, async (done) => { + const dbUser = user === 'alice' ? dbAlice : dbBob; const { data } = await axios.patch<MongoUser>( - `${BACKEND_URL}/api/users/${dbAlice._id}`, + `${BACKEND_URL}/api/users/${dbUser._id}`, input, axiosConfig ); @@ -64,70 +72,86 @@ describe('users::updateUser', () => { await connectToDatabase(); await User.deleteMany(); - const { data } = await axios.post<MongoUser>( - `${BACKEND_URL}/api/users`, - alice, - axiosConfig - ); - await axios.post<MongoUser>( - `${BACKEND_URL}/api/users`, - bob, - axiosConfig - ); + dbAlice = ( + await axios.post<MongoUser>( + `${BACKEND_URL}/api/users`, + alice, + axiosConfig + ) + ).data; - dbAlice = data; + dbBob = ( + await axios.post<MongoUser>( + `${BACKEND_URL}/api/users`, + bob, + axiosConfig + ) + ).data; done(); }); - testGoodInput('empty input', {}); + testGoodInput('empty input', 'alice', {}); testBadInput( 'no lastStationId', + 'alice', { lastStationId: null }, 'Path `lastStationId` is required' ); testBadInput( 'invalid lastStationId', + 'alice', { lastStationId: 'foo' }, - 'lastStationId: foo is not a valid universalId' + 'lastStationId: foo is not a valid stationId' ); testBadInput( 'no timezone', + 'alice', { ...alice, timezone: null }, 'Path `timezone` is required' ); testBadInput( 'invalid timezone', + 'alice', { timezone: 'foo' }, 'timezone: `foo` is not a valid enum value for path `timezone`' ); + + // Email-specific tests. testBadInput( 'no email', + 'alice', { emailReport: { email: null } }, 'emailReport.email: Path `email` is required' ); testBadInput( 'bad email', + 'alice', { emailReport: { email: 'foo' } }, 'emailReport.email: Please enter a valid email' ); testBadInput( 'wrong email frequency', + 'alice', { emailReport: { frequency: 'foo' } }, 'emailReport.frequency: `foo` is not a valid enum value for path `frequency`' ); - testGoodInput('change emailReport frequency', { + testGoodInput('change emailReport frequency', 'alice', { emailReport: { frequency: 'monthly' }, }); testBadInput( - 'no emailReport', + 'no emailReport nor expoReport', + 'bob', { - emailReport: { email: null }, + emailReport: null, }, - 'Path `email` is required' + 'emailReport: either email or expo report must be set, but not both' ); + + // Expo-specific tests. testBadInput( 'no expoPushToken', + 'bob', { expoReport: { expoPushToken: null }, }, @@ -135,30 +159,41 @@ describe('users::updateUser', () => { ); testBadInput( 'wrong expo frequency', + 'bob', { expoReport: { frequency: 'foo' } }, 'expoReport.frequency: `foo` is not a valid enum value for path `frequency`' ); - testGoodInput('change expoReport frequency', { + testGoodInput('change expoReport frequency', 'bob', { expoReport: { frequency: 'monthly' }, }); - testGoodInput('no expoReport', { - expoReport: null, - }); + testBadInput( + 'no expoReport nor emailReport', + 'bob', + { + expoReport: null, + }, + 'emailReport: Path `emailReport` is required., expoReport: Path `expoReport` is required.' + ); + // Cannot change from email to expo, or vice versa. testBadInput( 'duplicate expoPushToken', + 'alice', { + emailReport: null, expoReport: { expoPushToken: bob.expoReport.expoPushToken }, }, - 'E11000 duplicate key error collection: shootismoke.users index: expoReport.expoPushToken_1 dup key' + 'either email or expo report must be set, but not both' ); testBadInput( 'duplicate email', + 'bob', { - emailReport: { email: bob.emailReport.email }, + expoReport: null, + emailReport: { email: alice.emailReport.email }, }, - 'E11000 duplicate key error collection: shootismoke.users index: emailReport.email_1 dup key' + 'either email or expo report must be set, but not both' ); afterAll(() => connection.close()); diff --git a/test/e2e/backend/util/testdata.ts b/test/e2e/backend/util/testdata.ts index f298f090..0f8049df 100644 --- a/test/e2e/backend/util/testdata.ts +++ b/test/e2e/backend/util/testdata.ts @@ -30,19 +30,11 @@ export const alice = { email: 'alice@example.org', frequency: 'daily', }, - expoReport: { - expoPushToken: 'expo_token_alice', - frequency: 'weekly', - }, lastStationId: 'openaq|FR04143', timezone: 'Europe/Berlin', }; export const bob = { - emailReport: { - email: 'bob@example.org', - frequency: 'daily', - }, expoReport: { expoPushToken: 'ExponentPushToken[0zK3-xM3PgLEfe31-AafjB]', // real one, unused', frequency: 'monthly',