From 9005ce0628a0823d8ec2d966175ded405c85a2b7 Mon Sep 17 00:00:00 2001
From: DaddyWarbucks <techtbeau@gmail.com>
Date: Wed, 27 Mar 2024 17:39:17 -0500
Subject: [PATCH] Remove applyScope and update paramsToAdapter

---
 src/adapter.ts     | 46 ++++++++++++++++++++++++----------------------
 test/index.test.ts | 10 ++++++++--
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/src/adapter.ts b/src/adapter.ts
index 4b29d02..678c48e 100644
--- a/src/adapter.ts
+++ b/src/adapter.ts
@@ -111,10 +111,7 @@ export class SequelizeAdapter<
     return this.options.Model;
   }
 
-  /**
-   * @deprecated use 'service.ModelWithScope' instead. 'applyScope' will be removed in a future release.
-   */
-  applyScope (params?: ServiceParams) {
+  ModelWithScope (params: ServiceParams) {
     const Model = this.getModel(params);
     if (params?.sequelize?.scope) {
       return Model.scope(params.sequelize.scope);
@@ -122,10 +119,6 @@ export class SequelizeAdapter<
     return Model;
   }
 
-  ModelWithScope (params: ServiceParams) {
-    return this.applyScope(params);
-  }
-
   convertOperators (q: any): Query {
     if (Array.isArray(q)) {
       return q.map(subQuery => this.convertOperators(subQuery));
@@ -181,25 +174,36 @@ export class SequelizeAdapter<
     const params = _params || {} as ServiceParams;
     const { filters, query: where } = this.filterQuery(params);
 
-    const sequelize: FindOptions = {
+    // Until Sequelize fix all the findAndCount issues, a few 'hacks' are needed to get the total count correct
+
+    // Adding an empty include changes the way the count is done
+    // See: https://github.com/sequelize/sequelize/blob/7e441a6a5ca44749acd3567b59b1d6ceb06ae64b/lib/model.js#L1780-L1782
+    // sequelize.include = sequelize.include || [];
+
+    const defaults = {
       where,
-      order: getOrder(filters.$sort),
-      limit: filters.$limit,
-      offset: filters.$skip,
       attributes: filters.$select,
       distinct: true,
       returning: true,
       raw: this.raw,
       ...params.sequelize
-    };
+    }
 
-    // Until Sequelize fix all the findAndCount issues, a few 'hacks' are needed to get the total count correct
+    if (id === null) {
+      return {
+        order: getOrder(filters.$sort),
+        limit: filters.$limit,
+        offset: filters.$skip,
+        ...defaults
+      } as FindOptions
+    }
 
-    // Adding an empty include changes the way the count is done
-    // See: https://github.com/sequelize/sequelize/blob/7e441a6a5ca44749acd3567b59b1d6ceb06ae64b/lib/model.js#L1780-L1782
-    // sequelize.include = sequelize.include || [];
+    const sequelize: FindOptions = {
+      limit: 1,
+      ...defaults
+    };
 
-    if (id === null || where[this.id] === id) {
+    if (where[this.id] === id) {
       return sequelize;
     }
 
@@ -447,15 +451,13 @@ export class SequelizeAdapter<
       return result;
     }
 
-    /* TODO: This _get is really only needed to get all of the properties that are not being updated. We can't do similar to _update where we skip the _get (use count instead) and build a fresh instance. With _update we are building ALL the properties and can select/return the right values, with _patch the instance is only partially full. The  instance.update does not return the other properties. So, the returned instance wouldn't contain all properties. We have to get a full instance. This could be optimized to maybe skip this _get given some update props and select circumstances */
+
     const instance = await this._get(id, {
       ...params,
       sequelize: { ...params.sequelize, raw: false }
     }) as Model;
 
-    Object.keys(values).forEach(key => {
-      instance.set(key, values[key]);
-    });
+    instance.set(values)
 
     await instance
       .save(sequelize)
diff --git a/test/index.test.ts b/test/index.test.ts
index aadf36f..6d658c0 100644
--- a/test/index.test.ts
+++ b/test/index.test.ts
@@ -403,6 +403,12 @@ describe('Feathers Sequelize Service', () => {
         assert.strictEqual(updatedPerson.name, updateName);
       });
 
+      it('does not use $skip in get()', async () => {
+        const result = await people.get(kirsten.id, { query: { $skip: 10 } });
+
+        assert.strictEqual(result.id, kirsten.id);
+      })
+
       it('filterQuery does not convert dates and symbols', () => {
         const mySymbol = Symbol('test');
         const date = new Date();
@@ -449,7 +455,7 @@ describe('Feathers Sequelize Service', () => {
           .then(() => people.remove(null, { query: {} }))
       );
 
-      it('find() returns correct total when using includes for non-raw requests (#137)', async () => {
+      it('find() returns correct total when using includes for non-raw requests #137', async () => {
         const options = { sequelize: { raw: false, include: Order } };
 
         const result = await people.find(options) as Paginated<any>;
@@ -670,7 +676,7 @@ describe('Feathers Sequelize Service', () => {
       });
     });
 
-    it('can set the scope of an operation#130', async () => {
+    it('can set the scope of an operation #130', async () => {
       const people = app.service('people');
       const data = { name: 'Active', status: 'active' };
       const SCOPE_TO_ACTIVE = { sequelize: { scope: 'active' } };