Technical debt

[iragm]

A large chunk of this project was written in a single week. It was the first django project I'd ever built, and it shows. Now that auction.fish has grown to the point where others are contributing code, there's hundreds of daily visitors, and dozens of clubs actively using the site, it's time to think about refactoring some of the petco-tier code.

First, the site is "feature-complete", everything (?) works, and there's no roadmap towards a "version 2".

Second, I get near-daily requests for new features and a lot of these are really excellent ideas worth implementing, so development seems to be ongoing.

Here is a list of some of the stuff that I'm not super proud of:

• Tests. The tests are very incomplete, use sqlite instead of mysql, and need to be reafactored to use a single base class that sets up an auction, users, etc. ChatGPT can bang out test for most views in seconds once the base class has been created. I don't see a need to have 100% test coverage, but certainly a test for each model property and a couple tests per view (logged in, not logged in, joined auction, auction admin) are needed.
• Views. There are still some function based views that need to be rewritten as class based viewed. And many, many views look for auction in dispatch and can be refactored to use a common AuctionViewMixin
• Signals. Need to be split into signals.py.
• Location. GeoDjango didn't work when I played around with it 4 years ago, so location stuff uses a function called distance_to(). It works great, but there's quite a bit of redundancy in the code. Either move to GeoDjango, or create a new class that adds annotates get_queryset() with distance_to(), sets that latitude and longitude from cookies/userdata as appropriate, and puts up a "hey set your location" message if needed.
• Emails. It's quite hard to test things as they are. There's a lot of code that's currently in management commands; this should be moved to a model properties. As an example, see auctiontos_notifications.py, which does a lot of stuff. Instead, it could just fetch all auctiontos where x email has not been sent, loop over them, send the email and mark sent if appropriate and that's it.
• On the note of emails, we need to move from cronjobs to celery - there's enough different tasks at this point that it's worth working on this, although the cronjobs all work without issue.
• Directly related to model properties, celery, and management commands, endauctions.py is a critical piece of this project and it's a mess of code.
  • Moving actions to model properties would make it much more readable
  • Sold lots could be omitted (if we set active=False when buying now)
  • The filter that's used to find lots could easily select only lots that are ending gte=now, and a separate filter could be used to send the "ending soon" websocket message
  • Again, because it works as-is, this isn't a high priority, but it could just be so much more efficient
• Security. Overall the security is pretty good, but it's managed in several different functions (for images, lots, auctiontos, auctions, etc.). An issue was reported (UI for editing lots is inconsistent #193) and although this issue caused a user that should have had permission to not have permission, it's possible that other issues exist. (I did review the code and didn't see anything alarming, but still). While the site does not handle payments and the worst that can happen is leaking someone's email, there's absolutely no reason not to refactor the security code and add full test coverage for it. Rather than the single class and then some random functions called here and there, there should be a single class which calls a permissions_check in dispatch.
• Try/except code: A lot of the try except code can be removed altogether, for example the UserData is now always created as soon as the user is saved, so it can be accessed at user.userdata without a try except block. There's also a lot of places where a dict.get() with a default value is a lot cleaner than try except.

All of this stuff has been back-burned for years now, but PRs to fix anything are more than welcome.

[jamescurtin]

Happy to help here; will submit a PR (or two) to kick this off sometime later this weekend.