Merge pull request #1592 from serlo/krVaR-kulla-2024-06-15-10-23

refactor(deleted-entities): Migrate to SQL query

__tests__/__utils__/handlers.ts

@@ -21,37 +21,6 @@ const ForDefinitions = {
       given('UuidQuery').withPayload({ id: uuid.id }).returns(uuid)
     }
   },
-  DeletedEntitiesQuery(entities: Model<'AbstractEntity'>[]) {
-    given('UuidQuery').for(
-      entities.map((entity) => {
-        return { ...entity, trashed: true }
-      }),
-    )
-    given('DeletedEntitiesQuery').isDefinedBy(async ({ request }) => {
-      const body = await request.json()
-      const { first, after, instance } = body.payload
-
-      const entitiesByInstance = instance
-        ? entities.filter((entity) => entity.instance === instance)
-        : entities
-
-      const entitiesByAfter = after
-        ? entitiesByInstance.filter(
-            (entity) => new Date(entity.date) > new Date(after),
-          )
-        : entitiesByInstance
-
-      const entitiesByFirst = entitiesByAfter.slice(0, first)
-
-      const deletedEntities = entitiesByFirst.map((entity) => {
-        return {
-          id: entity.id,
-          dateOfDeletion: entity.date,
-        }
-      })
-      return HttpResponse.json({ deletedEntities })
-    })
-  },
 }
 type ForDefinitions = typeof ForDefinitions
 type ForArg<M> = M extends keyof ForDefinitions

__tests__/schema/entity/deleted-entities.ts

@@ -1,10 +1,6 @@
 import gql from 'graphql-tag'
 
-import {
-  article as baseArticle,
-  coursePage as baseCoursePage,
-} from '../../../__fixtures__'
-import { given, Client } from '../../__utils__'
+import { Client } from '../../__utils__'
 import { Instance } from '~/types'
 
 const query = new Client().prepareQuery({
@@ -24,39 +20,17 @@ const query = new Client().prepareQuery({
   `,
 })
 
-const coursePage = { ...baseCoursePage, instance: Instance.En }
-const article = { ...baseArticle, date: '2015-03-01T20:45:56Z' }
-
-beforeEach(() => {
-  given('DeletedEntitiesQuery').for(article, coursePage)
+beforeEach(async () => {
+  // The database dump does not contain any deleted entities, thus we create some here
+  await databaseForTests.mutate('update uuid set trashed = 1 where id = 1615')
 })
 
 test('returns deleted entities', async () => {
   await query.shouldReturnData({
     entity: {
       deletedEntities: {
         nodes: [
-          {
-            dateOfDeletion: article.date,
-            entity: { id: article.id },
-          },
-          {
-            dateOfDeletion: coursePage.date,
-            entity: { id: coursePage.id },
-          },
-        ],
-      },
-    },
-  })
-
-  await query.withVariables({ first: 1 }).shouldReturnData({
-    entity: {
-      deletedEntities: {
-        nodes: [
-          {
-            dateOfDeletion: article.date,
-            entity: { id: article.id },
-          },
+          { entity: { id: 1615 }, dateOfDeletion: '2014-04-21T14:40:28.000Z' },
         ],
       },
     },
@@ -68,37 +42,20 @@ test('paginates with `after` parameter { entityId, dateOfDeletion}, ', async ()
     .withVariables({
       after: Buffer.from(
         JSON.stringify({
-          id: coursePage.id,
-          dateOfDeletion: coursePage.date,
+          id: 1615,
+          dateOfDeletion: '2014-04-21T14:40:28.000Z',
         }),
       ).toString('base64'),
     })
     .shouldReturnData({
-      entity: {
-        deletedEntities: {
-          nodes: [
-            {
-              dateOfDeletion: article.date,
-              entity: { id: article.id },
-            },
-          ],
-        },
-      },
+      entity: { deletedEntities: { nodes: [] } },
     })
 })
 
 test('filters by instance', async () => {
-  await query
-    .withVariables({
-      instance: Instance.En,
-    })
-    .shouldReturnData({
-      entity: {
-        deletedEntities: {
-          nodes: [{ entity: { id: coursePage.id } }],
-        },
-      },
-    })
+  await query.withVariables({ instance: Instance.En }).shouldReturnData({
+    entity: { deletedEntities: { nodes: [] } },
+  })
 })
 
 test('fails when `first` is too high', async () => {
@@ -111,23 +68,8 @@ test('fails when `after` is malformed', async () => {
   await query
     .withVariables({
       after: Buffer.from(
-        JSON.stringify({
-          id: article.id,
-          dateOfDeletion: 'foo',
-        }),
+        JSON.stringify({ id: 1865, dateOfDeletion: 'foo' }),
       ).toString('base64'),
     })
     .shouldFailWithError('BAD_USER_INPUT')
 })
-
-test('fails when database layer returns a 400er response', async () => {
-  given('DeletedEntitiesQuery').returnsBadRequest()
-
-  await query.shouldFailWithError('BAD_USER_INPUT')
-})
-
-test('fails when database layer has an internal error', async () => {
-  given('DeletedEntitiesQuery').hasInternalServerError()
-
-  await query.shouldFailWithError('INTERNAL_SERVER_ERROR')
-})

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

@@ -20,22 +20,6 @@ export const spec = {
     }),
     canBeNull: false,
   },
-  DeletedEntitiesQuery: {
-    payload: t.type({
-      first: t.number,
-      after: t.union([t.string, t.undefined]),
-      instance: t.union([InstanceDecoder, t.undefined]),
-    }),
-    response: t.type({
-      deletedEntities: t.array(
-        t.type({
-          id: t.number,
-          dateOfDeletion: t.string,
-        }),
-      ),
-    }),
-    canBeNull: false,
-  },
   EntitySortMutation: {
     payload: t.type({ childrenIds: t.array(t.number), entityId: t.number }),
     response: t.type({ success: t.boolean }),

packages/server/src/model/serlo.ts

@@ -117,16 +117,6 @@ export function createSerloModel({
     },
   })
 
-  const getDeletedEntities = createRequest({
-    type: 'DeletedEntitiesQuery',
-    decoder: DatabaseLayer.getDecoderFor('DeletedEntitiesQuery'),
-    async getCurrentValue(
-      payload: DatabaseLayer.Payload<'DeletedEntitiesQuery'>,
-    ) {
-      return DatabaseLayer.makeRequest('DeletedEntitiesQuery', payload)
-    },
-  })
-
   const sortEntity = createMutation({
     type: 'EntitySortMutation',
     decoder: DatabaseLayer.getDecoderFor('EntitySortMutation'),
@@ -164,7 +154,6 @@ export function createSerloModel({
     executePrompt,
     getActiveReviewerIds,
     getActivityByType,
-    getDeletedEntities,
     getPotentialSpamUsers,
     getUsersByRole,
     getPages,

packages/server/src/schema/uuid/abstract-entity/resolvers.ts

@@ -63,19 +63,39 @@ export const resolvers: Resolvers = {
   EntityQuery: {
     async deletedEntities(_parent, payload, context) {
       const LIMIT = 1000
-      const { first = 100, after, instance } = payload
+      const { first = 100, after, instance = null } = payload
 
       if (first > LIMIT)
         throw new UserInputError(`'first' may not be higher than ${LIMIT}`)
 
-      const deletedAfter = after ? decodeDateOfDeletion(after) : undefined
-
-      const { deletedEntities } =
-        await context.dataSources.model.serlo.getDeletedEntities({
-          first: first + 1,
-          after: deletedAfter,
-          instance,
-        })
+      const deletedAfter = after
+        ? decodeDateOfDeletion(after)
+        : new Date('9999-12-31')
+
+      const deletedEntities = await context.database.fetchAll<{
+        id: number
+        dateOfDeletion: Date
+      }>(
+        `
+        SELECT
+          uuid.id as id,
+          MAX(event.date) AS dateOfDeletion
+        FROM event
+        JOIN uuid ON uuid.id = event.uuid_id
+        JOIN entity ON entity.id = uuid.id
+        JOIN instance ON instance.id = entity.instance_id
+        WHERE uuid.id = event.uuid_id
+            AND event.date < ?
+            AND (? is null OR instance.subdomain = ?)
+            AND event.event_type_id = 10
+            AND uuid.trashed = 1
+            AND entity.type_id NOT IN (35, 39, 40, 41, 42, 43, 44)
+        GROUP BY uuid.id
+        ORDER BY dateOfDeletion DESC
+        LIMIT ?;
+        `,
+        [deletedAfter, instance, instance, first.toString()],
+      )
 
       const nodes = await Promise.all(
         deletedEntities.map(async (node) => {
@@ -85,7 +105,7 @@ export const resolvers: Resolvers = {
               { id: node.id },
               context,
             ),
-            dateOfDeletion: node.dateOfDeletion,
+            dateOfDeletion: node.dateOfDeletion.toISOString(),
           }
         }),
       )
@@ -538,7 +558,7 @@ function decodeDateOfDeletion(after: string) {
       'The encoded dateOfDeletion in `after` should be a string in date format',
     )
 
-  return new Date(dateOfDeletion).toISOString()
+  return new Date(dateOfDeletion)
 }
 
 async function isAutoreviewEntity(