Repository: ViacomInc/data-point (js)

Issue:
normalize transform entity
From the readme:
> All entities share a common API (except for Transform)

> A Transform entity is meant to be used as a 'snippet' entity that you can re-use in other entities. It does not expose the common before/after/error/params API that other entities have.

Transforms _do_ expose the `before`, `after`, and `error` callbacks though. Either the readme or the code should be updated for accuracy.

Pull request:
fix(data-point/entities): normalize transform entity
transform entity should only accept a TransformExpression as its value

closes #51

<!--
Thanks for your interest in the project. We appreciate bugs filed and PRs submitted!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**: fixes #51

<!-- Why are these changes necessary? -->
**Why**:  current behavior is not as described in the documentation

<!-- How were these changes implemented? -->
**How**: 

1. not casting to object if value is not an object under core/factory.js

https://github.com/ViacomInc/data-point/blob/3e3da6ad20e2f6c1740e55a7f313cffc6b3be565/packages/data-point/lib/core/normalize-entities.js#L56-L60

2. entity extension only applied to entities that require it

https://github.com/ViacomInc/data-point/blob/3e3da6ad20e2f6c1740e55a7f313cffc6b3be565/packages/data-point/lib/core/normalize-entities.js#L39-L42


<!-- Have you done all of these things?  -->
**Checklist**:
<!-- add "N/A" to the end of each line that's irrelevant to your changes -->
<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->
- [x] Documentation
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->

<!-- feel free to add additional comments -->


Patch:
diff --git a/packages/data-point/README.md b/packages/data-point/README.md
index 1890e73..8763dc8 100644
--- a/packages/data-point/README.md
+++ b/packages/data-point/README.md
@@ -845,6 +845,8 @@ A Transform entity is meant to be used as a 'snippet' entity that you can re-use
 
 The value of a Transform entity is a [TransformExpression](#transform-expression).
 
+IMPORTANT: Transform Entities **do not support** (extension)[#extending-entities].
+
 **SYNOPSIS**
 
 ```js
@@ -2656,11 +2658,12 @@ function RenderTemplate () {
 /**
  * Entity Factory
  * @param {*} spec - Entity Specification
+ * @param {string} id - Entity id
  * @return {RenderTemplate} RenderTemplate Instance
  */
-function create (spec) {
+function create (spec, id) {
   // create an entity instance
-  const entity = DataPoint.createEntity(RenderTemplate, spec)
+  const entity = DataPoint.createEntity(RenderTemplate, spec, id)
   // set/create template from spec.template value
   entity.template = _.template(_.defaultTo(spec.template, ''))
   return entity
diff --git a/packages/data-point/examples/custom-entity-type.js b/packages/data-point/examples/custom-entity-type.js
index 5376eea..2811bac 100644
--- a/packages/data-point/examples/custom-entity-type.js
+++ b/packages/data-point/examples/custom-entity-type.js
@@ -8,11 +8,12 @@ function RenderTemplate () {}
 /**
  * Entity Factory
  * @param {*} spec - Entity Specification
+ * @param {string} id - Entity id
  * @return {RenderTemplate} RenderTemplate Instance
  */
-function create (spec) {
+function create (spec, id) {
   // create an entity instance
-  const entity = DataPoint.createEntity(RenderTemplate, spec)
+  const entity = DataPoint.createEntity(RenderTemplate, spec, id)
   // set/create template from spec.template value
   entity.template = _.template(_.defaultTo(spec.template, ''))
   return entity
diff --git a/packages/data-point/lib/core/normalize-entities.js b/packages/data-point/lib/core/normalize-entities.js
index 874708e..fb96fd9 100644
--- a/packages/data-point/lib/core/normalize-entities.js
+++ b/packages/data-point/lib/core/normalize-entities.js
@@ -37,6 +37,9 @@ function getAncestors (spec, specs) {
 module.exports.getAncestors = getAncestors
 
 function extendSpec (spec, ancestors, sources) {
+  if (ancestors.length === 0) {
+    return spec
+  }
   const ancestorSpecs = ancestors.map(parentId => {
     return sources[parentId].spec
   })
@@ -46,16 +49,6 @@ function extendSpec (spec, ancestors, sources) {
 
 module.exports.extendSpec = extendSpec
 
-function normalizeSpecSource (source) {
-  if (_.isPlainObject(source)) {
-    return source
-  }
-
-  return {
-    value: source
-  }
-}
-
 function normalizeSpec (specItemId, source) {
   const tokens = specItemId.split(' -> ')
   const id = tokens[0]
@@ -63,7 +56,7 @@ function normalizeSpec (specItemId, source) {
   return {
     id: id,
     parentId: parentId,
-    spec: normalizeSpecSource(source[specItemId]),
+    spec: source[specItemId],
     ancestors: []
   }
 }
diff --git a/packages/data-point/lib/entity-types/entity-collection/factory.js b/packages/data-point/lib/entity-types/entity-collection/factory.js
index 850c3c9..82b2327 100644
--- a/packages/data-point/lib/entity-types/entity-collection/factory.js
+++ b/packages/data-point/lib/entity-types/entity-collection/factory.js
@@ -51,10 +51,16 @@ function validateComposeVsInlineModifiers (spec, invalidInlinesKeys) {
   }
 }
 
-function create (spec) {
+/**
+ * Creates new Entity Object
+ * @param  {Object} spec - spec
+ * @param {string} id - Entity id
+ * @return {Collection} Entity Object
+ */
+function create (spec, id) {
   validateComposeVsInlineModifiers(spec, modifierKeys)
 
-  const entity = helpers.createEntity(Collection, spec)
+  const entity = helpers.createEntity(Collection, spec, id)
 
   const compose = parseCompose.parse(spec, modifierKeys)
   entity.compose = createCompose(compose)
diff --git a/packages/data-point/lib/entity-types/entity-control/factory.js b/packages/data-point/lib/entity-types/entity-control/factory.js
index c945477..f98d6ad 100644
--- a/packages/data-point/lib/entity-types/entity-control/factory.js
+++ b/packages/data-point/lib/entity-types/entity-control/factory.js
@@ -70,12 +70,13 @@ function parseSwitch (spec) {
 module.exports.parseSwitch = parseSwitch
 
 /**
- * Creates new ControlEntity based on spec
- * @param  {Object} spec - control spec
- * @return {ControlEntity}
+ * Creates new Entity Object
+ * @param  {Object} spec - spec
+ * @param {string} id - Entity id
+ * @return {EntityControl} Entity Object
  */
-function create (spec) {
-  const entity = helpers.createEntity(EntityControl, spec)
+function create (spec, id) {
+  const entity = helpers.createEntity(EntityControl, spec, id)
   entity.select = parseSwitch(spec)
   return Object.freeze(entity)
 }
diff --git a/packages/data-point/lib/entity-types/entity-entry/factory.js b/packages/data-point/lib/entity-types/entity-entry/factory.js
index 2f003c1..0529679 100644
--- a/packages/data-point/lib/entity-types/entity-entry/factory.js
+++ b/packages/data-point/lib/entity-types/entity-entry/factory.js
@@ -11,12 +11,13 @@ function EntityEntry () {}
 module.exports.EntityEntry = EntityEntry
 
 /**
- * Creates new ControlEntity based on spec
- * @param  {Object} spec - control spec
- * @return {ControlEntity}
+ * Creates new Entity Object
+ * @param  {Object} spec - spec
+ * @param {string} id - Entity id
+ * @return {EntityEntry} Entity Object
  */
-function create (spec) {
-  const entity = helpers.createEntity(EntityEntry, spec)
+function create (spec, id) {
+  const entity = helpers.createEntity(EntityEntry, spec, id)
   return Object.freeze(entity)
 }
 
diff --git a/packages/data-point/lib/entity-types/entity-hash/factory.js b/packages/data-point/lib/entity-types/entity-hash/factory.js
index e60dc65..b70abe0 100644
--- a/packages/data-point/lib/entity-types/entity-hash/factory.js
+++ b/packages/data-point/lib/entity-types/entity-hash/factory.js
@@ -93,10 +93,16 @@ function validateCompose (entityId, compose, validKeys) {
   })
 }
 
-function create (spec) {
+/**
+ * Creates new Entity Object
+ * @param  {Object} spec - spec
+ * @param {string} id - Entity id
+ * @return {Object} Entity Object
+ */
+function create (spec, id) {
   validateComposeVsInlineModifiers(spec, modifierKeys)
 
-  const entity = helpers.createEntity(Hash, spec)
+  const entity = helpers.createEntity(Hash, spec, id)
 
   const compose = parseCompose.parse(spec, modifierKeys)
   validateCompose(entity.id, compose, modifierKeys)
diff --git a/packages/data-point/lib/entity-types/entity-request/factory.js b/packages/data-point/lib/entity-types/entity-request/factory.js
index 78fc861..7e28fcc 100644
--- a/packages/data-point/lib/entity-types/entity-request/factory.js
+++ b/packages/data-point/lib/entity-types/entity-request/factory.js
@@ -37,10 +37,11 @@ module.exports.unsetTransformKeys = unsetTransformKeys
 /**
  * creates new Request based on spec
  * @param  {Object} spec - request spec
+ * @param {string} id - Entity id
  * @return {Request}
  */
-function create (spec) {
-  const entity = helpers.createEntity(Request, spec)
+function create (spec, id) {
+  const entity = helpers.createEntity(Request, spec, id)
   const options = _.defaultTo(spec.options, {})
   entity.url = _.defaultTo(spec.url, '')
   entity.beforeRequest = createTransform(spec.beforeRequest)
diff --git a/packages/data-point/lib/entity-types/entity-schema/factory.js b/packages/data-point/lib/entity-types/entity-schema/factory.js
index 5b61609..58705bb 100644
--- a/packages/data-point/lib/entity-types/entity-schema/factory.js
+++ b/packages/data-point/lib/entity-types/entity-schema/factory.js
@@ -15,12 +15,13 @@ function EntitySchema () {
 module.exports.EntitySchema = EntitySchema
 
 /**
- * @param  {Object} spec - entity spec
- * @return {ControlEntity}
+ * Creates new Entity Object
+ * @param  {Object} spec - spec
+ * @param {string} id - Entity id
+ * @return {EntitySchema} Entity Object
  */
-
-function create (spec) {
-  const entity = helpers.createEntity(EntitySchema, spec)
+function create (spec, id) {
+  const entity = helpers.createEntity(EntitySchema, spec, id)
   entity.schema = deepFreeze(_.defaultTo(spec.schema, {}))
   entity.options = deepFreeze(_.defaultTo(spec.options, {}))
 
diff --git a/packages/data-point/lib/entity-types/entity-transform/factory.js b/packages/data-point/lib/entity-types/entity-transform/factory.js
index 99d563a..e7c835a 100644
--- a/packages/data-point/lib/entity-types/entity-transform/factory.js
+++ b/packages/data-point/lib/entity-types/entity-transform/factory.js
@@ -10,9 +10,20 @@ function Transform () {}
 
 module.exports.Transform = Transform
 
+/**
+ * Creates new Entity Object
+ * @param  {*} spec - spec
+ * @param {string} id - Entity id
+ * @return {Transform} Entity Object
+ */
 function create (spec, id) {
-  const entity = helpers.createEntity(Transform, spec)
-  entity.id = id
+  const entity = helpers.createEntity(
+    Transform,
+    {
+      value: spec
+    },
+    id
+  )
   return Object.freeze(entity)
 }
 
diff --git a/packages/data-point/lib/helpers/helpers.js b/packages/data-point/lib/helpers/helpers.js
index c1cf1d9..5f68657 100644
--- a/packages/data-point/lib/helpers/helpers.js
+++ b/packages/data-point/lib/helpers/helpers.js
@@ -48,11 +48,12 @@ module.exports.resolveEntity = resolveEntity
 /**
  * @param {function} Factory - factory function to create the entity
  * @param {Object} spec - spec for the Entity
+ * @param {string} id - Entity's id
  */
-function createEntity (Factory, spec) {
+function createEntity (Factory, spec, id) {
   const entity = new Factory(spec)
 
-  entity.id = spec.id
+  entity.id = id
   entity.value = createTransform(spec.value)
   entity.before = createTransform(spec.before)
   entity.error = createTransform(spec.error)
diff --git a/packages/data-point/lib/stores/store-manager.js b/packages/data-point/lib/stores/store-manager.js
index c8ac8ab..1f619ee 100644
--- a/packages/data-point/lib/stores/store-manager.js
+++ b/packages/data-point/lib/stores/store-manager.js
@@ -1,7 +1,6 @@
 'use strict'
 
 const _ = require('lodash')
-const utils = require('../utils')
 
 /**
  * get all added models
@@ -58,8 +57,7 @@ function add (manager, errorInfoCb, factory, id, spec) {
     throw e
   }
 
-  const objSpec = utils.set(spec, 'id', id)
-  const item = _.attempt(factory, objSpec, id)
+  const item = _.attempt(factory, spec, id)
 
   if (item instanceof Error) {
     item.entityId = id


Test patch:
diff --git a/packages/data-point/lib/helpers/helpers.test.js b/packages/data-point/lib/helpers/helpers.test.js
index 92fa7f8..40b6ec8 100644
--- a/packages/data-point/lib/helpers/helpers.test.js
+++ b/packages/data-point/lib/helpers/helpers.test.js
@@ -77,13 +77,16 @@ describe('helpers.mockReducer', () => {
 describe('helpers.createEntity', () => {
   test('It should create entity defaults', () => {
     function FooEntity () {}
-    const entity = helpers.createEntity(FooEntity, {
-      id: 'foo',
-      before: '$.',
-      value: '$.',
-      error: '$.',
-      after: '$.'
-    })
+    const entity = helpers.createEntity(
+      FooEntity,
+      {
+        before: '$.',
+        value: '$.',
+        error: '$.',
+        after: '$.'
+      },
+      'foo'
+    )
 
     expect(entity).toHaveProperty('id', 'foo')
     expect(entity).toHaveProperty('value.typeOf', 'TransformExpression')