use pagination for version downloads

[Eh2406]

This is draft of fix for issues/557. Inspired by @carols10cents comment I tried to use the same pagination code as is used for the other endpoints.

[sgrif]

I wouldn't call this offset and limit, since it's not used for either offset or limit. Not sure it makes sense to permit the per_page param here either.

[Eh2406]

I did it to be consistent with the other uses. The advantage of per_page is the consistency with other endpoints. All in all, I am not strongly wedded to this solution.

[sgrif]

I'm not sure that the concept of a page makes as much sense here. Perhaps we should just have the parameter be called before_date, which takes the date directly, and only allow the 90 day interval.

[Eh2406]

That would make more seance. What is a good format for the date as url query fragment? And, how do we parce it into a type for the sql query?

[sgrif]

YYYY-MM-DD. I'd just pass it directly as a bind param into the query if Rust postgres lets us. If not, strptime it up

[sgrif]

What do you think about doing WHERE date BETWEEN $1 AND $2 instead?

[Eh2406]

Love it, I will make that change at the next opportunity!

[sgrif]

Looks good. I'd like to see a test that demonstrates data being returned as well.

[Eh2406]

I tried to structure it with a bindparam into the query but that requires doing the -89 in the sql and that gives me postgresql errors. Improvements to this code can be done in future PRs, if I figure out how to get around the errors.

[sgrif]

- '89 days' in SQL seems fine to me.

[Eh2406]

Using WHERE date BETWEEN $1 - '89 days' AND $1 AND version_id = $2 gives 'middle.call(&mut req) failed with: Error reported by Postgres: ERROR: operator is not unique: unknown - unknown'

Using WHERE date BETWEEN $1 - interval '89 days' AND $1 AND version_id = $2 gives 'middle.call(&mut req) failed with: Error reported by Postgres: ERROR: operator does not exist: timestamp without time zone >= interval'

[sgrif]

Oh right, sorry I'm used to Diesel handling this. XD

If you're using rust-postgres you'll need to tell PG what the type of the bind parameter is, so $1::timestamp.

[sgrif]

I think it's probably fine to just keep doing this in Rust though.