diff --git a/app/eventyay/cfp/flow.py b/app/eventyay/cfp/flow.py index 551cc31297..ad46a5360f 100644 --- a/app/eventyay/cfp/flow.py +++ b/app/eventyay/cfp/flow.py @@ -11,7 +11,7 @@ from django.core.files.storage import FileSystemStorage from django.core.files.uploadedfile import UploadedFile from django.db.models import Q -from django.forms import ValidationError +from django.forms import FileField, MultipleChoiceField, ModelMultipleChoiceField, ValidationError from django.http import HttpResponseNotAllowed from django.shortcuts import redirect from django.urls import reverse @@ -59,6 +59,33 @@ def i18n_string(data, locales): return LazyI18nString(data) +class SavedFileWrapper: + """Wrapper for saved files to display filename in form widgets. + + This class is used to represent files that have been saved to session storage + during multi-step form navigation. It mimics FieldFile behavior for display + purposes while preventing these objects from being passed as form data (which + would cause validation errors). + + Attributes: + is_saved_file: Always True, used to identify SavedFileWrapper instances + name: The filename to display in the widget + + The __bool__ method ensures the wrapper evaluates as truthy when it has a name, + allowing template conditionals to work correctly. + """ + is_saved_file = True + + def __init__(self, name): + self.name = name + + def __str__(self): + return self.name + + def __bool__(self): + return bool(self.name) + + def serialize_value(value): if getattr(value, 'pk', None): return value.pk @@ -109,8 +136,13 @@ def is_applicable(self, request): def is_completed(self, request): raise NotImplementedError() - @cached_property + @property def cfp_session(self): + """Get session data for this CfP submission. + + Note: This is a regular property (not cached) to ensure we always + get fresh session data, especially important for back navigation. + """ return cfp_session(self.request) def get_next_applicable(self, request): @@ -138,7 +170,7 @@ def get_next_url(self, request): return next_step.get_step_url(request) def get_step_url(self, request, query=None): - kwargs = request.resolver_match.kwargs + kwargs = request.resolver_match.kwargs.copy() kwargs['step'] = self.identifier url = reverse('cfp:event.submit', kwargs=kwargs) new_query = request.GET.copy() @@ -208,24 +240,38 @@ def get(self, request): def identifier(self): raise NotImplementedError() - class FormFlowStep(TemplateFlowStep): form_class = None file_storage = FileSystemStorage(str(Path(settings.MEDIA_ROOT) / 'cfp_uploads')) + def _mark_session_modified(self): + """Mark the session as modified to ensure changes are persisted.""" + self.request.session.modified = True + def get_form_initial(self): - initial_data = self.cfp_session.get('initial', {}).get(self.identifier, {}) - previous_data = self.cfp_session.get('data', {}).get(self.identifier, {}) - return copy.deepcopy({**initial_data, **previous_data}) + session_data = self.cfp_session + initial_data = session_data.get('initial', {}).get(self.identifier, {}) + previous_data = session_data.get('data', {}).get(self.identifier, {}) + result = copy.deepcopy({**initial_data, **previous_data}) + result.update(self.get_saved_file_objects()) + return result + + def get_saved_file_objects(self): + saved_files = self.cfp_session['files'].get(self.identifier, {}) + return { + field: SavedFileWrapper(name=(info.get('name') or _('Previously uploaded file'))) + for field, info in saved_files.items() + } def get_form(self, from_storage=False): if self.request.method == 'GET' or from_storage: - return self.form_class( - data=self.get_form_initial() if from_storage else None, - initial=self.get_form_initial(), - files=self.get_files(), - **self.get_form_kwargs(), - ) + initial_data = self.get_form_initial() + if from_storage: + filtered = {k: v for k, v in initial_data.items() if not isinstance(v, SavedFileWrapper)} + return self.form_class( + data=filtered, initial=filtered, files=self.get_files(), **self.get_form_kwargs() + ) + return self.form_class(initial=initial_data, **self.get_form_kwargs()) return self.form_class(data=self.request.POST, files=self.request.FILES, **self.get_form_kwargs()) def is_completed(self, request): @@ -241,7 +287,25 @@ def get_context_data(self, **kwargs): def post(self, request): self.request = request + action = request.POST.get('action', 'submit') + clear_file = request.POST.get('clear_file') + if clear_file: + form = self.get_form() + if clear_file in form.fields and isinstance(form.fields[clear_file], FileField): + self._clear_file(clear_file) + return self.get(request) + messages.error(request, _('Invalid field for file clearing.')) + return self.get(request) + form = self.get_form() + + if action == 'back': + self._save_partial_data(form) + if request.FILES: + self.set_files(request.FILES) + prev_url = self.get_prev_url(request) + return redirect(prev_url) if prev_url else self.get(request) + if not form.is_valid(): error_message = '\n\n'.join( (f'{form.fields[key].label}: ' if key != '__all__' else '') + ' '.join(values) @@ -249,29 +313,98 @@ def post(self, request): ) messages.error(self.request, error_message) return self.get(request) - self.set_data(form.cleaned_data) + self.set_data(form.cleaned_data, fields=form.fields) self.set_files(form.files) next_url = self.get_next_url(request) return redirect(next_url) if next_url else None - def set_data(self, data): - self.cfp_session['data'][self.identifier] = json.loads( - json.dumps( - {key: value for key, value in data.items() if not getattr(value, 'file', None)}, - default=serialize_value, - ) + def _clear_file(self, field_name): + """Remove a file from session and storage, or mark existing file for clearing.""" + session_data = self.cfp_session + saved_files = session_data['files'].get(self.identifier, {}) + + # If we have a session-stored file for this field, clear only that session file. + # Do not set the "clear_files" flag in this case (that flag is for clearing existing DB files). + if field_name in saved_files: + file_info = saved_files[field_name] + if 'tmp_name' in file_info: + try: + self.file_storage.delete(file_info['tmp_name']) + except OSError as e: + logger.warning("Failed to delete file '%s': %s", file_info['tmp_name'], e) + messages.error(self.request, _("Could not remove the uploaded file. Please try again.")) + return + del saved_files[field_name] + session_data['files'][self.identifier] = saved_files + + # If this field was previously marked for clearing an existing file, unmark it. + clear_files = session_data.get('clear_files', {}) + if (step_flags := clear_files.get(self.identifier)) and field_name in step_flags: + step_flags.remove(field_name) + if not step_flags: + clear_files.pop(self.identifier, None) + self._mark_session_modified() + return + + # No session-stored file: mark the existing DB-backed file for clearing. + clear_flags = session_data.setdefault('clear_files', {}).setdefault(self.identifier, []) + if field_name not in clear_flags: + clear_flags.append(field_name) + self._mark_session_modified() + + def _save_partial_data(self, form): + """Save form data for back navigation (even if incomplete).""" + is_valid = form.is_valid() + cleaned_data = getattr(form, 'cleaned_data', {}) if is_valid else {} + data_to_save = {} + for field_name, field in form.fields.items(): + if isinstance(field, FileField): + continue + if field_name in cleaned_data: + data_to_save[field_name] = cleaned_data[field_name] + elif field_name in self.request.POST: + if isinstance(field, (MultipleChoiceField, ModelMultipleChoiceField)): + data_to_save[field_name] = self.request.POST.getlist(field_name) + else: + data_to_save[field_name] = self.request.POST.get(field_name) + session_data = self.cfp_session + session_data['data'][self.identifier] = json.loads(json.dumps(data_to_save, default=serialize_value)) + self._mark_session_modified() + + def set_data(self, data, fields=None): + fields = fields or getattr(self.form_class, 'base_fields', {}) + file_field_names = {name for name, field in fields.items() if isinstance(field, FileField)} + data = {k: v for k, v in data.items() if k not in file_field_names} + session_data = self.cfp_session + session_data['data'][self.identifier] = json.loads( + json.dumps(data, default=serialize_value) ) + self._mark_session_modified() def get_files(self): + """Retrieve saved files from session storage. + + If a file cannot be opened (e.g., deleted from storage), it is skipped + with a warning. This allows the form to render gracefully even if some + files are missing, rather than failing completely. The user can re-upload + the missing file if needed. + """ saved_files = self.cfp_session['files'].get(self.identifier, {}) files = {} for field, field_dict in saved_files.items(): field_dict = field_dict.copy() tmp_name = field_dict.pop('tmp_name') - files[field] = UploadedFile(file=self.file_storage.open(tmp_name), **field_dict) + try: + uploaded_file = UploadedFile(file=self.file_storage.open(tmp_name), **field_dict) + files[field] = uploaded_file + except OSError as e: + logger.warning("Could not open file '%s' for field '%s': %s", tmp_name, field, e) return files or None def set_files(self, files): + if not files: + return + session_data = self.cfp_session for field, field_file in files.items(): tmp_filename = self.file_storage.save(field_file.name, field_file) file_dict = { @@ -281,9 +414,13 @@ def set_files(self, files): 'size': field_file.size, 'charset': field_file.charset, } - data = self.cfp_session['files'].get(self.identifier, {}) + data = session_data['files'].get(self.identifier, {}) data[field] = file_dict - self.cfp_session['files'][self.identifier] = data + session_data['files'][self.identifier] = data + clear_files = session_data.get('clear_files', {}) + if self.identifier in clear_files and field in clear_files[self.identifier]: + clear_files[self.identifier].remove(field) + self._mark_session_modified() class GenericFlowStep: @@ -518,7 +655,9 @@ def get_form_kwargs(self): if not result.get('user') and self.request.user.is_authenticated: result['user'] = self.request.user user = result.get('user') - result['name'] = user.fullname if user else user_data.get('register_name') + saved_profile_data = self.cfp_session.get('data', {}).get(self.identifier, {}) + saved_fullname = (saved_profile_data.get('fullname') or '').strip() + result['name'] = saved_fullname or (user.fullname if user else user_data.get('register_name')) result['read_only'] = False result['essential_only'] = True return result @@ -531,13 +670,29 @@ def get_context_data(self, **kwargs): email = data.get('register_email', '') if email: result['gravatar_parameter'] = User(email=email).gravatar_parameter + saved_files = self.cfp_session.get('files', {}).get(self.identifier, {}) + clear_flags = self.cfp_session.get('clear_files', {}).get(self.identifier, []) + if 'avatar' in saved_files: + result['saved_avatar_name'] = saved_files['avatar'].get('name') or _('Previously uploaded') + if self.request.user.is_authenticated and self.request.user.avatar: + avatar_name = self.request.user.avatar.name + result['avatar_basename'] = Path(avatar_name).name + # Hide existing avatar if marked for clearing + if 'avatar' in clear_flags: + result['avatar_cleared'] = True return result def done(self, request, draft=False): form = self.get_form(from_storage=True) - form.is_valid() + if not form.is_valid(): + raise ValidationError(_("Profile form is invalid.")) form.user = request.user + avatar_uploaded = bool(form.files and form.files.get('avatar')) form.save() + # Clear avatar if marked for clearing + clear_flags = self.cfp_session.get('clear_files', {}).get(self.identifier, []) + if 'avatar' in clear_flags and request.user.avatar and not avatar_uploaded: + request.user.avatar.delete(save=True) @property def label(self): diff --git a/app/eventyay/cfp/templates/cfp/event/submission_base.html b/app/eventyay/cfp/templates/cfp/event/submission_base.html index 5284913e1d..f785411a34 100644 --- a/app/eventyay/cfp/templates/cfp/event/submission_base.html +++ b/app/eventyay/cfp/templates/cfp/event/submission_base.html @@ -14,6 +14,7 @@ {% include "cfp/includes/forms_header.html" %} {% compress js %} + {% endcompress %} {% block cfp_submission_header %}{% endblock cfp_submission_header %} {% endblock cfp_header %} @@ -76,9 +77,9 @@