From 8e7bffe351ca7d1fd22308e0da0632f4a7cc04e3 Mon Sep 17 00:00:00 2001
From: jannyHou <juehou@ca.ibm.com>
Date: Wed, 27 Nov 2019 21:10:44 -0500
Subject: [PATCH] fixup!: address feedback

---
 ...-inclusion-resolver.relation.acceptance.ts |  5 +---
 .../src/repositories/legacy-juggler-bridge.ts | 26 +++++++++++--------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts b/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts
index 9dbb65ff97e9..944786393724 100644
--- a/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts
+++ b/packages/repository-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts
@@ -244,12 +244,9 @@ export function hasManyInclusionResolverAcceptance(
         });
 
         const pizza = await orderRepo.findById(thorOrder.id.toString());
-        let reheatedPizza = {...pizza};
+        const reheatedPizza = {...pizza};
         reheatedPizza.description = 'Reheated pizza';
 
-        // The replace here changes the order's foreignKey from thor.id to odin.id
-        // which will be rejected by the fk constraint check in jugger
-        // So I modified this test to replace `description` only.
         await orderRepo.replaceById(thorOrder.id, reheatedPizza);
         const odinPizza = await orderRepo.findById(thorOrder.id);
 
diff --git a/packages/repository/src/repositories/legacy-juggler-bridge.ts b/packages/repository/src/repositories/legacy-juggler-bridge.ts
index cf5929dbb866..003e2d52365a 100644
--- a/packages/repository/src/repositories/legacy-juggler-bridge.ts
+++ b/packages/repository/src/repositories/legacy-juggler-bridge.ts
@@ -536,18 +536,28 @@ export class DefaultCrudRepository<
     return includeRelatedModels<T, Relations>(this, entities, include, options);
   }
 
+  /**
+   * This function works as a persist hook.
+   * It converts an entity from the CRUD operations' caller
+   * to a persistable data that can will be stored in the
+   * back-end database.
+   *
+   * User can extend `DefaultCrudRepository` then override this
+   * function to execute custom persist hook.
+   * @param entity The entity passed from CRUD operations' caller.
+   * @param options
+   */
   protected async entityToData<R extends T>(
     entity: R | DataObject<R>,
     options = {},
   ): Promise<legacy.ModelData<legacy.PersistedModel>> {
-    this.ensurePersistable(entity, options);
-    return entity;
+    return this.ensurePersistable(entity, options);
   }
 
   /** Converts an entity object to a JSON object to check if it contains navigational property.
    * Throws an error if `entity` contains navigational property.
    *
-   * @param entity
+   * @param entity The entity passed from CRUD operations' caller.
    * @param options
    */
   protected ensurePersistable<R extends T>(
@@ -563,8 +573,8 @@ export class DefaultCrudRepository<
       typeof entity.toJSON === 'function' ? entity.toJSON() : {...entity};
     */
 
-    // proposal solution from agnes: delete the id property in the target dtat
-    // when runs replaceById
+    // proposal solution from agnes: delete the id property in the
+    // target data when runs replaceById
 
     const data: AnyObject = new this.entityClass(entity);
 
@@ -577,12 +587,6 @@ export class DefaultCrudRepository<
         );
       }
     }
-    // if (options.replacement === true) {
-    //   const idProperty = this.entityClass.getIdProperties()[0];
-    //   if (!data[idProperty] === undefined) {
-    //     throw new Error(`id property should not be included in ${data}`);
-    //   }
-    // }
     return data;
   }