implemented suggestions

Author: Saksham-Sirohi

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 MultipleChoiceField, ModelMultipleChoiceField, ValidationError
+from django.forms import FileField, MultipleChoiceField, ModelMultipleChoiceField, ValidationError
 from django.http import HttpResponseNotAllowed
 from django.shortcuts import redirect
 from django.urls import reverse
@@ -60,7 +60,20 @@ def i18n_string(data, locales):
 
 
 class SavedFileWrapper:
-    """Wrapper for saved files to display filename in form widgets."""
+    """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):
@@ -227,11 +240,14 @@ 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):
         session_data = self.cfp_session
         initial_data = session_data.get('initial', {}).get(self.identifier, {})
@@ -243,16 +259,17 @@ def get_form_initial(self):
     def get_saved_file_objects(self):
         saved_files = self.cfp_session['files'].get(self.identifier, {})
         return {
-            field: SavedFileWrapper(name=info.get('name', 'Previously uploaded file'))
+            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:
             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=initial_data, initial=initial_data, files=self.get_files(), **self.get_form_kwargs()
+                    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())
@@ -271,18 +288,21 @@ def get_context_data(self, **kwargs):
     def post(self, request):
         self.request = request
         action = request.POST.get('action', 'submit')
-        
-        # Handle file clearing - clear and re-render the page
         clear_file = request.POST.get('clear_file')
         if clear_file:
-            self._clear_file(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)
-            self.set_files(form.files)
+            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)
 
@@ -293,7 +313,7 @@ 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
@@ -302,56 +322,83 @@ 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 Exception:
-                    pass
+                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
-        # Mark existing file for clearing (for files from DB, not session)
+
+            # 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.request.session['cfp'] = self.request.session.get('cfp', {})
-        self.request.session.modified = True
+        self._mark_session_modified()
 
     def _save_partial_data(self, form):
         """Save form data for back navigation (even if incomplete)."""
-        form.is_valid()
+        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 field_name in getattr(form, 'cleaned_data', {}):
-                data_to_save[field_name] = form.cleaned_data[field_name]
+            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)
-        data_to_save = {k: v for k, v in data_to_save.items() if not getattr(v, 'file', None)}
         session_data = self.cfp_session
         session_data['data'][self.identifier] = json.loads(json.dumps(data_to_save, default=serialize_value))
-        self.request.session['cfp'] = self.request.session.get('cfp', {})
-        self.request.session.modified = True
+        self._mark_session_modified()
 
-    def set_data(self, data):
+    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({k: v for k, v in data.items() if not getattr(v, 'file', None)}, default=serialize_value)
+            json.dumps(data, default=serialize_value)
         )
-        self.request.session['cfp'] = self.request.session.get('cfp', {})
-        self.request.session.modified = True
+        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')
-            uploaded_file = UploadedFile(file=self.file_storage.open(tmp_name), **field_dict)
-            files[field] = uploaded_file
+            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):
@@ -370,12 +417,10 @@ def set_files(self, files):
             data = session_data['files'].get(self.identifier, {})
             data[field] = file_dict
             session_data['files'][self.identifier] = data
-            # Remove clear flag if new file is uploaded
-            clear_flags = session_data.get('clear_files', {}).get(self.identifier, [])
-            if field in clear_flags:
-                clear_flags.remove(field)
-        self.request.session['cfp'] = self.request.session.get('cfp', {})
-        self.request.session.modified = True
+            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:
@@ -611,7 +656,8 @@ def get_form_kwargs(self):
             result['user'] = self.request.user
         user = result.get('user')
         saved_profile_data = self.cfp_session.get('data', {}).get(self.identifier, {})
-        result['name'] = saved_profile_data.get('fullname') or (user.fullname if user else user_data.get('register_name'))
+        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
@@ -627,20 +673,25 @@ def get_context_data(self, **kwargs):
         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', 'Previously uploaded')
+            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:
+        if 'avatar' in clear_flags and request.user.avatar and not avatar_uploaded:
             request.user.avatar.delete(save=True)
 
     @property

app/eventyay/cfp/templates/cfp/event/submission_base.html

@@ -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 "common/js/fileInput.js" %}"></script>
     {% endcompress %}
     {% block cfp_submission_header %}{% endblock cfp_submission_header %}
 {% endblock cfp_header %}
@@ -76,7 +77,7 @@ <h2>{% block submission_step_title %}{{ title|default:'' }}{% endblock submissio
                     </div>
                     <div class="col-md-3">
                         {% if prev_url %}
-                            <button type="submit" class="btn btn-block btn-info btn-lg" name="action" value="back">
+                            <button type="submit" class="btn btn-block btn-info btn-lg" name="action" value="back" formnovalidate>
                                 « {{ phrases.base.back_button }}
                             </button>
                         {% endif %}

app/eventyay/cfp/templates/cfp/event/user_submission_edit.html

@@ -17,6 +17,7 @@
         <script defer src="{% static "js/jquery.js" %}"></script>
         <script defer src="{% static "js/jquery.formset.js" %}"></script>
         <script defer src="{% static "cfp/js/animateFormset.js" %}"></script>
+        <script defer src="{% static "common/js/fileInput.js" %}"></script>
     {% endcompress %}
 {% endblock cfp_header %}
 

app/eventyay/cfp/views/user.py

@@ -361,15 +361,6 @@ def object(self):
         return self.get_object()
 
     def post(self, request, *args, **kwargs):
-        clear_field = request.POST.get('clear_file')
-        if clear_field == 'image':
-            submission = self.object
-            if submission and getattr(submission, 'image', None):
-                submission.image.delete(save=False)
-                submission.image = None
-                submission.save(update_fields=['image'])
-            return redirect(request.path)
-
         form = self.get_form()
         if form.is_valid() and self.qform.is_valid():
             return self.form_valid(form)

app/eventyay/common/forms/widgets.py

@@ -89,8 +89,11 @@ def url(self):
 
     def get_context(self, name, value, attrs):
         ctx = super().get_context(name, value, attrs)
-        if value and not getattr(value, 'is_saved_file', False):
-            ctx['widget']['value'] = self.FakeFile(value)
+        if value:
+            if getattr(value, 'is_saved_file', False):
+                ctx['widget']['value'] = None
+            else:
+                ctx['widget']['value'] = self.FakeFile(value)
         return ctx
 
 
@@ -102,8 +105,13 @@ def get_context(self, name, value, attrs):
         if value and getattr(value, 'is_saved_file', False):
             ctx['widget']['is_saved_file'] = True
             ctx['widget']['saved_filename'] = Path(value.name).name if getattr(value, 'name', None) else ''
-        elif value and getattr(value, 'name', None):
-            ctx['widget']['saved_filename'] = Path(value.name).name
+        elif value:
+            if getattr(value, 'name', None):
+                ctx['widget']['saved_filename'] = Path(value.name).name
+            elif getattr(value, 'url', None):
+                ctx['widget']['saved_filename'] = Path(value.url).name
+            else:
+                ctx['widget']['saved_filename'] = str(value)
         return ctx
 
 

app/eventyay/common/templates/common/avatar.html

@@ -13,25 +13,25 @@
                     <div class="form-saved-file mb-2" id="avatar-saved-info">
                         <span class="text-muted">{% translate "Currently selected" %}: </span>
                         <strong>{{ saved_avatar_name }}</strong>
-                        <button type="submit" name="clear_file" value="avatar" class="btn btn-sm btn-outline-danger ml-2">{% translate "Clear" %}</button>
+                        <button type="submit" name="clear_file" value="avatar" class="btn btn-sm btn-outline-danger ml-2" formnovalidate>{% translate "Clear" %}</button>
                     </div>
                     {% elif user.avatar and not avatar_cleared %}
                     <div class="form-saved-file mb-2" id="avatar-saved-info">
                         <span class="text-muted">{% translate "Currently selected" %}: </span>
-                        <strong>{{ user.avatar.name|cut:"avatars/" }}</strong>
-                        <button type="submit" name="clear_file" value="avatar" class="btn btn-sm btn-outline-danger ml-2">{% translate "Clear" %}</button>
+                        <strong>{% if avatar_basename %}{{ avatar_basename }}{% else %}{{ user.avatar.name }}{% endif %}</strong>
+                        <button type="submit" name="clear_file" value="avatar" class="btn btn-sm btn-outline-danger ml-2" formnovalidate>{% translate "Clear" %}</button>
                     </div>
                     {% endif %}
                     <div class="form-file-selected mb-2" id="avatar-selected-info" style="display: none;">
                         <span class="text-muted">{% translate "Selected" %}: </span>
                         <strong id="avatar-selected-name"></strong>
-                        <button type="button" class="btn btn-sm btn-outline-danger ml-2" onclick="clearFileInput('avatar')">{% translate "Clear" %}</button>
+                        <button type="button" class="btn btn-sm btn-outline-danger ml-2" data-clear-file="avatar">{% translate "Clear" %}</button>
                     </div>
                     {{ form.avatar.as_field_group }}
                 </div>
                 {{ form.get_gravatar.as_field_group }}
             </div>
-            <div class="form-image-preview {% if not user.avatar or avatar_cleared %}{% if not user.get_gravatar %}d-none{% endif %}{% endif %}">
+            <div class="form-image-preview {% if not user.get_gravatar %}{% if not user.avatar or avatar_cleared %}d-none{% endif %}{% endif %}">
                 <a href="{% if user.avatar and not avatar_cleared %}{{ user.avatar|thumbnail:"default" }}{% endif %}" data-lightbox="{% if user.avatar and not avatar_cleared %}{{ user.avatar.url }}{% endif %}">
                     <img loading="lazy"
                          class="avatar"
@@ -51,4 +51,3 @@
 {% compress js %}
     <script defer src="{% static "cfp/js/profile.js" %}"></script>
 {% endcompress %}
-<script src="{% static 'common/js/fileInput.js' %}"></script>