Remove topic sort options

sync-002501 commented 4 years ago

This change forces all threads ("topics") to be viewed in newest-first order. This is achieved by:

Removing the sort order dropdown on the topic page
Removing the comment_order parameter from the topic view and hardcoding the sort order to NEWEST
Removing the default comment sort order option from the settings page, user model, and web API
Creating an Alembic migration removing the setting from the DB schema

Closes #5

sync-002501 commented 4 years ago

This is just a quick and dirty solution to get familiar with how the application is structured. Will consider a more configurable/upstreamable approach over the weekend.

Deimos commented 4 years ago

From my perspective (as the dev/owner/whatever of Tildes), I think changes like this one might be annoying to make upstreamable. You'd need global-ish settings for whether users are allowed to change comment sorting and what the default order is. You probably wouldn't want to make any database changes, since you wouldn't want to require re-adding database columns if the setting is toggled.

That's not a huge amount of work, but it's just additional effort and complexity that doesn't really have any direct benefit for you except maybe reducing future merge conflicts when you bring in new code from upstream. There are a fair amount of changes that you're planning to make overall, so it might be more effective to just go with the quick solutions like this initially, where you can just delete chunks of code and do it the easy way, and then worry about redoing some of it later if it ends up causing a lot of merge issues in practice.

If you want to have the site ready to use relatively soon, I think it's probably going to be best to just get things done the easy way, and it can always be redone later once the site's already running.

sync-002501 commented 4 years ago

Makes sense to me! I'll leave this one as-is unless our admins want changes for the MVP.

zorbathut commented 4 years ago

Yeah, agreed with @Deimos here; especially given that we might not even end up using this site, it seems not worth jumping through huge hoops for configurability. I may regret this if we end up having to go back and rewrite stuff later, but I'm fine doing that if that's what it takes.

I was originally looking at this and thinking "hmm, I don't know if a database schema change is good, maybe we should just keep the column and ignore it", but y'know what, the column removal is such a small part of the change, and this works as a great testbed for schema changes, so let's just go ahead with it and see what happens.

Merging!

themotte / tildes

Remove topic sort options #20