Repository: ViacomInc/data-point (js)

Issue:
should be able to fully override request.param.inspect
<!--
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).

Please fill/un-comment out this template with all the relevant information so we can
understand what's going on and fix the issue.
-->

**Problem description**:

Wehn attempting to override request.params.inspect with a function the inspect works but it outputs other information aside from the custom function. 

**Suggested solution**:

When inspect is overridden it should pass the necessary information to print:
- entityId
- options
- acc

and the output should be 100% delegated to this method

<!-- 
REPORTING BUG:

If you are reporting a bug, please provide the full error message/screenshots/anything.
-->

**What happened**:

Code below was expected to only print out 'hello':

```js
'request:foo': {
  url: 'http://some.url.com',
  params: {
    inspect: (acc) => {
       console.log('hello')
    }
  }
}
```

But it printed: 

```bash
inspect: request:foo
options: {
  "method": "GET",
  "json": true,
  "url": "http://some.url.com"
}
value: {}

custom:
hello
```

**Reproduction**: 

```js
const dp = require('data-point').create({
  entities: {
    'request:getVehicle': {
      url: 'https://swapi.co/api/vehicles/14/',
      params: {
        inspect: (acc) => {
          console.log('hello')
        }
      }
    }
  }
})

dp.resolve('request:getVehicle', {})
```
<!--If you have a link or example that reproduces your issue please include it. A reproduction with as little code as possible is better.-->

**Environment**:

- `data-point` version: 3.1.0
- `node` version: 8
- `npm` version: N/A


Pull request description:
fix(data-point): fix request.params.inspect
inspect was adding extra logging, this offloads the entire behaviour to the custom function

closes #263

<!--
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!

BREAKING CHANGES:

If your PR includes a breaking change, please submit it with a codemod under
data-point-codemods that will help users upgrade their codebase.

Breaking changes without a codemod will not be accepted unless a codemod is not
viable or does not apply to the specific situation.
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**:
 
fixes #263 

<!-- How were these changes implemented? -->
**How**:

offloading any inspect functionality to the custom inspect function

<!-- 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" -->
- [ ] Has Breaking changes
- [ ] Documentation
- [X] Tests
- [X] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [ ] Added username to **all-contributors** list

<!-- feel free to add additional comments -->


Patch:
diff --git a/packages/data-point/lib/entity-types/entity-request/resolve.js b/packages/data-point/lib/entity-types/entity-request/resolve.js
index 9939159..5714c70 100644
--- a/packages/data-point/lib/entity-types/entity-request/resolve.js
+++ b/packages/data-point/lib/entity-types/entity-request/resolve.js
@@ -85,12 +85,22 @@ module.exports.resolveUrl = resolveUrl
  * @param {Accumulator} acc
  */
 function inspect (acc) {
-  if (acc.params && acc.params.inspect) {
+  const paramInspect = acc.params && acc.params.inspect
+
+  if (typeof paramInspect === 'function') {
+    paramInspect(acc)
+    return true
+  }
+
+  if (paramInspect === true) {
     utils.inspect(acc, {
       options: acc.options,
       value: acc.value
     })
+    return true
   }
+
+  return false
 }
 
 module.exports.inspect = inspect
diff --git a/packages/data-point/lib/utils/index.js b/packages/data-point/lib/utils/index.js
index 2bfa176..871a58f 100644
--- a/packages/data-point/lib/utils/index.js
+++ b/packages/data-point/lib/utils/index.js
@@ -59,11 +59,6 @@ function inspect (acc, data) {
   }
 
   console.info.apply(null, log)
-
-  if (typeof acc.params.inspect === 'function') {
-    console.info('\ncustom:')
-    _.attempt(acc.params.inspect, acc)
-  }
 }
 
 module.exports.inspect = inspect


Test patch:
diff --git a/packages/data-point/lib/entity-types/entity-request/resolve.test.js b/packages/data-point/lib/entity-types/entity-request/resolve.test.js
index cf4e5a9..fb48840 100644
--- a/packages/data-point/lib/entity-types/entity-request/resolve.test.js
+++ b/packages/data-point/lib/entity-types/entity-request/resolve.test.js
@@ -12,6 +12,7 @@ const ResolveEntity = require('../base-entity/resolve')
 const FixtureStore = require('../../../test/utils/fixture-store')
 
 const helpers = require('../../helpers')
+const utils = require('../../utils')
 
 let dataPoint
 let resolveReducerBound
@@ -274,24 +275,41 @@ describe('inspect', () => {
   afterEach(() => {
     console.info = consoleInfo
   })
-  test('It should not execute utils.inspect', () => {
-    console.info = jest.fn()
+  test('It should not execute custom inspect or utils.inspect', () => {
+    const utilsIinspectSpy = jest.spyOn(utils, 'inspect')
     const acc = getAcc()
     acc.params.inspect = undefined
+
     Resolve.inspect(acc)
-    expect(console.info).not.toBeCalled()
+    expect(utilsIinspectSpy).not.toBeCalled()
+
+    utilsIinspectSpy.mockReset()
+    utilsIinspectSpy.mockRestore()
   })
-  test('It should not execute utils.inspect', () => {
-    console.info = jest.fn()
-    Resolve.inspect(getAcc())
-    expect(console.info.mock.calls[0]).toContain('test:test')
+  test('It should execute custom inspect when provided, not execute utils.inspect', () => {
+    const utilsIinspectSpy = jest.spyOn(utils, 'inspect')
+    const acc = getAcc()
+    acc.params.inspect = jest.fn()
+
+    Resolve.inspect(acc)
+    expect(utilsIinspectSpy).not.toBeCalled()
+    expect(acc.params.inspect).toBeCalledWith(acc)
+
+    utilsIinspectSpy.mockReset()
+    utilsIinspectSpy.mockRestore()
   })
-  test('It should output options', () => {
-    console.info = jest.fn()
+  test('It should execute utils.inspect when params.inspect === true', () => {
+    const utilsIinspectSpy = jest
+      .spyOn(utils, 'inspect')
+      .mockImplementation(() => true)
     const acc = getAcc()
-    _.set(acc, 'options', { options: 1 })
+    acc.params.inspect = true
+
     Resolve.inspect(acc)
-    expect(console.info.mock.calls[0]).toContain('\noptions:')
+    expect(utilsIinspectSpy).toBeCalled()
+
+    utilsIinspectSpy.mockReset()
+    utilsIinspectSpy.mockRestore()
   })
 })
 
diff --git a/packages/data-point/lib/utils/utils.test.js b/packages/data-point/lib/utils/utils.test.js
index 43a1a1a..0c2d798 100644
--- a/packages/data-point/lib/utils/utils.test.js
+++ b/packages/data-point/lib/utils/utils.test.js
@@ -90,14 +90,6 @@ describe('inspect', () => {
     expect(console.info.mock.calls[0]).toContain('\nfoo:')
     expect(console.info.mock.calls[0]).toContain('"bar"')
   })
-  test('It should execute custom inspect if function', () => {
-    console.info = jest.fn()
-    const acc = getAcc()
-    acc.params.inspect = () => console.info('baz')
-    utils.inspect(acc)
-    expect(console.info.mock.calls[1]).toContain('\ncustom:')
-    expect(console.info.mock.calls[2]).toContain('baz')
-  })
 })
 
 describe('inspectProperties', () => {