diff --git a/packages/api/src/datalayer/user/index.ts b/packages/api/src/datalayer/user/index.ts index 95e81000a..a4f6058b2 100644 --- a/packages/api/src/datalayer/user/index.ts +++ b/packages/api/src/datalayer/user/index.ts @@ -329,11 +329,15 @@ class UserModel extends DataModel { } @logMethod - deleteUser( + async delete( userId: string, - tx: Knex.Transaction + tx?: Knex.Transaction ): Promise { - return super.delete(userId, tx) + if (tx) { + return super.delete(userId, tx) + } + + return this.kx.transaction((tx) => super.delete(userId, tx)) } } diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 2a1d39641..013cab0d1 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -306,7 +306,7 @@ export const createArticleResolver = authorized< let uploadFileUrlOverride = '' if (uploadFileId) { const uploadFileData = await authTrx(async (tx) => { - return await models.uploadFile.setFileUploadComplete(uploadFileId, tx) + return models.uploadFile.setFileUploadComplete(uploadFileId, tx) }) if (!uploadFileData || !uploadFileData.id || !uploadFileData.fileName) { return pageError( diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index c39524338..cf57a201a 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -38,7 +38,6 @@ import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' import { comparePassword, hashPassword } from '../../utils/auth' -import type { UserData } from '../../datalayer/user/model' import { deletePagesByParam } from '../../elastic/pages' export const updateUserResolver = authorized< @@ -366,9 +365,8 @@ export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, MutationDeleteAccountArgs ->(async (_, { userID }, { models, claims, log, authTrx, pubsub }) => { +>(async (_, { userID }, { models, claims, log, pubsub }) => { const user = await models.user.get(userID) - if (!user) { return { errorCodes: [DeleteAccountErrorCode.UserNotFound], @@ -381,13 +379,6 @@ export const deleteAccountResolver = authorized< } } - const deleteUserResult = await authTrx((tx) => - models.user.deleteUser(claims.uid, tx) - ) - - // delete this user's pages in elastic - await deletePagesByParam({ userId: userID }, { uid: userID, pubsub }) - log.info('Deleting a user account', { userID, labels: { @@ -397,11 +388,17 @@ export const deleteAccountResolver = authorized< }, }) - if ((deleteUserResult as UserData).id !== undefined) { - return { userID } - } else { + const deletedUser = await models.user.delete(userID) + if ('error' in deletedUser) { + log.error('Error deleting user account', deletedUser.error) + return { - errorCodes: [DeleteAccountErrorCode.Forbidden], + errorCodes: [DeleteAccountErrorCode.UserNotFound], } } + + // delete this user's pages in elastic + await deletePagesByParam({ userId: userID }, { uid: userID, pubsub }) + + return { userID } }) diff --git a/packages/api/test/resolvers/user_delete_account.test.ts b/packages/api/test/resolvers/user_delete_account.test.ts index 12a2b52a5..6859217ca 100644 --- a/packages/api/test/resolvers/user_delete_account.test.ts +++ b/packages/api/test/resolvers/user_delete_account.test.ts @@ -1,4 +1,4 @@ -import { createTestUser } from '../db' +import { createTestUser, deleteTestUser } from '../db' import { graphqlRequest, request } from '../util' import * as chai from 'chai' import { expect } from 'chai' @@ -13,12 +13,10 @@ const deleteAccountRequest = async (authToken: string, userId: string) => { const mutation = ` mutation { deleteAccount( - input: { - userId: "${userId}", - } + userID: "${userId}", ) { ... on DeleteAccountSuccess { - userId + userID } ... on DeleteAccountError { errorCodes @@ -30,7 +28,7 @@ const deleteAccountRequest = async (authToken: string, userId: string) => { } describe('the deleteAccount API', () => { - const username = 'fakeUser' + const username = 'newFakeUser' let authToken: string let user: User @@ -44,6 +42,10 @@ describe('the deleteAccount API', () => { authToken = res.body.authToken }) + after(async () => { + await deleteTestUser(username) + }) + context('deleting a user that exists', () => { it('should return a unauthorized error if authToken is invalid', async () => { const res = await deleteAccountRequest('invalid-auth-token', user.id) @@ -54,7 +56,7 @@ describe('the deleteAccount API', () => { it('should return the user id after a successful user deletion', async () => { const res = await deleteAccountRequest(authToken, user.id) - expect(res.body.data.deleteAccount.userId).to.eql(user.id) + expect(res.body.data.deleteAccount.userID).to.eql(user.id) }) })