Fix avatar image bug. Fixes #586#595
Conversation
|
""" WalkthroughThe profile page template was updated to merge the avatar upload and profile update into a single form. JavaScript was added to manage avatar input changes, ensuring the avatar field is only included in submissions when modified. This streamlines profile updates and optimizes avatar upload handling. Additionally, debugging print statements were added to the profile view to log POST and FILES data on form submission. Changes
Assessment against linked issues
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @abdelrahman390, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses a bug related to updating the avatar image on the user profile page, as described in issue #586. The changes involve modifying the profile template to correctly handle avatar updates. Specifically, the form structure has been adjusted to ensure the avatar is only included in the form data if it has been changed by the user. This prevents unnecessary avatar uploads and potential issues with the update process.
Highlights
- Bug Fix: Addresses a bug where updating the avatar image was not working correctly.
- Template Modification: Adjusts the profile template (profile.html) to handle avatar updates more efficiently.
- Conditional Avatar Submission: Implements logic to only include the avatar in the form submission if it has been changed, optimizing the update process.
Changelog
- web/templates/profile.html
- The profile picture section is now wrapped within the main form.
- The main form is now identified with the id 'main-form'.
- A hidden avatar field is added for form processing.
- JavaScript code is added to handle avatar changes and conditional form submission.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A profile's face,
New image, a fresh embrace,
Bugs squashed with care.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request addresses a bug related to avatar image updates. The changes involve modifying the profile template to include the avatar upload form within a larger form and adding JavaScript to handle avatar changes. Overall, the approach seems reasonable, but there are a few areas that could be improved for clarity and efficiency.
Summary of Findings
- Form submission handling: The JavaScript code intercepts the form submission to conditionally include the avatar file. This approach might introduce complexity and potential issues with form validation and other form elements. Consider alternative approaches, such as using a hidden field to indicate whether the avatar has been changed.
- Session storage usage: The use of session storage to track avatar changes might not be reliable across different browsers or sessions. Consider using a more robust mechanism, such as a hidden input field within the form.
Merge Readiness
The pull request addresses the reported issue, but the current implementation introduces some complexity. Addressing the feedback regarding form submission handling and session storage usage would improve the code's reliability and maintainability. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/templates/profile.html(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Tests
🔇 Additional comments (4)
web/templates/profile.html (4)
33-38: Consolidated form structure correctly merges avatar and profile updates
Wrapping both avatar upload and profile fields inside a single<form>withenctype="multipart/form-data"and including{% csrf_token %}is the right approach. Defaulting the action to the current URL is acceptable if your view handles both avatar and profile changes.
41-45: Ensure SVG output is safe from XSS
You’re rendering raw SVG via{{ user.profile.custom_avatar.svg|safe }}. Please verify thatcustom_avatar.svgis sanitized server-side (e.g., via a whitelist sanitizer) before marking it safe to avoid potential XSS.
47-47: Ordering of avatar conditions is correct
Usingelif user.profile.avatarensures that a standard uploaded avatar only displays when there’s nocustom_avatar. The fallback priority looks solid.
79-81: Username and full name display logic is sound
Conditionally rendering{{ user.get_full_name }}only when nonempty enhances clarity without clutter. Nicely done.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
web/templates/profile.html (1)
72-77:⚠️ Potential issueRemove inline
onchangeto avoid premature submission
The inlineonchange="this.form.submit()"triggers an immediate submission, bypassing your sessionStorage logic and submit handler. Remove this attribute so the JS listener exclusively manages form submissions:-<input type="file" - id="avatar-upload" - name="avatar" - accept="image/*" - class="hidden" - onchange="this.form.submit()" /> +<input type="file" + id="avatar-upload" + name="avatar" + accept="image/*" + class="hidden" />
♻️ Duplicate comments (1)
web/templates/profile.html (1)
61-64: Add accessible label for initial/avatar fallback
The fallback<div>(showing the user’s initial) still lacks an ARIA role and label. Screen readers will ignore it without an explicit role oraria-label. Please add, for example:<div role="img" aria-label="{{ user.username }}'s avatar" class="w-32 h-32 rounded-full bg-orange-500 …"> … </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/templates/profile.html(3 hunks)web/views.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (4)
web/templates/profile.html (4)
33-37: Form merge andenctypeare correctly applied
Consolidating the avatar upload and profile update into a single<form>withid="main-form"andenctype="multipart/form-data"is the right approach for handling file uploads alongside other fields. Good job on ensuring CSRF protection remains in place.
530-537: JSchangelistener correctly flags avatar updates
The event listener onavatarInputcleanly setssessionStorage.avatarChangedwhen a user selects a new file. This separation of concerns (setting a flag, deferring actual submission) is well done.
539-545: JSsubmithandler disables unused input
Disabling the file input whenavatarChanged≠'true'is an effective way to omit it from the submission. Usingform.submit()afterward ensures the disabled field is not serialized.
547-550: Reset flag and submit via JS
ClearingsessionStorage.avatarChangedand invokingmainForm.submit()(which bypasses the JS listener) is correct. This guarantees that the disabled state doesn’t lock the form for future submissions.
|
@coderabbitai review and approve |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@A1L13N I'll review this PR for you. Let me gather the necessary information about the changes first. 🏁 Script executed: Length of output: 97758 @A1L13N After examining the PR changes, I'll review this fix for the avatar image bug (issue #586). 🏁 Script executed: Length of output: 5633 @A1L13N I've completed my review of the PR that fixes the avatar image bug (issue #586). This is a good solution to the problem. What was the bug?When users tried to update their avatar image, all other profile fields were being cleared because the avatar was submitting in a separate form that didn't include the other field values. How does this fix work?
This approach preserves all profile field values when updating the avatar image, which fixes the issue described in #586. The implementation is clean and maintains a good user experience. I approve this pull request. |
A1L13N
dismissed
gemini-code-assist[bot]’s stale review
@coderabbitai full review
2 participants
Related issues
Fixes #586
Checklist
Fixed avatar image bug that happens when trying to update the avatar.
Summary by CodeRabbit
New Features
Enhancements