Venkat knex

[anandvenkat4]

Knex style queries.

[lakesare]

That's exactly what we want 👍
Don't forget to delete the CourseModel.select.countAllPublic method as we no longer need it.

[lakesare]

It's ES6, let's use const for all vars that do not change, and let for all vars that do get reassigned.
Fyi, - const and let are preferable, because they let us have {} scope instead of function(){} scope, - and that lets us use them easily within loops e.g..

[anandvenkat4]

Sure, I knew I had to change this, but sent it for the functional check. Thats a good learning.

[lakesare]

Looks nice!
We can improve it by factoring out this piece of code:

knex('course').count('id', {as: 'amount_of_public_courses'})
    .where('if_public', 'true')
    .andWhere(2, "<=", subquery); 

E.g. we can do (please check if this actually works though :-)):

const courseCategoryId = request.query.courseCategoryId;
let amountPromise = knex('course').count('id', {as: 'amount_of_public_courses'})
    .where('if_public', 'true')
    .andWhere(2, "<=", subquery); 

if (courseCategoryId) {
  amountPromise = amountPromise.andWhere('course_category_id', courseCategoryId)
}

const amountOfAllCourses = await amountPromise;

[anandvenkat4]

I will check and revert. Also, will change the 'allPublic' so that we can completely get rid of older syntax and then will commit the changes. Thank you for reviewing.

[anandvenkat4]

This is done in the new commit.

[anandvenkat4]

That's exactly what we want 👍
Don't forget to delete the CourseModel.select.countAllPublic method as we no longer need it.

Done.

[lakesare]

Does this actually work locally? Do we not need to access 'amount_of_public_courses' somehow?

[anandvenkat4]

Sorry, my bad. I did not push the latest change. Done now. Please check.

[lakesare]

👍Lgtm
I'll merge this now, but for the future, - we can do const amountOfAllCourses = (await amountofAllCoursesQuery)[0].amountOfPublicCourses; whenever it looks better.

[anandvenkat4]

Sure. Thanks :)