Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
63 changes: 59 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 GET requests, use unbound forms with initial data
# This correctly displays saved values from the session
return self.form_class(
data=self.get_form_initial() if from_storage else None,
initial=self.get_form_initial(),
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=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
# New uploads take precedence over session files
for field, file_list in self.request.FILES.lists():
files.setlist(field, file_list)

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

Comment on lines 243 to 257
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

There's a potential issue with file preservation logic. When a user clicks back without uploading any new files, the session files are merged into form.files (lines 243-256). However, these session files are UploadedFile objects pointing to already-saved temporary files. When set_files is called with these files on line 295, it will call file_storage.save again, creating duplicate temporary files in storage without cleaning up the originals. Consider checking if files come from session (e.g., by comparing with self.get_files()) before saving them again, or only save files that are actually new uploads from request.FILES.

Suggested change
# 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
# New uploads take precedence over session files
for field, file_list in self.request.FILES.lists():
files.setlist(field, file_list)
return self.form_class(data=self.request.POST, files=files, **self.get_form_kwargs())
# For POST requests, only use newly uploaded files from the request.
# Previously uploaded files are preserved in session storage and
# should not be passed back as fresh uploads to avoid re-saving
# already stored temporary files.
files = self.request.FILES or None
return self.form_class(
data=self.request.POST,
files=files,
**self.get_form_kwargs(),
)

Copilot uses AI. Check for mistakes.
def is_completed(self, request):
self.request = request
Expand All @@ -237,11 +264,39 @@ 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, {})
result['uploaded_files'] = {
field: file_dict.get('name') for field, file_dict in saved_files.items()
} if saved_files else {}
return result

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

# For "back" action, save data even if form is invalid
if action == 'back':
if form.is_valid():
self.set_data(form.cleaned_data)
else:
# Save raw POST data for invalid forms (user might be going back to fix something)
# Filter out non-field data and serialize appropriately
partial_data = {
field_name: request.POST.get(field_name)
for field_name in form.fields.keys()
if field_name in request.POST
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

The comment mentions filtering out non-field data, but the implementation only filters based on whether field_name is in request.POST. This doesn't actually filter out non-field POST data like 'csrfmiddlewaretoken' or 'action'. Consider adding a check to exclude these special fields, or the comment should be updated to accurately reflect what the code does.

Copilot uses AI. Check for mistakes.
}
if partial_data:
self.set_data(partial_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>{{ filename }}</strong> {% translate "(will be preserved if you don't upload a new file)" %}</li>
{% 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