CTFd / CTFd

CTFs as you need them
https://ctfd.io
Apache License 2.0
5.54k stars 2.06k forks source link

Support Select Fields for Custom Fields #2102

Open Matir opened 2 years ago

Matir commented 2 years ago

It would be nice to have select fields available for custom fields. This can be used for things like:

I'm happy to send a PR with a naive implementation that does the following:

  1. Adds a new field to the DB table Fields to support storing the choices as a serialized JSON object (db.JSON)
  2. Adds the necessary UI for managing the custom fields, with a textarea for choices (one per line)
  3. Modifies attach_custom_team_fields to provide a select field

Please let me know if this seems reasonable. I can do it as a plugin, but it feels broadly useful and easier to implement in core.

ColdHeat commented 2 years ago

I think this is a good idea but the implementation likely requires some finesse. I actually avoided implementing select because it wasn't clear where to store the options. I didn't really want to create a new table for it because of how it would affect the export/import process but the JSON fields themselves also kind of affect the import process.

I think I am happy to take your PR but I do not think it's something that would be able to be released until the next planned minor release (3.6).

Matir commented 2 years ago

That's fair. Can you explain how JSON fields affect the import/export process? (At least in the context of this.)

I tried to implement it completely as a plugin, but it seems plugins are loaded after the schema for existing tables is fixed, so neither adding a column by monkey patching (i.e., models.Fields.newcol = db...) nor creating a new Polymorphic table on models.Fields with additional columns works.

Obviously an implementation with its own tables is quite possible, but it comes with baggage.

(Separately, is there a way to get any of these fields to the scoreboard view? I think not currently without overloading the view.)

ColdHeat commented 2 years ago

Hi sorry for the delay

  1. On JSON fields: Different databases store JSON differently. MySQL & Postgres have a dedicated JSON type but MariaDB treats it as a string. During import/export some glitches can happen when moving between databases and JSON fields will be loaded as strings or not as strings. See code sections like https://github.com/CTFd/CTFd/blob/073d4b7cf90327cdcedbea41bd78cac6fc9d8b4d/CTFd/utils/exports/__init__.py#L316-L342 where CTFd needs to make some educated guesses about how to handle the data. We might be able to improve the export code into properly serializing everything but it technically relies on a library called dataset that I haven't gotten around to removing but I do want to.

  2. On plugins: I think you can do it in a plugin but it will be tricky. Instead of adding a new field you would add a new relationship so the the Fields have a back reference to a second table. I think this is probably a bad idea and I do want to support this in CTFd directly.

  3. On scoreboard view: It's possible but you need to customize the get_standings() function. The issue is that it's been really optimized and hard to customize: https://github.com/CTFd/CTFd/blob/073d4b7cf90327cdcedbea41bd78cac6fc9d8b4d/CTFd/utils/scores/__init__.py#L11. I think I did have an issue I was working on where get_standings could be converted to return a SQLAlchemy object so you could query relationships but I think I failed for some reason or another. Instead I think I would recommend creating some kind of sidecar function that gets you the data you want and then render it altogether in scoreboard.html.

aarchangel64 commented 3 weeks ago

Hey, sorry to necro an old issue but I was wondering if there's any updates on this. I'd also be willing to help develop a PR, but I just wanted to know if any of the information here has changed. Thanks!

ColdHeat commented 3 weeks ago

There are no changes on this so far. I would like the feature myself but have not had the time to implement it so if you have the spare time to figure out a PR go for it.