Make search random across all pages

[hemal7735]

This PR addresses the concerns shown here: #51 (comment)

How it is done?

I see there is a field called total_count in the response. We can utilize that to figure out the number of pages as default is 30 items per page. So the idea is:

1. figure out the number of pages
2. select random page
3. select random issue from that page

Note:

• We will be making 2 calls to octokit apis for this. I'm not sure how it will affect rate-limiting.
• changes to tests in search.spec.js.

[bnb]

Don't worry about build failures – issue with our .travis.yml

[hemal7735]

@bnb I was thinking of using filter operation first and then reduce, rather than filtering inside reduce.

[bnb]

I think that makes sense 🤔

[ghost]

If you filter outside the reduce its a bit less performant because you're looping more than once over the items no?

[hemal7735]

If you filter outside the reduce its a bit less performant because you're looping more than once over the items no?

Yes, it might be a tiny bit slower, however, it will be negligible due to the size of the dataset(30 max) that we have. Then it comes down to the readability of code, and that is what pipe and filter pattern excels at. Tomorrow you might want to change the filters based on the user demands, in that case we just need to swap the current filter with something else.

[bnb]

FWIW it still seems like I'm only getting 1st page with good-first-issue feeling-lucky – all the results are within the last hour (and were the same last night as well). Not sure if you've implemented random selection of pages yet 🤔

[hemal7735]

@bnb great catch. Found it, just es6 things where key can be skipped incase of variable name matches to key!
Sorry for wasting your time! 😞
Initially I had the same keys, then linting asked me to use camelCase .

[bnb]

@hemal7735 no worries! ❤️

[bnb]

Looks like it's working! I got a few from ~3 hours ago, and one from ~11 months ago. 💥

Only comment is that it looks like some lines don't have test coverage. If you could add that, this should be good to go!

[hemal7735]

@bnb It is 100% now. Can you check with Travis CI?

[bnb]

Travis checks look good to me. I'll pull this down and test it out a bit, but when I was testing the other day it looked good. Sanity check and we're good to go 👍

[bnb]

Out of 10-15 tries I got mostly issues within the last 48 hours with good-first-issue feeling-lucky. Not necessarily an issue on the part of this code, just not sure if that's intended or not 🤔

[bnb]

So is it possible to change the starting page (PAGE)? It doesn't seem like we're doing that, which would be a vital part of full randomization right? 🤔

Perhaps I'm just not fully understanding what this code is doing.

[hemal7735]

These are the constants and defaults referred in getSearchParams function. Any calls made to octokit.search.issues should use getSearchParams function inorder to properly generate search-params.

now, although these are defaults, one can override them by passing their search param values.
perhaps, I should use Object.assign 😞

[hemal7735]

So is it possible to change the starting page (PAGE)?

Yes, we are doing it in the second call made to issues search.

[bnb]

LGTM. Going to merge this + ship it in v0.17.0 👍