fixup!: address feedback

loopbackio · Nov 28, 2019 · 8e7bffe · 8e7bffe
1 parent cb9d9d6
commit 8e7bffe
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 15 deletions.
diff --git a/...ry-tests/src/crud/relations/acceptance/has-many-inclusion-resolver.relation.acceptance.ts b/...ry-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
@@ -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;
   }