use most recent order on the homepage #3

ben · 2018-07-20T21:58:56Z

ben commented

2018-07-20 21:58:56 +00:00

also loads all quotes (instead of just 10)

khuxkm commented

2018-07-20 22:02:04 +00:00

The choice to only load 10 quotes was intentional. I don't want to have a really long scrolling page.

ben commented

2018-07-20 22:02:56 +00:00

That's fair. We should add pagination or another view that loads all.

/top currently loads all quotes in the db.

That's fair. We should add pagination or another view that loads all. /top currently loads all quotes in the db.

khuxkm commented

2018-07-20 22:08:35 +00:00

suggestion:

Add get_quotes_by_recent(limit=None), where if limit is not None:, add a "LIMIT "+limit to the end of the query string, and ORDER BY ID DESC.
Have the homepage use get_quotes_by_recent(10).
Add a new endpoint: @app.route("/new") that uses just get_quotes_by_recent().

If you don't object, I'll do this on my own (in this branch).

suggestion: - Add `get_quotes_by_recent(limit=None)`, where `if limit is not None:`, add a `"LIMIT "+limit` to the end of the query string, and `ORDER BY ID DESC`. - Have the homepage use `get_quotes_by_recent(10)`. - Add a new endpoint: `@app.route("/new")` that uses just `get_quotes_by_recent()`. If you don't object, I'll do this on my own (in this branch).

ben was assigned by khuxkm

2018-07-20 22:08:42 +00:00

khuxkm self-assigned this 2018-07-20 22:08:43 +00:00

ben commented

2018-07-20 22:10:10 +00:00

I don't object to that. What do you think of pagination?

khuxkm commented

2018-07-21 08:08:54 +00:00

If you can figure out how to do pagination in SQL, then sure! Otherwise, we'll figure it out after I finish working on this.

khuxkm commented

2018-07-21 08:11:46 +00:00

Also, in case you're wondering why I'm insisting on the functions being seperate: when I start work on the API, I want to be able to at least have 1:1 functionality with the homepage. That's why I treat the pages as skins for the quotes: it just makes it easier to build an API.

ben commented

2018-07-21 20:44:49 +00:00

Oh I didn't realize you were planning on adding an API :P

khuxkm commented

2018-07-26 18:33:22 +00:00

I legitimately just realized that something similar to this was in my workflow and just hadn't been committed yet. I like the idea we've come up with here more, though, so we'll go with this plan.

ben commented

2018-07-26 18:35:10 +00:00

Are you thinking pagination then?

Looks like it wouldn't be too bad to do:

SO explanation here

sqlite wiki

there might already be an easy way to do this with the python sqlite library so that might be worth looking into as well

Are you thinking pagination then? Looks like it wouldn't be *too* bad to do: [SO explanation here](https://stackoverflow.com/a/14468878/6352706) [sqlite wiki](https://www.sqlite.org/rowvalue.html#scrolling_window_queries) there might already be an easy way to do this with the python sqlite library so that might be worth looking into as well

khuxkm commented

2018-08-19 10:57:23 +00:00

Alright, this LGTM!

khuxkm closed this pull request

2018-08-19 10:58:31 +00:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: tildeverse/quotes#3