search

lionleaf / dwitter

Social network for short js demos
https://www.dwitter.net
Apache License 2.0
771 stars 67 forks source link

Add feeds for top week/month/year/all-time #427

Closed lionleaf closed 5 years ago

lionleaf commented 5 years ago

Add 4 variants of the top feed; top of the week (last 7 days), top of the month (last 30 days), top of the year (last 365 days) and top of all time (the old top behaviour).

Also adds a dropdown menu to select between the new feeds.

And adds new unit tests.

screen shot 2018-11-25 at 11 16 33 pm screen shot 2018-11-25 at 11 08 23 pm screen shot 2018-11-25 at 11 08 29 pm screen shot 2018-11-25 at 11 08 43 pm

Default top feed is "top of the month", which will hopefully be more dynamic than top of all time.

Note; I made a conscious decision to pick a handful of specific feeds (top/week, top/month, top/year, top/all) rather than a more flexible url-parameter (eg. top?days=23). This gives us fewer "official" rankings with nice URLs that the whole community get the same small handful of lists. I know it's easy to want full configurability; but I'm opting to keep things simple for now :D

Would be great to get some feedback on the code :)

lionleaf commented 5 years ago

View code has some possibilities for more sharing of code between "top" view classes, but it's not important to fix for now, imo.

  • @stianjensen

Yeah, I know I could reduce the code reuse significantly there; but I didn't want to go overboard with sub-classes; opting for more repitions in the hope that it might be more straight-forward to understand for other people dabbling in dwitter. That said; I'd love to get your take on the best balance and how you would do that? I was considering a new "TopDweetFeed" class with the common new parts (context etc) and then subclass that.

stianjensen commented 5 years ago

Yeah, I know I could reduce the code reuse significantly there; but I didn't want to go overboard with sub-classes; opting for more repitions in the hope that it might be more straight-forward to understand for other people dabbling in dwitter. That said; I'd love to get your take on the best balance and how you would do that? I was considering a new "TopDweetFeed" class with the common new parts (context etc) and then subclass that.

One thing I was looking at was that the num_likes annotation was repeated across both top and non-top feeds, and is basically needed in all views. So somehow reworking the get_dweet_list helper on the base class to always annotate, and make the other views just attach more filtering to that instead of starting from Dweet.objects could be nice.

lionleaf commented 5 years ago

Did a little refactoring and cleanup of feed/views.py . What do you think @stianjensen ?

stianjensen commented 5 years ago

LGTM, ship it!