Merge pull request #563 from adopted-ember-addons/dev/bugfixes

mfeckie · web-flow · commit 7035d8ba398f · 2026-05-05T20:48:13.000+08:00
Fix bugs and design issues found in audit
diff --git a/addon/src/-private.js b/addon/src/-private.js
@@ -2,6 +2,8 @@
 
 import FixtureBuilderFactory from './builder/fixture-builder-factory';
 import MockStoreRequest from './mocks/mock-store-request';
+import RequestWrapper from './mocks/request-wrapper';
+import Sequence from './sequence';
 
 import {
   isEmptyObject,
@@ -13,6 +15,8 @@ import {
 export {
   FixtureBuilderFactory,
   MockStoreRequest,
+  RequestWrapper,
+  Sequence,
   isEmptyObject,
   param,
   isEquivalent,
diff --git a/addon/src/factory-guy.js b/addon/src/factory-guy.js
@@ -450,6 +450,10 @@ class FactoryGuy {
   }
 
   reset() {
+    if (this._cacheOnlyOriginalAdapterFor && this.store) {
+      this.store.adapterFor = this._cacheOnlyOriginalAdapterFor;
+      this._cacheOnlyOriginalAdapterFor = null;
+    }
     this.store = null;
     this.requestManager?.stop();
     this.requestManager = null;
@@ -499,6 +503,7 @@ class FactoryGuy {
    */
   cacheOnlyMode({ except = [] } = {}) {
     let store = this.store;
+    this._cacheOnlyOriginalAdapterFor = store.adapterFor;
     let findAdapter = store.adapterFor.bind(store);
 
     store.adapterFor = function (name) {
@@ -592,7 +597,7 @@ class FactoryGuy {
 
     assert(
       `[ember-data-factory-guy] Can't find that factory named [ ${name} ]`,
-      !definition && assertItExists,
+      !assertItExists,
     );
   }
 
diff --git a/addon/src/mocks/exposed-request-functions.js b/addon/src/mocks/exposed-request-functions.js
@@ -23,8 +23,7 @@ export function mock({ type = 'GET', url, responseText, status } = {}) {
 export function mockLinks(model, relationshipKey) {
   assert(
     '[ember-data-factory-guy] mockLinks requires at least model and relationshipKey',
-    model,
-    relationshipKey,
+    model && relationshipKey,
   );
 
   return new MockLinksRequest(model, relationshipKey);
@@ -108,10 +107,7 @@ export function mockReload(...args) {
     let record = args[0];
     modelName = record.constructor.modelName;
     id = record.id;
-  } else if (
-    typeof args[0] === 'string' &&
-    typeof parseInt(args[1]) === 'number'
-  ) {
+  } else if (typeof args[0] === 'string' && args[1] != null) {
     modelName = args[0];
     id = args[1];
   }
diff --git a/addon/src/mocks/mock-request.js b/addon/src/mocks/mock-request.js
@@ -3,7 +3,7 @@ import { isEmptyObject, param } from '../utils/helper-functions';
 import FactoryGuy from '../factory-guy';
 import { isMatch, isEqual } from 'lodash';
 
-export default class {
+export default class MockRequest {
   constructor() {
     this.status = 200;
     this.responseHeaders = new Headers();
diff --git a/addon/src/mocks/mock-store-request.js b/addon/src/mocks/mock-store-request.js
@@ -4,7 +4,7 @@ import FactoryGuy from '../factory-guy';
 import MockRequest from './mock-request';
 import { isEmptyObject, isEquivalent } from '../utils/helper-functions';
 
-export default class extends MockRequest {
+export default class MockStoreRequest extends MockRequest {
   constructor(modelName, requestType) {
     super();
     this.modelName = modelName;
diff --git a/addon/src/mocks/request-manager-msw.js b/addon/src/mocks/request-manager-msw.js
@@ -44,12 +44,15 @@ export default class RequestManagerMSW extends RequestManager {
       wrapper = this.wrappers[key];
 
     if (!wrapper) {
-      wrapper = new RequestWrapper(); // this generates & returns the handler function
+      wrapper = new RequestWrapper();
       const mswHandler = http[type.toLowerCase()](
         url,
         async ({ request, params }) => {
           await delay(this._settings.delay);
-          return await wrapper({ request: request.clone(), params });
+          return await wrapper.handleRequest({
+            request: request.clone(),
+            params,
+          });
         },
       );
       this.worker.use(mswHandler);
diff --git a/addon/src/mocks/request-manager-pretender.js b/addon/src/mocks/request-manager-pretender.js
@@ -31,7 +31,7 @@ export default class RequestManagerPretender extends RequestManager {
    * For now, you can only set the response delay.
    */
   settings(settings = {}) {
-    super.setttings({
+    super.settings({
       ...settings,
       delay: settings.responseTime ?? settings.delay ?? 0,
     });
@@ -75,7 +75,7 @@ export default class RequestManagerPretender extends RequestManager {
             return cl;
           };
 
-          const response = await wrapper({
+          const response = await wrapper.handleRequest({
             request: request.clone(),
             params: fakeRequest.params,
           });
diff --git a/addon/src/mocks/request-wrapper.js b/addon/src/mocks/request-wrapper.js
@@ -1,51 +1,12 @@
 /**
- * This request wrapper controls what will be returned by one url / http verb
- * Normally when you set up pretender, you give it one function to handle one url / verb.
- *
- * So, for example, you would:
- *
- *  ```
- *    pretender.get('/users', myfunction )
- *  ```
- *
- *  to mock a [GET /users] call
- *
- *  This wrapper allows that GET /users call to be handled my many functions
- *  instead of just one, since this request handler hold the ability to take
- *  a list of hanlders.
- *
- *  That way you can setup a few mocks like
- *
- *  ```
- *    mockFindAll('user')
- *    mockQuery('user', {name: 'Dude'})
- *  ```
- *
- *  and both of these hanlders will reside in the list for the wrapper that
- *  belongs to [GET /users]
+ * This request wrapper controls what will be returned by one url / http verb.
+ * Holds a list of handlers for a given url/verb pair so multiple mocks (e.g.
+ * mockFindAll and mockQuery) can share the same route.
  */
 export default class RequestWrapper {
   constructor() {
     this.index = 0;
     this.handlers = [];
-    return this.generateRequestHandler();
-  }
-
-  /**
-   * Generating a function that we can hand off to pretender that
-   * will handle the request.
-   *
-   * Before passing back that function, add some other functions
-   * to control the handlers array
-   *
-   * @returns {function(this:T)}
-   */
-  generateRequestHandler() {
-    const requestHandler = this.handleRequest.bind(this);
-    requestHandler.getHandlers = this.getHandlers.bind(this);
-    requestHandler.addHandler = this.addHandler.bind(this);
-    requestHandler.removeHandler = this.removeHandler.bind(this);
-    return requestHandler;
   }
 
   /**
diff --git a/addon/src/scenario.js b/addon/src/scenario.js
@@ -18,7 +18,7 @@ import {
   mock,
 } from './mocks/exposed-request-functions';
 
-export default class {
+export default class Scenario {
   constructor() {
     this.make = make;
     this.makeNew = makeNew;
diff --git a/addon/src/sequence.js b/addon/src/sequence.js
@@ -1,9 +1,15 @@
-export default function (fn) {
-  let index = 1;
-  this.next = function () {
-    return fn.call(this, index++);
-  };
-  this.reset = function () {
-    index = 1;
-  };
+export default class Sequence {
+  constructor(fn) {
+    this._fn = fn;
+    this._index = 1;
+  }
+
+  next() {
+    const fn = this._fn;
+    return fn(this._index++);
+  }
+
+  reset() {
+    this._index = 1;
+  }
 }
diff --git a/test-app/tests/unit/factory-guy-test.js b/test-app/tests/unit/factory-guy-test.js
@@ -1057,4 +1057,49 @@ module('FactoryGuy', function (hooks) {
       assert.strictEqual(method, 'PUT');
     });
   });
+
+  module('FactoryGuy#cacheOnlyMode', function (subHooks) {
+    inlineSetup(subHooks, '-json-api');
+
+    test('restores store.adapterFor on reset', function (assert) {
+      const store = FactoryGuy.store;
+      const originalAdapterFor = store.adapterFor;
+
+      FactoryGuy.cacheOnlyMode();
+      assert.notStrictEqual(
+        store.adapterFor,
+        originalAdapterFor,
+        'adapterFor is patched',
+      );
+
+      FactoryGuy.reset();
+      assert.strictEqual(
+        store.adapterFor,
+        originalAdapterFor,
+        'adapterFor is restored after reset',
+      );
+    });
+  });
+
+  module('FactoryGuy.lookupDefinitionForFixtureName', function (subHooks) {
+    inlineSetup(subHooks, '-json-api');
+
+    test('returns undefined without throwing when name not found and assertItExists is false', function (assert) {
+      const FactoryGuyClass = FactoryGuy.constructor;
+      const result = FactoryGuyClass.lookupDefinitionForFixtureName(
+        'NoSuchFactory',
+        false,
+      );
+      assert.strictEqual(result, undefined);
+    });
+
+    test('throws when name not found and assertItExists is true', function (assert) {
+      const FactoryGuyClass = FactoryGuy.constructor;
+      assert.throws(
+        () =>
+          FactoryGuyClass.lookupDefinitionForFixtureName('NoSuchFactory', true),
+        /Can't find that factory named \[ NoSuchFactory \]/,
+      );
+    });
+  });
 });
diff --git a/test-app/tests/unit/mocks/mock-links-test.js b/test-app/tests/unit/mocks/mock-links-test.js
@@ -19,6 +19,12 @@ module('MockLinks', function (hooks) {
     }, 'requires at least model and a relationshipKey');
   });
 
+  test('with only model parameter throws the guard assertion message', function (assert) {
+    assert.throws(function () {
+      mockLinks(make('user'));
+    }, /mockLinks requires at least model and relationshipKey/);
+  });
+
   test('getUrl and status for belongsTo links', function (assert) {
     let companyLink = '/users/1/company',
       user = make('user', { links: { company: companyLink } }),
diff --git a/test-app/tests/unit/mocks/request-manager-pretender-test.js b/test-app/tests/unit/mocks/request-manager-pretender-test.js
@@ -0,0 +1,37 @@
+import { module, test } from 'qunit';
+import { setupTest } from 'ember-qunit';
+import { RequestManagerPretender } from 'ember-data-factory-guy';
+
+const fakePretender = {
+  get: () => {},
+  post: () => {},
+  put: () => {},
+  patch: () => {},
+  delete: () => {},
+  head: () => {},
+  shutdown: () => {},
+};
+
+module('RequestManagerPretender', function (hooks) {
+  setupTest(hooks);
+
+  module('#settings', function () {
+    test('does not throw when called', function (assert) {
+      const rm = new RequestManagerPretender(fakePretender);
+      rm.settings({ delay: 50 });
+      assert.ok(true, 'settings call completed without error');
+    });
+
+    test('passes delay through to internal settings', function (assert) {
+      const rm = new RequestManagerPretender(fakePretender);
+      rm.settings({ delay: 100 });
+      assert.strictEqual(rm._settings.delay, 100);
+    });
+
+    test('maps responseTime to delay', function (assert) {
+      const rm = new RequestManagerPretender(fakePretender);
+      rm.settings({ responseTime: 200 });
+      assert.strictEqual(rm._settings.delay, 200);
+    });
+  });
+});
diff --git a/test-app/tests/unit/sequence-test.js b/test-app/tests/unit/sequence-test.js
@@ -0,0 +1,44 @@
+import { module, test } from 'qunit';
+import { Sequence } from 'ember-data-factory-guy/-private';
+
+module('Sequence', function () {
+  test('is a class (has a name)', function (assert) {
+    const seq = new Sequence((n) => n);
+    assert.strictEqual(seq.constructor.name, 'Sequence');
+  });
+
+  test('calls fn with incrementing index starting at 1', function (assert) {
+    const received = [];
+    const seq = new Sequence((n) => {
+      received.push(n);
+      return n * 10;
+    });
+
+    assert.strictEqual(seq.next(), 10);
+    assert.strictEqual(seq.next(), 20);
+    assert.strictEqual(seq.next(), 30);
+    assert.deepEqual(received, [1, 2, 3]);
+  });
+
+  test('does not bind this to the Sequence instance inside fn', function (assert) {
+    let capturedThis;
+    const seq = new Sequence(function (n) {
+      capturedThis = this;
+      return n;
+    });
+    seq.next();
+    assert.notStrictEqual(
+      capturedThis,
+      seq,
+      'fn should not receive Sequence as this',
+    );
+  });
+
+  test('reset restarts index at 1', function (assert) {
+    const seq = new Sequence((n) => n);
+    seq.next();
+    seq.next();
+    seq.reset();
+    assert.strictEqual(seq.next(), 1);
+  });
+});
diff --git a/test-app/tests/unit/shared-adapter-behaviour.js b/test-app/tests/unit/shared-adapter-behaviour.js
