Cleanup and add tests

Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>

packages/strapi-connector-bookshelf/lib/buildQuery.js

@@ -1,6 +1,8 @@
 const _ = require('lodash');
 const { singular } = require('pluralize');
 
+const BOOLEAN_OPERATORS = ['or'];
+
 /**
  * Build filters on a bookshelf query
  * @param {Object} options - Options
@@ -136,6 +138,7 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => {
     };
   };
 
+  // tree made to create the joins strucutre
   const tree = {
     alias: model.collectionName,
     assoc: null,
@@ -143,6 +146,12 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => {
     joins: {},
   };
 
+  /**
+   * Returns the SQL path for a qery field.
+   * Adds table to the joins tree
+   * @param {string} field a field used to filter
+   * @param {Object} tree joins tree
+   */
   const generateNestedJoins = (field, tree) => {
     let [key, ...parts] = field.split('.');
 
@@ -168,11 +177,18 @@ const buildJoinsAndFilter = (qb, model, whereClauses) => {
     return generateNestedJoins(parts.join('.'), tree.joins[key]);
   };
 
+  /**
+   * Format every where clauses whith the right table name aliases.
+   * Add table joins to the joins list
+   * @param {Array<{field, operator, value}>} whereClauses a list of where clauses
+   * @param {Object} context
+   * @param {Object} context.model model on which the query is run
+   */
   const buildWhereClauses = (whereClauses, { model }) => {
     return whereClauses.map(whereClause => {
       const { field, operator, value } = whereClause;
 
-      if (operator === 'or') {
+      if (BOOLEAN_OPERATORS.includes(operator)) {
         return { field, operator, value: value.map(v => buildWhereClauses(v, { model })) };
       }
 
@@ -216,13 +232,13 @@ const buildWhereClause = ({ qb, field, operator, value }) => {
     case 'or':
       return qb.where(orQb => {
         value.forEach(orClause => {
-          orQb.orWhere(q => {
+          orQb.orWhere(subQb => {
             if (Array.isArray(orClause)) {
               orClause.forEach(orClause =>
-                q.where(qq => buildWhereClause({ qb: qq, ...orClause }))
+                subQb.where(andQb => buildWhereClause({ qb: andQb, ...orClause }))
               );
             } else {
-              buildWhereClause({ qb: q, ...orClause });
+              buildWhereClause({ qb: subQb, ...orClause });
             }
           });
         });

packages/strapi/__tests__/filtering.test.e2e.js

@@ -171,9 +171,7 @@ describe('Filtering API', () => {
         });
 
         expect(res.body).toEqual(
-          expect.arrayContaining(
+          expect.arrayContaining(data.products.map(o => expect.objectContaining(o)))
-            data.products.map(o => expect.objectContaining(o))
-          )
         );
       });
 
@@ -186,9 +184,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res.body).toEqual(
+        expect(res.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
       });
     });
 
@@ -235,9 +231,7 @@ describe('Filtering API', () => {
         });
 
         expect(res1.body).toEqual(
-          expect.arrayContaining(
+          expect.arrayContaining(data.products.map(o => expect.objectContaining(o)))
-            data.products.map(o => expect.objectContaining(o))
-          )
         );
 
         const res2 = await rq({
@@ -285,9 +279,7 @@ describe('Filtering API', () => {
         });
 
         expect(res.body).toEqual(
-          expect.arrayContaining(
+          expect.arrayContaining(data.products.map(o => expect.objectContaining(o)))
-            data.products.map(o => expect.objectContaining(o))
-          )
         );
 
         const res2 = await rq({
@@ -426,9 +418,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res.body).toEqual(
+        expect(res.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
       });
 
       test('Should return an array without the values matching when an array of values is provided', async () => {
@@ -440,9 +430,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res.body).toEqual(
+        expect(res.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
       });
 
       test('Should return an array with values that do not match the filter', async () => {
@@ -468,9 +456,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res.body).toEqual(
+        expect(res.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
 
         const res2 = await rq({
           method: 'GET',
@@ -552,9 +538,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res2.body).toEqual(
+        expect(res2.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
       });
 
       test('Should work with integers', async () => {
@@ -616,9 +600,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res.body).toEqual(
+        expect(res.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
 
         const res2 = await rq({
           method: 'GET',
@@ -700,9 +682,7 @@ describe('Filtering API', () => {
           },
         });
 
-        expect(res2.body).toEqual(
+        expect(res2.body).toEqual(expect.not.arrayContaining([data.products[0]]));
-          expect.not.arrayContaining([data.products[0]])
-        );
       });
 
       test('Should work with integers', async () => {
@@ -756,6 +736,108 @@ describe('Filtering API', () => {
   });
 
   describe('Or filtering', () => {
+    describe('_or filter', () => {
+      test('Supports simple or', async () => {
+        const res = await rq({
+          method: 'GET',
+          url: '/products',
+          qs: {
+            _where: {
+              _or: [
+                {
+                  rank: 42,
+                },
+                {
+                  rank: 82,
+                },
+              ],
+            },
+          },
+        });
+
+        expect(res.body).toEqual(expect.arrayContaining([data.products[0], data.products[1]]));
+      });
+
+      test('Supports simple or on different fields', async () => {
+        const res = await rq({
+          method: 'GET',
+          url: '/products',
+          qs: {
+            _where: {
+              _or: [
+                {
+                  rank: 42,
+                },
+                {
+                  price_gt: 28,
+                },
+              ],
+            },
+          },
+        });
+
+        expect(res.body).toEqual(
+          expect.arrayContaining([data.products[0], data.products[1], data.products[2]])
+        );
+      });
+
+      test('Supports or with nested and', async () => {
+        const res = await rq({
+          method: 'GET',
+          url: '/products',
+          qs: {
+            _where: {
+              _or: [
+                {
+                  rank: 42,
+                },
+                [
+                  {
+                    price_gt: 28,
+                  },
+                  {
+                    rank: 91,
+                  },
+                ],
+              ],
+            },
+          },
+        });
+
+        expect(res.body).toEqual(expect.arrayContaining([data.products[0], data.products[2]]));
+      });
+
+      test('Supports or with nested or', async () => {
+        const res = await rq({
+          method: 'GET',
+          url: '/products',
+          qs: {
+            _where: {
+              _or: [
+                {
+                  rank: 42,
+                },
+                [
+                  {
+                    price_gt: 28,
+                  },
+                  {
+                    _or: [
+                      {
+                        rank: 91,
+                      },
+                    ],
+                  },
+                ],
+              ],
+            },
+          },
+        });
+
+        expect(res.body).toEqual(expect.arrayContaining([data.products[0], data.products[2]]));
+      });
+    });
+
     test('Filter equals', async () => {
       const res = await rq({
         method: 'GET',
@@ -934,9 +1016,7 @@ describe('Filtering API', () => {
         },
       });
 
-      expect(res.body).toEqual(
+      expect(res.body).toEqual(expect.not.arrayContaining([data.products[1], data.products[2]]));
-        expect.not.arrayContaining([data.products[1], data.products[2]])
-      );
       expect(res.body).toEqual(expect.arrayContaining([data.products[0]]));
 
       res = await rq({
@@ -976,9 +1056,7 @@ describe('Filtering API', () => {
       });
 
       expect(res.body).toEqual(
-        expect.arrayContaining(
+        expect.arrayContaining(data.products.slice(0).sort((a, b) => a.rank - b.rank))
-          data.products.slice(0).sort((a, b) => a.rank - b.rank)
-        )
       );
     });
 
@@ -992,9 +1070,7 @@ describe('Filtering API', () => {
       });
 
       expect(res.body).toEqual(
-        expect.arrayContaining(
+        expect.arrayContaining(data.products.slice(0).sort((a, b) => a.rank - b.rank))
-          data.products.slice(0).sort((a, b) => a.rank - b.rank)
-        )
       );
 
       const res2 = await rq({
@@ -1006,9 +1082,7 @@ describe('Filtering API', () => {
       });
 
       expect(res2.body).toEqual(
-        expect.arrayContaining(
+        expect.arrayContaining(data.products.slice(0).sort((a, b) => b.rank - a.rank))
-          data.products.slice(0).sort((a, b) => b.rank - a.rank)
-        )
       );
     });
 
@@ -1021,14 +1095,11 @@ describe('Filtering API', () => {
         },
       });
 
-      [
+      [data.products[3], data.products[0], data.products[2], data.products[1]].forEach(
-        data.products[3],
+        expectedPost => {
-        data.products[0],
-        data.products[2],
-        data.products[1],
-      ].forEach(expectedPost => {
           expect(res.body).toEqual(expect.arrayContaining([expectedPost]));
-      });
+        }
+      );
     });
   });
 
@@ -1055,9 +1126,7 @@ describe('Filtering API', () => {
         },
       });
 
-      expect(res.body).toEqual(
+      expect(res.body).toEqual(expect.arrayContaining([data.products[data.products.length - 1]]));
-        expect.arrayContaining([data.products[data.products.length - 1]])
-      );
     });
 
     test('Offset', async () => {
@@ -1082,9 +1151,7 @@ describe('Filtering API', () => {
         },
       });
 
-      expect(res.body).toEqual(
+      expect(res.body).toEqual(expect.arrayContaining(data.products.slice(1, 2)));
-        expect.arrayContaining(data.products.slice(1, 2))
-      );
     });
   });