Added profile page, avatar handling, navbar synchronization and tests#65
Open
ghanshyam2005singh wants to merge 2 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 38 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThis PR introduces a complete user profile system: users can edit their profile (name, bio, expertise, avatar, social links, visibility), view others' public profiles, and discover members through a searchable directory. The backend adds profile/avatar/account API endpoints with public discovery gating, the database schema expands to store profile fields and avatar history, and the frontend adds a profile editor, public profile viewer, member directory, and enhanced navigation auth UI with avatar display. ChangesUser Profiles & Public Discovery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
public/partials/navbar.html (2)
11-13: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd aria-label to mobile menu button for screen readers.
The mobile menu toggle button should have an
aria-labelattribute to describe its purpose for assistive technology users.♿ Proposed accessibility improvement
-<button class="md:hidden p-2 hover:bg-teal-700 rounded-lg" onclick="toggleMobileMenu()"> +<button class="md:hidden p-2 hover:bg-teal-700 rounded-lg" onclick="toggleMobileMenu()" aria-label="Toggle mobile menu"> <i class="fas fa-bars text-xl"></i> </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 `@public/partials/navbar.html` around lines 11 - 13, The mobile menu toggle button lacks an accessible name; add an aria-label attribute to the button element that calls toggleMobileMenu() (e.g., aria-label="Toggle main menu" or "Open main menu") so screen readers can describe its purpose; ensure the attribute is placed on the <button> that currently has class "md:hidden p-2 hover:bg-teal-700 rounded-lg" and consider updating the label dynamically inside the toggleMobileMenu() handler if you change state (open/close).
224-232: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd ARIA attributes to profile dropdown button.
The profile dropdown button should include
aria-haspopup="true"andaria-expandedto communicate its interactive state to screen readers. Additionally, the button and dropdown menu should be associated with proper ARIA roles.♿ Proposed accessibility improvement
-<button onclick="toggleProfileDropdown()" class="w-full flex items-center space-x-2 p-2 hover:bg-teal-700 rounded-lg transition"> +<button onclick="toggleProfileDropdown()" class="w-full flex items-center space-x-2 p-2 hover:bg-teal-700 rounded-lg transition" aria-haspopup="true" aria-expanded="false" aria-controls="profile-dropdown"> <!-- Profile Avatar with First Letter --> <div id="profile-avatar" class="w-8 h-8 rounded-full bg-orange-500 flex items-center justify-center font-bold text-white text-sm">Then update the
toggleProfileDropdown()function in layout.js to toggle thearia-expandedattribute:window.toggleProfileDropdown = function () { const dropdown = document.getElementById('profile-dropdown'); const button = document.querySelector('[aria-controls="profile-dropdown"]'); if (dropdown) { const isHidden = dropdown.classList.toggle('hidden'); if (button) button.setAttribute('aria-expanded', !isHidden); } };🤖 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 `@public/partials/navbar.html` around lines 224 - 232, Add proper ARIA attributes to the profile dropdown button and associate it with the dropdown: update the button markup (the element that calls toggleProfileDropdown()) to include aria-haspopup="true", aria-expanded="false", and aria-controls="profile-dropdown" so screen readers can discover the controlled element; add role="menu" to the element with id="profile-dropdown" and role="menuitem" to each dropdown entry. Then update the toggleProfileDropdown() function to find the button via document.querySelector('[aria-controls="profile-dropdown"]') and toggle its aria-expanded value whenever it shows/hides the element with id "profile-dropdown" (use the existing dropdown.classList.toggle('hidden') result to set aria-expanded to the inverse of the hidden 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 `@public/js/layout.js`:
- Around line 104-113: The _setNavButtonAvatar function injects unescaped
user-derived initials into an inline onerror attribute, creating an XSS risk;
remove the inline event handler and instead create the IMG element via DOM APIs
(document.createElement('img')), set its src/class/alt normally, and attach a
safe onerror handler (img.onerror = () => { el.textContent = initials; }) or set
el.textContent using a properly escaped value before/after insertion; ensure you
only use element.textContent (not innerHTML) to render initials and reference
_setNavButtonAvatar and the avatarElId/initials variables when updating the
implementation.
In `@public/partials/navbar.html`:
- Around line 235-282: The logout button inside the profile dropdown (element id
"profile-dropdown") currently uses <button onclick="logout()">; add
type="button" to that button to prevent accidental form submission (i.e., change
the button with onclick="logout()" to include type="button"); also scan the
dropdown for any other plain <button> elements and add explicit type attributes
as needed to be safe.
- Around line 330-368: The logout button lacks an explicit type which can cause
unintended form submissions; update the button element that calls logout() (the
element with onclick="logout()") to include type="button" so it won't submit any
parent form — locate the button with the logout() onclick in the mobile auth
panel (currently inside the `#mobile-auth-logged-in` block) and add the
type="button" attribute.
In `@public/profile.html`:
- Around line 80-84: Add an explicit type="button" attribute to every button
that is not intended to submit a form to prevent accidental form submissions:
update the Upload Photo button that triggers
document.getElementById('avatar-file-input').click(), the button with id
"btn-remove-avatar" (onclick removeAvatar()), and likewise add type="button" to
the toggle switch buttons, the profile Save button, the Delete Account button,
and both modal buttons (confirm/cancel) so none default to type="submit".
In `@public/public-profile.html`:
- Around line 190-198: Add an onerror fallback to the avatar image element so a
broken avatar URL falls back to initials: when setting
document.getElementById('profile-avatar-img').src from p.avatar_url, also attach
an onerror handler that hides the image, shows
document.getElementById('profile-initials'), and sets its textContent to the
computed initials (same logic as the else branch); ensure the handler is
idempotent (removes itself or checks to avoid loops) and uses the existing
initials computation based on p.name || p.username || '?' so the page matches
the users.html behavior.
In `@public/users.html`:
- Around line 51-63: Add accessible labeling and explicit button type: associate
the search input (id="search-input", used by filterUsers()) with an accessible
label (either add a visually-hidden <label for="search-input"> or set aria-label
on the input) so screen readers announce its purpose, and set the filter button
(id="filter-teachers", used by toggleTeacherFilter()) to type="button" to
prevent implicit form submission; ensure you use the existing sr-only utility
(or Tailwind's sr-only) if adding a hidden label.
In `@tests/test_api_profile.py`:
- Around line 359-407: Add a new test in the TestUploadAvatar class that
exercises the R2-enabled path by providing a mocked R2 binding on the env and
asserting the returned avatar_url is an R2 URL (instead of the data: URL); reuse
the helpers used in this file (make_env, MockDB, make_stmt, _req,
api_upload_avatar) to construct env with db=MockDB([...]) and set env.R2 to a
mock object that implements the expected put/upload behavior, call
worker.api_upload_avatar with a small base64 PNG payload (like in
test_no_r2_falls_back_to_base64), assert r.status == 200, and assert
_parse(r)["data"]["avatar_url"] matches the expected R2 URL pattern (or contains
the mock R2 key returned by your mock).
In `@wrangler.toml`:
- Around line 22-26: Update the wrangler.toml R2 bucket comment block to include
documentation for the required R2_PUBLIC_URL environment variable so avatar URLs
resolve in production; explicitly instruct users to create the bucket (wrangler
r2 bucket create learn-avatars), then set R2_PUBLIC_URL in their Cloudflare
Workers environment (Workers → Settings → Variables) to the bucket’s public URL
(e.g., https://pub-xxxxx.r2.dev), and keep the existing [[r2_buckets]] example
(binding = "R2", bucket_name = "learn-avatars"); reference R2_PUBLIC_URL and the
avatar URL logic in worker.py (around lines ~2750-2751) so maintainers see why
the variable is needed.
---
Outside diff comments:
In `@public/partials/navbar.html`:
- Around line 11-13: The mobile menu toggle button lacks an accessible name; add
an aria-label attribute to the button element that calls toggleMobileMenu()
(e.g., aria-label="Toggle main menu" or "Open main menu") so screen readers can
describe its purpose; ensure the attribute is placed on the <button> that
currently has class "md:hidden p-2 hover:bg-teal-700 rounded-lg" and consider
updating the label dynamically inside the toggleMobileMenu() handler if you
change state (open/close).
- Around line 224-232: Add proper ARIA attributes to the profile dropdown button
and associate it with the dropdown: update the button markup (the element that
calls toggleProfileDropdown()) to include aria-haspopup="true",
aria-expanded="false", and aria-controls="profile-dropdown" so screen readers
can discover the controlled element; add role="menu" to the element with
id="profile-dropdown" and role="menuitem" to each dropdown entry. Then update
the toggleProfileDropdown() function to find the button via
document.querySelector('[aria-controls="profile-dropdown"]') and toggle its
aria-expanded value whenever it shows/hides the element with id
"profile-dropdown" (use the existing dropdown.classList.toggle('hidden') result
to set aria-expanded to the inverse of the hidden 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: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b659b84-2209-46d9-9e33-968c6e34af85
⛔ Files ignored due to path filters (1)
migrations/0004_add_user_profiles.sqlis excluded by!**/migrations/**
📒 Files selected for processing (12)
public/js/layout.jspublic/partials/navbar.htmlpublic/profile.htmlpublic/public-profile.htmlpublic/users.htmlschema.sqlsrc/worker.pytests/test_api_activities.pytests/test_api_admin.pytests/test_api_auth.pytests/test_api_profile.pywrangler.toml
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
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.
Summary
This PR added the profile system, avatar, user info and expanding profile test coverage.
Changes
Profile page
Navbar Profile Updates
Avatar Storage
learn-avatarsR2 bucket.Testing
Notes
To enable production avatar storage:
wrangler.toml.R2_PUBLIC_URL.The Base64 fallback remains available for local development and environments where R2 is not configured.
Summary
This PR introduces a comprehensive user profile system, enabling users to manage their profiles, upload avatars, and discover other community members.
Key Features & Changes
Profile Management
profile.html) allowing users to update bio, set social media usernames (GitHub, Discord, Slack), manage expertise tags, toggle teacher status and profile visibility, and delete their accountpublic-profile.html) displaying user information when profiles are set to publicusers.html) with search and "Teachers Only" filter for discovering community membersNavbar & Authentication
Avatar Handling
Backend API
GET /api/profile,PATCH /api/profile,DELETE /api/accountPOST /api/profile/avatar,DELETE /api/profile/avatarGET /api/users,GET /api/users/:usernameDatabase Schema
userstable with profile fields: bio, avatar_url, social usernames, expertise, teaching flag, and profile visibility toggleavatarstable tracking avatar history per userTesting
test_api_profile.py) covering profile CRUD operations, avatar uploads, public directory filtering, and access controlTechnical Notes
R2 bucket configuration is currently commented out in
wrangler.toml; enabling production avatar storage requires creating thelearn-avatarsbucket and configuring theR2_PUBLIC_URLenvironment variable.User Experience Impact