From a3d536fcf15d4a6c8f5997720093d4c6d2f403e0 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 10 Aug 2022 16:00:30 +0800 Subject: [PATCH 01/10] Separate migrations from test --- .github/workflows/run-tests.yaml | 14 ++++++++++++-- packages/api/test/db.ts | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 6da610fee..30fe32cc2 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -63,6 +63,16 @@ jobs: run: | source ~/.nvm/nvm.sh yarn install --frozen-lockfile + - name: Database Migration + run: | + yarn workspace @omnivore/db migrate + psql --host localhost --port ${{ job.services.postgres.ports[5432] }} --user postgres --password -c "CREATE USER app_user WITH ENCRYPTED PASSWORD 'app_pass';GRANT omnivore_user to app_user;" + env: + PG_HOST: localhost + PG_PORT: ${{ job.services.postgres.ports[5432] }} + PG_USER: postgres + PG_PASSWORD: postgres + PG_DB: omnivore_test - name: TypeScript, Lint, Tests run: | source ~/.nvm/nvm.sh @@ -72,8 +82,8 @@ jobs: env: PG_HOST: localhost PG_PORT: ${{ job.services.postgres.ports[5432] }} - PG_USER: postgres - PG_PASSWORD: postgres + PG_USER: app_user + PG_PASSWORD: app_pass PG_DB: omnivore_test PG_POOL_MAX: 10 ELASTIC_URL: http://localhost:${{ job.services.elastic.ports[9200] }}/ diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts index d1e752c17..fc6a68b26 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -42,7 +42,8 @@ const runMigrations = async () => { } export const createTestConnection = async (): Promise => { - await runMigrations() + // need to manually run migrations before creating the connection + // await runMigrations() AppDataSource.setOptions({ type: 'postgres', From 3a39ec55cc9f177c0dffa6e0ad1cbecced4c0510 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 10 Aug 2022 16:36:31 +0800 Subject: [PATCH 02/10] Delete user with right permission --- packages/api/test/db.ts | 13 +++++------ .../api/test/gql/sanitize-directive.test.ts | 5 ++-- packages/api/test/resolvers/api_key.test.ts | 6 ++--- packages/api/test/resolvers/article.test.ts | 5 ++-- .../resolvers/article_saving_request.test.ts | 5 ++-- packages/api/test/resolvers/highlight.test.ts | 5 ++-- .../api/test/resolvers/integrations.test.ts | 4 ++-- packages/api/test/resolvers/labels.test.ts | 2 +- .../api/test/resolvers/newsletters.test.ts | 8 +++---- .../api/test/resolvers/popular_reads.test.ts | 6 ++--- packages/api/test/resolvers/reminders.test.ts | 8 +++---- packages/api/test/resolvers/report.test.ts | 6 ++--- .../send_install_instructions.test.ts | 13 ++++------- .../api/test/resolvers/subscriptions.test.ts | 6 ++--- packages/api/test/resolvers/update.test.ts | 6 ++--- .../resolvers/upload_file_request.test.ts | 5 ++-- packages/api/test/resolvers/user.test.ts | 7 +++--- .../resolvers/user_delete_account.test.ts | 5 ++-- .../test/resolvers/user_device_tokens.test.ts | 6 ++--- .../test/resolvers/user_feed_article.test.ts | 7 +++--- packages/api/test/resolvers/webhooks.test.ts | 6 ++--- packages/api/test/routers/article.test.ts | 8 +++---- packages/api/test/routers/auth.test.ts | 17 ++++++++------ packages/api/test/routers/emails.test.ts | 5 ++-- .../api/test/routers/integrations.test.ts | 2 +- .../api/test/routers/pdf_attachments.test.ts | 5 ++-- packages/api/test/routers/reminders.test.ts | 6 ++--- packages/api/test/routers/webhooks.test.ts | 5 ++-- .../api/test/services/create_user.test.ts | 23 +++++++++++++++---- packages/api/test/services/labels.test.ts | 7 +++--- packages/api/test/services/save_email.test.ts | 11 ++++++--- .../services/save_newsletter_email.test.ts | 5 ++-- packages/api/test/utils/parser.test.ts | 2 +- 33 files changed, 111 insertions(+), 119 deletions(-) diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts index fc6a68b26..3447d859c 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -9,7 +9,7 @@ import { UserDeviceToken } from '../src/entity/user_device_tokens' import { Label } from '../src/entity/label' import { Subscription } from '../src/entity/subscription' import { AppDataSource } from '../src/server' -import { getRepository } from '../src/entity/utils' +import { getRepository, setClaims } from '../src/entity/utils' import { createUser } from '../src/services/create_user' import { SnakeNamingStrategy } from 'typeorm-naming-strategies' import { SubscriptionStatus } from '../src/generated/graphql' @@ -61,12 +61,11 @@ export const createTestConnection = async (): Promise => { await AppDataSource.initialize() } -export const deleteTestUser = async (name: string) => { - await AppDataSource.createQueryBuilder() - .delete() - .from(User) - .where({ email: `${name}@omnivore.app` }) - .execute() +export const deleteTestUser = async (userId: string) => { + await AppDataSource.transaction(async (t) => { + await setClaims(t, userId) + await t.getRepository(User).delete({ id: userId }) + }) } export const createTestUser = async ( diff --git a/packages/api/test/gql/sanitize-directive.test.ts b/packages/api/test/gql/sanitize-directive.test.ts index d4d8820b7..1699407ec 100644 --- a/packages/api/test/gql/sanitize-directive.test.ts +++ b/packages/api/test/gql/sanitize-directive.test.ts @@ -5,7 +5,6 @@ import { hashPassword } from '../../src/utils/auth' import 'mocha' describe('Sanitize Directive', () => { - const username = 'fake_user' const correctPassword = 'fakePassword' let authToken: string @@ -13,7 +12,7 @@ describe('Sanitize Directive', () => { before(async () => { const hashedPassword = await hashPassword(correctPassword) - user = await createTestUser(username, '', hashedPassword) + user = await createTestUser('fake_user', '', hashedPassword) const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -22,7 +21,7 @@ describe('Sanitize Directive', () => { }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Update user with a bio that is too long', () => { diff --git a/packages/api/test/resolvers/api_key.test.ts b/packages/api/test/resolvers/api_key.test.ts index 11452fdf4..e1f85f3d8 100644 --- a/packages/api/test/resolvers/api_key.test.ts +++ b/packages/api/test/resolvers/api_key.test.ts @@ -25,8 +25,6 @@ const testAPIKey = (apiKey: string): supertest.Test => { } describe('Api Key resolver', () => { - const username = 'fake_user' - let authToken: string let user: User let query: string @@ -36,7 +34,7 @@ describe('Api Key resolver', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fake_user') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -46,7 +44,7 @@ describe('Api Key resolver', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('generate api key', () => { diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index 440b8107b..b62dc0a02 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -343,14 +343,13 @@ const typeaheadSearchQuery = (keyword: string) => { } describe('Article API', () => { - const username = 'fakeUser' let authToken: string let user: User let ctx: PageContext before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -366,7 +365,7 @@ describe('Article API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('CreateArticle', () => { diff --git a/packages/api/test/resolvers/article_saving_request.test.ts b/packages/api/test/resolvers/article_saving_request.test.ts index 3cadd9a59..a0ea2c203 100644 --- a/packages/api/test/resolvers/article_saving_request.test.ts +++ b/packages/api/test/resolvers/article_saving_request.test.ts @@ -49,14 +49,13 @@ const createArticleSavingRequestMutation = (url: string) => ` ` describe('ArticleSavingRequest API', () => { - const username = 'fakeUser' let authToken: string let user: User let ctx: PageContext before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -72,7 +71,7 @@ describe('ArticleSavingRequest API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('createArticleSavingRequest', () => { diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index 98d40c775..ca5e85967 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -102,7 +102,6 @@ const mergeHighlightQuery = ( } describe('Highlights API', () => { - const username = 'fakeUser' let authToken: string let user: User let pageId: string @@ -110,7 +109,7 @@ describe('Highlights API', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -121,7 +120,7 @@ describe('Highlights API', () => { }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) if (pageId) { await deletePage(pageId, ctx) } diff --git a/packages/api/test/resolvers/integrations.test.ts b/packages/api/test/resolvers/integrations.test.ts index f024e0eb0..6383438d2 100644 --- a/packages/api/test/resolvers/integrations.test.ts +++ b/packages/api/test/resolvers/integrations.test.ts @@ -30,7 +30,7 @@ describe('Integrations resolvers', () => { }) after(async () => { - await deleteTestUser(loginUser.name) + await deleteTestUser(loginUser.id) }) describe('setIntegration API', () => { @@ -199,7 +199,7 @@ describe('Integrations resolvers', () => { }) after(async () => { - await deleteTestUser(otherUser.name) + await deleteTestUser(otherUser.id) await getRepository(Integration).delete({ id: existingIntegration.id, }) diff --git a/packages/api/test/resolvers/labels.test.ts b/packages/api/test/resolvers/labels.test.ts index 6b81a36c8..e4dfaa2c3 100644 --- a/packages/api/test/resolvers/labels.test.ts +++ b/packages/api/test/resolvers/labels.test.ts @@ -40,7 +40,7 @@ describe('Labels API', () => { after(async () => { // clean up - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) describe('GET labels', () => { diff --git a/packages/api/test/resolvers/newsletters.test.ts b/packages/api/test/resolvers/newsletters.test.ts index 465fcc9aa..46f2ce22d 100644 --- a/packages/api/test/resolvers/newsletters.test.ts +++ b/packages/api/test/resolvers/newsletters.test.ts @@ -6,19 +6,19 @@ import { } from '../db' import { generateFakeUuid, graphqlRequest, request } from '../util' import { NewsletterEmail } from '../../src/entity/newsletter_email' +import { User } from '../../src/entity/user' import { expect } from 'chai' import { DeleteNewsletterEmailErrorCode } from '../../src/generated/graphql' import 'mocha' describe('Newsletters API', () => { - const username = 'fakeUser' - + let user: User let authToken: string let newsletterEmails: NewsletterEmail[] before(async () => { // create test user and login - const user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -39,7 +39,7 @@ describe('Newsletters API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Get newsletter emails', () => { diff --git a/packages/api/test/resolvers/popular_reads.test.ts b/packages/api/test/resolvers/popular_reads.test.ts index 895eae646..22d216b6e 100644 --- a/packages/api/test/resolvers/popular_reads.test.ts +++ b/packages/api/test/resolvers/popular_reads.test.ts @@ -6,8 +6,6 @@ import { User } from '../../src/entity/user' import { getPageByParam } from '../../src/elastic/pages' describe('PopularReads API', () => { - const username = 'fakeUser' - let user: User let authToken: string @@ -28,7 +26,7 @@ describe('PopularReads API', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -38,7 +36,7 @@ describe('PopularReads API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('addPopularRead', () => { diff --git a/packages/api/test/resolvers/reminders.test.ts b/packages/api/test/resolvers/reminders.test.ts index 16e616ca5..26645748d 100644 --- a/packages/api/test/resolvers/reminders.test.ts +++ b/packages/api/test/resolvers/reminders.test.ts @@ -12,6 +12,7 @@ import { } from '../db' import { expect } from 'chai' import { Reminder } from '../../src/entity/reminder' +import { User } from '../../src/entity/user' import { CreateReminderErrorCode, ReminderErrorCode, @@ -22,15 +23,14 @@ import 'mocha' import { Page } from '../../src/elastic/types' describe('Reminders API', () => { - const username = 'fakeUser' - let authToken: string let page: Page let reminder: Reminder + let user: User before(async () => { // create test user and login - const user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -44,7 +44,7 @@ describe('Reminders API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Get reminder', () => { diff --git a/packages/api/test/resolvers/report.test.ts b/packages/api/test/resolvers/report.test.ts index 1a4ba6ea1..0724eefef 100644 --- a/packages/api/test/resolvers/report.test.ts +++ b/packages/api/test/resolvers/report.test.ts @@ -8,15 +8,13 @@ import { expect } from 'chai' import { getRepository } from '../../src/entity/utils' describe('Report API', () => { - const username = 'fakeUser' - let user: User let authToken: string let page: Page before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -29,7 +27,7 @@ describe('Report API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('reportItem', () => { diff --git a/packages/api/test/resolvers/send_install_instructions.test.ts b/packages/api/test/resolvers/send_install_instructions.test.ts index 6d28d7cdd..e0e5d257f 100644 --- a/packages/api/test/resolvers/send_install_instructions.test.ts +++ b/packages/api/test/resolvers/send_install_instructions.test.ts @@ -1,18 +1,15 @@ -import { - createTestUser, - deleteTestUser, -} from '../db' +import { createTestUser, deleteTestUser } from '../db' import { graphqlRequest, request } from '../util' import 'mocha' +import { User } from '../../src/entity/user' describe('Send Install Instructions API', () => { - const username = 'fakeUser' - let authToken: string + let user: User before(async () => { // create test user and login - const user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -22,7 +19,7 @@ describe('Send Install Instructions API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Send install instructions', () => { diff --git a/packages/api/test/resolvers/subscriptions.test.ts b/packages/api/test/resolvers/subscriptions.test.ts index a79fb0678..c6b34ff86 100644 --- a/packages/api/test/resolvers/subscriptions.test.ts +++ b/packages/api/test/resolvers/subscriptions.test.ts @@ -6,15 +6,13 @@ import 'mocha' import { User } from '../../src/entity/user' describe('Subscriptions API', () => { - const username = 'fakeUser' - let user: User let authToken: string let subscriptions: Subscription[] before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -29,7 +27,7 @@ describe('Subscriptions API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('GET subscriptions', () => { diff --git a/packages/api/test/resolvers/update.test.ts b/packages/api/test/resolvers/update.test.ts index a742454fb..7ac09d563 100644 --- a/packages/api/test/resolvers/update.test.ts +++ b/packages/api/test/resolvers/update.test.ts @@ -6,15 +6,13 @@ import { User } from '../../src/entity/user' import { Page } from '../../src/elastic/types' describe('Update API', () => { - const username = 'fakeUser' - let user: User let authToken: string let page: Page before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -25,7 +23,7 @@ describe('Update API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('update page', () => { diff --git a/packages/api/test/resolvers/upload_file_request.test.ts b/packages/api/test/resolvers/upload_file_request.test.ts index 583afe825..50f57956c 100644 --- a/packages/api/test/resolvers/upload_file_request.test.ts +++ b/packages/api/test/resolvers/upload_file_request.test.ts @@ -46,14 +46,13 @@ const uploadFileRequest = async ( } describe('uploadFileRequest API', () => { - const username = 'fakeUser' let authToken: string let user: User let ctx: PageContext before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -68,7 +67,7 @@ describe('uploadFileRequest API', () => { }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('UploadFileRequest', () => { diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index 58ebd8cb6..9939a6f64 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -10,7 +10,6 @@ import { hashPassword } from '../../src/utils/auth' import 'mocha' describe('User API', () => { - const username = 'fake_user' const correctPassword = 'fakePassword' const anotherUsername = 'newFakeUser' @@ -21,7 +20,7 @@ describe('User API', () => { before(async () => { const hashedPassword = await hashPassword(correctPassword) // create test user and login - user = await createTestUser(username, '', hashedPassword) + user = await createTestUser('fake_user', '', hashedPassword) const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -34,8 +33,8 @@ describe('User API', () => { after(async () => { // clean up - await deleteTestUser(username) - await deleteTestUser(anotherUsername) + await deleteTestUser(user.id) + await deleteTestUser(anotherUser.id) }) describe('Update user', () => { diff --git a/packages/api/test/resolvers/user_delete_account.test.ts b/packages/api/test/resolvers/user_delete_account.test.ts index 501c37057..fa87f980e 100644 --- a/packages/api/test/resolvers/user_delete_account.test.ts +++ b/packages/api/test/resolvers/user_delete_account.test.ts @@ -28,13 +28,12 @@ const deleteAccountRequest = async (authToken: string, userId: string) => { } describe('the deleteAccount API', () => { - const username = 'newFakeUser' let authToken: string let user: User before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('newFakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -43,7 +42,7 @@ describe('the deleteAccount API', () => { }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) context('deleting a user that exists', () => { diff --git a/packages/api/test/resolvers/user_device_tokens.test.ts b/packages/api/test/resolvers/user_device_tokens.test.ts index cdedd4bc3..bf1201ffb 100644 --- a/packages/api/test/resolvers/user_device_tokens.test.ts +++ b/packages/api/test/resolvers/user_device_tokens.test.ts @@ -13,15 +13,13 @@ import { User } from '../../src/entity/user' import { getRepository } from '../../src/entity/utils' describe('Device tokens API', () => { - const username = 'fakeUser' - let authToken: string let deviceToken: UserDeviceToken let user: User before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -34,7 +32,7 @@ describe('Device tokens API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Set device token', () => { diff --git a/packages/api/test/resolvers/user_feed_article.test.ts b/packages/api/test/resolvers/user_feed_article.test.ts index b4abb1863..69e6dfdee 100644 --- a/packages/api/test/resolvers/user_feed_article.test.ts +++ b/packages/api/test/resolvers/user_feed_article.test.ts @@ -12,10 +12,11 @@ import { Link } from '../../src/entity/link' import { Highlight } from '../../src/entity/highlight' import 'mocha' import { getRepository } from '../../src/entity/utils' +import { User } from '../../src/entity/user' describe('User feed article API', () => { const existingUsername = 'fakeUser' - + let user: User let authToken: string let page: Page let link: Link @@ -23,7 +24,7 @@ describe('User feed article API', () => { before(async () => { // create test user and login - const user = await createTestUser(existingUsername) + user = await createTestUser(existingUsername) const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -44,7 +45,7 @@ describe('User feed article API', () => { after(async () => { // clean up - await deleteTestUser(existingUsername) + await deleteTestUser(user.id) }) describe('get shared article', () => { diff --git a/packages/api/test/resolvers/webhooks.test.ts b/packages/api/test/resolvers/webhooks.test.ts index e0e91b027..a22ca3d8d 100644 --- a/packages/api/test/resolvers/webhooks.test.ts +++ b/packages/api/test/resolvers/webhooks.test.ts @@ -8,14 +8,12 @@ import { Webhook } from '../../src/entity/webhook' import { getRepository } from '../../src/entity/utils' describe('Webhooks API', () => { - const username = 'fakeUser' - let user: User let authToken: string before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -39,7 +37,7 @@ describe('Webhooks API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('Get webhook', () => { diff --git a/packages/api/test/routers/article.test.ts b/packages/api/test/routers/article.test.ts index 2e2581218..ec0c133f7 100644 --- a/packages/api/test/routers/article.test.ts +++ b/packages/api/test/routers/article.test.ts @@ -4,10 +4,10 @@ import { expect } from 'chai' import nock from 'nock' import 'mocha' import { env } from '../../src/env' +import { User } from '../../src/entity/user' describe('/article/save API', () => { - const username = 'fakeUser' - + let user: User let authToken: string // We need to mock the pupeeteer-parse @@ -17,7 +17,7 @@ describe('/article/save API', () => { before(async () => { // create test user and login - const user = await createTestUser(username) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -27,7 +27,7 @@ describe('/article/save API', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('POST /article/save', () => { diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index ed8560ddd..b2d4310d1 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -55,7 +55,8 @@ describe('auth router', () => { }) afterEach(async () => { - await deleteTestUser(username) + const user = await getRepository(User).findOne({ where: { name } }) + await deleteTestUser(user!.id) }) context('when confirmation email sent', () => { @@ -112,15 +113,17 @@ describe('auth router', () => { }) context('when user exists', () => { + let user: User + before(async () => { username = 'Some_username' - const user = await createTestUser(username) + user = await createTestUser(username) email = user.email password = 'Some password' }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) it('redirects to sign up page with error code USER_EXISTS', async () => { @@ -170,7 +173,7 @@ describe('auth router', () => { }) after(async () => { - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) context('when email and password are valid', () => { @@ -289,7 +292,7 @@ describe('auth router', () => { after(async () => { sinon.restore() - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) context('when token is valid', () => { @@ -377,7 +380,7 @@ describe('auth router', () => { }) after(async () => { - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) context('when email is verified', () => { @@ -485,7 +488,7 @@ describe('auth router', () => { }) after(async () => { - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) context('when token is valid', () => { diff --git a/packages/api/test/routers/emails.test.ts b/packages/api/test/routers/emails.test.ts index 2f78483a7..e8bdf5f15 100644 --- a/packages/api/test/routers/emails.test.ts +++ b/packages/api/test/routers/emails.test.ts @@ -13,7 +13,6 @@ import * as sendNotification from '../../src/utils/sendNotification' import * as sendEmail from '../../src/utils/sendEmail' describe('Emails Router', () => { - const username = 'fakeUser' const newsletterEmail = 'fakeUser@omnivore.app' let user: User @@ -21,7 +20,7 @@ describe('Emails Router', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') await createTestNewsletterEmail(user, newsletterEmail) token = process.env.PUBSUB_VERIFICATION_TOKEN! @@ -29,7 +28,7 @@ describe('Emails Router', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) sinon.restore() }) diff --git a/packages/api/test/routers/integrations.test.ts b/packages/api/test/routers/integrations.test.ts index 20d927ab5..dbf598fab 100644 --- a/packages/api/test/routers/integrations.test.ts +++ b/packages/api/test/routers/integrations.test.ts @@ -85,7 +85,7 @@ describe('Integrations routers', () => { }) after(async () => { - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) context('when integration not found', () => { diff --git a/packages/api/test/routers/pdf_attachments.test.ts b/packages/api/test/routers/pdf_attachments.test.ts index 6110ef4e3..595948ea3 100644 --- a/packages/api/test/routers/pdf_attachments.test.ts +++ b/packages/api/test/routers/pdf_attachments.test.ts @@ -11,7 +11,6 @@ import { expect } from 'chai' import { getPageById } from '../../src/elastic/pages' describe('PDF attachments Router', () => { - const username = 'fakeUser' const newsletterEmail = 'fakeEmail@omnivore.app' let user: User @@ -19,7 +18,7 @@ describe('PDF attachments Router', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') await createTestNewsletterEmail(user, newsletterEmail) authToken = jwt.sign(newsletterEmail, process.env.JWT_SECRET || '') @@ -27,7 +26,7 @@ describe('PDF attachments Router', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('upload', () => { diff --git a/packages/api/test/routers/reminders.test.ts b/packages/api/test/routers/reminders.test.ts index 899648c13..b7e8d6216 100644 --- a/packages/api/test/routers/reminders.test.ts +++ b/packages/api/test/routers/reminders.test.ts @@ -13,15 +13,13 @@ 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) + user = await createTestUser('fakeUser') const res = await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -35,7 +33,7 @@ describe('Reminders Router', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('trigger reminders', () => { diff --git a/packages/api/test/routers/webhooks.test.ts b/packages/api/test/routers/webhooks.test.ts index 60e0c2bd0..20e76606f 100644 --- a/packages/api/test/routers/webhooks.test.ts +++ b/packages/api/test/routers/webhooks.test.ts @@ -8,7 +8,6 @@ import { expect } from 'chai' import nock from 'nock' describe('Webhooks Router', () => { - const username = 'fakeUser' const token = process.env.PUBSUB_VERIFICATION_TOKEN || '' const webhookBaseUrl = 'https://localhost:3000' const webhookPath = `/webhooks` @@ -18,7 +17,7 @@ describe('Webhooks Router', () => { before(async () => { // create test user and login - user = await createTestUser(username) + user = await createTestUser('fakeUser') await request .post('/local/debug/fake-user-login') .send({ fakeEmail: user.email }) @@ -32,7 +31,7 @@ describe('Webhooks Router', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) describe('trigger webhooks', () => { diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 029d6027e..d2f10df95 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -17,6 +17,8 @@ import sinonChai from 'sinon-chai' import sinon from 'sinon' import * as util from '../../src/utils/sendEmail' import { MailDataRequired } from '@sendgrid/helpers/classes/mail' +import { User } from '../../src/entity/user' +import { getRepository } from '../../src/entity/utils' chai.use(sinonChai) @@ -24,8 +26,14 @@ describe('create user', () => { context('create a user with an invite', () => { it('follows the other user in the group', async () => { after(async () => { - await deleteTestUser(testOwner) - await deleteTestUser(testUser) + const testUser = await getRepository(User).findOneBy({ + name: 'testuser', + }) + await deleteTestUser(testUser!.id) + const testOwner = await getRepository(User).findOneBy({ + name: 'testowner', + }) + await deleteTestUser(testOwner!.id) }) const testOwner = 'testowner' @@ -46,7 +54,10 @@ describe('create user', () => { it('creates profile when user exists but profile not', async () => { after(async () => { - await deleteTestUser(name) + const user = await getRepository(User).findOneBy({ + name: 'userWithoutProfile', + }) + await deleteTestUser(user!.id) }) const name = 'userWithoutProfile' @@ -71,7 +82,8 @@ describe('create user', () => { afterEach(async () => { sinon.restore() - await deleteTestUser(name) + const user = await getRepository(User).findOneBy({ name }) + await deleteTestUser(user!.id) }) it('creates the user with pending status and correct name', async () => { @@ -95,7 +107,8 @@ describe('create user', () => { after(async () => { sinon.restore() - await deleteTestUser(name) + const user = await getRepository(User).findOneBy({ name }) + await deleteTestUser(user!.id) }) it('rejects with error', async () => { diff --git a/packages/api/test/services/labels.test.ts b/packages/api/test/services/labels.test.ts index 30994c45f..3afc6dd84 100644 --- a/packages/api/test/services/labels.test.ts +++ b/packages/api/test/services/labels.test.ts @@ -13,15 +13,16 @@ import { Label } from '../../src/entity/label' import { Link } from '../../src/entity/link' import { labelsLoader } from '../../src/services/labels' import { getRepository } from '../../src/entity/utils' +import { User } from '../../src/entity/user' describe('batch get labels from linkIds', () => { - let username = 'testUser' + let user: User let labels: Label[] = [] let link: Link before(async () => { // create test user - const user = await createTestUser(username) + user = await createTestUser('fakeUser') // Create some test links const page = await createTestPage() @@ -41,7 +42,7 @@ describe('batch get labels from linkIds', () => { after(async () => { // clean up - await deleteTestUser(username) + await deleteTestUser(user.id) }) it('should return a list of label from one link', async () => { diff --git a/packages/api/test/services/save_email.test.ts b/packages/api/test/services/save_email.test.ts index 4b0b345d8..975c9ad0d 100644 --- a/packages/api/test/services/save_email.test.ts +++ b/packages/api/test/services/save_email.test.ts @@ -6,13 +6,19 @@ import { SaveContext, saveEmail } from '../../src/services/save_email' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' import nock from 'nock' +import { User } from '../../src/entity/user' describe('saveEmail', () => { - const username = 'fakeUser' const fakeContent = 'fake content' + let user: User + + before(async () => { + // create test user + user = await createTestUser('fakeUser') + }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) it('doesnt fail if saved twice', async () => { @@ -21,7 +27,6 @@ describe('saveEmail', () => { const url = 'https://blog.omnivore.app/fake-url' const title = 'fake title' const author = 'fake author' - const user = await createTestUser(username) const ctx: SaveContext = { pubsub: createPubSubClient(), uid: user.id, diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index 0b2d874fe..0f9bb3f99 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -12,7 +12,6 @@ import { getPageByParam } from '../../src/elastic/pages' import nock from 'nock' describe('saveNewsletterEmail', () => { - const username = 'fakeUser' const fakeContent = 'fake content' const title = 'fake title' const author = 'fake author' @@ -22,7 +21,7 @@ describe('saveNewsletterEmail', () => { let ctx: SaveContext before(async () => { - user = await createTestUser(username) + user = await createTestUser('fakeUser') email = await createNewsletterEmail(user.id) ctx = { pubsub: createPubSubClient(), @@ -32,7 +31,7 @@ describe('saveNewsletterEmail', () => { }) after(async () => { - await deleteTestUser(username) + await deleteTestUser(user.id) }) it('adds the newsletter to the library', async () => { diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index 0a2f80aa4..bf5889155 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -113,7 +113,7 @@ describe('isProbablyArticle', () => { }) after(async () => { - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) it('returns true when email is signed up with us', async () => { From b59db0629d9528e52487c9a9d88042a918b43878 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 13 Oct 2022 11:38:09 +0800 Subject: [PATCH 03/10] Add ELASTIC_URL in migration --- .github/workflows/run-tests.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 30fe32cc2..5a22f661d 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -73,6 +73,7 @@ jobs: PG_USER: postgres PG_PASSWORD: postgres PG_DB: omnivore_test + ELASTIC_URL: http://localhost:${{ job.services.elastic.ports[9200] }}/ - name: TypeScript, Lint, Tests run: | source ~/.nvm/nvm.sh From f43ad01b89beea9733197b00df1b0ea41ae19ae0 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 13 Oct 2022 11:56:06 +0800 Subject: [PATCH 04/10] Set PGPASSWORD in env to call psql without password prompt --- .github/workflows/run-tests.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 5a22f661d..2f9b2e5be 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -66,7 +66,7 @@ jobs: - name: Database Migration run: | yarn workspace @omnivore/db migrate - psql --host localhost --port ${{ job.services.postgres.ports[5432] }} --user postgres --password -c "CREATE USER app_user WITH ENCRYPTED PASSWORD 'app_pass';GRANT omnivore_user to app_user;" + psql -p ${{ job.services.postgres.ports[5432] }} -U postgres -c "CREATE USER app_user WITH ENCRYPTED PASSWORD 'app_pass';GRANT omnivore_user to app_user;" env: PG_HOST: localhost PG_PORT: ${{ job.services.postgres.ports[5432] }} @@ -74,6 +74,7 @@ jobs: PG_PASSWORD: postgres PG_DB: omnivore_test ELASTIC_URL: http://localhost:${{ job.services.elastic.ports[9200] }}/ + PGPASSWORD: postgres # This is required for the psql command to work without a password prompt - name: TypeScript, Lint, Tests run: | source ~/.nvm/nvm.sh From ea737f1c4ff0bdbe3b18e3885bc276995dab06ef Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 13 Oct 2022 12:26:38 +0800 Subject: [PATCH 05/10] Add HOST option in psql cmd --- .github/workflows/run-tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 2f9b2e5be..0086a086e 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -66,7 +66,7 @@ jobs: - name: Database Migration run: | yarn workspace @omnivore/db migrate - psql -p ${{ job.services.postgres.ports[5432] }} -U postgres -c "CREATE USER app_user WITH ENCRYPTED PASSWORD 'app_pass';GRANT omnivore_user to app_user;" + psql -h localhost -p ${{ job.services.postgres.ports[5432] }} -U postgres -c "CREATE USER app_user WITH ENCRYPTED PASSWORD 'app_pass';GRANT omnivore_user to app_user;" env: PG_HOST: localhost PG_PORT: ${{ job.services.postgres.ports[5432] }} From 14ddb369a90371f7c3d2ae8c5ae4abdefb68926a Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 13 Oct 2022 12:36:36 +0800 Subject: [PATCH 06/10] Fix tests --- packages/api/test/resolvers/recent_searches.test.ts | 2 +- packages/api/test/utils/stub.test.ts | 0 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 packages/api/test/utils/stub.test.ts diff --git a/packages/api/test/resolvers/recent_searches.test.ts b/packages/api/test/resolvers/recent_searches.test.ts index 4d6a95c67..8598a2c6e 100644 --- a/packages/api/test/resolvers/recent_searches.test.ts +++ b/packages/api/test/resolvers/recent_searches.test.ts @@ -29,7 +29,7 @@ describe('recent_searches resolver', () => { after(async () => { // clean up - await deleteTestUser(user.name) + await deleteTestUser(user.id) }) describe('recentSearches API', () => { diff --git a/packages/api/test/utils/stub.test.ts b/packages/api/test/utils/stub.test.ts deleted file mode 100644 index e69de29bb..000000000 From 05ecac7c52a45822c54b6c4d580ba529537b21fe Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 13 Oct 2022 14:13:59 +0800 Subject: [PATCH 07/10] Fix tests --- .../entity/reports/content_display_report.ts | 8 +++-- .../reports/content_display_report_created.ts | 2 +- packages/api/test/db.ts | 29 ++++++++++++++++ .../api/test/resolvers/integrations.test.ts | 18 ++++------ packages/api/test/resolvers/labels.test.ts | 33 ++++++++++++++----- packages/api/test/resolvers/report.test.ts | 6 ---- .../test/resolvers/user_feed_article.test.ts | 3 +- packages/api/test/routers/auth.test.ts | 26 ++++----------- 8 files changed, 76 insertions(+), 49 deletions(-) diff --git a/packages/api/src/entity/reports/content_display_report.ts b/packages/api/src/entity/reports/content_display_report.ts index b15b45d41..627a83870 100644 --- a/packages/api/src/entity/reports/content_display_report.ts +++ b/packages/api/src/entity/reports/content_display_report.ts @@ -2,17 +2,21 @@ import { Column, CreateDateColumn, Entity, + JoinColumn, + ManyToOne, PrimaryGeneratedColumn, UpdateDateColumn, } from 'typeorm' +import { User } from '../user' @Entity() export class ContentDisplayReport { @PrimaryGeneratedColumn('uuid') id?: string - @Column('text') - userId!: string + @ManyToOne(() => User, { onDelete: 'CASCADE' }) + @JoinColumn({ name: 'user_id' }) + user!: User @Column('text') pageId?: string diff --git a/packages/api/src/events/reports/content_display_report_created.ts b/packages/api/src/events/reports/content_display_report_created.ts index d8d1ab44e..add7b1d80 100644 --- a/packages/api/src/events/reports/content_display_report_created.ts +++ b/packages/api/src/events/reports/content_display_report_created.ts @@ -20,7 +20,7 @@ export class ContentDisplayReportSubscriber async afterInsert(event: InsertEvent): Promise { const report = event.entity const message = `A new content display report was created by: - ${report.userId} for URL: ${report.originalUrl} + ${report.user.id} for URL: ${report.originalUrl} ${report.reportComment}` console.log(message) diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts index 3447d859c..54f40dfd2 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -13,6 +13,8 @@ import { getRepository, setClaims } from '../src/entity/utils' import { createUser } from '../src/services/create_user' import { SnakeNamingStrategy } from 'typeorm-naming-strategies' import { SubscriptionStatus } from '../src/generated/graphql' +import { Integration } from '../src/entity/integration' +import { FindOptionsWhere } from 'typeorm' const runMigrations = async () => { const migrationDirectory = __dirname + '/../../db/migrations' @@ -202,3 +204,30 @@ export const createTestSubscription = async ( status: SubscriptionStatus.Active, }) } + +export const deleteTestLabels = async ( + userId: string, + criteria: string[] | FindOptionsWhere