From 26388b8527239f34b6c2591e0a8daaf0d754b5d8 Mon Sep 17 00:00:00 2001
From: Randall Morey <randall@randallmorey.com>
Date: Thu, 23 Jul 2020 19:31:34 -0400
Subject: [PATCH] refactor: deduce scope path prefix in adapter rather than
 passing it via the model save method

---
 addons/api/addon/adapters/application.js      | 32 +++++++++++++------
 addons/api/addon/models/base.js               | 28 ----------------
 .../tests/unit/adapters/application-test.js   | 10 +++---
 3 files changed, 29 insertions(+), 41 deletions(-)

diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js
index bfc109b3f..09b772cb7 100644
--- a/addons/api/addon/adapters/application.js
+++ b/addons/api/addon/adapters/application.js
@@ -59,8 +59,13 @@ export default class ApplicationAdapter extends RESTAdapter.extend(
   // =methods
 
   /**
-   * Prepends a scope to the URL prefix, if a scope is passed via the
-   * snapshot's `adapterOptions.scopeID` field.
+   * Prepends a scope to the URL prefix.  Precedence is given to the ID passed
+   * via `adapterOptions.scopeID`.  If this value isn't found in
+   * `adapterOptions`, we look to the `scope.scope_id` field on the snapshot,
+   * which most resources have.
+   *
+   * Note:  scopes themselves never receive a scope URL prefix.
+   *
    * @override
    * @param {string} path
    * @param {string} parentURL
@@ -69,18 +74,27 @@ export default class ApplicationAdapter extends RESTAdapter.extend(
    * @param {object} snapshot
    * @return {string}
    */
-  urlPrefix(path, parentURL, modelName, id, snapshot={}) {
+  urlPrefix(path, parentURL, modelName, id, snapshot) {
     const prefix = super.urlPrefix(...arguments);
     const isScope = modelName === 'scope';
     let scopePath = '';
+    let scopeID = '';
+    if (snapshot) {
+      // Not all snapshots have `attr` (such as array snapshots),
+      // so we do this sort of ugly check.
+      if (snapshot.attr) {
+        const parentScope = snapshot.attr('scope');
+        if (parentScope) scopeID = parentScope.attr('scope_id');
+      }
+      // Attempt to get scopeID from adapterOptions and fallback on its current
+      // value if it wasn't passed through options.
+      scopeID = getWithDefault(snapshot, 'adapterOptions.scopeID', scopeID);
+    }
     // Only non-scope resources need a scope path, since scope resources
     // aren't technically "scoped" the same way.
-    if (!isScope) {
-      scopePath = getWithDefault(snapshot, 'adapterOptions.scopeID', '');
-      // Ensure a slash is added between prefix + scope path if needed.
-      if (scopePath && prefix.charAt(prefix.length - 1) !== '/') {
-        scopePath = `/scopes/${scopePath}`;
-      }
+    // Ensure a slash is added between prefix + scope path if needed.
+    if (!isScope && scopeID && prefix.charAt(prefix.length - 1) !== '/') {
+      scopePath = `/scopes/${scopeID}`;
     }
     return `${prefix}${scopePath}`;
   }
diff --git a/addons/api/addon/models/base.js b/addons/api/addon/models/base.js
index 635d3b831..cfbcce14b 100644
--- a/addons/api/addon/models/base.js
+++ b/addons/api/addon/models/base.js
@@ -84,32 +84,4 @@ export default class BaseModel extends Model {
     return !this.canSave;
   }
 
-  // =methods
-
-  /**
-   * If this model instance has a related scope, the scope's ID will be passed
-   * via `adapterOptions.scopeID` automatically.  End users may optionally
-   * pass a custom scope ID to override.
-   *
-   * @example
-   *   model.save(); // passes the model's current scope ID
-   *   model.save({adapterOptions: {scopeID: 'global'}}); // override scope ID
-   *
-   * End users may specify a _custom method_ via `adapterOptions.method`.
-   * Custom methods are appended to the record's URL by the adapter.
-   *
-   * @example
-   *   // Results in a URL: `/v1/model-name/id:my-custom-method`
-   *   model.save({adapterOptions: {method: 'my-custom-method'}});
-   *
-   * @return {Promise}
-   */
-  save(options={adapterOptions: {}}) {
-    const scopeID = this.scopeID;
-    if (scopeID && !options.adapterOptions.scopeID) {
-      options.adapterOptions.scopeID = scopeID;
-    }
-    return super.save(options);
-  }
-
 }
diff --git a/addons/api/tests/unit/adapters/application-test.js b/addons/api/tests/unit/adapters/application-test.js
index bea9b215c..c3d72d469 100644
--- a/addons/api/tests/unit/adapters/application-test.js
+++ b/addons/api/tests/unit/adapters/application-test.js
@@ -29,7 +29,7 @@ module('Unit | Adapter | application', function (hooks) {
     const scopeID = 'global';
     const adapter = this.owner.lookup('adapter:application');
     const prefix = adapter.urlPrefix(null, null, null, null, {
-      adapterOptions: {scopeID}
+      adapterOptions: { scopeID }
     });
     assert.ok(config.api.namespace);
     assert.equal(prefix, `v1/scopes/${scopeID}`);
@@ -40,7 +40,7 @@ module('Unit | Adapter | application', function (hooks) {
     const scopeID = 'global';
     const adapter = this.owner.lookup('adapter:application');
     const prefix = adapter.urlPrefix(null, null, 'scope', null, {
-      adapterOptions: {scopeID}
+      adapterOptions: { scopeID }
     });
     assert.equal(prefix, `v1`);
   });
@@ -56,7 +56,7 @@ module('Unit | Adapter | application', function (hooks) {
     const method = 'set-something';
     const adapter = this.owner.lookup('adapter:application');
     const suffix = adapter.urlSuffix(null, null, {
-      adapterOptions: {method}
+      adapterOptions: { method }
     });
     assert.ok(config.api.namespace);
     assert.equal(suffix, ':set-something');
@@ -95,7 +95,9 @@ module('Unit | Adapter | application', function (hooks) {
       assert.ok(true, 'Scoped resource URL was requested.');
       return {items: []};
     });
-    await store.findAll('group', {adapterOptions: {scopeID: 'p_456'}});
+    await store.findAll('group', {
+      adapterOptions: { scopeID: 'p_456' }
+    });
   });
 
   test('it rewrites PUT to PATCH, but leaves others unchanged', function (assert) {