From e3386cc6280347bbb77b2bbe8fd77115ae37010b Mon Sep 17 00:00:00 2001
From: Joey Perrott <josephperrott@gmail.com>
Date: Tue, 19 Apr 2022 15:03:49 -0700
Subject: [PATCH] fix(apps): move the getConverter logic into the base class

Moving the getConverter logic into the base class as it returns an object which while not typed as
each individual firestore data converter, it matches the type signatures and removes the duplicated
code.
---
 apps/shared/models/app-models.ts    | 22 +++-------------------
 apps/shared/models/base.ts          | 20 ++++++++++++++++----
 apps/shared/models/server-models.ts | 20 +-------------------
 3 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/apps/shared/models/app-models.ts b/apps/shared/models/app-models.ts
index 28b57692e..657555fbd 100644
--- a/apps/shared/models/app-models.ts
+++ b/apps/shared/models/app-models.ts
@@ -17,29 +17,13 @@ function forApp<
   const staticModel = model as unknown as typeof BaseModel;
   staticModel.markAsDecorated();
 
-  /** The converter object for performing conversions in and out of Firestore. */
-  const converter = {
-    fromFirestore: (snapshot: any) => {
-      return new model(snapshot.data());
-    },
-    toFirestore: (model: any) => {
-      return model.data;
-    },
-  };
-
-  /**
-   * Class method to get the converter object, ensuring that the converter returned is always
-   * the converter from the specific class definition rather than a parent class.
-   */
-  model.prototype.getConverter = function () {
-    return converter;
-  };
-
   /**
    * Gets the model referenced by the provided FirestoreReference, returning the same reference
    * as previously queried if the instance cache finds an entry.
    */
   model.prototype.getByReference = function (ref: FirestoreReference<TBase>) {
-    return getDoc(doc(getFirestore(), fromFirestoreReference(ref)).withConverter(converter));
+    return getDoc(
+      doc(getFirestore(), fromFirestoreReference(ref)).withConverter(staticModel.getConverter()),
+    );
   };
 }
diff --git a/apps/shared/models/base.ts b/apps/shared/models/base.ts
index f2f825c17..dd4c4400c 100644
--- a/apps/shared/models/base.ts
+++ b/apps/shared/models/base.ts
@@ -29,14 +29,26 @@ export abstract class BaseModel<T> {
   }
 
   /** A function to get the converter for the model */
-  static getConverter() {
-    return this.prototype.getConverter();
+  static getConverter<T>() {
+    return this.prototype.getConverter<T>(this as unknown as Constructor<T>);
   }
-  protected getConverter(): {
+
+  /**
+   * Class method to get the converter object, ensuring that the converter returned is always
+   * the converter from the specific class definition rather than a parent class.
+   */
+  private getConverter<T>(model: Constructor<T>): {
     fromFirestore: (snapshot: any) => T;
     toFirestore: (model: any) => any;
   } {
-    throw Error('This was not implemented');
+    return {
+      fromFirestore: (snapshot: any) => {
+        return new model(snapshot.data());
+      },
+      toFirestore: (model: any) => {
+        return model.data;
+      },
+    };
   }
 
   /**
diff --git a/apps/shared/models/server-models.ts b/apps/shared/models/server-models.ts
index c3025feec..ccf3ef756 100644
--- a/apps/shared/models/server-models.ts
+++ b/apps/shared/models/server-models.ts
@@ -29,29 +29,11 @@ function forServer<
   const staticModel = model as unknown as typeof GithubBaseModel;
   staticModel.markAsDecorated();
 
-  /** The converter object for performing conversions in and out of Firestore. */
-  const converter = {
-    fromFirestore: (snapshot: any) => {
-      return new model(snapshot.data());
-    },
-    toFirestore: (model: any) => {
-      return model.data;
-    },
-  };
-
-  /**
-   * Class method to get the converter object, ensuring that the converter returned is always
-   * the converter from the specific class definition rather than a parent class.
-   */
-  model.prototype.getConverter = function () {
-    return converter;
-  };
-
   /**
    * Gets the model referenced by the provided FirestoreReference.
    */
   model.prototype.getByReference = async function (ref: FirestoreReference<TBase>) {
-    return firestore().doc(fromFirestoreReference(ref)).withConverter(converter);
+    return firestore().doc(fromFirestoreReference(ref)).withConverter(this.getConverter());
   };
 
   if (staticModel.githubHelpers !== undefined) {