Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 49 additions & 4 deletions app/eventyay/cfp/flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.db.models import Q
from django.forms import ValidationError
from django.http import HttpResponseNotAllowed
from django.utils.datastructures import MultiValueDict
from django.shortcuts import redirect
from django.urls import reverse
from django.utils.functional import Promise, cached_property
Expand Down Expand Up @@ -219,14 +220,40 @@ def get_form_initial(self):
return copy.deepcopy({**initial_data, **previous_data})

def get_form(self, from_storage=False):
if self.request.method == 'GET' or from_storage:
# Cache form initial data to avoid repeated work
form_initial = self.get_form_initial()

if self.request.method == 'GET':
# For initial GET requests, use an unbound form populated from session data
# This shows saved values but intentionally does not display validation errors
return self.form_class(
data=None,
initial=form_initial,
files=None,
**self.get_form_kwargs(),
)
if from_storage:
# For validation checks, create a bound form with session data
return self.form_class(
data=self.get_form_initial() if from_storage else None,
initial=self.get_form_initial(),
data=form_initial,
initial=form_initial,
files=self.get_files(),
**self.get_form_kwargs(),
)
return self.form_class(data=self.request.POST, files=self.request.FILES, **self.get_form_kwargs())
# For POST requests, merge new uploads with existing session files
# This allows users to navigate back without losing previously uploaded files
session_files = self.get_files() or {}

# Preserve MultiValueDict semantics for proper multi-file field support
files = MultiValueDict()
# Add session files first
for field, file_obj in session_files.items():
files[field] = file_obj
# For each field, new uploads completely replace any existing session files
for field, file_list in self.request.FILES.lists():
Comment on lines +247 to +253
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Merging session_files into MultiValueDict can break multi-file fields and value types.

The merge assumes session_files.items() returns a single file per field, but get_files() likely returns a MultiValueDict-like object or lists for multi-file fields. Assigning with files[field] = file_obj causes MultiValueDict.__setitem__ to wrap values in a list; if file_obj is already a list, you get nested lists. And if session_files is a MultiValueDict, iterating over .items() drops all but the last file per field.

To preserve multi-file semantics, build files using setlist/lists() instead of plain item assignment, e.g.:

files = MultiValueDict()
if isinstance(session_files, MultiValueDict):
    for field, file_list in session_files.lists():
        files.setlist(field, list(file_list))
else:
    for field, file_obj in session_files.items():
        files[field] = file_obj

for field, file_list in self.request.FILES.lists():
    files.setlist(field, file_list)

This preserves existing multi-file values and avoids nested lists when there’s no new upload for a field.

files.setlist(field, file_list)

return self.form_class(data=self.request.POST, files=files, **self.get_form_kwargs())

def is_completed(self, request):
self.request = request
Expand All @@ -237,11 +264,29 @@ def get_context_data(self, **kwargs):
result['form'] = self.get_form()
previous_data = self.cfp_session.get('data')
result['submission_title'] = previous_data.get('info', {}).get('title')
# Add information about uploaded files for display in templates
saved_files = self.cfp_session.get('files', {}).get(self.identifier, {}) or {}
result['uploaded_files'] = {
field: file_dict.get('name') for field, file_dict in saved_files.items()
}
return result

def post(self, request):
self.request = request
form = self.get_form()
action = request.POST.get('action', 'submit')

# For "back" action, only save data if form is valid
if action == 'back':
if form.is_valid():
self.set_data(form.cleaned_data)
# Always save files if present
if form.files:
self.set_files(form.files)
prev_url = self.get_prev_url(request)
return redirect(prev_url) if prev_url else redirect(request.path)

# For "submit" and "draft" actions, validate as before
if not form.is_valid():
error_message = '\n\n'.join(
(f'{form.fields[key].label}: ' if key != '__all__' else '') + ' '.join(values)
Expand Down
14 changes: 12 additions & 2 deletions app/eventyay/cfp/templates/cfp/event/submission_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ <h2>{% block submission_step_title %}{{ title|default:'' }}{% endblock submissio
</div>
{% block submission_step_text %}<p>{{ text|rich_text }}</p>{% endblock %}
{% block inner %}
{% if uploaded_files %}
<div class="alert alert-info mb-3">
<i class="fa fa-info-circle"></i> {% translate "Previously uploaded files:" %}
<ul class="mb-0 mt-1">
{% for field_name, filename in uploaded_files.items %}
<li><strong>{{ field_name|default:'' }}</strong>: {{ filename }} {% translate "(will be preserved if you don't upload a new file)" %}</li>
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field name is used directly in the template display without any sanitization or prettification. Field names like "id_image" or "proposal_file" will be displayed as-is to users. Consider either using the field's verbose_name/label from the form definition, or adding a filter to make the field name more user-friendly.

Suggested change
<li><strong>{{ field_name|default:'' }}</strong>: {{ filename }} {% translate "(will be preserved if you don't upload a new file)" %}</li>
<li><strong>{{ field_name|default:''|capfirst }}</strong>: {{ filename }} {% translate "(will be preserved if you don't upload a new file)" %}</li>

Copilot uses AI. Check for mistakes.
{% endfor %}
</ul>
</div>
{% endif %}
{{ form }}
{% endblock inner %}
{% block buttons %}
Expand Down Expand Up @@ -76,9 +86,9 @@ <h2>{% block submission_step_title %}{{ title|default:'' }}{% endblock submissio
</div>
<div class="col-md-3">
{% if prev_url %}
<a href="{{ prev_url }}" class="btn btn-block btn-info btn-lg">
<button type="submit" class="btn btn-block btn-info btn-lg" name="action" value="back">
« {{ phrases.base.back_button }}
</a>
</button>
{% endif %}
</div>
</div>
Expand Down
4 changes: 4 additions & 0 deletions app/eventyay/cfp/views/wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ def dispatch(self, request, *args, **kwargs):
or step.identifier == 'user'
],
)
if request.method == 'POST' and request.POST.get('action') == 'back':
# When clicking Back, the step's POST handler has already saved the data
# Now redirect to the previous step
Comment on lines +75 to +76
Copy link

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "When clicking Back, the step's POST handler has already saved the data" but this is only partially true. Looking at the flow.py implementation, the data is only saved if the form is valid (line 281). If the form is invalid, no data is saved but the user is still redirected back. Consider updating this comment to reflect this nuance or ensuring the back button behavior is consistent with the comment.

Suggested change
# When clicking Back, the step's POST handler has already saved the data
# Now redirect to the previous step
# When clicking Back, the step's POST handler has already processed the data
# (and, if valid, saved it). Now redirect to the previous step.

Copilot uses AI. Check for mistakes.
return result
if request.method == 'GET' or (step.get_next_applicable(request) or not step.is_completed(request)):
if result and (csp_change := step.get_csp_update(request)):
result._csp_update = csp_change
Expand Down