chore: adds tests to validate queries

2023-04-17 16:25:07 -04:00
parent d187b809d7
commit 995054d46b
5 changed files with 383 additions and 88 deletions
--- a/src/errors/QueryError.ts
+++ b/src/errors/QueryError.ts
@@ -0,0 +1,16 @@
+import httpStatus from 'http-status';
+import type { TFunction } from 'i18next';
+import APIError from './APIError';
+
+class QueryError extends APIError {
+  constructor(results: { path: string }[], t?: TFunction) {
+    const message = t ? t('error:unspecific', { count: results.length }) : `The following path${results.length === 1 ? '' : 's'} cannot be queried:`;
+    super(
+      `${message} ${results.map((err) => err.path).join(', ')}`,
+      httpStatus.BAD_REQUEST,
+      results,
+    );
+  }
+}
+
+export default QueryError;
--- a/src/mongoose/buildQuery.ts
+++ b/src/mongoose/buildQuery.ts
@@ -12,6 +12,7 @@ import { CollectionPermission, FieldPermissions, GlobalPermission } from '../aut
 import flattenFields from '../utilities/flattenTopLevelFields';
 import { getEntityPolicies } from '../utilities/getEntityPolicies';
 import { SanitizedConfig } from '../config/types';
+import QueryError from '../errors/QueryError';

 const validOperators = ['like', 'contains', 'in', 'all', 'not_in', 'greater_than_equal', 'greater_than', 'less_than_equal', 'less_than', 'not_equals', 'equals', 'exists', 'near'];

@@ -46,8 +47,6 @@ type ParamParserArgs = {
  overrideAccess?: boolean
 }

-type QueryError = { path: string }
-
 export class ParamParser {
  collectionSlug?: string

@@ -74,7 +73,7 @@ export class ParamParser {
    };
  }

-  errors: QueryError[]
+  errors: { path: string }[]

  constructor({
    req,
@@ -566,7 +565,11 @@ const getBuildQueryPlugin = ({
        overrideAccess,
      });
      const result = await paramParser.parse();
-      // TODO: throw errors here
+
+      if (this.errors.length > 0) {
+        throw new QueryError(this.errors);
+      }
+
      return result;
    }
    modifiedSchema.statics.buildQuery = buildQuery;
--- a/test/collections-rest/config.ts
+++ b/test/collections-rest/config.ts
@@ -35,6 +35,29 @@ export const customIdNumberSlug = 'custom-id-number';
 export const errorOnHookSlug = 'error-on-hooks';

 export default buildConfig({
+  endpoints: [
+    {
+      path: '/send-test-email',
+      method: 'get',
+      handler: async (req, res) => {
+        await req.payload.sendEmail({
+          from: 'dev@payloadcms.com',
+          to: devUser.email,
+          subject: 'Test Email',
+          html: 'This is a test email.',
+          // to recreate a failing email transport, add the following credentials
+          // to the `email` property of `payload.init()` in `../dev.ts`
+          // the app should fail to send the email, but the error should be handled without crashing the app
+          // transportOptions: {
+          //   host: 'smtp.ethereal.email',
+          //   port: 587,
+          // },
+        });
+
+        res.status(200).send('Email sent');
+      },
+    },
+  ],
  collections: [
    {
      slug,
@@ -78,6 +101,13 @@ export default buildConfig({
          relationTo: [relationSlug, 'dummy'],
          hasMany: true,
        },
+        {
+          name: 'restrictedField',
+          type: 'text',
+          access: {
+            read: () => false,
+          },
+        },
      ],
    },
    {
@@ -91,7 +121,23 @@ export default buildConfig({
      ],
    },
    collectionWithName(relationSlug),
-    collectionWithName('dummy'),
+    {
+      slug: 'dummy',
+      access: openAccess,
+      fields: [
+        {
+          type: 'text',
+          name: 'title',
+        },
+        {
+          type: 'text',
+          name: 'name',
+          access: {
+            read: () => false,
+          },
+        },
+      ],
+    },
    {
      slug: customIdSlug,
      access: openAccess,
--- a/test/collections-rest/int.spec.ts
+++ b/test/collections-rest/int.spec.ts
@@ -2,7 +2,7 @@ import mongoose from 'mongoose';
 import { randomBytes } from 'crypto';
 import { initPayloadTest } from '../helpers/configHelpers';
 import type { Relation } from './config';
-import config, { customIdNumberSlug, customIdSlug, slug, relationSlug, pointSlug, errorOnHookSlug } from './config';
+import config, { customIdNumberSlug, customIdSlug, errorOnHookSlug, pointSlug, relationSlug, slug } from './config';
 import payload from '../../src';
 import { RESTClient } from '../helpers/rest';
 import type { ErrorOnHook, Post } from './payload-types';
@@ -49,10 +49,16 @@ describe('collections-rest', () => {
    });

    it('should update existing', async () => {
-      const { id, description } = await createPost({ description: 'desc' });
+      const {
+        id,
+        description,
+      } = await createPost({ description: 'desc' });
      const updatedTitle = 'updated-title';

-      const { status, doc: updated } = await client.update<Post>({
+      const {
+        status,
+        doc: updated,
+      } = await client.update<Post>({
        id,
        data: { title: updatedTitle },
      });
@@ -62,14 +68,18 @@ describe('collections-rest', () => {
      expect(updated.description).toEqual(description); // Check was not modified
    });

+    describe('Bulk operations', () => {
      it('should bulk update', async () => {
        await mapAsync([...Array(11)], async (_, i) => {
          await createPost({ description: `desc ${i}` });
        });

        const description = 'updated';
-      const { status, docs } = await client.updateMany<Post>({
-        query: { title: { equals: 'title' } },
+        const {
+          status,
+          docs,
+        } = await client.updateMany<Post>({
+          where: { title: { equals: 'title' } },
          data: { description },
        });

@@ -79,6 +89,208 @@ describe('collections-rest', () => {
        expect(docs.pop().description).toEqual(description);
      });

+      it('should not bulk update with a bad query', async () => {
+        await mapAsync([...Array(2)], async (_, i) => {
+          await createPost({ description: `desc ${i}` });
+        });
+
+        const description = 'updated';
+
+        const { status, docs: noDocs, errors } = await client.updateMany<Post>({
+          where: { missing: { equals: 'title' } },
+          data: { description },
+        });
+
+        expect(status).toEqual(400);
+        expect(noDocs).toBeUndefined();
+        expect(errors).toHaveLength(1);
+
+        const { docs } = await payload.find({
+          collection: slug,
+        });
+
+        expect(docs[0].description).not.toEqual(description);
+        expect(docs.pop().description).not.toEqual(description);
+      });
+
+      it('should not bulk update with a bad relationship query', async () => {
+        await mapAsync([...Array(2)], async (_, i) => {
+          await createPost({ description: `desc ${i}` });
+        });
+
+        const description = 'updated';
+        const { status: relationFieldStatus, docs: relationFieldDocs, errors: relationFieldErrors } = await client.updateMany<Post>({
+          where: { 'relationField.missing': { equals: 'title' } },
+          data: { description },
+        });
+
+        console.log({ relationFieldStatus, relationFieldDocs, relationFieldErrors });
+
+        const { status: relationMultiRelationToStatus } = await client.updateMany<Post>({
+          where: { 'relationMultiRelationTo.missing': { equals: 'title' } },
+          data: { description },
+        });
+
+        const { docs } = await payload.find({
+          collection: slug,
+        });
+
+        expect(relationFieldStatus).toEqual(400);
+        expect(relationMultiRelationToStatus).toEqual(400);
+        expect(docs[0].description).not.toEqual(description);
+        expect(docs.pop().description).not.toEqual(description);
+      });
+
+      it('should not bulk update with a read restricted field query', async () => {
+        const { id } = await payload.create({
+          collection: slug,
+          data: {
+            restrictedField: 'restricted',
+          },
+        });
+
+        const description = 'description';
+        const { status } = await client.updateMany<Post>({
+          query: { restrictedField: { equals: 'restricted' } },
+          data: { description },
+        });
+
+        const doc = await payload.findByID({
+          collection: slug,
+          id,
+        });
+
+        expect(status).toEqual(400);
+        expect(doc.description).toBeUndefined();
+      });
+
+      it('should bulk update with a relationship field that exists in one collection and not another', async () => {
+        const relationOne = await payload.create({
+          collection: 'dummy',
+          data: {
+            title: 'title',
+          },
+        });
+        const relationTwo = await payload.create({
+          collection: 'relation',
+          data: {
+            name: 'name',
+          },
+        });
+
+        const description = 'desc';
+        const relationPost = await payload.create({
+          collection: slug,
+          data: {
+            description,
+            relationMultiRelationTo: {
+              value: relationTwo.id,
+              relationTo: 'relation',
+            },
+          },
+        });
+
+        const relationToDummyPost = await payload.create({
+          collection: slug,
+          data: {
+            description,
+            relationMultiRelationTo: {
+              value: relationOne.id,
+              relationTo: 'dummy',
+            },
+          },
+        });
+
+        const updatedDescription = 'updated';
+
+        const { status: relationMultiRelationToStatus, docs: updated } = await client.updateMany<Post>({
+          where: {
+            'relationMultiRelationTo.title': {
+              equals: relationOne.title,
+            },
+          },
+          data: { description: updatedDescription },
+        });
+
+        const updatedDoc = await payload.findByID({
+          collection: slug,
+          id: relationToDummyPost.id,
+        });
+
+        const otherDoc = await payload.findByID({
+          collection: slug,
+          id: relationPost.id,
+        });
+
+        expect(relationMultiRelationToStatus).toEqual(200);
+        expect(updated).toHaveLength(1);
+        expect(updated[0].id).toEqual(relationToDummyPost.id);
+        expect(updatedDoc.description).toEqual(updatedDescription);
+        expect(otherDoc.description).toEqual(description);
+      });
+
+      it('should bulk update with a relationship field that exists in one collection and is restricted in another', async () => {
+        const name = 'name';
+        const relationOne = await payload.create({
+          collection: 'dummy',
+          data: {
+            title: 'title',
+            name, // read access: () => false
+          },
+        });
+        const relationTwo = await payload.create({
+          collection: 'relation',
+          data: {
+            name,
+          },
+        });
+
+        const description = 'desc';
+        const relationPost = await payload.create({
+          collection: slug,
+          data: {
+            description,
+            relationMultiRelationTo: {
+              value: relationTwo.id,
+              relationTo: 'relation',
+            },
+          },
+        });
+
+        const relationToDummyPost = await payload.create({
+          collection: slug,
+          data: {
+            description,
+            relationMultiRelationTo: {
+              value: relationOne.id,
+              relationTo: 'dummy',
+            },
+          },
+        });
+
+        const updatedDescription = 'updated';
+
+        const { status } = await client.updateMany<Post>({
+          where: { 'relationMultiRelationTo.name': { equals: name } },
+          data: { description: updatedDescription },
+        });
+
+
+        const updatedDoc = await payload.findByID({
+          collection: slug,
+          id: relationPost.id,
+        });
+
+        const otherDoc = await payload.findByID({
+          collection: slug,
+          id: relationToDummyPost.id,
+        });
+
+        expect(status).toEqual(200);
+        expect(updatedDoc.description).toEqual(updatedDescription);
+        expect(otherDoc.description).toEqual(description);
+      });
+
      it('should return formatted errors for bulk updates', async () => {
        const text = 'bulk-update-test-errors';
        const errorDoc = await payload.create({
@@ -100,7 +312,7 @@ describe('collections-rest', () => {

        const result = await client.updateMany<ErrorOnHook>({
          slug: errorOnHookSlug,
-        query: { text: { equals: text } },
+          where: { text: { equals: text } },
          data: { text: update },
        });

@@ -120,7 +332,7 @@ describe('collections-rest', () => {
        });

        const { status, docs } = await client.deleteMany<Post>({
-        query: { title: { eq: 'title' } },
+          where: { title: { eq: 'title' } },
        });

        expect(status).toEqual(200);
@@ -146,7 +358,7 @@ describe('collections-rest', () => {

        const result = await client.deleteMany({
          slug: errorOnHookSlug,
-        query: { text: { equals: 'test' } },
+          where: { text: { equals: 'test' } },
        });

        expect(result.status).toEqual(400);
@@ -155,6 +367,7 @@ describe('collections-rest', () => {
        expect(result.errors[0].message).toBeDefined();
        expect(result.errors[0].id).toBeDefined();
      });
+    });

    describe('Custom ID', () => {
      describe('string', () => {
@@ -342,7 +555,24 @@ describe('collections-rest', () => {
          expect(result.totalDocs).toEqual(1);
        });

-        it.todo('nested by property value');
+        it('nested by property value', async () => {
+          const post1 = await createPost({
+            relationMultiRelationTo: { relationTo: relationSlug, value: relation.id },
+          });
+          await createPost();
+
+          const { status, result } = await client.find<Post>({
+            query: {
+              'relationMultiRelationTo.value.name': {
+                equals: relation.name,
+              },
+            },
+          });
+
+          expect(status).toEqual(200);
+          expect(result.docs).toEqual([post1]);
+          expect(result.totalDocs).toEqual(1);
+        });
      });

      describe('relationTo multi hasMany', () => {
--- a/test/helpers/rest.ts
+++ b/test/helpers/rest.ts
@@ -57,7 +57,7 @@ type UpdateManyArgs<T = any> = {
  slug?: string;
  data: Partial<T>;
  auth?: boolean;
-  query: any;
+  where: any;
 };

 type DeleteArgs = {
@@ -69,7 +69,7 @@ type DeleteArgs = {
 type DeleteManyArgs = {
  slug?: string;
  auth?: boolean;
-  query: any;
+  where: any;
 };

 type FindGlobalArgs<T = any> = {
@@ -205,9 +205,9 @@ export class RESTClient {
  }

  async updateMany<T = any>(args: UpdateManyArgs<T>): Promise<DocsResponse<T>> {
-    const { slug, data, query } = args;
+    const { slug, data, where } = args;
    const formattedQs = qs.stringify({
-      ...(query ? { where: query } : {}),
+      ...(where ? { where } : {}),
    }, {
      addQueryPrefix: true,
    });
@@ -225,9 +225,9 @@ export class RESTClient {
  }

  async deleteMany<T = any>(args: DeleteManyArgs): Promise<DocsResponse<T>> {
-    const { slug, query } = args;
+    const { slug, where } = args;
    const formattedQs = qs.stringify({
-      ...(query ? { where: query } : {}),
+      ...(where ? { where } : {}),
    }, {
      addQueryPrefix: true,
    });