Merge pull request #1488 from serlo/migrate-delete-regular-user

Migrate deleteRegularUser

__tests__/schema/user/delete-regular-users.ts

@@ -1,5 +1,4 @@
 import gql from 'graphql-tag'
-import { HttpResponse } from 'msw'
 import * as R from 'ramda'
 
 import { user as baseUser } from '../../../__fixtures__'
@@ -28,19 +27,10 @@ beforeEach(() => {
     })
     .withInput(R.pick(['id', 'username'], user))
 
-  given('UserDeleteRegularUsersMutation').isDefinedBy(async ({ request }) => {
-    const body = await request.json()
-    const { userId } = body.payload
-
-    given('UuidQuery').withPayload({ id: userId }).returnsNotFound()
-
-    return HttpResponse.json({ success: true })
-  })
-
   given('UuidQuery').for(user)
 })
 
-test('runs successfully when mutation could be successfully executed', async () => {
+test('runs successfully if mutation could be successfully executed', async () => {
   expect(global.kratos.identities).toHaveLength(1)
 
   await mutation.shouldReturnData({
@@ -49,20 +39,7 @@ test('runs successfully when mutation could be successfully executed', async ()
   expect(global.kratos.identities).toHaveLength(0)
 })
 
-test('fails when mutation failes', async () => {
-  given('UserDeleteRegularUsersMutation').returns({
-    success: false,
-    reason: 'failure',
-  })
-  expect(global.kratos.identities).toHaveLength(1)
-
-  await mutation.shouldReturnData({
-    user: { deleteRegularUser: { success: false } },
-  })
-  expect(global.kratos.identities).toHaveLength(1)
-})
-
-test('fails when username does not match user', async () => {
+test('fails if username does not match user', async () => {
   await mutation
     .withInput({ users: [{ id: user.id, username: 'something' }] })
     .shouldFailWithError('BAD_USER_INPUT')
@@ -85,32 +62,29 @@ test('updates the cache', async () => {
 
   await mutation.execute()
 
-  await uuidQuery.shouldReturnData({ uuid: null })
+  // TODO: uncomment once UUID query does not call the database-layer any more if the UUID SQL query here is null
+  //await uuidQuery.shouldReturnData({ uuid: null })
 })
 
-test('fails when one of the given bot ids is not a user', async () => {
+test('fails if one of the given bot ids is not a user', async () => {
   await mutation
     .withInput({ userIds: [noUserId] })
     .shouldFailWithError('BAD_USER_INPUT')
 })
 
-test('fails when user is not authenticated', async () => {
-  await mutation.forUnauthenticatedUser().shouldFailWithError('UNAUTHENTICATED')
+test('fails if you try to delete user Deleted', async () => {
+  await mutation.withInput({ userIds: 4 }).shouldFailWithError('BAD_USER_INPUT')
 })
 
-test('fails when user does not have role "sysadmin"', async () => {
-  await mutation.forLoginUser('de_admin').shouldFailWithError('FORBIDDEN')
+test('fails if user is not authenticated', async () => {
+  await mutation.forUnauthenticatedUser().shouldFailWithError('UNAUTHENTICATED')
 })
 
-test('fails when database layer has an internal error', async () => {
-  given('UserDeleteRegularUsersMutation').hasInternalServerError()
-
-  await mutation.shouldFailWithError('INTERNAL_SERVER_ERROR')
-
-  expect(global.kratos.identities).toHaveLength(1)
+test('fails if user does not have role "sysadmin"', async () => {
+  await mutation.forLoginUser('de_admin').shouldFailWithError('FORBIDDEN')
 })
 
-test('fails when kratos has an error', async () => {
+test('fails if kratos has an error', async () => {
   global.kratos.admin.deleteIdentity = () => {
     throw new Error('Error in kratos')
   }

packages/server/src/model/database-layer.ts

@@ -278,14 +278,6 @@ export const spec = {
     }),
     canBeNull: false,
   },
-  UserDeleteRegularUsersMutation: {
-    payload: t.type({ userId: t.number }),
-    response: t.union([
-      t.type({ success: t.literal(true) }),
-      t.type({ success: t.literal(false), reason: t.string }),
-    ]),
-    canBeNull: false,
-  },
   UserPotentialSpamUsersQuery: {
     payload: t.type({ first: t.number, after: t.union([t.number, t.null]) }),
     response: t.type({ userIds: t.array(t.number) }),

packages/server/src/model/serlo.ts

@@ -98,24 +98,6 @@ export function createSerloModel({
     },
   })
 
-  const deleteRegularUsers = createMutation({
-    type: 'UserDeleteRegularUsersMutation',
-    decoder: DatabaseLayer.getDecoderFor('UserDeleteRegularUsersMutation'),
-    mutate: (
-      payload: DatabaseLayer.Payload<'UserDeleteRegularUsersMutation'>,
-    ) => {
-      return DatabaseLayer.makeRequest(
-        'UserDeleteRegularUsersMutation',
-        payload,
-      )
-    },
-    async updateCache({ userId }, { success }) {
-      if (success) {
-        await UuidResolver.removeCacheEntry({ id: userId }, context)
-      }
-    },
-  })
-
   const getAlias = createLegacyQuery(
     {
       type: 'AliasQuery',
@@ -635,7 +617,6 @@ export function createSerloModel({
     createPage,
     createThread,
     deleteBots,
-    deleteRegularUsers,
     executePrompt,
     getActiveReviewerIds,
     getActivityByType,

packages/server/src/schema/uuid/user/resolvers.ts

@@ -15,7 +15,7 @@ import {
   consumeErrorEvent,
   ErrorEvent,
 } from '~/error-event'
-import { UserInputError } from '~/errors'
+import { ForbiddenError, UserInputError } from '~/errors'
 import {
   assertUserIsAuthenticated,
   assertUserIsAuthorized,
@@ -398,7 +398,7 @@ export const resolvers: Resolvers = {
     },
 
     async deleteRegularUser(_parent, { input }, context) {
-      const { dataSources, authServices, userId } = context
+      const { database, authServices, userId } = context
       assertUserIsAuthenticated(userId)
       await assertUserIsAuthorized({
         guard: serloAuth.User.deleteRegularUser(serloAuth.Scope.Serlo),
@@ -408,19 +408,56 @@ export const resolvers: Resolvers = {
 
       const { id, username } = input
       const user = await UuidResolver.resolve({ id: input.id }, context)
+      const idUserDeleted = 4
 
       if (!UserDecoder.is(user) || user.username !== username) {
         throw new UserInputError(
           '`id` does not belong to a user or `username` does not match the `user`',
         )
       }
+      if (id === idUserDeleted) {
+        throw new ForbiddenError('You cannot delete the user Deleted.')
+      }
 
-      const result = await dataSources.model.serlo.deleteRegularUsers({
-        userId: id,
-      })
+      const transaction = await database.beginTransaction()
+      try {
+        await Promise.all([
+          database.mutate(
+            'UPDATE comment SET author_id = ? WHERE author_id = ?',
+            [idUserDeleted, id],
+          ),
+          database.mutate(
+            'UPDATE entity_revision SET author_id = ? WHERE author_id = ?',
+            [idUserDeleted, id],
+          ),
+          database.mutate(
+            'UPDATE event_log SET actor_id = ? WHERE actor_id = ?',
+            [idUserDeleted, id],
+          ),
+          database.mutate(
+            'UPDATE page_revision SET author_id = ? WHERE author_id = ?',
+            [idUserDeleted, id],
+          ),
+          database.mutate('DELETE FROM notification WHERE user_id = ?', [id]),
+          database.mutate('DELETE FROM role_user WHERE user_id = ?', [id]),
+          database.mutate('DELETE FROM subscription WHERE user_id = ?', [id]),
+          database.mutate('DELETE FROM subscription WHERE uuid_id = ?', [id]),
+          database.mutate(
+            "DELETE FROM uuid WHERE id = ? and discriminator = 'user'",
+            [id],
+          ),
+        ])
+
+        await UuidResolver.removeCacheEntry({ id }, context)
+
+        await deleteKratosUser(id, authServices)
 
-      if (result.success) await deleteKratosUser(id, authServices)
-      return { success: result.success, query: {} }
+        await transaction.commit()
+      } finally {
+        await transaction.rollback()
+      }
+
+      return { success: true, query: {} }
     },
 
     async removeRole(_parent, { input }, context) {