fix(model): skip commit callbacks when callbacks = false

changelog.d/callbacks-false-commit-callbacks.fixed.md

@@ -0,0 +1 @@
+- `save(callbacks=false)` and `delete(callbacks=false)` now skip `afterCommit` and `afterRollback` too, like every other callback. They used to queue them. With callbacks on, a save that changes nothing still queues `afterCommit`

vendor/wheels/model/callbacks.cfm

@@ -425,13 +425,15 @@
 	 * fire; or, with no real transaction (none/false mode), fires afterCommit now
 	 * because the write is already committed (decision B1). Inside a foreign raw
 	 * transaction{} (detectable engines only) it skips both callbacks and warns once.
+	 * With `callbacks = false` (a save or delete that skips its callbacks) only the saved-change
+	 * restore is queued.
 	 */
-	public void function $enqueueTransactionCallbacks(required string operation, struct savedBefore) {
+	public void function $enqueueTransactionCallbacks(required string operation, struct savedBefore, boolean callbacks = true) {
 		// A delete changes no saved-change state, so a rollback puts back the current one.
 		if (!StructKeyExists(arguments, "savedBefore")) {
 			arguments.savedBefore = $savedChangesState();
 		}
-		if (!$hasTransactionCallbacks()) {
+		if (!arguments.callbacks || !$hasTransactionCallbacks()) {
 			// No callbacks to fire, but a rollback must still put back what savedChanges() reported
 			// before this save (F49), so the save is queued for that alone.
 			$enqueueSavedChangesRestore(operation = arguments.operation, savedBefore = arguments.savedBefore);

vendor/wheels/model/create.cfm

@@ -181,7 +181,7 @@
 						local.rv = true;
 						// v4.2.0: queue afterCommit/afterRollback (fires at the outermost
 						// transaction resolve, or immediately in none/false mode).
-						$enqueueTransactionCallbacks(operation = "create", savedBefore = local.savedBefore);
+						$enqueueTransactionCallbacks(operation = "create", savedBefore = local.savedBefore, callbacks = arguments.callbacks);
 					} else {
 						$restoreSavedChanges(local.savedBefore);
 						if (local.rollback) {
@@ -214,7 +214,7 @@
 						}
 						local.rv = true;
 						// v4.2.0: queue afterCommit/afterRollback (see create branch).
-						$enqueueTransactionCallbacks(operation = "update", savedBefore = local.savedBefore);
+						$enqueueTransactionCallbacks(operation = "update", savedBefore = local.savedBefore, callbacks = arguments.callbacks);
 					} else {
 						$restoreSavedChanges(local.savedBefore);
 					}

vendor/wheels/model/delete.cfm

@@ -226,7 +226,7 @@
 				local.rv = true;
 				// v4.2.0: queue afterCommit/afterRollback (fires at the outermost
 				// transaction resolve, or immediately in none/false mode).
-				$enqueueTransactionCallbacks(operation = "delete");
+				$enqueueTransactionCallbacks(operation = "delete", callbacks = arguments.callbacks);
 			}
 		}
 		return local.rv;

vendor/wheels/tests/specs/model/callbacksFalseAfterCommitSpec.cfc

@@ -0,0 +1,83 @@
+/**
+ * `callbacks = false` on save() / delete() also skips afterCommit and afterRollback, like every other
+ * callback. A save that changes nothing still queues afterCommit (unchanged, documented). The test
+ * runner uses transactionMode = "none", so specs pass `transaction` explicitly.
+ */
+component extends="wheels.WheelsTest" {
+
+	function beforeAll() {
+		variables.g = application.wo;
+	}
+
+	function run() {
+
+		describe("callbacks = false and the commit callbacks", () => {
+
+			beforeEach(() => {
+				request.$acLog = [];
+				variables.g.model("tag").$registerCallback(type = "afterCommit", methods = "recordAfterCommit");
+				variables.g.model("tag").$registerCallback(type = "afterRollback", methods = "recordAfterRollback");
+			});
+
+			afterEach(() => {
+				variables.g.model("tag").$clearCallbacks(type = "afterCommit");
+				variables.g.model("tag").$clearCallbacks(type = "afterRollback");
+				variables.g.model("tag").deleteAll(where = "name LIKE 'cbfalse-%'", instantiate = false, callbacks = false, transaction = "commit");
+			});
+
+			it("skips afterCommit on a create with callbacks = false", () => {
+				variables.g.model("tag").new(name = "cbfalse-create").save(callbacks = false, transaction = "commit");
+				expect(ArrayLen(request.$acLog)).toBe(0);
+			});
+
+			it("skips afterCommit on an update with callbacks = false", () => {
+				var t = variables.g.model("tag").new(name = "cbfalse-update");
+				t.save(callbacks = false, transaction = "commit");
+				t.name = "cbfalse-update-2";
+				t.save(callbacks = false, transaction = "commit");
+				expect(ArrayLen(request.$acLog)).toBe(0);
+			});
+
+			it("skips afterCommit on a delete with callbacks = false", () => {
+				var t = variables.g.model("tag").new(name = "cbfalse-delete");
+				t.save(callbacks = false, transaction = "commit");
+				t.delete(callbacks = false, transaction = "commit");
+				expect(ArrayLen(request.$acLog)).toBe(0);
+			});
+
+			it("skips afterRollback on a rolled-back save with callbacks = false", () => {
+				variables.g.model("tag").new(name = "cbfalse-rollback").save(callbacks = false, transaction = "rollback");
+				expect(ArrayLen(request.$acLog)).toBe(0);
+			});
+
+			it("still puts back savedChanges() when a callbacks = false save rolls back", () => {
+				var t = variables.g.model("tag").new(name = "cbfalse-saved");
+				t.save(callbacks = false, transaction = "commit");
+				var before = Duplicate(t.savedChanges());
+				t.name = "cbfalse-saved-undone";
+				t.save(callbacks = false, transaction = "rollback");
+				expect(t.savedChanges()).toBe(before);
+				expect(ArrayLen(request.$acLog)).toBe(0);
+			});
+
+			it("still runs them with callbacks = true", () => {
+				var t = variables.g.model("tag").new(name = "cbfalse-on");
+				t.save(transaction = "commit");
+				t.delete(transaction = "commit");
+				variables.g.model("tag").new(name = "cbfalse-on-rb").save(transaction = "rollback");
+				expect(ArrayToList(request.$acLog)).toBe("commit:cbfalse-on,commit:cbfalse-on,rollback:cbfalse-on-rb");
+			});
+
+			it("still queues afterCommit for a save that changes nothing", () => {
+				var t = variables.g.model("tag").new(name = "cbfalse-nochange");
+				t.save(transaction = "commit");
+				request.$acLog = [];
+				t.save(transaction = "commit");
+				expect(ArrayLen(request.$acLog)).toBe(1);
+			});
+
+		});
+
+	}
+
+}