search

StateVoicesNational / Spoke

mass-contact text/SMS distribution tool
Other
462 stars 402 forks source link

Update all queries to use knex directly #281

Open sreynen opened 6 years ago

sreynen commented 6 years ago

Spoke is currently using a custom adapter to translate thinky syntax into knex queries. This adds a bit of complexity to the code, increasing the opportunity for bugs and raising the barrier to entry for contributors. I suggest we aim to eventually remove this layer.

By my count, currently 17 queries already use knex directly (r.knex) and 68 still use thinky syntax (r.table). That's a lot of queries to update, some pretty complex, which makes this a large, complex task. This issue is intended as a long-term goal more than a practical task someone might take on, maybe best accomplished in the context of other issues that involve thinky queries.

Frydafly commented 3 years ago

Checking in with @sreynen & @jmcarp

Looks like this might still be an issue, since the rethink-knex-adapter is still in use.

The long term goal is to move all queries and changes to knex. Would be great to get work done on this, but yes, is definitely a complex ask!

Relevant link: https://github.com/MoveOnOrg/Spoke/blob/7ebf78378e7ae2b3d49ccb884a3a7dba08c5ec8c/docs/EXPLANATION-development-guidelines.md#understanding-dborm-calls

schuyler1d commented 3 years ago

Marking as good first issue -- We'll take PRs removing any non-knex queries -- it doesn't need to be all of them at once.