fix: two bugs which affect batchMode

src/hooks/trigger.ts

@@ -247,7 +247,7 @@ const triggerBefore = async <H extends HookContext, T = Record<string, any>>(
 
     const before = await changesByIdBefore(context, {
       skipHooks: false,
-      params: () => (sub.params ? sub.paramsResolved : null),
+      params: () => (sub.paramsResolved ? sub.paramsResolved : null),
       deleteParams: ["trigger"],
       fetchBefore: sub.fetchBefore || sub.conditionsBefore !== true,
     });
@@ -391,7 +391,7 @@ const triggerAfter = async <H extends HookContext>(context: H): Promise<H> => {
       }
     }
 
-    if (isSubscriptionInBatchMode(sub)) {
+    if (isSubscriptionInBatchMode(sub) && batchActionArguments.length) {
       const promise = sub.action(batchActionArguments, context);
 
       if (sub.isBlocking) {

test/hooks/base-mock.ts

@@ -0,0 +1,46 @@
+import type { HookTriggerOptions } from "../../src";
+import { trigger } from "../../src";
+import { MemoryService } from "@feathersjs/memory";
+import type { HookContext } from "@feathersjs/feathers";
+import { feathers } from "@feathersjs/feathers";
+import type { MethodName } from "../../src/types.internal";
+
+export function mock(
+  hookNames: MethodName | MethodName[],
+  options: HookTriggerOptions,
+  beforeHook?: (context: HookContext) => Promise<HookContext>,
+  afterHook?: (context: HookContext) => Promise<HookContext>,
+) {
+  hookNames = Array.isArray(hookNames) ? hookNames : [hookNames];
+  const app = feathers();
+  app.use("/tests", new MemoryService({ multi: true }));
+  const service = app.service("tests");
+  const hook = trigger(options);
+
+  const beforeAll = [hook];
+  if (beforeHook) {
+    beforeAll.push(beforeHook);
+  }
+
+  const afterAll = [hook];
+  if (afterHook) {
+    afterAll.push(afterHook);
+  }
+
+  const hooks = {
+    before: {},
+    after: {},
+  };
+
+  hookNames.forEach((hookName) => {
+    hooks.before[hookName] = beforeAll;
+    hooks.after[hookName] = afterAll;
+  });
+
+  service.hooks(hooks);
+
+  return {
+    app,
+    service,
+  };
+}

test/hooks/trigger-batchmode.test.ts

@@ -0,0 +1,156 @@
+import { mock } from "./base-mock";
+
+describe("trigger batch mode", () => {
+  it("create: triggers on multi create without condition in batch mode", async function () {
+    let cbCount = 0;
+    let changeCount = 0;
+    const { service } = mock("create", {
+      method: "create",
+      service: "tests",
+      batchMode: true,
+      action: (changes) => {
+        cbCount++;
+        changeCount = changes.length;
+      },
+    });
+
+    await service.create([
+      { id: 0, test: true },
+      { id: 1, test: true },
+      { id: 2, test: true },
+    ]);
+    assert.strictEqual(cbCount, 1, "action cb was called only a single time");
+    assert.strictEqual(
+      changeCount,
+      3,
+      "action cb was called with three change tuples",
+    );
+  });
+
+  it("patch: triggers on multi create with conditions in batch mode", async function () {
+    let cbCount = 0;
+    let changeCount = 0;
+    const { service } = mock(["create", "patch"], {
+      service: "tests",
+      batchMode: true,
+      conditionsBefore: {
+        test: true,
+      },
+      conditionsData: {
+        test: false,
+      },
+      conditionsResult: {
+        test: false,
+      },
+      action: (changes) => {
+        cbCount++;
+        changeCount = changes.length;
+      },
+    });
+
+    await service.create([
+      { id: 0, test: true },
+      { id: 1, test: true },
+      { id: 2, test: true },
+    ]);
+    assert.strictEqual(cbCount, 0, "action cb was not called");
+
+    await service.patch(null, {
+      test: false,
+    });
+
+    assert.strictEqual(cbCount, 1, "action cb was called only a single time");
+    assert.strictEqual(
+      changeCount,
+      3,
+      "action cb was called with three change tuples",
+    );
+
+    await service.patch(null, {
+      test: false,
+    });
+
+    assert.strictEqual(cbCount, 1, "action cb was called only a single time");
+    assert.strictEqual(
+      changeCount,
+      3,
+      "action cb was called with three change tuples",
+    );
+  });
+
+  it("patch: triggers on multi create with complex conditions in batch mode", async function () {
+    let cbCount = 0;
+    let changeCount = 0;
+    const { service } = mock(["create", "patch"], {
+      service: "tests",
+      batchMode: true,
+      conditionsBefore: {
+        submittedAt: {
+          $ne: null,
+        },
+        approvedAt: null,
+      },
+      conditionsData: {
+        approvedAt: {
+          $ne: null,
+        },
+      },
+      conditionsResult: {
+        approvedAt: {
+          $ne: null,
+        },
+        declinedAt: null,
+      },
+      action: (changes) => {
+        cbCount++;
+        changeCount = changes.length;
+      },
+    });
+
+    await service.create([
+      { id: 0, submittedAt: null, approvedAt: null, declinedAt: null },
+      { id: 1, submittedAt: null, approvedAt: null, declinedAt: null },
+      { id: 2, submittedAt: null, approvedAt: null, declinedAt: null },
+      { id: 12, submittedAt: null, approvedAt: null, declinedAt: null },
+      { id: 13, submittedAt: null, approvedAt: null, declinedAt: null },
+      { id: 14, submittedAt: null, approvedAt: null, declinedAt: null },
+    ]);
+
+    await service.patch(
+      null,
+      {
+        submittedAt: new Date(),
+      },
+      {
+        query: {
+          id: {
+            $in: [12, 13, 14],
+          },
+        },
+      },
+    );
+
+    assert.strictEqual(cbCount, 0, "action cb was not called");
+
+    await service.patch(
+      null,
+      {
+        approvedAt: new Date(),
+      },
+      {
+        query: {
+          id: {
+            $in: [12, 13, 14],
+          },
+        },
+      },
+    );
+
+    assert.strictEqual(cbCount, 1, "action cb was called only a single time");
+    assert.strictEqual(
+      changeCount,
+      3,
+      "action cb was called with three change tuples",
+    );
+  });
+});

test/hooks/trigger.test.ts

@@ -1,53 +1,10 @@
 import assert from "assert";
-import type { HookTriggerOptions, Subscription, Action } from "../../src";
-import { trigger } from "../../src";
-import { MemoryService } from "@feathersjs/memory";
-import type { HookContext } from "@feathersjs/feathers";
-import { feathers } from "@feathersjs/feathers";
+import type { Subscription, Action } from "../../src";
 import type { MethodName } from "../../src/types.internal";
+import { mock } from "./base-mock";
 
 import { addDays, isBefore } from "date-fns";
 
-function mock(
-  hookNames: MethodName | MethodName[],
-  options: HookTriggerOptions,
-  beforeHook?: (context: HookContext) => Promise<HookContext>,
-  afterHook?: (context: HookContext) => Promise<HookContext>,
-) {
-  hookNames = Array.isArray(hookNames) ? hookNames : [hookNames];
-  const app = feathers();
-  app.use("/tests", new MemoryService({ multi: true }));
-  const service = app.service("tests");
-  const hook = trigger(options);
-
-  const beforeAll = [hook];
-  if (beforeHook) {
-    beforeAll.push(beforeHook);
-  }
-
-  const afterAll = [hook];
-  if (afterHook) {
-    afterAll.push(afterHook);
-  }
-
-  const hooks = {
-    before: {},
-    after: {},
-  };
-
-  hookNames.forEach((hookName) => {
-    hooks.before[hookName] = beforeAll;
-    hooks.after[hookName] = afterAll;
-  });
-
-  service.hooks(hooks);
-
-  return {
-    app,
-    service,
-  };
-}
-
 describe("hook - trigger", function () {
   describe("general", function () {
     it("throws without options", function () {
@@ -296,32 +253,6 @@ describe("hook - trigger", function () {
       assert.strictEqual(cbCount, 3, "action cb was called three times");
     });
 
-    it("create: triggers on multi create without condition in batch mode", async function () {
-      let cbCount = 0;
-      let changeCount = 0;
-      const { service } = mock("create", {
-        method: "create",
-        service: "tests",
-        batchMode: true,
-        action: (changes) => {
-          cbCount++;
-          changeCount = changes.length;
-        },
-      });
-
-      await service.create([
-        { id: 0, test: true },
-        { id: 1, test: true },
-        { id: 2, test: true },
-      ]);
-      assert.strictEqual(cbCount, 1, "action cb was called only a single time");
-      assert.strictEqual(
-        changeCount,
-        3,
-        "action cb was called with three change tuples",
-      );
-    });
-
     it("create: does not trigger with service mismatch", async function () {
       let cbCount = 0;
       const { service } = mock("create", {
@@ -907,6 +838,44 @@ describe("hook - trigger", function () {
       );
     });
 
+    it("patch: triggers on multi create with all conditions", async function () {
+      let cbCount = 0;
+      const { service } = mock(["create", "patch"], {
+        service: "tests",
+        conditionsBefore: {
+          test: true,
+        },
+        conditionsData: {
+          test: false,
+        },
+        conditionsResult: {
+          test: false,
+        },
+        action: (change, option) => {
+          cbCount++;
+        },
+      });
+
+      await service.create([
+        { id: 0, test: true },
+        { id: 1, test: true },
+        { id: 2, test: true },
+      ]);
+      assert.strictEqual(cbCount, 0, "action cb was not called");
+
+      await service.patch(null, {
+        test: false,
+      });
+
+      assert.strictEqual(cbCount, 3, "action cb was called three times");
+
+      await service.patch(null, {
+        test: false,
+      });
+
+      assert.strictEqual(cbCount, 3, "action cb was still called three times");
+    });
+
     it("patch: triggers with custom view", async function () {
       let cbCount = 0;
       const { service } = mock("patch", {