Skip to content

feat(smart actions): endpoint that handle forms' load hooks#546

Merged
rap2hpoutre merged 9 commits intomasterfrom
handle-load-hook-route
Nov 26, 2020
Merged

feat(smart actions): endpoint that handle forms' load hooks#546
rap2hpoutre merged 9 commits intomasterfrom
handle-load-hook-route

Conversation

@rap2hpoutre
Copy link
Contributor

@rap2hpoutre rap2hpoutre commented Nov 20, 2020

Developer' agents have an endpoint that handle smart actions forms' load hooks

  • The routes verb is POST
  • The routes path is: actions/smart-action-endpoint/hooks/load (or the smart action endpoint)
  • The routes handles parameters from request
    • It should transform a recordId to a record.
    • It should get fields from request or get it from schema when missing (we could discuss about it later).
  • The routes sends fields and record to the callback (the function defined by the user).
  • The route has to consider the return value from the callback then trigger error in UI with 500 code and console when something went wrong, e.g:
    • Error has been thrown in the function
    • Nothing is returned
    • load is not a function
    • a field is deleted or added (new key or missing key in fields)
    • Add a test for each error case
  • Add a test for valid case

Almost all lines of code are covered (90.3%, not sure why it's not 100%).

I suggest reviewing code by opening src/routes/actions.js rather than reading the diff.

Pull Request checklist:

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes (not that simple, I will do this during review)
  • Review my own code (indentation, syntax, style, simplicity, readability)
  • Wonder if you can improve the existing code

Tests

routes > actions
  ✓ should not create a route when no actions is present (3ms)
  ✓ should not create a route when actions.values or actions.hooks.* are missing present (1ms)
  when actions.values is present
    ✓ should create a route (1ms)
    ✓ should create a valid route callback (1ms)
    ✓ should handle async values function (1ms)
  when action.hooks is present
    when action.hooks.load is present
      ✓ should create a route
      when calling the route controller
        when action.hooks.load is invalid
          ✓ should fail with message when action.hooks.load is not a function (2ms)
          ✓ should fail with message when action.hooks.load does not return an object (2ms)
          ✓ should fail with message when action.hooks.load returned fields are not consistent
        when action.hooks.load is valid
          ✓ should respond success with the updated fields

@rap2hpoutre rap2hpoutre self-assigned this Nov 20, 2020
@forest-bot
Copy link
Member

Copy link
Contributor

@GuillaumeCisco GuillaumeCisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work! 🔥
Great thing we can go further in this topic.

I added some possible improvements and some questions.

Copy link
Contributor

@GuillaumeCisco GuillaumeCisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this refacto, code is far more easier to read.
I've added some improvements.
Especially in term of complexity.

I'm available for talking about it :)

* @param {Number} step current step (internal var)
* @returns {Boolean}
*/
function isSameDataStructure(object, other, deep = 0, step = 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function isSameDataStructure has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.

GuillaumeCisco
GuillaumeCisco previously approved these changes Nov 26, 2020
Copy link
Contributor

@GuillaumeCisco GuillaumeCisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

I will check if we can get rid of the codeclimate warning.
seems overkill to me.

@GuillaumeCisco GuillaumeCisco self-assigned this Nov 26, 2020
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 689bb60 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 95.8% (49% is the threshold).

This pull request will bring the total coverage in the repository to 50.9% (0.6% change).

View more on Code Climate.

Copy link
Contributor

@GuillaumeCisco GuillaumeCisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✨

@rap2hpoutre rap2hpoutre merged commit 3e3c018 into master Nov 26, 2020
@rap2hpoutre rap2hpoutre deleted the handle-load-hook-route branch November 26, 2020 09:40
forest-bot added a commit that referenced this pull request Nov 26, 2020
# [7.7.0](v7.6.0...v7.7.0) (2020-11-26)

### Features

* **smart actions:** endpoint that handle forms' load hooks ([#546](#546)) ([3e3c018](3e3c018))
@forest-bot
Copy link
Member

🎉 This PR is included in version 7.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

forest-bot added a commit that referenced this pull request Dec 2, 2020
# [8.0.0-beta.4](v8.0.0-beta.3...v8.0.0-beta.4) (2020-12-02)

### Bug Fixes

* **schema:** do not generate `framework`, `framework_version` to ensure equality across environments ([#556](#556)) ([30ee17a](30ee17a))
* **smart fields:** compute properly smart fields ([#570](#570)) ([923c968](923c968))
* **smart-actions:** transform legacy widgets in hooks ([#571](#571)) ([f58b867](f58b867))
* **technical:** remove useless data property from load hook controller ([#562](#562)) ([7465982](7465982))

### Features

* **smart actions:** endpoint that handle forms' load hooks ([#546](#546)) ([3e3c018](3e3c018))
* **smart actions:** endpoint that handle forms' load hooks ([#565](#565)) ([824a670](824a670))

### Reverts

* **related-data:** use same reference on record for dataValues and direct attributes ([#569](#569)) ([5e7a689](5e7a689))
@forest-bot
Copy link
Member

🎉 This PR is included in version 8.0.0-beta.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments