Chore: Cfp flow movement preservation #1475
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideRefactors the CfP submission flow to better preserve form and file state across navigation, adds explicit handling for clearing previously uploaded files (including avatars), and updates templates/widgets/JS to show current file names and support client-side file selection UI. Sequence diagram for CfP back navigation with partial data and file preservationsequenceDiagram
actor User
participant Browser
participant CfpWizardView as CfpWizardView
participant FormFlowStep as FormFlowStep
participant Session as DjangoSession
User->>Browser: Click Back button
Browser->>CfpWizardView: POST action=back
CfpWizardView->>CfpWizardView: dispatch(request)
CfpWizardView->>FormFlowStep: get_step(request)
note over CfpWizardView,FormFlowStep: request.method == POST and action == back
CfpWizardView-->>FormFlowStep: result (step response)
FormFlowStep->>FormFlowStep: post(request)
FormFlowStep->>FormFlowStep: get_form()
FormFlowStep->>FormFlowStep: _save_partial_data(form)
FormFlowStep->>Session: update cfp_session[data][step]
FormFlowStep->>FormFlowStep: set_files(form.files)
FormFlowStep->>Session: update cfp_session[files][step]
FormFlowStep->>FormFlowStep: get_prev_url(request)
FormFlowStep-->>Browser: redirect(prev_url) or render current step
Browser->>CfpWizardView: GET prev_url
CfpWizardView->>FormFlowStep: step.get(request)
FormFlowStep->>FormFlowStep: get_form_initial()
FormFlowStep->>Session: read cfp_session initial/data/files
FormFlowStep-->>Browser: Render form with restored fields and file names
Sequence diagram for clearing saved CfP files and avatarssequenceDiagram
actor User
participant Browser
participant FormFlowStep as FormFlowStep
participant ProfileView as ProfileStepView
participant FileStorage as FileSystemStorage
participant Session as DjangoSession
participant DB as Database
rect rgb(235, 245, 255)
note over User,Browser: Clear file in CfP step (session-based upload)
User->>Browser: Click Clear on saved file
Browser->>FormFlowStep: POST clear_file=field_name
FormFlowStep->>FormFlowStep: post(request)
FormFlowStep->>FormFlowStep: _clear_file(field_name)
FormFlowStep->>Session: read cfp_session[files][step][field_name]
alt tmp_name present
FormFlowStep->>FileStorage: delete(tmp_name)
end
FormFlowStep->>Session: remove files[step][field_name]
FormFlowStep->>Session: append field_name to clear_files[step]
FormFlowStep->>Session: mark request.session.modified
FormFlowStep-->>Browser: get(request) -> re-render step
end
rect rgb(235, 255, 235)
note over User,Browser: Clear avatar in profile step (DB file)
User->>Browser: Click Clear avatar
Browser->>ProfileView: POST clear_file=avatar
ProfileView->>ProfileView: post(request)
ProfileView->>DB: load user
ProfileView->>FileStorage: user.avatar.delete(save=True)
ProfileView->>DB: set user.avatar=None and save
ProfileView-->>Browser: redirect to same path
Browser->>ProfileView: GET profile page
ProfileView->>Session: read clear_files flags
ProfileView-->>Browser: Render form with avatar cleared
end
Updated class diagram for CfP flow steps and file widgetsclassDiagram
class FlowStepBase {
+request
+identifier
+get_next_applicable(request)
+get_prev_url(request)
+get_next_url(request)
+get_step_url(request, query)
+is_applicable(request)
+is_completed(request)
+cfp_session
}
class FormFlowStep {
+file_storage
+get_form_initial()
+get_saved_file_objects()
+get_form(from_storage)
+post(request)
+_clear_file(field_name)
+_save_partial_data(form)
+set_data(data)
+get_files()
+set_files(files)
}
class SavedFileWrapper {
+is_saved_file
+name
+__init__(name)
+__str__()
+__bool__()
}
class BasenameFileInput {
+FakeFile
+get_context(name, value, attrs)
}
class FakeFile {
+file
+name
+__init__(file)
+__str__()
+url
}
class ImageInput {
+get_context(name, value, attrs)
}
class ProfileForm {
+user
+user_fields
+__init__(*args, name, **kwargs)
}
FlowStepBase <|-- FormFlowStep
BasenameFileInput <|-- ImageInput
BasenameFileInput *-- FakeFile
FormFlowStep o-- SavedFileWrapper
ProfileForm o-- FormFlowStep
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The session update logic is duplicated and slightly inconsistent across
_clear_file,_save_partial_data,set_data, andset_files(e.g. repeatedly doingsession_data = self.cfp_sessionand thenself.request.session['cfp'] = self.request.session.get('cfp', {})); consider centralizing this into a small helper and avoid what appears to be a no-op reassignment that could become problematic ifcfp_sessionever returns a copy instead of a live dict. - The functions in
fileInput.js(e.g.handleFileSelect) assume that elements likename + '-selected-info'andname + '-selected-name'always exist, which will cause runtime errors if the script is used with inputs that don't render the matching markup; add null checks or guard clauses before dereferencing these elements. fileInput.jsis included both from the image input widget and fromavatar.html, so it may be loaded multiple times on the same page; consider loading it once via the base layout or the asset pipeline to avoid duplicate global definitions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The session update logic is duplicated and slightly inconsistent across `_clear_file`, `_save_partial_data`, `set_data`, and `set_files` (e.g. repeatedly doing `session_data = self.cfp_session` and then `self.request.session['cfp'] = self.request.session.get('cfp', {})`); consider centralizing this into a small helper and avoid what appears to be a no-op reassignment that could become problematic if `cfp_session` ever returns a copy instead of a live dict.
- The functions in `fileInput.js` (e.g. `handleFileSelect`) assume that elements like `name + '-selected-info'` and `name + '-selected-name'` always exist, which will cause runtime errors if the script is used with inputs that don't render the matching markup; add null checks or guard clauses before dereferencing these elements.
- `fileInput.js` is included both from the image input widget and from `avatar.html`, so it may be loaded multiple times on the same page; consider loading it once via the base layout or the asset pipeline to avoid duplicate global definitions.
## Individual Comments
### Comment 1
<location> `app/eventyay/common/templates/common/widgets/image_input.html:28` </location>
<code_context>
{% compress js %}
<script defer src="{% static "cfp/js/profile.js" %}"></script>
{% endcompress %}
+<script src="{% static 'common/js/fileInput.js' %}"></script>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Including `fileInput.js` inside the widget template can lead to multiple redundant script loads and event bindings.
Since this template is rendered once per file field, each instance will inject another `fileInput.js` `<script>` tag. The browser may only fetch it once, but it will execute on every inclusion, registering duplicate listeners and handlers. Please move the script tag to a layout/base template (or another once-per-page block) and keep this widget responsible only for its HTML markup.
Suggested implementation:
```
<div class="form-file-selected mb-2" id="{{ widget.name }}-selected-info" style="display: none;">
<span class="text-muted">{% translate "Selected" %}: </span>
<strong id="{{ widget.name }}-selected-name"></strong>
<button type="button" class="btn btn-sm btn-outline-danger ml-2" onclick="clearFileInput('{{ widget.name }}')">{% translate "Clear" %}</button>
</div>
<input type="{{ widget.type }}" name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %} onchange="handleFileSelect(this)">
```
To fully implement your review comment, you’ll also need to:
1. Include `fileInput.js` once per page, e.g. in a base or layout template in the appropriate JS block:
- For example, in a base template:
`{% block extra_js %}`
` <script src="{% static 'common/js/fileInput.js' %}"></script>`
`{% endblock %}`
2. Ensure that any pages using this widget extend that base template or otherwise pull in `fileInput.js` so `handleFileSelect` and `clearFileInput` are available.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements CFP submission flow preservation to maintain user input and file selections when navigating between wizard steps, addressing issue #1391. The implementation adds partial form data persistence, file upload handling with session storage, and UI controls for clearing previously uploaded files and avatars.
- Converts back navigation from anchor links to form submissions to enable data preservation
- Adds JavaScript-driven file selection UI with clear functionality across forms
- Implements session-based file storage for CFP wizard with SavedFileWrapper for display
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| app/eventyay/static/common/js/fileInput.js | New JavaScript module for handling file input interactions and display state management |
| app/eventyay/person/forms/profile.py | Updates profile form initialization to prioritize session data over user model defaults for back navigation |
| app/eventyay/common/templates/common/widgets/image_input.html | Redesigns image input widget with file selection status display and clear buttons |
| app/eventyay/common/templates/common/avatar.html | Adds avatar file selection UI with saved file display and clearing functionality |
| app/eventyay/common/forms/widgets.py | Extends ImageInput widget context to support SavedFileWrapper and display filenames |
| app/eventyay/cfp/views/wizard.py | Adds early return handling for back navigation and file clearing actions |
| app/eventyay/cfp/views/user.py | Implements submission image clearing via clear_file POST parameter |
| app/eventyay/cfp/templates/cfp/event/submission_base.html | Changes back button from link to submit button for form data preservation |
| app/eventyay/cfp/flow.py | Major refactoring: converts cfp_session to uncached property, adds partial data saving, file clearing logic, and SavedFileWrapper class for session-stored files |
Comments suppressed due to low confidence (1)
app/eventyay/cfp/flow.py:311
- 'except' clause does nothing but pass and there is no explanatory comment.
except Exception:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Saksham-Sirohi
force-pushed
the
cfp-flow-movement-preservation
branch
from
b94a521 to
1ce62b8
Compare
Saksham-Sirohi
force-pushed
the
cfp-flow-movement-preservation
branch
from
1ce62b8 to
3a66db7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1391
Summary by Sourcery
Improve CFP submission flow to preserve user input and file selections across navigation, and enhance file/avatar handling UX.
New Features:
Bug Fixes:
Enhancements: