Replace global custom team data field with per-hunt questions#39
Replace global custom team data field with per-hunt questions#39dlareau wants to merge 2 commits into
Conversation
Registration and leaderboard "custom team data" was a single site-wide constance field (text/boolean only), shared across every hunt and mislabeled on archived hunts once the setting changed. Replaces it with per-hunt TeamDataQuestion/TeamDataAnswer models supporting multiple questions with text/boolean/select types, a dedicated staff config page, generalized leaderboard columns/grouping, and a data migration to carry over existing custom_data values before the old field is removed.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
dlareau
left a comment
dlareau
left a comment
There was a problem hiding this comment.
Mostly good, some things done in odd ways that I want to discuss.
| return val | ||
|
|
||
|
|
||
| @register.filter() |
There was a problem hiding this comment.
Where is this used? It isn't always bad to make a get_item filter, but usually it can indicate bad view/template separation.
| return {} | ||
| result = {} | ||
| for a in TeamDataAnswer.objects.filter(team__in=teams_queryset, question__in=questions).select_related('question'): | ||
| result.setdefault(a.team_id, {})[a.question_id] = a.display_value |
There was a problem hiding this comment.
Why is this bulk function needed vs doing database functions like select_related?
Also is this the only place we do defaults? I feel like this is an out of the way place to have defaulting behavior
| return redirect('puzzlehunt:staff:team_data_questions', hunt.pk) | ||
|
|
||
|
|
||
| def _apply_question_form_data(question, request): |
There was a problem hiding this comment.
why does this method exist instead of using django's form capabilities?
| return [opt.strip() for opt in raw_options.replace('\n', ',').split(',') if opt.strip()] | ||
|
|
||
|
|
||
| def _question_list_response(request, hunt): |
There was a problem hiding this comment.
This doesn't feel like enough logic to be worth a helper
| return render(request, "staff_hunt_puzzles.html", context) | ||
|
|
||
|
|
||
| def _parse_question_options(raw_options): |
There was a problem hiding this comment.
If this needs to be made a helper move it to utils and make it more generic.
| question: TeamDataQuestion instance being edited, or None when adding a new question | ||
| action_url: URL the form should POST to | ||
| {% endcomment %} | ||
| <form hx-post="{{ action_url }}" hx-target="#team-data-question-list" hx-swap="innerHTML"> |
There was a problem hiding this comment.
Do we use form generators in the rest of this project? Like forms made from python data then inserted via tag? If so, why do we not use them here?
- Replace hand-rolled POST parsing + raw HTML staff form with a proper TeamDataQuestionForm (crispy-forms), matching every other form in the app. Drops _apply_question_form_data, _parse_question_options, and the standalone form template entirely. - Replace the manual per-team answer dict cache and get_item template filter with prefetch_related + a positionally-ordered data_answer_values list, so templates iterate columns directly with no ID-keyed lookups. - Add TeamDataAnswer.NO_ANSWER_DISPLAY as the single canonical "no answer" placeholder, referenced by both the model's own blank-value case and the view-side missing-row case.
1 participant
No description provided.