Story 2449: Webpage Integration: Edit User Profile - Profile Links#2514
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesProfile links feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
users/serializers.py (1)
29-45: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNo server-side validation for
profile_links— writable field accepts arbitrary/malformed JSON.
profile_linksis now writable viaPATCH /api/v1/users/me/with zero validation: no allowlist of keys (github/website/slack), no enforcement of HTTPS-only URLs for github/website (a stated PR requirement, currently only enforced client-side in Alpine.js), and no type checking on values. A client can persist{"slack": null}or{"anything": {...}}, which crashesV3UserProfileForm._display_link_valueinusers/forms.py(unconditionalvalue.startswith(...)) on the very next edit-profile page load, and unvalidated URL schemes are a latent XSS/open-redirect risk if these links are later rendered ashrefon public profiles.🛡️ Suggested fix: add serializer-level validation
+ ALLOWED_LINK_TYPES = {"github", "website", "email", "slack"} + + def validate_profile_links(self, value): + if not isinstance(value, dict): + raise serializers.ValidationError("profile_links must be an object.") + unknown = set(value) - self.ALLOWED_LINK_TYPES + if unknown: + raise serializers.ValidationError(f"Unsupported link types: {sorted(unknown)}") + for key in ("github", "website"): + val = value.get(key) + if val and not (isinstance(val, str) and val.startswith("https://")): + raise serializers.ValidationError({key: "Must be a secure https:// URL."}) + for key, val in value.items(): + if val is not None and not isinstance(val, str): + raise serializers.ValidationError({key: "Must be a string."}) + return value + class Meta: model = User🤖 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/serializers.py` around lines 29 - 45, `UserSerializer` currently allows `profile_links` to be written with no server-side checks, so add serializer-level validation for that field. In the serializer (and any dedicated `validate_profile_links`/update logic), restrict keys to the approved set (`github`, `website`, `email`, `slack`), require string values where appropriate, and enforce HTTPS-only URLs for `github` and `website` before saving. Also reject nulls and unexpected nested objects so `V3UserProfileForm._display_link_value` and later profile rendering only receive well-formed data.
🤖 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 `@templates/v3/user_profile_edit.html`:
- Line 145: The Save Changes button in user_profile_edit.html now calls save()
directly, but that method only PATCHes profile_links and skips the rest of the
profile form state. Update the save flow so the button submits the full profile
payload, or extend save() to include all edited profile fields along with links;
use the existing save(), profile_links, and the form’s profile data bindings as
the entry points.
- Around line 445-454: The Slack normalization in collectLinks() is out of sync
with the persisted contract: it currently converts the input into a full team
URL, while users/models.py documents Slack as a bare identifier. Update the
collectLinks() Slack branch to preserve the stripped identifier if that is the
intended storage format, or otherwise update the model/help text and any
display/prefill builders to consistently use the URL format. Make sure the
normalization also handles pasted full Slack URLs without creating double
prefixes.
- Around line 436-438: The profile link validator currently accepts any value
that merely starts with https://, which still allows invalid or unusable URLs to
pass. Update the github/website validation branch in the user_profile_edit.html
form logic to parse the value as a real URL and validate the protocol/host,
instead of relying on a prefix check, so only usable secure links are accepted.
In `@users/forms.py`:
- Around line 248-256: Guard `_display_link_value` against non-string inputs
before calling `startswith`, since `value` can be `None` or another JSON type
and crash the edit-profile render. Update the static method
`_display_link_value` in `users/forms.py` to first verify `value` is a string,
then apply the Slack URL trimming logic only for string values; otherwise return
the original value unchanged. Keep the existing `V3ProfileLinkChoices.SLACK` and
`SLACK_TEAM_URL_PREFIX` handling intact.
In `@users/models.py`:
- Around line 264-271: The help text on profile_links is describing Slack
storage backwards. Update the JSONField help_text in the profile_links
definition so it matches the actual behavior in users/forms.py and the template
logic: Slack links are stored as the full canonical team URL and only converted
to a display-friendly value by _display_link_value/SLACK_TEAM_URL_PREFIX and
collectLinks(). Keep the wording aligned with that storage/display flow and
reference profile_links as the field to update.
---
Outside diff comments:
In `@users/serializers.py`:
- Around line 29-45: `UserSerializer` currently allows `profile_links` to be
written with no server-side checks, so add serializer-level validation for that
field. In the serializer (and any dedicated `validate_profile_links`/update
logic), restrict keys to the approved set (`github`, `website`, `email`,
`slack`), require string values where appropriate, and enforce HTTPS-only URLs
for `github` and `website` before saving. Also reject nulls and unexpected
nested objects so `V3UserProfileForm._display_link_value` and later profile
rendering only receive well-formed data.
🪄 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: 4d624b59-40a3-47e2-adc1-c2a459eb4ba6
📒 Files selected for processing (8)
templates/v3/includes/_button.htmltemplates/v3/includes/header/_header_avatar.htmltemplates/v3/user_profile_edit.htmlusers/forms.pyusers/migrations/0022_user_profile_links.pyusers/models.pyusers/serializers.pyusers/views.py
| {% comment %} Persists the Profile links to the current user via PATCH /api/v1/users/me/. {% endcomment %} | ||
| <div class="card__cta_section"> | ||
| {% include 'v3/includes/_button.html' with style='primary' type='submit' label='Save Changes' only %} | ||
| {% include 'v3/includes/_button.html' with style='primary' type='button' label='Save Changes' alpine_disabled="saving" alpine_click="save()" only %} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve saving for the rest of the profile form.
The new button bypasses the previous submit flow, but save() PATCHes only profile_links. Edits to the other profile fields in this card will be dropped instead of being saved together with the links.
Also applies to: 475-481
🤖 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 145, The Save Changes button in
user_profile_edit.html now calls save() directly, but that method only PATCHes
profile_links and skips the rest of the profile form state. Update the save flow
so the button submits the full profile payload, or extend save() to include all
edited profile fields along with links; use the existing save(), profile_links,
and the form’s profile data bindings as the entry points.
There was a problem hiding this comment.
Intentional for this PR. On the V3 edit page the "Save Changes" button never persisted the other Profile fields. So save() isn't dropping anything that previously worked. Instead, it adds the first real persistence (links).
The other Profile-card fields persist elsewhere: tagline/biography in #2502, and role/avatar/badges/visibility have no backend yet and are their own upcoming stories (out of scope here).
This follows the same profileForm → PATCH /api/v1/users/me/ pattern as #2502 (built fresh on develop, not a code dependency). Once #2502 merges, the two profileForm definitions will be reconciled into a single save().
There was a problem hiding this comment.
@javiercoronadonarvaez Thanks for the detailed context — that makes sense. Since the "Save Changes" button never actually persisted the other Profile-card fields before this PR (tagline/biography handled separately via #2502, and role/avatar/badges/visibility have no backend yet), save() scoped to profile_links isn't a regression. I'll withdraw this concern.
✏️ Learnings added
Learnt from: javiercoronadonarvaez
Repo: boostorg/website-v2 PR: 2514
File: templates/v3/user_profile_edit.html:145-145
Timestamp: 2026-07-02T16:29:33.677Z
Learning: In `templates/v3/user_profile_edit.html`, the "Save Changes" button in the Profile card previously never persisted the Profile-card fields (it was a plain submit with no backing view logic for those fields). This PR (boostorg/website-v2 `#2514`) introduces the first real persistence for that card, scoped only to `profile_links` via the `window.profileForm` Alpine component's `save()` method, which PATCHes `/api/v1/users/me/`. Tagline/biography persistence is handled separately (see PR `#2502`, which uses a similar `profileForm` → `PATCH /api/v1/users/me/` pattern but was built independently on `develop`). Role/avatar/badges/visibility fields have no backend persistence yet and are out of scope. Once `#2502` merges, the two separate `profileForm` Alpine component definitions are expected to be reconciled into a single `save()` implementation that persists all profile fields together.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
users/views.py (1)
111-115: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle empty
profile_linksbefore iterating the formset
V3UserProfileForm.__init__only createsself.link_formsetinsideif links:, but later always iteratesself.link_formset. Users with no profile links will hit anAttributeErroron the profile edit page.🤖 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 111 - 115, V3UserProfileForm currently only sets link_formset inside the conditional in __init__, but other code always iterates it, so users with no profile_links can hit an AttributeError. Update V3UserProfileForm so link_formset is always initialized, even when user_links is empty, and make the iteration logic in the form initialization/render path safely handle an empty list using the V3UserProfileForm and self.link_formset symbols as the main fix points.
🤖 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.
Outside diff comments:
In `@users/views.py`:
- Around line 111-115: V3UserProfileForm currently only sets link_formset inside
the conditional in __init__, but other code always iterates it, so users with no
profile_links can hit an AttributeError. Update V3UserProfileForm so
link_formset is always initialized, even when user_links is empty, and make the
iteration logic in the form initialization/render path safely handle an empty
list using the V3UserProfileForm and self.link_formset symbols as the main fix
points.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bc893060-6507-4c9f-b62e-723ee0d6d125
📒 Files selected for processing (8)
core/context_processors.pytemplates/v3/includes/_field_text.htmltemplates/v3/user_profile_edit.htmlusers/forms.pyusers/serializers.pyusers/tests/test_forms.pyusers/tests/test_serializers.pyusers/views.py
🚧 Files skipped from review as they are similar to previous changes (5)
- users/forms.py
- core/context_processors.py
- users/serializers.py
- templates/v3/includes/_field_text.html
- templates/v3/user_profile_edit.html
36655c7 to
6209ff9
Compare
There was a problem hiding this comment.
Hi @herzog0 ! I think the error text for the GitHub and Website fields are currently displayed next to the button instead of underneath the input fields. Would you mind directing error text to the correct slots?
Edit: I see that removing the Slack display shortener was intentional, all good!
Thanks a lot for your help here!
julhoang
left a comment
julhoang
left a comment
There was a problem hiding this comment.
Looks great to me, thank you for all the updates! 🙌
d17dd96 to
80214bc
Compare
feat: expose and prefill profile_links via CurrentUser API and edit view feat: persist links on Save Changes via PATCH to /api/v1/users/me/ endpoint fix: redirect to http://localhost:8000/users/me/?edit=True when clicking Edit Profile button feat: persist profile links on Save Changes with inline validation fix: normalize Slack input to avoid double-prefixed team URL fix: validate profile links as real HTTPS URLs, not just a prefix fix: guard _display_link_value against non-string profile_links values fix: help text matches actual functionality fix: reject non-dict profile_links payloads in CurrentUserSerializer fix: match migration help_text to the profile_links model field fix: restore link length validation dropped by the alpine_error rendering path fix: key profile link inputs by data-link-type instead of array position fix: match Slack prefix stripping case-insensitivity between JS and Python fix: build edit-profile CTA URL in the header context processor, not the template feat: increase url max length fix: clear saved-links indicator when a link is edited afterward fix: allowlist profile_links keys and bound value length in API fix: add aria-label to profile link value inputs fix: dedupe edit-profile CTA URL construction into a shared helper fix: pass SLACK_TEAM_URL_PREFIX from Python instead of hardcoding in JS test: cover Slack link normalization and profile_links validation fix: lint fix: build Slack profile links as messages/<username> URLs fix: clarify Slack placeholder refers to the CPPLang workspace fix: shorten Slack placeholder text fix: lint fix: enforce secure URL check for profile links on backend fix: build Slack profile links via app_redirect using the Member ID fix: validate Slack link format on frontend and backend fix: lint fix: match profile_links migration help_text to model field fix: escape Slack URL prefix for JS context and surface save errors in UI feat: switch Slack profile link to cpplang.slack.com/team/<id> scheme fix: lint fix: make Slack link placeholder more intuitive fix: show full Slack URL in edit form and use CPPLang-specific validation messages fix: route backend link validation errors to their own field, not the save-button banner
80214bc to
e8be1cd
Compare
4 participants
Issue: #2449
Summary & Context
Adds the Profile Links section to the edit profile page (GitHub, Website, Email, Slack), with inline validation and persistence. Links are batched into a single Save Changes request alongside the rest of the Profile section, persisted to the
Userrecord, and pre-filled from the saved data on load.Changes
profile_links(JSONField, keyed by link type) field to theUsermodel via migration0022.profile_linksonCurrentUserSerializer(writable) and pre-fill the edit form fromuser.profile_links(replacing the previous hardcoded placeholder).PATCH /api/v1/users/me/, with a Saved / error indicator on the button.Please add a secure link. The error clears as the user edits the field. Email and Slack accept free-form input.@vinniefalco); the frontend strips a leading@and stores the canonical team URLhttps://cpplang.slack.com/team/[identifier]. On prefill the URL is reversed back to the bare identifier so the field matches the design.alpine_clickto the shared_button.htmlinclude.Please list any potential risks or areas that need extra attention during review/testing
Kept
PATCH /api/v1/users/me/instead of the ticket's/api/user/profileendpoint (same reasoning as the sibling bio/tagline PR #2502):PATCHis the correct partial-update verb, the endpoint already exists and is tested (CurrentUserAPIView+CurrentUserSerializer), and it keeps the codebase's/api/v1/versioning convention.Links are stored in a single
profile_linksJSONField keyed by type, rather than four separate columns. This maps 1:1 to the existingV3UserProfileForm(user_links={...})dict interface (clean prefill), batches all four links into one PATCH, and lives in its own field so it never clobbers the shareddataJSONField.Slack is persisted as the full constructed URL, not the raw identifier. The
SLACK_TEAM_URL_PREFIXconstant inusers/forms.pyis the single source of truth. The JS construction (user_profile_edit.html) and the Python prefill reversal are kept in sync against it. If the prefix ever changes, update both.Validation is client-side and inline only — no server-side URL validation. Per the ticket, moderation/flagging of suspicious URLs and reachability/safety checks are explicitly out of scope.
Migration numbering. This PR adds
0022_user_profile_linkson top of0021; PR #2502 also adds a0022_*. They don't collide on disk, but whichever merges second may need a Django--mergemigration (or a renumber to0023).Out of scope (intentionally deferred): rendering the saved links on the public profile (
social_media_linksstill points to#), the design's "+ Add Another" affordance (the four types are fixed/locked per the acceptance criteria), and batching avatar/role/badges into the same save request.Screenshots
LocalStorage and DB Persistence
ProfileLinksPersistence.mov
Self-review Checklist
Frontend
Summary by CodeRabbit
Summary by CodeRabbit