search

lakesare / memcode

Spaced-repetition: with real formatting.
http://memcode.com
MIT License
335 stars 72 forks source link

Backend: migrate legacy endpoint structure to new endpoint structure #65

Open lakesare opened 4 years ago

lakesare commented 4 years ago

Migrating the backend endpoints of this form: https://github.com/lakesare/memcode/blob/master/backend/api/CourseApi/index.js#L10 into endpoints of this form: https://github.com/lakesare/memcode/blob/master/backend/api/CourseApi/rate.js is always a #todo.

It's fine to PR just a few endpoints migrated.

If migrating to knex is too hard (e.g. SQL query is more easily written in raw SQL), - it’s fine to leave pg-promise.

anandvenkat4 commented 4 years ago

@lakesare What is your thoughts on this: https://medium.com/@gajus/stop-using-knex-js-and-earn-30-bf410349856c

lakesare commented 4 years ago

@anandvenkat4, I think that's the author of slonik 🙃 I think we can see the value of knex for ourselves by comparing the before (raw sql with pg-promise) / after code (with knex) of any controller. Fyi pg-promise is very close to slonik syntax-wise, - you also write raw SQL.

anandvenkat4 commented 4 years ago

Got it :)

anandvenkat4 commented 4 years ago

To give a flavour of my changes and to align with what is expected, I am sending you a PR which replaces 'countAllPublic' to a knex like syntax. Please let me know your comments.

lakesare commented 4 years ago

@anandvenkat4, great PR 👍 We should also be moving these routes into separate files (see /CourseApi/rate.js).

And in the future, - if some query is too complex in SQL (see the getCoursesWithStats()), - it can be better to leave it as such. Sometimes it’s indeed easier to read the raw SQL.