9895fa1bba
changed in it the comments.html template (along with submission.html) has numerous undesirable properties which i will describe now. unless you are very familiar with the codebase, it can be extremely difficult to grok. this is pretty insane as there is nothing fundamentally complex about the goal of comments.html: return a component that shows a username and info, reports if any, comment content, and actions a user can take. this behemeoth was initially 886 lines in the old version of this codebase, and this is with awards and a lot of other cruft removed. anyway, the maintainability of this file is about on par with some legacy application that keels over and dies if you sneeze vaguely in its direction. the nicest thing i can say about it is that it isn't currently crashing. anyway some of the problems include: * large, splittable components, are not split into separate files. this makes it incredibly difficult to find or make changes across the template and makes it nearly impossible to find or change a specific thing. this is most easily exemplified in the modals, which should by all accounts be separate templates, just inlined into comments.html. * the nesting is oftentimes incorrect. inexplicably, probably out of laziness from when the code was first written, things will end up fully left aligned, while multiple layers deep into a nesting context. if an if statement or an endif is changed, it is *incredibly* difficult to figure out where the error was. you can't trust the nesting. * multiple repeated checks for things that are always true. this is probably a symptom of the above two problems but it's very noticeable once you fix the nesting. for example there is a block near the very top of the actions bar which checks for parent_submission which effectively checks "is this in a post" (this commit won't complain about parent_submission checks but i do have opinions on those). all of the action buttons further down the chain also check for parent_submission, or even check inconsistently (by using if c.post) within this context this is a completely unnecessary check in this context. while it is potentially useful (and in fact because #251 requires we dismantle the assumption a little bit) to have these checks now, the fact that they were initially added shows that when the code was all initial written, there was little care into thinking about comment state. * mobile actions are duplicated and duplicated inline. i actually do find it probably pretty hard to support this normally given the codebase's DOM so whatever, duplicate the things, *but* if we're going to do that, inlining it into the middle of an incredibly long template is really difficult to comprehend as a design decision. ...anyway yeah this PR intends to fix these problems and enable work to be done on #251. this is a "perfect is the enemy of good" commit. it doesn't change much fundamental and is not intended to erase the sins of the original file, but at least make it maintainable. this also fixes a minor bug with #473 where the GIF modal was left in by accident. |
||
|---|---|---|
| .github/workflows | ||
| bootstrap | ||
| files | ||
| migrations | ||
| thirdparty/sqlalchemy-easy-profile | ||
| util | ||
| .dockerignore | ||
| .editorconfig | ||
| .gitattributes | ||
| .gitignore | ||
| dependabot.yml | ||
| docker-compose-operation.yml | ||
| docker-compose.yml | ||
| Dockerfile | ||
| LICENSE | ||
| poetry.lock | ||
| pyproject.toml | ||
| readme.md | ||
| site_settings.json | ||
This code runs https://www.themotte.org .
Installation (Windows/Linux/MacOS)
1 - Install a container runtime and the Docker commandline tools on your machine.
On Windows, Docker will pester you to pay them money for licensing. If you want something less proprietary, consider Container Desktop or Stevedore instead.
2 - Install Git. If you're on Windows and want a GUI, Github Desktop is quite nice.
3 - Run the following commands in the terminal or command line for first-time setup:
git clone https://github.com/themotte/rDrama/
cd rDrama
4 - Run the following command to start the site:
docker-compose up --build
The first time you do this, it will take a while. It'll be (much) faster next time.
4 - That's it! Visit localhost in your browser.
Code edits will be reflected (almost) immediately. If you make any setup changes or database changes, you'll need to ctrl-C the docker-compose status log and run docker-compose up --build again.
Run the E2E tests:
./util/test.py
Database Stuff
What is a migration?
Prior to the fork of themotte from rDrama, there were no database migrations, and the database schema was stored in schema.sql. Any time a column or such was added to a model, one hoped that the author remembered to update schema.sql to add that column. One of the first changes we made after forking this repo was to add database migrations using Alembic.
Database migrations are instructions for how to convert an out-of-date database into an up-to-date database. This can involve changing the schema, or changing data within the database.
Why use database migrations
Database migrations allow us to specify where data moves when there are schema changes. This is important when we're live -- if we rename the comments.ban_reason column to comments.reason_banned for naming consistency or whatever, and we do this by dropping the ban_reason column and adding a reason_banned column, we will lose all user data in that column. We don't want to do this. With migrations, we could instead specify that the operation in question should be a column rename, or, if the database engine does not support renaming columns, that we should do a three-step process of "add new column, migrate data over, drop old column".
Database schema change workflow
As an example, let's say we want to add a column is_flagged to the comments table.
- Update the
Commentmodel infiles/classes/comment.py
from files.__main__ import Base
class Comment(Base):
__tablename__ = "comments"
id = Column(Integer, primary_key=True)
...
+ is_flagged = Column(Boolean, default=False, nullable=False)
- Autogenerate a migration with a descriptive message. To do this, run
./util/command.py db revision --autogenerate --message="add is_flagged field to comments"
This will create a migration in the migrations/versions directory with a name like migrations/versions/2022_05_23_05_38_40_9c27db0b3918_add_is_flagged_field_to_comments.py and content like
"""add is_flagged field to comments
Revision ID: 9c27db0b3918
Revises: 16d6335dd9a3
Create Date: 2022-05-23 05:38:40.398762+00:00
"""
from alembic import op
import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = '9c27db0b3918'
down_revision = '16d6335dd9a3'
branch_labels = None
depends_on = None
def upgrade():
op.add_column('comments', sa.Column('is_flagged', sa.Boolean(), nullable=False))
def downgrade():
op.drop_column('comments', 'is_flagged')
-
Examine the autogenerated migration to make sure that everything looks right (it adds the column you expected it to add and nothing else, all constraints are named, etc.) If you see a
Nonein one of the alembic operations, e.g.op.create_foreign_key_something(None, 'usernotes', 'users', ['author_id']), please replace it with a descriptive string before you commit the migration. -
Restart the Docker container to make sure it works.
docker-compose up --build
So what's up with original-schema.sql, can I just change that?
No, please do not do that. Instead, please make a migration as described above.