buggyrace / buggy-race-server Goto Github PK

Most obviously, remove the py library import I introduced in the admin work.

It's not being used: it's effectively a redundant dev library:

NOTE: this library is in maintenance mode and should not be used in new code.

I suspect its inclusion is due to the IDE adding it without me realising due to some keystroke error, genuinely baffled.

We added it to requirements deploy here e795601 because this was a breaking change, but this is the wrong solution.

I haven't yet found a way to get the placeholder text for the Jinja template to reflect the institution short name.

Looks like it was taken out of the registration form, but it's useful when setting up a new site (unless we make injecting users/SQL directly into the DB which I don't recommend — this feels too useful when setting up.

When downloading buggies, it might be sensible to (automatically? optionally?) exclude any non-enrolled students' buggies from the list.

It's almost certainly wrong to include buggies from any student accounts that aren't active (since that's effectively how students are removed from the course).

A server might need to run collecting:

• server username
• institution's username
• email

and any combination of those.

Actually using institution's username (and password) for login auth (LDAP etc) ought to be an option too but that's a chunk of worry.

The buttons are in but not wired up yet.

Remember to demand an authorisation code.

It would be good to consolidate all the places we're using environment variables — Flask is loading them and exposing them via settings.py so that feels like the right place to be getting them all from — I've reached directly into os.env elsewhere in the code and that means I'm straying away from knowing what and where the defaults are. It already made me suspicious that the FORCE_REDIRECT_HTTP_TO_HTTPS setting might not have been working.

Maybe the only thing to do is backtick to code conversion (maybe bold too?)

currently there's a code change to announce project message — would be better to have these in the database so admin can set them up without code changes. It's very likely we'll need them again.

The existing solution used its own div but maybe could use flash messages + CSS?

It's very likely this will be a useful customisation for different installations

The VS code integration for the last RHUL run (yay!) also introduced a requirement for ENV var CS1999_PORT to be set.

Make sure the default — in the event of the environment variable not being set — is to just work. Must be able to run the buggy editor standalone (defaulting to Flask's default port) — even if we don't expect it to be used in that way.

This might include being clear about how we're using VS Code and what config is needed on the institution's server instance — affects documentation as well as possibly other things I don't know about.

This is in addition to how docs may be customised.

It's also possible that a module code is not the same as the name of the project (at RHUL it happened to be the same), so maybe replace with two settings – PROJECT_SHORT_NAME and PROJECT_NAME.

Now using get_tasks_as_issues_csv in the utils.py module when the server is running... but we were doing this "offline". Need to make sure the code is identical (some refactoring to do); the new version needs the application configuration to know how to construct the task URLs.

When the auth code config setting is stored in the database, it's in plaintext and shouldn't be

Now each task is in the database, add capability for students to record the "report" notes on the server as they go along.

And allow downloads of HTML (or other formats?) for the poster.

Need to make these configurable and customisable (and not display if empty)

• link URL
• link text
• description?
• icon

The idea was that there's a race log that shows the results.

Maybe a static log file that is manually added to the server? Obviously this should be automated.

Relevant to: 5-RACELOG

In flash messages (e.g., failure to change a user's password) should run username through a util method that titlecases it if IS_PRETTY_USERNAME_TITLECASE is set — no point having it if we're not being consistent throughout the whole site.

When displaying user table to admin, the user can choose which columns are visible. The buttons are filled/outlined depending on whether or not they are selected.

Currently, pressing "all columns" correctly toggles the state of the other buttons, but the state of the "all columns" button should be deselected as soon as all columns are not being displayed.

The "poster" requirement is a silly legacy from the original marking scheme for an "individual scientific project".

Currently we've got a very simple environment-variable driven mechanism: if your username is in the (comma-separated) list of usernames in ADMIN_USERNAMES then you're admin.

Really this would be better (more robust) as an is_admin field in the user model.

The old requirements.txt got removed in issue #21 however Heroku requires this file in the root.

For now, I have just made it recursively call requirements/prod.txt. We can tweak this later.

In effect this means — where possible — the settings should be stored in the database. The config settings should still exist and be used as defaults when the project/database is initialised (also, there are some core settings — such as database credentials — where they can't belong).

But it's going to greatly facilitate setup for staff users if settings that are currently shown in admin/settings/ were editable there too. (Obvious examples are adding social media links, etc, but also institution names, etc.

We've already set up the Cloudflare https page rules on the domains but need to tidy up (or decide to remove) the warnings in the Flask code — requests that originated as http but are handled by Cloudflare seem to be insecure from Flask's POV; check what is feasible here to remove confusing warnings on the page.

By default, static pages are hosted (successfully!) on the server itself now. But it must be possible to pass that out so, ultimately, anything can be overridden, especially in the tech notes which can be quite opinionated (and we're not offering a mechanism for updating them through the web interface).

Currently there's a misleadingly named (legacy) setting GITHUB_PAGES_URL — this is no longer sensible (and shouldn't be in the "GitHub" settings group either (probably makes most sense in "Server") because it's not github pages. Rename to something like TECH_NOTES_URL, and should be blank if it's on this server (the default). However, that doesn't allow tech notes to be entirely suppressed.

(Currently the other item like this is the PROJECT_WORKFLOW_URL, but that's a single-page case. Also, dangerously, that currently has three behaviours:

1. if it's empty, suppress the link entirely (there is no project workflow page)
2. if it starts with http then it's an external URL
3. if it's anything else force the default of project/workflow (yes even binky)

This seems wrong, so maybe should validate the entry to makes sure if it's not 1. or 2. it can only be project/workflow. This example is probably the model for the TECH_NOTES_URL

There is presumably an equivalent case for the three (or maybe two) project pages being hosted externally:

• project index
• project report
• project tasks → this seems less relevant, since we're allowing task content to be edited on the server

Ultimately this is all about when the admin can and cannot edit text that's presented to students through the server.

Flask is sometimes (correctly) borking with a 400: "The CSRF token has expired" after long period of inactivity.

Should present that with a nicer error handler (as we have for, eg. 500 and 403) and if that's the message explain it — because we know people seeing it are learning about HTTP!

(add a link to https://en.wikipedia.org/wiki/Cross-site_request_forgery )

This isn't needed for RHUL, because it will be used for that, so this issue is also to ensure that the existing mechanism exists (e.g., having moved tasks into the database, there's no longer an issues.csv in the repo, so need to check we're picking the generated one up correctly.

There might be quite a lot of work to do checking how much technotes (and workflow?) is affected by not having the GitHub integration (my suspicion is: there's a lot, and this becomes a big issue).

Deployments are failing due to buildpacks being incompatible with the version of Python we are using.

The docs* specify that Python 3.9.13 is supported on all stacks, hence we should switch to this one. 3.10.5 is also available however bleeding edge is not what we're going for.

*https://devcenter.heroku.com/articles/python-support#supported-runtimes

This makes deployment, i.e., the server, the standalone and complete project server.

Proposed solution: to generate the docs /info/ directory that is server as static HTML using Jekyll as that requires minimal change to existing info pages, of which there are plenty).

This could be an issue on https://github.com/RHUL-CS-Projects/CS1999-buggy-race-editor but I don't want to do that while the project is running.

It turns out my belief that there are not useful diagnostics coming back from SQLite is simply wrong (maybe it's changed since 18 months ago? hard to believe) — so maybe that except block should only be catching sqlite3.OperationalError exceptions.

This would have been much kinder to the students because it does send back errors like:

sqlite3.OperationalError: no such column: qty_bananas

This just needs fixing with something like this:

        except Exception as e:
            con.rollback()
            msg = f"error in update operation: {e}"

or

        except sqlite3.OperationalError as e:
            con.rollback()
            msg = f"error in update operation: {e}"

Argh! this would have been much kinder!

There's a problem with the bulk upload timing out but maybe we can replace this with a mechanism for injecting CSV → SQL directory (i.e., not by uploading to the web).

Didn't investigate why the existing method is _so inefficient though — but it is: times out if too many (and yet, not that many) users are added in one go.

There are currently a couple of places in admin (generating tech docs, working with tasks) that use forms that are missing an authorisation code input.

Need to check there are no unexpected gotchas with latest version of Flask maybe regarding Python version.

Specifically, added the buggy illustrations with hardcoded URLs: fix that

How does this work in CSS assets? (answer: presumably don't worry about it)

Can't do this until #52 is in place. Not sure how hard we should look to determine if text is meaningful.

Because we have an opportunity to refactor, we should seize it

It's currently all over the place for no good reason, maybe tidy it up.
One consequence is the views are disorganised so it's harder than it should be to find the right method/route in the code.

The requirements/prod.txt want Flask 1.1.2 but Flask V1.X.X is no longer supported.

Recommend: go up to 2.1.2

Note that currently the 1.1.2 requirement breaks the build, because (from stackoverflow):

Jinja is a dependency of Flask and Flask V1.X.X uses the escape module from Jinja, however recently support for the escape module was dropped in newer versions of Jinja

⚠️ Note: we need to update the Buggy Editor code too ⚠️

e.g. in init_db.py

print("- Opened database successfully in file \"{}\"".format(DATABASE_FILE))

f-strings are much friendlier to new programmers than Python's old clunky way.

I think we can be generous and allow for a demo buggy race server, open for general registration on the main page to allow people to get a feel for how it works. This would just require some adaptations to the configs but we could run https://demo.buggyrace.net.

We are already deploying to a dev environment, so in the end it makes sense to move this over to a fully functional (or partly mock-up, i.e. admin features) demo environment.

And currently it's false which is stupidly showing up as 5 characters.

Might be tailwind

am not enjoying using bootstrap and I think I could do a nicer job if I was out of it... but it's going to be quite a big job.

History: Bootstrap was part of the cookie-cutter repo that seeded this one, and did what it said on the tin. I just... don't like using it very much.

Currently the explicit warning of no https is distracting and potentially wrong (because the Flask app might not know about the https status since it's being applied outside — I think we saw this before).

Either take it out completely or make sure it's conditional on the (probably best OFF) FORCE_REDIRECT_HTTP_TO_HTTPS setting that isn't doing much except getting in the way at the moment.

The API page helpfully links to task 5-API (which is when students need it¹) but that link will break if the task list is edited so either the phase or (less likely?) name are changed.

So "TASK_FOR_API" might be a config setting so, if it's changed, the docs/pages can keep up to date.

There are a few other places where the tech notes mention tasks too: need to think about this because currently admins can edit the tasks but not the tech_notes.

¹ ...although there's a good case for that being in phase 4 because it's interesting and less involved than the user stuff)

See DEVELOPMENT.md for current state-of-play

Doesn't run especially publicaly but the results should be published.

If it's not automated yet, needs to be published as HTML on the server by hand.

General idea:

• check eligibility of buggies:
  1. rules are not violated (only even numbers of wheels)
  2. calculated cost of buggy is below race limit
• manage in turns
• each turn calculate
  • acceleration depending on mass and power
  • risk of puncture
  • risk of catastrophe
  • update mass by removing spent fuel
  • switch to aux power if the primary is used up
  • stop when either no buggies are running or one has... won?

If a user forks the editor repo and then deletes it from their own account, if they fork it again (because our button correctly reappears), then the new (second and beyond) repo gets created with -1, -2-, -3 etc appended on the end. That is, GitHub remembers that this repo used to exist, and doesn't create the new one with a name clash. It looks like this is breaking the subsequent work on the repo injecting images.

I think this is new behaviour on GitHub, because I deleted and re-forked the repo multiple times when we were first setting this up, and it didn't used to happen.

Heroku should deploy the additional postgres resource automatically.

This was always the intention: in addition to the API secret that the user sets for themselves — admin should be able to set an API key for any user. This more closely models a useful application (that the API key is granted on a per-user basis by admin, and also that the secret set by the user is similar to e.g., payment or transaction info) and also notifies admin/staff when a student is attempting this task (5-API)

Should always be inserting PROJECT_SLUG as a prefix if there is one, otherwise a slugified PROJECT_CODE if there is one (otherwise: no prefix).

Now we have quite a few download files — both for admin and students — this should be available in the utils module.

The responsive navbar from cookiecutter-flask never seemed to work so need to fix that, because it's obvious (current bodge is to jump to the footer which contains the same links)

This is what the buggyrace site is for.

1. Most inefficient: render it live on every request, which includes markdown conversion. Implement this first...
2. Then call that render_template once, and store the output in a known file, which is then served by flask. This is probably the best middle way
3. Maybe? Investigate optionally adding it to tech notes — but I don't think there's any need if 2 works well enough.

The only concern is lack of persistent filestore in some implementations (e.g., heroku), which means should render the template on startup. This might be correct; and then record the timestamp when it happened.