Story: 2437 User Profile Edit UI#2478
Open
jlchilders11 wants to merge 8 commits into
Open
Conversation
julioest
approved these changes
Jun 5, 2026
herzog0
requested changes
Jun 9, 2026
herzog0
left a comment
herzog0
left a comment
Collaborator
There was a problem hiding this comment.
Heya! Thanks for this, the UI is looking great :D
Just a few things I think should be adjusted before shipping
Collaborator
There was a problem hiding this comment.
Heya! Is there any specific reason we need to duplicate the wysiwyg component here?
I also noticed there are a lot of imports not being used and a few function calls/refs that don't seem to exist in the scope of the script (e.g. isSafeUrl, parseMarkdownSafe and others..).
Collaborator
Author
There was a problem hiding this comment.
So this was the best solution I found for the requirements of limiting the features on the wysiwig. I'm not super happy with this solution, but it is the best one I discovered.
Collaborator
There was a problem hiding this comment.
Sure thing, no problem with that specifically. My comment is more related to the unused imports and the missing function references. Did you have the chance to take a look at those?
Comment on lines
+28
to
+30
| {% else %} | ||
| {{ type }} | ||
| {% endif %} |
Collaborator
There was a problem hiding this comment.
Should we really display the raw type when it doesn't fit any case above?
Collaborator
Author
There was a problem hiding this comment.
This is something of an opinionated option, but I think that all forms should be tested locally before they go live. This provides some feedback of "you have used an unsupported type" which the dev can then address.
| {% if error %}aria-invalid="true" aria-describedby="field-{{ name }}-error"{% elif help_text %}aria-describedby="field-{{ name }}-help"{% endif %} | ||
| x-show="!jsReady" | ||
| :disabled="jsReady" | ||
| :disabled="jsReady || {{disabled}}" |
Collaborator
There was a problem hiding this comment.
If we don't pass disabled=field.disabled in the call to _field_dropdown from the _field_include.html, then this expression evaluates to jsReady || and it throws an error:
Collaborator
Author
There was a problem hiding this comment.
Good catch, added a default to "false" if the value is not passed.
📝 WalkthroughWalkthroughThis PR introduces a complete v3 user profile edit interface featuring a TipTap-based WYSIWYG editor with Markdown support, comprehensive form field templates, backend form definitions for profile links and commit emails, view routing logic, a two-column layout template, and supporting CSS/design system enhancements for forms, avatars, and responsive layout. ChangesUser Profile Edit Feature
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/wysiwyg-editor.js (1)
35-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winExport additional functions required by user-profile-wysiwyg-editor.js.
The user-profile editor (imported at line 19 of
user-profile-wysiwyg-editor.js) calls several functions from this module that are not exported:
parseMarkdownSafe(line 35) - called at lines 157, 199, 239, 267, 287turndown(line 53) - used at lines 253, 308isSafeUrl(line 87) - called at line 79openModal(line 96) - called at line 75highlightPreviewCodeBlocks(line 655) - called at lines 240, 288renderMermaidPreview(line 669) - called at lines 241, 289Without exporting these, the user-profile editor will crash with
ReferenceErrorat runtime.📤 Proposed fix
-function parseMarkdownSafe(md) { +export function parseMarkdownSafe(md) { return DOMPurify.sanitize(marked.parse(md)); } -function sanitizeSvg(svgString) { +export function sanitizeSvg(svgString) { return DOMPurify.sanitize(svgString, { USE_PROFILES: { svg: true, svgFilters: true }, ADD_TAGS: ["use"] }); } // ... later in file ... -const turndown = new TurndownService({ +export const turndown = new TurndownService({ headingStyle: "atx", // ... later in file ... -const isSafeUrl = (url) => { +export const isSafeUrl = (url) => { try { // ... later in file ... -const openModal = (title, fields) => +export const openModal = (title, fields) => new Promise((resolve) => { // ... later in file ... -const highlightPreviewCodeBlocks = (container) => { +export const highlightPreviewCodeBlocks = (container) => { container.querySelectorAll("pre code[class*='language-']").forEach((codeEl) => { // ... later in file ... -const renderMermaidPreview = async (container) => { +export const renderMermaidPreview = async (container) => { const mermaidCodes = container.querySelectorAll("code.language-mermaid");Also applies to: 53-62, 87-94, 96-164, 655-667, 669-695
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/wysiwyg-editor.js` around lines 35 - 36, The module currently defines utility functions used by user-profile-wysiwyg-editor.js but doesn't export them, causing runtime ReferenceErrors; update the file to export the required functions—add named exports for parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, and renderMermaidPreview (or export them together at the bottom with export { parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, renderMermaidPreview };), ensuring the functions referenced (parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, renderMermaidPreview) remain defined and available to imports.templates/v3/includes/_avatar_v3.html (1)
31-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit intrinsic dimensions for
size="xxxl"images.Line 31 still falls back to
40x40forxxxl, while CSS renders112x112. Add anxxxlbranch so intrinsic size matches rendered size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/v3/includes/_avatar_v3.html` at line 31, The img tag rendering avatar intrinsic dimensions falls back to 40x40 for size=="xxxl"; update the width and height template expressions in the <img> that produces class "avatar__img" to add an elif branch for size == 'xxxl' and return 112 for both width and height so the intrinsic dimensions match the CSS-rendered 112x112.
♻️ Duplicate comments (2)
frontend/user-profile-wysiwyg-editor.js (1)
1-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused imports.
Several imports are not used in this file:
- Line 19:
handleKeyDownandhandlePasteare imported but never called (reimplemented inline at lines 177-187 and 188-207)- Lines 3, 6-10: TipTap extensions
CodeBlockLowlight,Table,TableRow,TableCell,TableHeader, andImageare imported but never configured in the editor (lines 166-171 only use StarterKit, Underline, TaskList, TaskItem)🧹 Proposed fix
import { Editor } from "`@tiptap/core`"; import StarterKit from "`@tiptap/starter-kit`"; -import CodeBlockLowlight from "`@tiptap/extension-code-block-lowlight`"; import Underline from "`@tiptap/extension-underline`"; -import Link from "`@tiptap/extension-link`"; -import Table from "`@tiptap/extension-table`"; -import TableRow from "`@tiptap/extension-table-row`"; -import TableCell from "`@tiptap/extension-table-cell`"; -import TableHeader from "`@tiptap/extension-table-header`"; -import Image from "`@tiptap/extension-image`"; import TaskList from "`@tiptap/extension-task-list`"; import TaskItem from "`@tiptap/extension-task-item`"; -import { common, createLowlight } from "lowlight"; -import { toHtml } from "hast-util-to-html"; -import { marked } from "marked"; -import DOMPurify from "dompurify"; -import TurndownService from "turndown"; -import { gfm } from "turndown-plugin-gfm"; -import {handleKeyDown, handlePaste, createToolbarButton, ICONS, setupMermaidEditMode, debounce} from './wysiwyg-editor'; +import { createToolbarButton, ICONS, setupMermaidEditMode, debounce } from './wysiwyg-editor';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/user-profile-wysiwyg-editor.js` around lines 1 - 19, Remove the unused imports or wire them up: either delete the unused symbols (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader, Image, handleKeyDown, handlePaste) from the top of frontend/user-profile-wysiwyg-editor.js, or instead reuse the shared implementations by calling the imported handleKeyDown and handlePaste functions where inline handlers are currently implemented (lines ~177-207) and add the missing TipTap extensions (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader, Image) into the Editor configuration (the array passed to new Editor / extensions at ~166-171) so they are actually used; update imports accordingly to keep only what the file uses.users/forms.py (1)
207-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstantiate nested formsets per
V3UserProfileForminstance, not as class attributes.
link_formsetandcommit_email_formsetare currently created at class scope (Line 239 and Line 282), andif commit_emails:(Line 218) skips rebuilding when an empty list is passed. That leaves shared/default state in place and will produce incorrect per-user formset state.Suggested fix
class V3UserProfileForm(forms.Form): def __init__(self, *args, **kwargs): - links = kwargs.pop("user_links", None) - commit_emails = kwargs.pop("commit_emails", None) + links = kwargs.pop("user_links", None) or {} + commit_emails = kwargs.pop("commit_emails", None) or [] super().__init__(*args, **kwargs) - if links: - self.link_formset = V3ProfileLinkFormset( - initial=[ - {"type": x, "value": links.get(x, "")} - for x in V3ProfileLinkChoices.values - ], - ) - if commit_emails: - self.commit_email_formset = V3CommitEmailFormSet( - initial=[ - { - "email": x, - } - for x in commit_emails - ], - ) + self.link_formset = V3ProfileLinkFormset( + initial=[ + {"type": x, "value": links.get(x, "")} + for x in V3ProfileLinkChoices.values + ] + ) + self.commit_email_formset = V3CommitEmailFormSet( + initial=[{"email": x} for x in commit_emails] + ) @@ - link_formset = V3ProfileLinkFormset( - initial=[{"type": x, "value": ""} for x in V3ProfileLinkChoices.values], - ) @@ - commit_email_formset = V3CommitEmailFormSet( - initial=[ - { - "email": "abc@example.com", - }, - ], - )Also applies to: 239-241, 282-288
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@users/forms.py` around lines 207 - 227, The V3UserProfileForm currently leaves link_formset and commit_email_formset as class-level/shared defaults and skips rebuilding when an empty list is passed; in V3UserProfileForm.__init__ always assign instance attributes self.link_formset and self.commit_email_formset (not rely on class attributes) and construct them unconditionally or when the corresponding kwargs is not None (use "if commit_emails is not None" to allow empty lists), creating V3ProfileLinkFormset (using V3ProfileLinkChoices.values for initial) and V3CommitEmailFormSet (using commit_emails for initial) so each form instance has its own fresh formsets instead of shared state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 89-91: The function handleDocClick in buildToolbar references
undefined variables tableWrapper and gridPopup; remove this dead code (delete
the handleDocClick definition and any registrations like
document.addEventListener('click', handleDocClick)) or, if behavior is required,
replace it with a safe implementation that uses existing toolbar elements
instead; ensure no leftover event listeners or references to
tableWrapper/gridPopup remain in buildToolbar.
- Line 21: Remove the unused lowlight initialization: delete the const lowlight
= createLowlight(common) line (and remove the createLowlight and common imports
if they are no longer used) because CodeBlockLowlight is not included in this
editor configuration; ensure no other references to lowlight or createLowlight
remain in the file (search for lowlight, createLowlight, and common) and run the
linter/TypeScript checks to confirm no unused-import errors.
- Around line 75-87: The Link action uses openModal and isSafeUrl but those
helpers are only defined (const isSafeUrl, const openModal) in another module
without exports, causing runtime failures; either export them from
frontend/wysiwyg-editor.js and import them into this file, or implement
equivalents locally in frontend/user-profile-wysiwyg-editor.js, then update the
Link command to call the imported/local openModal and isSafeUrl before calling
editor.chain().focus().setLink(...). Ensure the exported names match exactly:
openModal and isSafeUrl.
- Around line 157-160: The file references undefined symbols parseMarkdownSafe,
highlightPreviewCodeBlocks, renderMermaidPreview, and turndown causing
ReferenceErrors; fix by exporting those helpers (or a single helper that returns
turndown) from frontend/wysiwyg-editor.js and importing them into
frontend/user-profile-wysiwyg-editor.js, or alternatively implement equivalent
local versions in user-profile-wysiwyg-editor.js that match the behavior used
around initialContent parsing and preview rendering (update the catch block
usage of parseMarkdownSafe, the preview calls to
highlightPreviewCodeBlocks/renderMermaidPreview, and any turndown
instantiation/usage to use the imported or locally defined identifiers).
In `@templates/v3/includes/_field_checkbox.html`:
- Around line 16-21: Change the hard-coded id/for pair to use an optional
override variable so IDs can be unique when the partial is reused: update the
label's for and the input's id from "checkbox-{{ name }}" to use something like
"{{ input_id or ('checkbox-' ~ name) }}", so the template will accept an
explicit input_id (unique per option) but fall back to the existing name-based
id when not provided; then in multi-checkbox callers (e.g.,
checkboxselectmultiple loops) pass a unique input_id (forloop.counter0 or
similar) for each option.
In `@templates/v3/includes/_field_dropdown.html`:
- Around line 103-106: The dropdown trigger element with class
"dropdown__trigger" and role="button" currently keeps tabindex="0" even when the
template variable disabled is true; update the template so that when disabled is
true the element is not tabbable (remove tabindex or set tabindex="-1") and add
aria-disabled="true" to indicate non-interactive state for assistive tech; apply
the same change to the other occurrence around the second trigger (the block
referenced at line ~115) so both disabled triggers are non-focusable and
properly exposed as disabled.
- Line 95: The Alpine :disabled expression currently renders Python True/False
and breaks JS logic; change the serialization of the Django `disabled` context
used in the `:disabled` attribute to emit lowercase `true`/`false` (e.g. use the
template filter like `{{ disabled|default:"false"|lower }}` or `{{
disabled|yesno:"true,false" }}`) so the expression becomes `jsReady ||
true/false`; additionally, when the trigger is disabled ensure it is not
focusable and exposes state: add aria-disabled="true" and set tabindex="-1" when
`disabled` is true, and remove/restore tabindex/aria-disabled when false (update
the same template element that contains the `:disabled` attribute).
In `@templates/v3/includes/_field_include.html`:
- Line 15: The checkbox include is using bound_field.initial as the value which
doesn't set the checked state; update the include invocation in
_field_include.html to pass the current boolean state (bound_field.value) as the
checked argument (e.g., include 'v3/includes/_field_checkbox.html' with ...
checked=bound_field.value ...) so the checkbox reflects the bound field's actual
value; keep other parameters (name, label, required, disabled, help_text)
unchanged.
In `@templates/v3/user_profile_edit.html`:
- Line 28: The WYSIWYG include is wired to textarea_name='content' and
textarea_id='id_content' but the form field is user_profile_form.bio, so update
the include invocation (the call to v3/includes/_wysiwyg_editor.html) to use
textarea_name='bio' and textarea_id='id_bio' so the editor binds to
user_profile_form.bio; also search for any client-side code that references
id_content and update it to id_bio to keep JS submit/wiring working.
- Line 121: The included bound field name is misspelled: change the include that
references user_profile_form.ovverride_commit_author_email to use the correct
field name user_profile_form.override_commit_author_email so the template
renders and wires the override_commit_author_email field properly (update the
bound_field in the include call).
In `@users/forms.py`:
- Around line 250-261: These BooleanField profile checkboxes currently validate
as required and cause errors when left unchecked; update the field definitions
for hide_github, hide_ml, hide_ach, indicate_last_login_method,
override_commit_author_name, override_commit_author_email, and
allow_notification_terms_updated in users/forms.py to include required=False so
unchecked boxes are accepted as False, and keep existing labels/help_text
unchanged to preserve behavior.
In `@users/views.py`:
- Around line 108-109: The current check self.request.GET.get("edit", False)
treats any non-empty value (e.g. ?edit=false) as true; change to an explicit
parse such as reading val = self.request.GET.get("edit") and enabling edit mode
only when val is exactly "true" (case-insensitive) or when you intentionally
want presence semantics use "if 'edit' in self.request.GET" — update the
V3UserProfileForm branch and duplicate the same fix at the other occurrence (the
check around lines 419-420) so both locations use the same explicit comparison
(e.g., val and val.lower() == "true" or presence check).
---
Outside diff comments:
In `@frontend/wysiwyg-editor.js`:
- Around line 35-36: The module currently defines utility functions used by
user-profile-wysiwyg-editor.js but doesn't export them, causing runtime
ReferenceErrors; update the file to export the required functions—add named
exports for parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, and renderMermaidPreview (or export them together at
the bottom with export { parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, renderMermaidPreview };), ensuring the functions
referenced (parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, renderMermaidPreview) remain defined and available
to imports.
In `@templates/v3/includes/_avatar_v3.html`:
- Line 31: The img tag rendering avatar intrinsic dimensions falls back to 40x40
for size=="xxxl"; update the width and height template expressions in the <img>
that produces class "avatar__img" to add an elif branch for size == 'xxxl' and
return 112 for both width and height so the intrinsic dimensions match the
CSS-rendered 112x112.
---
Duplicate comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 1-19: Remove the unused imports or wire them up: either delete the
unused symbols (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader,
Image, handleKeyDown, handlePaste) from the top of
frontend/user-profile-wysiwyg-editor.js, or instead reuse the shared
implementations by calling the imported handleKeyDown and handlePaste functions
where inline handlers are currently implemented (lines ~177-207) and add the
missing TipTap extensions (CodeBlockLowlight, Table, TableRow, TableCell,
TableHeader, Image) into the Editor configuration (the array passed to new
Editor / extensions at ~166-171) so they are actually used; update imports
accordingly to keep only what the file uses.
In `@users/forms.py`:
- Around line 207-227: The V3UserProfileForm currently leaves link_formset and
commit_email_formset as class-level/shared defaults and skips rebuilding when an
empty list is passed; in V3UserProfileForm.__init__ always assign instance
attributes self.link_formset and self.commit_email_formset (not rely on class
attributes) and construct them unconditionally or when the corresponding kwargs
is not None (use "if commit_emails is not None" to allow empty lists), creating
V3ProfileLinkFormset (using V3ProfileLinkChoices.values for initial) and
V3CommitEmailFormSet (using commit_emails for initial) so each form instance has
its own fresh formsets instead of shared state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f3a887f-0e3b-46eb-816b-1eba3b5f28d3
📒 Files selected for processing (19)
frontend/user-profile-wysiwyg-editor.jsfrontend/wysiwyg-editor.jspackage.jsonstatic/css/v3/avatar.cssstatic/css/v3/buttons.cssstatic/css/v3/forms.cssstatic/css/v3/user-profile-page.cssstatic/css/v3/wysiwyg-editor.cssstatic/js/v3/user-profile-wysiwyg-editor.jstemplates/includes/icon.htmltemplates/v3/includes/_avatar_v3.htmltemplates/v3/includes/_field_checkbox.htmltemplates/v3/includes/_field_dropdown.htmltemplates/v3/includes/_field_include.htmltemplates/v3/includes/_form_include.htmltemplates/v3/includes/_wysiwyg_editor.htmltemplates/v3/user_profile_edit.htmlusers/forms.pyusers/views.py
| import { gfm } from "turndown-plugin-gfm"; | ||
| import {handleKeyDown, handlePaste, createToolbarButton, ICONS, setupMermaidEditMode, debounce} from './wysiwyg-editor'; | ||
|
|
||
| const lowlight = createLowlight(common); |
There was a problem hiding this comment.
Remove unused lowlight initialization.
The lowlight constant is created but never used, since CodeBlockLowlight is not included in this editor's configuration.
🧹 Proposed fix
-const lowlight = createLowlight(common);
-
const editorInstances = new Map();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const lowlight = createLowlight(common); | |
| const editorInstances = new Map(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/user-profile-wysiwyg-editor.js` at line 21, Remove the unused
lowlight initialization: delete the const lowlight = createLowlight(common) line
(and remove the createLowlight and common imports if they are no longer used)
because CodeBlockLowlight is not included in this editor configuration; ensure
no other references to lowlight or createLowlight remain in the file (search for
lowlight, createLowlight, and common) and run the linter/TypeScript checks to
confirm no unused-import errors.
Comment on lines
+75
to
+87
| const result = await openModal("Insert Link", [ | ||
| { name: "url", label: "URL", type: "url", placeholder: "https://example.com" }, | ||
| ]); | ||
| if (!result || !result.url) return; | ||
| if (!isSafeUrl(result.url)) { | ||
| window.alert("Only http, https, and mailto URLs are allowed."); | ||
| return; | ||
| } | ||
| editor.chain().focus().setLink({ href: result.url }).run(); | ||
| }, | ||
| isActive: () => editor.isActive("link"), | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that openModal and isSafeUrl are not exported from wysiwyg-editor.js
echo "=== Checking if openModal is exported ==="
rg -n "export.*openModal" frontend/wysiwyg-editor.js || echo "Not found as export"
echo ""
echo "=== Checking if isSafeUrl is exported ==="
rg -n "export.*isSafeUrl" frontend/wysiwyg-editor.js || echo "Not found as export"
echo ""
echo "=== Checking where openModal is defined ==="
rg -n "^(const|function|export const|export function)\s+openModal" frontend/wysiwyg-editor.js
echo ""
echo "=== Checking where isSafeUrl is defined ==="
rg -n "^(const|function|export const|export function)\s+isSafeUrl" frontend/wysiwyg-editor.jsRepository: boostorg/website-v2
Length of output: 350
Export/import openModal and isSafeUrl for the Link action to avoid runtime failures
frontend/user-profile-wysiwyg-editor.js (lines 75-87) uses openModal and isSafeUrl, but frontend/wysiwyg-editor.js only defines them (const isSafeUrl ... at 87 and const openModal ... at 96) and has no corresponding named exports, so these identifiers won’t be available in this module’s scope when the Link command runs.
Export openModal/isSafeUrl from frontend/wysiwyg-editor.js and import them here, or implement those helpers locally in frontend/user-profile-wysiwyg-editor.js.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/user-profile-wysiwyg-editor.js` around lines 75 - 87, The Link
action uses openModal and isSafeUrl but those helpers are only defined (const
isSafeUrl, const openModal) in another module without exports, causing runtime
failures; either export them from frontend/wysiwyg-editor.js and import them
into this file, or implement equivalents locally in
frontend/user-profile-wysiwyg-editor.js, then update the Link command to call
the imported/local openModal and isSafeUrl before calling
editor.chain().focus().setLink(...). Ensure the exported names match exactly:
openModal and isSafeUrl.
Comment on lines
+89
to
+91
| const handleDocClick = (e) => { | ||
| if (!tableWrapper.contains(e.target)) gridPopup.style.display = "none"; | ||
| }; |
There was a problem hiding this comment.
Remove dead code referencing undefined variables.
This handleDocClick function references tableWrapper and gridPopup which are not defined anywhere in the buildToolbar function scope. This appears to be leftover code from the full editor that wasn't cleaned up during the simplification.
🗑️ Proposed fix
});
- const handleDocClick = (e) => {
- if (!tableWrapper.contains(e.target)) gridPopup.style.display = "none";
- };
-
const mdBtn = document.createElement("button");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleDocClick = (e) => { | |
| if (!tableWrapper.contains(e.target)) gridPopup.style.display = "none"; | |
| }; | |
| }); | |
| const mdBtn = document.createElement("button"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/user-profile-wysiwyg-editor.js` around lines 89 - 91, The function
handleDocClick in buildToolbar references undefined variables tableWrapper and
gridPopup; remove this dead code (delete the handleDocClick definition and any
registrations like document.addEventListener('click', handleDocClick)) or, if
behavior is required, replace it with a safe implementation that uses existing
toolbar elements instead; ensure no leftover event listeners or references to
tableWrapper/gridPopup remain in buildToolbar.
Comment on lines
+157
to
+160
| initialContent = parseMarkdownSafe(initialContent); | ||
| } catch (_) { | ||
| initialContent = rawContent; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if these required functions are exported from wysiwyg-editor.js
echo "=== Checking exports ==="
for func in parseMarkdownSafe highlightPreviewCodeBlocks renderMermaidPreview turndown; do
echo "Checking $func:"
rg -n "export.*(const|function)\s+$func" frontend/wysiwyg-editor.js || echo " ❌ Not exported"
done
echo ""
echo "=== Where they are defined in wysiwyg-editor.js ==="
rg -n "^(const|function)\s+(parseMarkdownSafe|highlightPreviewCodeBlocks|renderMermaidPreview|turndown)" frontend/wysiwyg-editor.jsRepository: boostorg/website-v2
Length of output: 514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== user-profile-wysiwyg-editor.js imports/exports near top ==="
sed -n '1,220p' frontend/user-profile-wysiwyg-editor.js | nl -ba | sed -n '1,220p'
echo ""
echo "=== user-profile-wysiwyg-editor.js: locate references ==="
rg -n "\bparseMarkdownSafe\b|\bhighlightPreviewCodeBlocks\b|\brenderMermaidPreview\b|\bturndown\b" frontend/user-profile-wysiwyg-editor.js -n
echo ""
echo "=== user-profile-wysiwyg-editor.js: check if identifiers are defined locally ==="
rg -n "function\s+parseMarkdownSafe|const\s+parseMarkdownSafe|let\s+parseMarkdownSafe|var\s+parseMarkdownSafe" frontend/user-profile-wysiwyg-editor.js || true
rg -n "function\s+highlightPreviewCodeBlocks|const\s+highlightPreviewCodeBlocks|let\s+highlightPreviewCodeBlocks|var\s+highlightPreviewCodeBlocks" frontend/user-profile-wysiwyg-editor.js || true
rg -n "function\s+renderMermaidPreview|const\s+renderMermaidPreview|let\s+renderMermaidPreview|var\s+renderMermaidPreview" frontend/user-profile-wysiwyg-editor.js || true
rg -n "const\s+turndown|let\s+turndown|var\s+turndown" frontend/user-profile-wysiwyg-editor.js || true
echo ""
echo "=== wysiwyg-editor.js: check exports mechanism ==="
rg -n "export\s+(default\s+)?|module\.exports|exports\." frontend/wysiwyg-editor.js
echo ""
echo "=== wysiwyg-editor.js: show definitions for referenced identifiers ==="
rg -n "function\s+parseMarkdownSafe|const\s+turndown\s*=|const\s+highlightPreviewCodeBlocks\s*=|const\s+renderMermaidPreview\s*=" frontend/wysiwyg-editor.js -n
echo ""
echo "=== wysiwyg-editor.js: show surrounding code for the definitions ==="
for line in 35 53 655 669; do
echo "--- context around line $line ---"
sed -n "$((line-5)),$((line+20))p" frontend/wysiwyg-editor.js | nl -ba | sed -n '1,80p'
doneRepository: boostorg/website-v2
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== user-profile-wysiwyg-editor.js: show first ~240 lines with line numbers ==="
awk '{printf "%d:%s\n", NR, $0}' frontend/user-profile-wysiwyg-editor.js | sed -n '1,240p'
echo ""
echo "=== user-profile-wysiwyg-editor.js: locate references to the identifiers ==="
rg -n "\bparseMarkdownSafe\b|\bhighlightPreviewCodeBlocks\b|\brenderMermaidPreview\b|\bturndown\b" frontend/user-profile-wysiwyg-editor.js
echo ""
echo "=== user-profile-wysiwyg-editor.js: locate any imports for wysiwyg-editor.js ==="
rg -n "from\s+['\"].*wysiwyg-editor['\"]|require\(['\"].*wysiwyg-editor['\"]\)" frontend/user-profile-wysiwyg-editor.js || true
echo ""
echo "=== user-profile-wysiwyg-editor.js: check whether any of these identifiers are declared locally ==="
rg -n "(function|const|let|var)\s+parseMarkdownSafe\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(function|const|let|var)\s+highlightPreviewCodeBlocks\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(function|const|let|var)\s+renderMermaidPreview\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(const|let|var)\s+turndown\b" frontend/user-profile-wysiwyg-editor.js || true
echo ""
echo "=== wysiwyg-editor.js: check how these identifiers are declared and exported ==="
rg -n "function\s+parseMarkdownSafe|const\s+turndown\s*=|const\s+highlightPreviewCodeBlocks\s*=|const\s+renderMermaidPreview\s*=" frontend/wysiwyg-editor.js
echo ""
echo "=== wysiwyg-editor.js: check for exports of these identifiers ==="
rg -n "export\s*\{[^}]*\b(parseMarkdownSafe|highlightPreviewCodeBlocks|renderMermaidPreview|turndown)\b[^}]*\}|export\s+(const|function)\s+(parseMarkdownSafe|highlightPreviewCodeBlocks|renderMermaidPreview|turndown)\b|module\.exports|exports\." frontend/wysiwyg-editor.js || true
echo ""
echo "=== wysiwyg-editor.js: show context around each definition/export area ==="
for start in 1 20 40 620 640 650 660 670; do
:
done
for line in 35 53 655 669; do
echo "--- context around wysiwyg-editor.js:$line ---"
awk -v s="$((line-10))" -v e="$((line+25))" 'NR>=s && NR<=e {printf "%d:%s\n", NR, $0}' frontend/wysiwyg-editor.js
doneRepository: boostorg/website-v2
Length of output: 15757
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== user-profile-wysiwyg-editor.js: check for any local declarations (whole file) ==="
rg -n "(function|const|let|var)\s+parseMarkdownSafe\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(function|const|let|var)\s+highlightPreviewCodeBlocks\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(function|const|let|var)\s+renderMermaidPreview\b" frontend/user-profile-wysiwyg-editor.js || true
rg -n "(const|let|var)\s+turndown\b" frontend/user-profile-wysiwyg-editor.js || true
echo ""
echo "=== user-profile-wysiwyg-editor.js: show context around reported usages (150-220) ==="
awk 'NR>=140 && NR<=220 {printf "%d:%s\n", NR, $0}' frontend/user-profile-wysiwyg-editor.js
echo ""
echo "=== user-profile-wysiwyg-editor.js: show context around reported usages (230-320) ==="
awk 'NR>=220 && NR<=340 {printf "%d:%s\n", NR, $0}' frontend/user-profile-wysiwyg-editor.js
echo ""
echo "=== wysiwyg-editor.js: show exports section (if any) ==="
rg -n "export\s*\{|export\s+default|module\.exports|exports\." frontend/wysiwyg-editor.js || trueRepository: boostorg/website-v2
Length of output: 8029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for global assignments of these identifiers ==="
for sym in parseMarkdownSafe highlightPreviewCodeBlocks renderMermaidPreview turndown; do
echo "--- $sym ---"
rg -n "globalThis\.${sym}|window\.${sym}|global\.\${sym}|self\.${sym}" -S . || true
done
echo ""
echo "=== Search for re-exports from wysiwyg-editor.js ==="
rg -n "wysiwyg-editor['\"]" frontend -S || true
rg -n "export\s*\{[^}]*parseMarkdownSafe|export\s*\{[^}]*highlightPreviewCodeBlocks|export\s*\{[^}]*renderMermaidPreview|export\s*\{[^}]*turndown" frontend/wysiwyg-editor.js -S || true
echo ""
echo "=== Search for any import specifiers of parseMarkdownSafe etc from wysiwyg-editor ==="
rg -n "from\s+['\"].*wysiwyg-editor['\"]" frontend/user-profile-wysiwyg-editor.js -n
rg -n "parseMarkdownSafe|highlightPreviewCodeBlocks|renderMermaidPreview|turndown" frontend/user-profile-wysiwyg-editor.js -nRepository: boostorg/website-v2
Length of output: 2403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check for any ES-module exports of these symbols in wysiwyg-editor.js
echo "=== wysiwyg-editor.js: export statements referencing these symbols ==="
for sym in parseMarkdownSafe highlightPreviewCodeBlocks renderMermaidPreview turndown; do
echo "--- $sym ---"
rg -n "export\s+(const|function)\s+$sym\b" frontend/wysiwyg-editor.js || true
rg -n "export\s*\{[^}]*\b$sym\b[^}]*\}" frontend/wysiwyg-editor.js || true
rg -n "\bexport\s*\*\s*from\s*['\"][^'\"]*['\"]" frontend/wysiwyg-editor.js || true
done
echo ""
echo "=== wysiwyg-editor.js: all occurrences of 'export' (context) ==="
rg -n "\bexport\b" frontend/wysiwyg-editor.js
echo ""
# 2) Check for global/window assignments in frontend for these symbols
echo "=== frontend/: global assignments for these symbols ==="
for sym in parseMarkdownSafe highlightPreviewCodeBlocks renderMermaidPreview turndown; do
echo "--- $sym ---"
rg -n "globalThis\.${sym}\b|window\.${sym}\b|global\.${sym}\b|self\.${sym}\b" frontend -S || true
done
echo ""
echo "=== user-profile-wysiwyg-editor.js: any imports of these symbols from wysiwyg-editor ==="
rg -n "from\s+['\"]\./wysiwyg-editor['\"]" frontend/user-profile-wysiwyg-editor.js
rg -n "import\s*\{[^}]*\b(parseMarkdownSafe|highlightPreviewCodeBlocks|renderMermaidPreview|turndown)\b[^}]*\}\s*from\s+['\"]\./wysiwyg-editor['\"]" frontend/user-profile-wysiwyg-editor.js || true
echo ""
echo "=== user-profile-wysiwyg-editor.js: whether these symbols are declared locally ==="
for sym in parseMarkdownSafe highlightPreviewCodeBlocks renderMermaidPreview turndown; do
rg -n "(function|const|let|var)\s+$sym\b" frontend/user-profile-wysiwyg-editor.js || true
doneRepository: boostorg/website-v2
Length of output: 1010
Fix undefined identifiers in user-profile-wysiwyg-editor.js (will crash with ReferenceError)
frontend/user-profile-wysiwyg-editor.js calls parseMarkdownSafe(), highlightPreviewCodeBlocks(), renderMermaidPreview(), and uses turndown, but none of these are imported or defined in this file. They are defined in frontend/wysiwyg-editor.js but are not exported, so these references will throw at runtime when the code paths run.
Snippet at issue location (final state):
initialContent = parseMarkdownSafe(initialContent);
} catch (_) {
initialContent = rawContent;
}Impacted identifiers/usages:
parseMarkdownSafe()(157, 199, 239, 267, 287)highlightPreviewCodeBlocks()(240, 288)renderMermaidPreview()(241, 289)turndown(253, 308)
Solution: Export parseMarkdownSafe, highlightPreviewCodeBlocks, renderMermaidPreview, and turndown (or a helper that returns turndown) from frontend/wysiwyg-editor.js and import them here, or implement/instantiate the same logic locally in frontend/user-profile-wysiwyg-editor.js.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/user-profile-wysiwyg-editor.js` around lines 157 - 160, The file
references undefined symbols parseMarkdownSafe, highlightPreviewCodeBlocks,
renderMermaidPreview, and turndown causing ReferenceErrors; fix by exporting
those helpers (or a single helper that returns turndown) from
frontend/wysiwyg-editor.js and importing them into
frontend/user-profile-wysiwyg-editor.js, or alternatively implement equivalent
local versions in user-profile-wysiwyg-editor.js that match the behavior used
around initialContent parsing and preview rendering (update the catch block
usage of parseMarkdownSafe, the preview calls to
highlightPreviewCodeBlocks/renderMermaidPreview, and any turndown
instantiation/usage to use the imported or locally defined identifiers).
Comment on lines
+16
to
+21
| <label class="checkbox{% if disabled %} checkbox--disabled{% endif %}{% if extra_class %} {{ extra_class }}{% endif %}" | ||
| for="checkbox-{{ name }}"> | ||
| <input class="checkbox__input" | ||
| type="checkbox" | ||
| id="checkbox-{{ name }}" | ||
| name="{{ name }}" |
There was a problem hiding this comment.
Make checkbox IDs overridable to avoid duplicate IDs in multi-checkbox use.
id="checkbox-{{ name }}" is not unique when this partial is reused in loops (e.g., checkboxselectmultiple), which breaks label targeting and produces invalid HTML.
🔧 Proposed fix
- <label class="checkbox{% if disabled %} checkbox--disabled{% endif %}{% if extra_class %} {{ extra_class }}{% endif %}"
- for="checkbox-{{ name }}">
+ <label class="checkbox{% if disabled %} checkbox--disabled{% endif %}{% if extra_class %} {{ extra_class }}{% endif %}"
+ for="{{ input_id|default:'checkbox-'|add:name }}">
<input class="checkbox__input"
type="checkbox"
- id="checkbox-{{ name }}"
+ id="{{ input_id|default:'checkbox-'|add:name }}"
name="{{ name }}"And in the multi-checkbox caller, pass a unique input_id per option (for example using forloop.counter0).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <label class="checkbox{% if disabled %} checkbox--disabled{% endif %}{% if extra_class %} {{ extra_class }}{% endif %}" | |
| for="checkbox-{{ name }}"> | |
| <input class="checkbox__input" | |
| type="checkbox" | |
| id="checkbox-{{ name }}" | |
| name="{{ name }}" | |
| <label class="checkbox{% if disabled %} checkbox--disabled{% endif %}{% if extra_class %} {{ extra_class }}{% endif %}" | |
| for="{{ input_id|default:'checkbox-'|add:name }}"> | |
| <input class="checkbox__input" | |
| type="checkbox" | |
| id="{{ input_id|default:'checkbox-'|add:name }}" | |
| name="{{ name }}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/v3/includes/_field_checkbox.html` around lines 16 - 21, Change the
hard-coded id/for pair to use an optional override variable so IDs can be unique
when the partial is reused: update the label's for and the input's id from
"checkbox-{{ name }}" to use something like "{{ input_id or ('checkbox-' ~ name)
}}", so the template will accept an explicit input_id (unique per option) but
fall back to the existing name-based id when not provided; then in
multi-checkbox callers (e.g., checkboxselectmultiple loops) pass a unique
input_id (forloop.counter0 or similar) for each option.
| {% elif type == 'text' %} | ||
| {% include 'v3/includes/_field_text.html' with name=bound_field.name value=bound_field.initial type='text' label=bound_field.label required=field.required disabled=field.disabled help_text=field.help_text placeholder=widget.attrs.placeholder only %} | ||
| {% elif type == 'checkbox' %} | ||
| {% include 'v3/includes/_field_checkbox.html' with name=bound_field.name value=bound_field.initial label=bound_field.label required=field.required disabled=field.disabled help_text=field.help_text only %} |
There was a problem hiding this comment.
Bind checkbox checked state from bound_field.value.
Line 15 passes initial as value but never sets checked, so boolean fields can render unchecked even when true.
🔧 Proposed fix
- {% include 'v3/includes/_field_checkbox.html' with name=bound_field.name value=bound_field.initial label=bound_field.label required=field.required disabled=field.disabled help_text=field.help_text only %}
+ {% include 'v3/includes/_field_checkbox.html' with name=bound_field.name label=bound_field.label required=field.required disabled=field.disabled checked=bound_field.value help_text=field.help_text only %}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/v3/includes/_field_include.html` at line 15, The checkbox include
is using bound_field.initial as the value which doesn't set the checked state;
update the include invocation in _field_include.html to pass the current boolean
state (bound_field.value) as the checked argument (e.g., include
'v3/includes/_field_checkbox.html' with ... checked=bound_field.value ...) so
the checkbox reflects the bound field's actual value; keep other parameters
(name, label, required, disabled, help_text) unchanged.
| </div> | ||
| <hr class="card__hr" aria-hidden="true" /> | ||
| <div class="card__column px-large"> | ||
| {% include 'v3/includes/_wysiwyg_editor.html' with textarea_id='id_content' textarea_name='content' label='Biography' help_text='This text field supports Markdown and this content is what will appear on your public profile' only %} |
There was a problem hiding this comment.
Bind the WYSIWYG biography editor to user_profile_form.bio instead of content.
Line 28 hardcodes textarea_name='content', while the backing form defines bio. This breaks cross-layer field mapping and will drop/skip bio when wiring submit handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/v3/user_profile_edit.html` at line 28, The WYSIWYG include is wired
to textarea_name='content' and textarea_id='id_content' but the form field is
user_profile_form.bio, so update the include invocation (the call to
v3/includes/_wysiwyg_editor.html) to use textarea_name='bio' and
textarea_id='id_bio' so the editor binds to user_profile_form.bio; also search
for any client-side code that references id_content and update it to id_bio to
keep JS submit/wiring working.
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.country %} | ||
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.indicate_last_login_method %} | ||
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.override_commit_author_name %} | ||
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.ovverride_commit_author_email %} |
There was a problem hiding this comment.
Fix misspelled bound field name for commit-author email override.
Line 121 uses ovverride_commit_author_email, but the form field is override_commit_author_email. This leaves the field unrendered/miswired.
Suggested fix
- {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.ovverride_commit_author_email %}
+ {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.override_commit_author_email %}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.ovverride_commit_author_email %} | |
| {% include 'v3/includes/_field_include.html' with bound_field=user_profile_form.override_commit_author_email %} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/v3/user_profile_edit.html` at line 121, The included bound field
name is misspelled: change the include that references
user_profile_form.ovverride_commit_author_email to use the correct field name
user_profile_form.override_commit_author_email so the template renders and wires
the override_commit_author_email field properly (update the bound_field in the
include call).
Collaborator
There was a problem hiding this comment.
@jlchilders11 I think this is a good one to fix
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Comment on lines
+250
to
+261
| hide_github = forms.BooleanField( | ||
| label="Hide GitHub activity from your profile", | ||
| help_text="Links your login to an existing commit-author email after verification", | ||
| ) | ||
| hide_ml = forms.BooleanField( | ||
| label="Hide mailing list activity from your profile", | ||
| help_text="Links your login to an existing commit-author email after verification", | ||
| ) | ||
| hide_ach = forms.BooleanField( | ||
| label="Hide achievements & badges from your profile", | ||
| help_text="Links your login to an existing commit-author email after verification", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
ls -la users/forms.py
# Show the relevant sections with line numbers
sed -n '220,330p' users/forms.py | nl -ba | sed -n '1,130p'
# Also search for BooleanField definitions in the file
rg -n "forms\.BooleanField\(" users/forms.py
# Extract any occurrences that mention required= or validation around these booleans
rg -n "required\s*=" users/forms.pyRepository: boostorg/website-v2
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File =="
ls -la users/forms.py
echo "== Lines 235-315 (with line numbers) =="
sed -n '235,315p' users/forms.py | cat -n
echo "== BooleanField occurrences =="
rg -n "forms\.BooleanField\(" users/forms.py
echo "== Any required= usage nearby =="
rg -n "required\s*=" users/forms.pyRepository: boostorg/website-v2
Length of output: 4157
🌐 Web query:
Django forms.BooleanField default required value
💡 Result:
In Django forms, BooleanField has required=True by default (inherited from Field), so an “empty” value causes a required-field ValidationError unless you set required=False [1][2]. For the checkbox behavior itself, BooleanField normalizes values to Python True/False and defines an “empty value” of False [3]. That means when the checkbox isn’t checked (i.e., the field is omitted from POST data), it’s treated as False; however, whether that’s allowed depends on required: - required=True (default): the field validates that the value is True, so unchecked (False) fails validation [1][3]. - required=False: unchecked (False) is accepted (cleaned_data will contain False) [1][3]. So the “required value” question is: BooleanField’s default behavior is required=True (must be checked / True), and its empty value is False [1][3].
Citations:
- 1: https://docs.djangoproject.com/en/5.0/ref/forms/fields/
- 2: https://docs.djangoproject.com/en/6.0/ref/forms/fields/
- 3: https://django.readthedocs.io/en/latest/ref/forms/fields.html
Set required=False for these unchecked profile checkboxes to avoid validation errors
In users/forms.py, these forms.BooleanField fields don’t specify required=False, so leaving a checkbox unchecked (False) is treated as an empty value and raises a validation error instead of accepting False:
hide_github,hide_ml,hide_ach(lines ~250-261)indicate_last_login_method,override_commit_author_name,override_commit_author_email(lines ~271-279)allow_notification_terms_updated(lines ~303-305)
Suggested fix
hide_github = forms.BooleanField(
label="Hide GitHub activity from your profile",
help_text="Links your login to an existing commit-author email after verification",
+ required=False,
)
@@
hide_ml = forms.BooleanField(
label="Hide mailing list activity from your profile",
help_text="Links your login to an existing commit-author email after verification",
+ required=False,
)
@@
hide_ach = forms.BooleanField(
label="Hide achievements & badges from your profile",
help_text="Links your login to an existing commit-author email after verification",
+ required=False,
)
@@
indicate_last_login_method = forms.BooleanField(
- help_text="The login page will indicate the last method you used to login"
+ help_text="The login page will indicate the last method you used to login",
+ required=False,
)
@@
override_commit_author_name = forms.BooleanField(
- help_text="Globally replaces your git commit author name with username value set above"
+ help_text="Globally replaces your git commit author name with username value set above",
+ required=False,
)
@@
override_commit_author_email = forms.BooleanField(
- help_text="Links your login to an existing commit-author email after verification"
+ help_text="Links your login to an existing commit-author email after verification",
+ required=False,
)
@@
allow_notification_terms_updated = forms.BooleanField(
- label="The sites terms of use or privacy policy are changed"
+ label="The sites terms of use or privacy policy are changed",
+ required=False,
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@users/forms.py` around lines 250 - 261, These BooleanField profile checkboxes
currently validate as required and cause errors when left unchecked; update the
field definitions for hide_github, hide_ml, hide_ach,
indicate_last_login_method, override_commit_author_name,
override_commit_author_email, and allow_notification_terms_updated in
users/forms.py to include required=False so unchecked boxes are accepted as
False, and keep existing labels/help_text unchanged to preserve behavior.
Comment on lines
+108
to
+109
| if self.request.GET.get("edit", False): | ||
| ctx["user_profile_form"] = V3UserProfileForm( |
There was a problem hiding this comment.
edit query handling is too permissive.
Line 108 and Line 419 treat any non-empty edit value as enabled, so ?edit=false still renders edit mode. Parse explicitly for "true" (or presence semantics) to match the intended route contract.
Also applies to: 419-420
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@users/views.py` around lines 108 - 109, The current check
self.request.GET.get("edit", False) treats any non-empty value (e.g.
?edit=false) as true; change to an explicit parse such as reading val =
self.request.GET.get("edit") and enabling edit mode only when val is exactly
"true" (case-insensitive) or when you intentionally want presence semantics use
"if 'edit' in self.request.GET" — update the V3UserProfileForm branch and
duplicate the same fix at the other occurrence (the check around lines 419-420)
so both locations use the same explicit comparison (e.g., val and val.lower() ==
"true" or presence check).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Issue: #2437
Summary & Context
Adds an edit mode to the user profile for editing the individual components of the user profile.
NOTE: This includes no actual edit functionality, which will be added in several additional tickets.
Changes
v3/user_profile_edit.html, which is served when the appropriate query param is passedv3/includes/_field_include.htmlwhich accepts a django bound field and renders it using the appropriate include field template.field_dropdowncomponent with a disabled parameter and stylinguser/formsto capture the information from this page for processing.Please list any potential risks or areas that need extra attention during review/testing
Screenshots
Light Mode:
Dark Mode:
Mobile:
No Avatar Set:
Self-review Checklist
Frontend
Summary by CodeRabbit
New Features
Improvements