Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
17 changes: 14 additions & 3 deletions app/eventyay/cfp/templates/cfp/event/submission_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
{% include "cfp/includes/forms_header.html" %}
{% compress js %}
<script defer src="{% static "cfp/js/proposalTabTitles.js" %}"></script>
<script defer src="{% static "cfp/js/formAutoSave.js" %}"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Please use different quote style when nesting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks

{% endcompress %}
{% block cfp_submission_header %}{% endblock cfp_submission_header %}
{% endblock cfp_header %}
Expand All @@ -39,7 +40,7 @@
</div>
{% block cfp_form %}
{% if request.user.is_authenticated %}
<form method="post"{% if form.is_multipart %} enctype="multipart/form-data"{% endif %}>
<form id="cfp-submission-form" method="post"{% if form.is_multipart %} enctype="multipart/form-data"{% endif %}>
{% csrf_token %}
{{ wizard.management_form }}
<div class="d-flex align-items-start">
Expand All @@ -49,6 +50,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 +87,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
207 changes: 207 additions & 0 deletions app/eventyay/static/cfp/js/formAutoSave.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
/**
* Auto-save CfP form data to sessionStorage to preserve it during browser navigation
* This ensures data is not lost when users use the browser back button
*/

(function() {
Copy link
Member

Choose a reason for hiding this comment

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

This style of JS code (wrap the whole code in an anonymous function) is outdated and verbose.
Please use "<script type='module'>".

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably I'm outdated with respect to js ;-)

'use strict';

// Get unique storage key based on the current URL (includes tmpid and step)
function getStorageKey() {
const path = window.location.pathname;
return `cfp_form_data_${path}`;
}

// Debounce function to avoid saving too frequently
function debounce(func, wait) {
let timeout;
return function executedFunction(...args) {
const later = () => {
clearTimeout(timeout);
func(...args);
};
clearTimeout(timeout);
timeout = setTimeout(later, wait);
};
}

// Save form data to sessionStorage
function saveFormData() {
const form = document.getElementById('cfp-submission-form');
if (!form) return;

const formData = {};

// Find all form fields using querySelectorAll for reliability
const textareas = form.querySelectorAll('textarea');
const inputs = form.querySelectorAll('input:not([type="hidden"]):not([type="submit"])');
const selects = form.querySelectorAll('select');

// Process all textareas
textareas.forEach((textarea) => {
const name = textarea.name;
if (name && name !== 'csrfmiddlewaretoken') {
formData[name] = textarea.value;
}
});
Comment on lines +37 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Checkbox groups with the same name are not handled correctly in auto-save

The logic currently saves a single boolean per checkbox name, so groups with shared names overwrite each other and are all restored to the same value. For checkbox groups, you should instead persist an array of selected values (like multi-selects) and, on restore, set each checkbox’s checked state based on whether its value is in that array.


// Process all inputs
inputs.forEach((input) => {
const name = input.name;
const type = input.type;

if (!name || name === 'csrfmiddlewaretoken' || name === 'action') return;

if (type === 'checkbox') {
Comment on lines +48 to +55
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): Checkbox groups with the same name are not handled correctly when saving/restoring.

The current logic stores a single boolean per checkbox name (formData[name] = input.checked), so in a group only the last processed checkbox controls the stored value. On restore, that single boolean is then applied to every checkbox in the group, losing the actual selection.

For checkbox groups that represent multi-select fields, store an array of checked values keyed by name, similar to multiple selects, and restore based on value:

// saveFormData
if (type === 'checkbox') {
    if (!formData[name]) formData[name] = [];
    if (input.checked) formData[name].push(input.value);
}
// restore
if (elements.length > 1 && elements[0].type === 'checkbox' && Array.isArray(value)) {
    for (let i = 0; i < elements.length; i++) {
        elements[i].checked = value.includes(elements[i].value);
    }
    continue;
}

This preserves the correct set of checked options per group.

formData[name] = input.checked;
} else if (type === 'radio') {
if (input.checked) {
formData[name] = input.value;
}
} else if (type !== 'file') {
formData[name] = input.value;
}
});

// Process all selects
selects.forEach((select) => {
const name = select.name;
if (name && name !== 'csrfmiddlewaretoken') {
if (select.multiple) {
formData[name] = Array.from(select.selectedOptions).map(opt => opt.value);
} else {
formData[name] = select.value;
}
}
});

try {
sessionStorage.setItem(getStorageKey(), JSON.stringify(formData));
} catch (e) {
console.warn('Failed to save form data to sessionStorage:', e);
}
}

// Restore form data from sessionStorage
function restoreFormData() {
try {
const savedData = sessionStorage.getItem(getStorageKey());
if (!savedData) return;

const formData = JSON.parse(savedData);
const form = document.getElementById('cfp-submission-form');
if (!form) return;

// Check if the saved sessionStorage data matches the current form data
// If they match exactly, it means we successfully submitted and the server
// echoed back our data - in this case, clear sessionStorage
let allFieldsMatch = true;
let checkedFields = 0;

for (const [name, savedValue] of Object.entries(formData)) {
const elements = form.elements[name];
if (!elements) continue;

const element = elements.length !== undefined ? elements[0] : elements;
const currentValue = element.value || '';
const savedValueStr = String(savedValue || '');
Comment on lines +98 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (bug_risk): The "all fields match" detection for clearing sessionStorage is likely too weak and may never trigger.

This comparison only uses element.value from the first control per name, so it ignores checked state for checkboxes/radios and multi-select values, and may treat matching submissions as mismatches. That means allFieldsMatch can stay false even when the server echoed the submitted data, leaving stale sessionStorage around.

To make this reliable, derive currentData the same way as in saveFormData() and compare the serialized forms (e.g. JSON.stringify(currentData) === savedData) instead of relying on form.elements[name][0].value.

Suggested implementation:

            // Check if the saved sessionStorage data matches the current form data.
            // If they match exactly, it means we successfully submitted and the server
            // echoed back our data - in this case, clear sessionStorage.
            // We reconstruct the current form data in the same shape as what we store.
            const currentData = {};
            const formDataFromDom = new FormData(form);

            for (const [name, value] of formDataFromDom.entries()) {
                if (Object.prototype.hasOwnProperty.call(currentData, name)) {
                    const existing = currentData[name];
                    if (Array.isArray(existing)) {
                        existing.push(value);
                    } else {
                        currentData[name] = [existing, value];
                    }
                } else {
                    currentData[name] = value;
                }
            }

            const serializedCurrentData = JSON.stringify(currentData);

            // If the serialized current form data matches what we have stored,
            // it means the form was successfully submitted - clear sessionStorage.
            if (serializedCurrentData === savedData) {
                clearFormData();
                return;
            }

To be fully robust and future-proof, you should ensure that saveFormData() uses the same normalization/serialization logic as this block. The ideal approach is:

  1. Extract the currentData building logic into a shared helper (e.g. getNormalizedFormData(form)).
  2. Use that helper both in saveFormData() (before sessionStorage.setItem) and here (before comparison), so any changes to how form data is represented are made in one place.


checkedFields++;
if (currentValue.trim() !== savedValueStr.trim()) {
Comment on lines +101 to +110
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 comparison 'currentValue.trim() !== savedValueStr.trim()' assumes both values are strings with a trim() method. However, if the currentValue is from a checkbox or other non-text input, it might not have a trim() method. This could cause a runtime error. Consider adding type checks or handling non-string values appropriately.

Copilot uses AI. Check for mistakes.
allFieldsMatch = false;
break;
}
}

// If all saved fields match current values AND we checked some fields,
// it means the form was successfully submitted - clear sessionStorage
if (allFieldsMatch && checkedFields > 0) {
clearFormData();
return;
}
Comment on lines +95 to +121
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 logic here checks if allFieldsMatch and clears sessionStorage if true. However, this assumes that a match between saved and current values means successful submission. This could lead to false positives in edge cases where the user manually enters the exact same values that were in sessionStorage, then navigates back before submitting. Consider adding an additional indicator (e.g., a flag in sessionStorage) to explicitly track successful form submission.

Copilot uses AI. Check for mistakes.

// Otherwise, restore from sessionStorage
for (const [name, value] of Object.entries(formData)) {
const elements = form.elements[name];
if (!elements) continue;

// Handle NodeList (radio buttons, checkboxes with same name)
if (elements.length > 1) {
for (let i = 0; i < elements.length; i++) {
const element = elements[i];
if (element.type === 'checkbox') {
element.checked = value;
} else if (element.type === 'radio') {
element.checked = (element.value === value);
}
}
} else {
const element = elements.length !== undefined ? elements[0] : elements;
Comment on lines +101 to +139
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 logic for checking if an element is a NodeList and accessing the first element is duplicated in lines 105 and 139. The pattern 'elements.length !== undefined ? elements[0] : elements' appears twice. Consider extracting this into a helper function to reduce duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
const type = element.type;

if (type === 'checkbox') {
element.checked = value;
} else if (type === 'radio') {
element.checked = (element.value === value);
} else if (element.tagName === 'SELECT') {
if (element.multiple && Array.isArray(value)) {
for (let i = 0; i < element.options.length; i++) {
element.options[i].selected = value.includes(element.options[i].value);
}
} else {
element.value = value;
}
} else if (element.tagName === 'TEXTAREA' || element.tagName === 'INPUT') {
// Always restore - sessionStorage takes precedence
// If we got this far, the matching logic already determined
// this is new data that should be restored
element.value = value;
}
}
Comment on lines +124 to +160
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 code iterates through form.elements[name] which can be either a single element or a NodeList depending on whether multiple elements share the same name. The check 'elements.length !== undefined' is used to distinguish these cases, but this is fragile. NodeList and HTMLCollection both have a length property, but a single element does not. However, checking for 'undefined' specifically could fail if length exists but is 0. Consider using a more robust check like 'elements instanceof NodeList || elements instanceof RadioNodeList' or 'elements.tagName === undefined' to distinguish collections from single elements.

Copilot uses AI. Check for mistakes.
}
} catch (e) {
console.warn('Failed to restore form data from sessionStorage:', e);
}
}

// Clear saved form data (called after successful submission)
function clearFormData() {
try {
sessionStorage.removeItem(getStorageKey());
} catch (e) {
console.warn('Failed to clear form data from sessionStorage:', e);
}
}

// Initialize auto-save functionality
function init() {
const form = document.getElementById('cfp-submission-form');
if (!form) return;

// Restore form data on page load
restoreFormData();

// Create debounced save function (save 500ms after user stops typing)
const debouncedSave = debounce(saveFormData, 500);

// Attach event listeners to form elements
form.addEventListener('input', debouncedSave);
form.addEventListener('change', debouncedSave);

// Save before page unload (browser back button, refresh, close tab, etc.)
// This ensures data is preserved even when using browser navigation
window.addEventListener('beforeunload', saveFormData);

// Note: We don't clear sessionStorage on form submit because:
// 1. We can't reliably detect if submission succeeded (might have validation errors)
// 2. The restore logic already handles this by clearing sessionStorage when
// it detects the form has server data (successful previous submission)
}

// Initialize when DOM is ready
if (document.readyState === 'loading') {
document.addEventListener('DOMContentLoaded', init);
} else {
init();
}
})();
11 changes: 8 additions & 3 deletions app/eventyay/static/cfp/js/proposalTabTitles.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ const updateTitle = (newTitle) => {
}

const checkForTitle = () => {
const titleInput = document.getElementById("id_title").value
updateTitle(titleInput)
const titleInput = document.getElementById("id_title")
if (titleInput) {
updateTitle(titleInput.value)
}
}

if (titleParts.length !== 3) {
Expand All @@ -19,5 +21,8 @@ if (titleParts.length !== 3) {
)
} else {
onReady(checkForTitle)
document.getElementById("id_title").addEventListener("change", (ev) => { updateTitle(ev.target.value) })
const titleInput = document.getElementById("id_title")
if (titleInput) {
titleInput.addEventListener("change", (ev) => { updateTitle(ev.target.value) })
}
}