Skip to content
This repository has been archived by the owner on Sep 6, 2023. It is now read-only.

Fix use of Knex.modify() #177

Closed
AdamVig opened this issue Apr 5, 2017 · 1 comment
Closed

Fix use of Knex.modify() #177

AdamVig opened this issue Apr 5, 2017 · 1 comment
Assignees
Labels

Comments

@AdamVig
Copy link
Collaborator

AdamVig commented Apr 5, 2017

./services/filter-query.js, ./controllers/model.js, and ./services/endpoint.js use Knex's modify function incorrectly. See the documentation.

Instead of returning functions (in filter-query.js) and binding parameters (in endpoint.js), just call modify with a function as its first parameter, and the other parameters will be passed to the function.

@AdamVig AdamVig self-assigned this Apr 5, 2017
@AdamVig AdamVig changed the title Fix query filtering implementation Fix use of Knex.modify() Apr 5, 2017
@AdamVig AdamVig added chore and removed enhancement labels Jun 27, 2017
@AdamVig AdamVig added this to the November 20 Sprint milestone Nov 8, 2017
@AdamVig
Copy link
Collaborator Author

AdamVig commented Nov 9, 2017

This is easy to fix in services/filter-query.js, but difficult to fix in all of the individual controllers that supply knex.modify() functions.

The reason it is difficult is because the flow looks like this:

  1. controller declares a modify function ((req, queryBuilder) => { ... })
  2. controller passes modify function to endpoint service
  3. endpoint service binds req as the first parameter to the modify function
  4. endpoint service passes the modify function to the db service
  5. db service calls modify function like knex.modify(modifyFunction), expecting the function to accept queryBuilder as its first parameter (which it does, after req is bound in the endpoint service)

The fix would look like:

  • changing the parameters of each modify function to queryBuilder, req
  • not binding the modify function in the endpoint service
  • passing req into the db service so it could call the modify function like knex.modify(modifyFunction, req)

The problem with this is that it increases the coupling between the endpoint service and the db service, a problem that will be solved in #373 Refactor endpoint service. Once the endpoint service is refactored to not pass parameters through to the db service, each controller can directly pass req to the db service so that modify() can be used correctly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant