The comments schema, prior to December 2021, used parent_comment_id
instead of also storing top_comment_id. Comment pagination is based
now on top_comment_id. However, upstream never migrated their old
comments to populate tc_id, and thus retained two copies of pagination
logic, each using different limits to try to emulate similar behavior.
TheMotte foremost has no posts created prior to December 2021 (so
these branches never activated) and also has tc_id on all comments.
The dual limit pagination approach was already removed (there is
only one limit for paginating comments). This completes the removal of
this logic, since these are purely dead codepaths which have previously
caused confusion to contributors.
This is likely not an issue for production (since each request will
get its own SQLAlchemy session), but `scoped_session` results in the
tests reuseing the same Session across tests. The tests rely on
the default session expiry behavior.
Following #485, we began investigating post/comment rendering
bottlenecks. The most immediate issue is the eager comment loading
(merged in 23a8fb9663) did not seem fully operative: query logs
showed comments and associated FKs were being lazy loaded again
(linear query quantity in number of rendered comments). In fact,
CPU load seemed even worse than previous lazy loading.
Bisect revealed first bad commit: fb77cbcc2b
which fixed post view counters by committing the SQLAlchemy session
instead of flushing, following upstream's fix. However, committing
a session has the unfortunate side effect of dumping cached session
objects, such as the previously loaded comment objects and their
relationships, causing fallback to the old lazy behavior.
We fix this here by explicitly telling SQLAlchemy to not expire
the session on commit.
Hopefully this will simultaneously resolve the elevated DB CPU load
observed in production and speed up page rendering again.
The 15-line <pre> block at the bottom of the sidebar was a fix for
mobile hamburger menu behavior on upstream, where the long mobile
sidebar would be covered by the bottom navigation drawer.
In-place replacement would be `margin-bottom: 15em`, but it's not
actually necessary for the sidebar as used on TheMotte, and it adds
an unsightly block of whitespace to the bottom of the homepage.
Also the `style` attribute width was related to `GET /sidebar`
behavior (sidebar.html), which is unused on TheMotte, having been
replaced in the signup template with a link to `/rules`.
Fixes#483. Implement a Jinja template filter to shuffle a sequence
(which surprisingly doesn't already exist) and shuffle the relevant
lists in the sidebar.
