feat: add mentorship system with mentor profiles, requests, sessions, and ratings#1042
Open
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Open
feat: add mentorship system with mentor profiles, requests, sessions, and ratings#1042ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Conversation
Contributor
👀 Peer Review RequiredHi @ayesha1145! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
Contributor
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Models web/models.py |
Added MentorProfile, MentorshipRequest, MentorshipSession with User/Subject relations, availability/pricing/scheduling/status fields, validators, timestamps, ordering, and helper properties (average_rating, total_sessions). |
Admin web/admin.py |
Registered MentorProfile, MentorshipRequest, MentorshipSession with custom ModelAdmin classes (list_display, list_filter, search_fields) inserted alongside existing User admin registration. |
Views (logic) web/views.py |
Added ~11 login-protected view functions for mentor listing/filtering, profile, request create/cancel, mentor onboarding, mentor dashboard, respond to requests, schedule/complete sessions, and rate sessions; includes validation, state transitions, datetime parsing, and M2M subject assignment. |
URLs web/urls.py |
Added i18n-prefixed mentorship URL patterns mapping to the new views (mentor list, onboarding, dashboard, profile, request actions, scheduling, completion, rating). |
Templates — Discovery & Profiles web/templates/mentorship/mentor_list.html, .../mentor_profile.html, .../become_mentor.html |
New templates for browsing/filtering mentors, detailed mentor profile (reviews, availability, pricing), and creating/editing mentor profiles (subjects, bio, availability). |
Templates — Student Workflows web/templates/mentorship/request_mentorship.html, .../my_mentorship.html, .../rate_session.html |
Templates for sending mentorship requests, viewing a student's requests & sessions, and rating completed sessions. |
Templates — Mentor Workflows web/templates/mentorship/mentor_dashboard.html, .../schedule_session.html |
Mentor dashboard and scheduling form templates showing pending/accepted requests, upcoming sessions, scheduling inputs, and session completion actions. |
Sequence Diagram(s)
sequenceDiagram
actor Student
participant Browser as StudentClient
participant Views as WebViews
participant DB as Database
participant Email as OptionalEmailService
Student->>Browser: Open mentor list (GET /mentorship/)
Browser->>Views: GET mentor_list
Views->>DB: Query MentorProfile, Subjects, apply filters
DB-->>Views: Mentor list
Views-->>Browser: Render mentor_list.html
Browser->>Views: POST request_mentorship (form)
Views->>DB: Validate, create MentorshipRequest
DB-->>Views: MentorshipRequest created
Views->>Email: (optional) notify mentor
Views-->>Browser: Redirect to mentor_profile
sequenceDiagram
actor Mentor
participant Browser as MentorClient
participant Views as WebViews
participant DB as Database
Mentor->>Browser: Open mentor dashboard
Browser->>Views: GET mentor_dashboard
Views->>DB: Fetch pending/accepted requests, upcoming sessions
DB-->>Views: Aggregated data
Views-->>Browser: Render mentor_dashboard.html
Browser->>Views: POST respond_to_request (accept/decline)
Views->>DB: Update MentorshipRequest.status
DB-->>Views: Status updated
Browser->>Views: POST schedule_session (datetime)
Views->>DB: Validate, create MentorshipSession
DB-->>Views: Session created
Views-->>Browser: Redirect to mentor_dashboard
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The pull request title accurately and comprehensively describes the main change: adding a complete mentorship system with its core components (profiles, requests, sessions, ratings). |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/admin.py`:
- Around line 15-17: The imports for MentorProfile, MentorshipRequest, and
MentorshipSession were inserted between Badge and BlogComment, breaking the
file's alphabetical import order; move these three model imports so they appear
after Meme and before MembershipPlan to restore consistent alphabetical ordering
(look for the import block containing Badge, BlogComment, Meme, MembershipPlan
and relocate MentorProfile/MentorshipRequest/MentorshipSession accordingly).
- Around line 616-634: The admin classes MentorProfileAdmin,
MentorshipRequestAdmin, and MentorshipSessionAdmin should add raw_id_fields for
foreign-key lookups and useful readonly_fields to improve performance and UX
when there are many related records; update each class to include raw_id_fields
for user/mentor/student relation fields (e.g., raw_id_fields = ("user",) in
MentorProfileAdmin and raw_id_fields = ("student","mentor") in
MentorshipRequestAdmin and MentorshipSessionAdmin) and add readonly_fields for
auto-managed fields like "created_at" and any computed or status fields you
don't want edited in the admin.
In `@web/templates/mentorship/become_mentor.html`:
- Around line 25-31: The hourly_rate input (id="hourly_rate",
name="hourly_rate") lacks an accessible label; add a visible <label> for that
input or at minimum an aria-label/aria-labelledby so screen readers can identify
it. Update the template to include a label element (e.g., <label
for="hourly_rate">Hourly rate</label>) placed appropriately near the input, or
add aria-label="Hourly rate" to the input if a visible label is not desired, and
ensure any CSS/layout adjustments keep the form accessible.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Around line 79-82: The "Mark Complete" form (action using URL name
'complete_session' for session id s.id) needs a user confirmation before
submitting; add a confirmation step by attaching a JavaScript confirmation
handler to the form (either via an onsubmit attribute on the form element or by
selecting the form in JS and adding an event listener) that calls window.confirm
with a clear message (e.g., warning that completing is irreversible and will
trigger rating/billing), and prevent submission if the user cancels; ensure the
handler targets the specific form (e.g., by adding a unique id or data
attribute) so only the "Mark Complete" action is affected.
- Around line 10-21: The metric cards currently use <h3> for the numeric values
(pending_requests.count, upcoming_sessions.count, mentor.total_sessions) which
is semantically incorrect; change the markup so the descriptive label (e.g.,
"Pending Requests", "Upcoming Sessions", "Completed Sessions") is the heading
element (e.g., keep or use <h3> for the label) and render the numeric value in a
non-heading element (e.g., <p> or <div>) and/or add appropriate ARIA attributes
(role="status" or aria-live="polite") to the numeric element to convey dynamic
updates to assistive tech; update the three blocks containing
pending_requests.count, upcoming_sessions.count, and mentor.total_sessions
accordingly.
In `@web/templates/mentorship/mentor_list.html`:
- Around line 61-63: The template currently escapes the dollar sign in the price
rendering (you can find it in the span using mentor.is_free and
mentor.hourly_rate where it shows \${{ mentor.hourly_rate }}/hr); remove the
unnecessary backslash so the string is "${{ mentor.hourly_rate }}/hr" when
mentor.is_free is false, leaving the conditional around mentor.is_free
unchanged.
- Around line 41-47: The avatar img tag currently has an empty alt attribute;
update it to include a descriptive, accessible label using the mentor's name
(e.g., use mentor.user.get_full_name with a fallback to mentor.user.username) so
the alt reads something like "<mentor name>'s avatar" to aid screen readers;
locate the <img> using mentor.user.profile.avatar and replace the empty alt=""
with the generated descriptive text.
In `@web/templates/mentorship/mentor_profile.html`:
- Around line 52-54: The star icons in mentor_profile.html are purely visual and
marked aria-hidden, so add a visually hidden text node that announces the
numeric rating using the existing r.rating value (e.g., a span with class
"sr-only" containing "{{ r.rating }} out of 5 stars") adjacent to the star loop;
keep the <i> icons as aria-hidden="true" and place the sr-only span before or
after the <span class="text-yellow-500 text-xs"> so screen readers receive the
rating while visual output is unchanged.
- Around line 14-19: The avatar image currently has an empty alt ("") which is
bad for accessibility; update the <img> to include meaningful alt text like the
mentor's name (use the template variables such as mentor.user.get_full_name or
fallback to mentor.user.username) so the src="{{ mentor.user.profile.avatar.url
}}" image has alt="{{ mentor.user.get_full_name|default:mentor.user.username
}}"; also update the fallback placeholder element to provide an accessible label
(e.g., add an aria-label or visually hidden text using the same mentor name) so
screen readers can identify the avatar when no image is present.
In `@web/templates/mentorship/my_mentorship.html`:
- Around line 60-62: Replace the current star markup that only outputs filled
icons by iterating over the same "12345" loop and rendering a filled star (class
"fas fa-star") when forloop.counter <= s.rating and an empty/star-outline (class
"far fa-star") otherwise, ensure each <i> has aria-hidden="true", and add a
screen-reader-only label (e.g., <span class="sr-only">Rated {{ s.rating }} out
of 5</span>) adjacent to the stars so assistive tech reads the numeric rating;
update the template fragment that references s.rating and the forloop.counter to
implement these changes.
In `@web/templates/mentorship/rate_session.html`:
- Around line 13-24: The radio inputs for rating (input name="rating" within the
for loop using forloop.counter and currently using class "sr-only") lack
explicit accessible labels for screen readers; update each radio input to
include an aria-label (or aria-labelledby) that describes the option (e.g.,
"Rating 1", "Rating 2", etc.) using the forloop.counter so screen readers
announce the rating value, and ensure the required attribute remains and the
visible star span stays purely decorative with aria-hidden="true".
In `@web/templates/mentorship/schedule_session.html`:
- Around line 15-19: Add a minimum datetime to the scheduled_at input to prevent
selecting past dates: update the <input id="scheduled_at" name="scheduled_at">
to include a min attribute set from the view (e.g. min="{{ min_datetime }}"
where min_datetime is formatted as YYYY-MM-DDTHH:MM) or, if not available
server-side, add a small script that finds the element by id "scheduled_at" and
sets its min to the current local datetime formatted as YYYY-MM-DDTHH:MM; ensure
the view provides min_datetime when possible or the JS runs on page load to keep
the input constrained.
In `@web/urls.py`:
- Around line 97-109: The new mentorship URL patterns use single quotes instead
of the file's standard double quotes; update each path string (e.g., the
patterns passed to path like 'mentorship/', 'mentorship/become/',
'mentorship/<int:mentor_id>/request/', etc.) to use double quotes so the entries
(and their names) match the existing quote style and maintain consistency with
the rest of web/urls.py; no behavior changes needed—only replace single quotes
with double quotes for the path literals and route name strings if any.
In `@web/views.py`:
- Around line 8949-8979: The code is silently coercing malformed or negative
numeric fields for MentorProfile (experience_years, hourly_rate) instead of
validating them. Replace the manual parsing with a Django ModelForm (e.g.,
MentorProfileForm) or at minimum validate parsed values before saving: parse
experience_years and hourly_rate from request.POST, ensure they are non-negative
and within sensible bounds, and if validation fails return the form with errors
(or abort the save) rather than defaulting to 0; apply these checks when
updating fields on the existing MentorProfile (mentor.bio,
mentor.experience_years, mentor.hourly_rate, mentor.is_free,
mentor.availability) and when creating a new MentorProfile via
MentorProfile.objects.create, so invalid inputs are rejected and not persisted.
- Around line 8850-9106: Add short docstrings and type hints to all new view
functions (mentor_list, mentor_profile_view, request_mentorship, my_mentorship,
cancel_mentorship_request, become_mentor, mentor_dashboard, respond_to_request,
schedule_session, complete_session, rate_session): annotate the request
parameter as HttpRequest and return type as HttpResponse, import
HttpRequest/HttpResponse from django.http, and include one- or two-sentence
docstrings describing the purpose and primary behavior of each view (inputs and
important side-effects like redirects or DB changes); ensure any helper
variables in signatures (e.g., mentor_id, request_id, session_id) keep their
existing names and types (int) in the annotations.
- Line 8878: The current lookup uses
MentorshipRequest.objects.filter(mentor=mentor, student=request.user).first()
which matches historical (cancelled/declined) requests; change it to only
consider active/non-terminal requests by adding a status filter (for example use
status__in=['pending','accepted'] or exclude terminal statuses like
status__in=['cancelled','declined']) so only live requests are treated as
duplicates; apply the same change to the other occurrences around the
MentorshipRequest lookups (the ones at the referenced block around lines
8895-8898) to ensure profile rendering and duplicate checks only consider active
requests.
- Around line 9044-9062: The create branch currently accepts any parsed
scheduled_at and duration and directly calls MentorshipSession.objects.create,
so reject past timestamps, non-positive durations, and any overlapping sessions
for the same mentor or student before creating; specifically, use
django.utils.timezone.now() to ensure scheduled_at > now, ensure the parsed
duration_minutes (from request.POST["duration_minutes"]) is an int > 0, compute
new_end = scheduled_at + timedelta(minutes=duration) and run a query against
MentorshipSession (filtering by mentor=mentor OR
student=mentorship_request.student) to detect overlaps using the interval
intersection condition (existing.scheduled_at < new_end AND existing_end >
scheduled_at) where existing_end = existing.scheduled_at +
timedelta(minutes=existing.duration_minutes); if any validation fails, call
messages.error with a clear message and return redirect("schedule_session",
request_id=request_id) instead of creating the session; only call
MentorshipSession.objects.create when all checks pass.
- Around line 8900-8908: The code currently accepts any subject_id from
request.POST and creates a MentorshipRequest without verifying the mentor
actually teaches that subject; update the subject validation around
request.POST.get("subject") and Subject.objects.filter(...) so that after
loading subject you check that subject is one of the mentor's advertised
subjects (e.g. subject in mentor.subjects.all() or
mentor.advertised_subjects.all()); if the subject_id is missing or not part of
the mentor's subjects, either set subject=None or call messages.error(...) and
return redirect("request_mentorship", mentor_id=mentor_id) to prevent creating a
request for an invalid/tampered subject before calling
MentorshipRequest.objects.create(...).
- Around line 9076-9080: Currently the view fetches a MentorshipSession with
status="scheduled" and lets POST change session.status to "completed", which
allows mentors to complete future appointments; add a check after
get_object_or_404 (before the request.method == "POST" branch) that compares the
session's scheduled/start time (e.g., session.scheduled_time or
session.start_time) against django.utils.timezone.now() and disallow completion
if the scheduled time is in the future (return an appropriate
400/403/HttpResponse or raise Http404), keeping the
update_fields=["status","notes","updated_at"] save logic only for sessions whose
scheduled time is <= now.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b17de88-9d28-418f-b2b4-29cb588eb7eb
⛔ Files ignored due to path filters (1)
web/migrations/0064_add_mentorship_models.pyis excluded by!**/migrations/**
📒 Files selected for processing (12)
web/admin.pyweb/models.pyweb/templates/mentorship/become_mentor.htmlweb/templates/mentorship/mentor_dashboard.htmlweb/templates/mentorship/mentor_list.htmlweb/templates/mentorship/mentor_profile.htmlweb/templates/mentorship/my_mentorship.htmlweb/templates/mentorship/rate_session.htmlweb/templates/mentorship/request_mentorship.htmlweb/templates/mentorship/schedule_session.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/mentorship-system
branch
from
ad72cbd to
87ea607
Compare
Contributor
💬 Unresolved Review ConversationsHi @ayesha1145! 👋 This pull request currently has 2 unresolved review conversations. Please address all review feedback and push a new commit to resolve them before this PR can be merged. Steps to resolve:
Once all conversations are resolved, this notice will be removed automatically. Thank you! 🙏 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
ayesha1145
force-pushed
the
feat/mentorship-system
branch
from
87ea607 to
32c8cc0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
web/views.py (4)
8958-8969:⚠️ Potential issue | 🟠 MajorReject negative profile numbers instead of coercing them to zero.
max(0, ...)/max(0.0, ...)still silently rewrites bad input into public mentor data. A typo like-3years or-25rate becomes0with no feedback.🧰 Suggested fix
try: - experience_years = max(0, int(request.POST.get("experience_years", 0))) + experience_years = int(request.POST.get("experience_years", 0)) except (ValueError, TypeError): messages.error(request, "Please enter a valid number for years of experience.") return redirect("become_mentor") + if experience_years < 0: + messages.error(request, "Please enter a non-negative number for years of experience.") + return redirect("become_mentor") @@ try: - hourly_rate = max(0.0, float(request.POST.get("hourly_rate", 0))) + hourly_rate = float(request.POST.get("hourly_rate", 0)) except (ValueError, TypeError): messages.error(request, "Please enter a valid hourly rate.") return redirect("become_mentor") + if hourly_rate < 0: + messages.error(request, "Please enter a non-negative hourly rate.") + return redirect("become_mentor")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8958 - 8969, The current parsing for experience_years and hourly_rate uses max(...) which silently converts negative inputs to zero; instead, parse the inputs into integers/floats in the existing try/except blocks (remove the max(...) wrapping) and after successful parsing validate that experience_years >= 0 and hourly_rate >= 0.0; if either is negative, call messages.error with a clear message (e.g., "Please enter a non-negative value for years of experience/hourly rate") and return redirect("become_mentor"). Keep the existing ValueError/TypeError handling for invalid formats and update the code paths around the experience_years and hourly_rate variables to use these explicit validations.
9064-9078:⚠️ Potential issue | 🟠 MajorDon't silently turn bad duration input into a 60-minute booking.
This branch still schedules a session when
duration_minutesis malformed or< 15, and it still doesn't reject overlaps for the mentor or student. A stale or tampered form can therefore create the wrong slot and conflicting bookings in one POST. Please fail closed here and run the overlap check beforeMentorshipSession.objects.create(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9064 - 9078, Currently malformed or too-small duration is silently replaced with 60 and a session is created; instead, validate request.POST["duration_minutes"] strictly (parse to int and require >=15) and fail the request (raise/return a validation error) if invalid, and before calling MentorshipSession.objects.create run the overlap check for both mentor and mentorship_request.student using the same overlap-detection logic/service your app uses (i.e., check any existing MentorshipSession for conflicting scheduled_at/duration_minutes for mentor and student), and only call MentorshipSession.objects.create(mentor=mentor, student=mentorship_request.student, subject=..., scheduled_at=scheduled_at, duration_minutes=duration, notes=notes) after both validation and overlap checks pass.
8904-8917:⚠️ Potential issue | 🟠 MajorKeep subject selection mandatory for both mentor onboarding and request creation.
Nice improvement on checking the posted request subject against
mentor.subjects. The remaining gap is thatrequest_mentorship()still allowssubject=None, andbecome_mentor()can activate a profile with zero subjects. That leaves the subject-based workflow with incomplete data and active mentors who cannot be meaningfully filtered or requested by subject.🧰 Suggested fix
@@ message = request.POST.get("message", "").strip() subject_id = request.POST.get("subject") if not message: messages.error(request, "Please include a message with your request.") return redirect("request_mentorship", mentor_id=mentor_id) - subject = None - if subject_id: - subject = mentor.subjects.filter(id=subject_id).first() - if not subject: - messages.error(request, "Please select a valid subject from the mentor profile.") - return redirect("request_mentorship", mentor_id=mentor_id) + if not subject_id: + messages.error(request, "Please select a subject.") + return redirect("request_mentorship", mentor_id=mentor_id) + subject = mentor.subjects.filter(id=subject_id).first() + if not subject: + messages.error(request, "Please select a valid subject from the mentor profile.") + return redirect("request_mentorship", mentor_id=mentor_id) @@ availability = request.POST.get("availability", "flexible") if availability not in dict(MentorProfile.AVAILABILITY_CHOICES): availability = "flexible" subject_ids = request.POST.getlist("subjects") + valid_subjects = Subject.objects.filter(id__in=subject_ids) + if not valid_subjects.exists(): + messages.error(request, "Please select at least one subject.") + return redirect("become_mentor") @@ - mentor.subjects.set(Subject.objects.filter(id__in=subject_ids)) + mentor.subjects.set(valid_subjects)Also applies to: 8973-8991
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8904 - 8917, The request_mentorship flow currently allows subject=None and become_mentor can activate a profile with zero subjects; enforce subject as mandatory by validating request.POST.get("subject") in request_mentorship (reject and redirect with messages.error if missing or not found) before calling MentorshipRequest.objects.create, and update become_mentor to block activation (or require adding subjects) when mentor.subjects.count() == 0 — surface a clear messages.error and prevent saving/activating the profile until at least one subject is associated.
9093-9099:⚠️ Potential issue | 🟠 MajorGate completion on the scheduled end time, not the start time.
The new guard blocks future starts, but a 90-minute session can still be marked completed at minute 1. That unlocks ratings before the booked window has actually finished.
🧰 Suggested fix
if request.method == "POST": - if session.scheduled_at > timezone.now(): - messages.error(request, "You cannot complete a session that has not started yet.") + session_end = session.scheduled_at + timedelta(minutes=session.duration_minutes) + if session_end > timezone.now(): + messages.error(request, "You can only complete a session after it ends.") return redirect("mentor_dashboard")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9093 - 9099, The guard currently prevents completion if session.scheduled_at is in the future, but it should block completion until the scheduled end time; change the check to compare the current time against the session's scheduled end (use session.scheduled_end if that attribute exists, otherwise compute end = session.scheduled_at + timedelta(minutes=session.duration or session.duration_minutes)), ensuring you use timezone-aware arithmetic with django.utils.timezone.now(); keep the rest of the POST flow (setting status = "completed", notes, save) the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/models.py`:
- Around line 3208-3215: The average_rating and total_sessions properties on the
model (which call self.sessions.aggregate(...) and
self.sessions.filter(...).count()) cause an N+1 when mentor_list.html evaluates
them per card; instead annotate these values on the queryset and read the
annotations in the template. Update the view that builds the mentors queryset to
use .annotate(avg_rating=Avg("sessions__rating",
filter=Q(sessions__rating__isnull=False)), total_sessions=Count("sessions",
filter=Q(sessions__status='completed'))) and change the template to use the
annotated fields (avg_rating and total_sessions) rather than calling the
average_rating and total_sessions instance properties; you may keep the
properties but have them fall back to the annotated attributes if present to
preserve compatibility.
- Around line 3236-3238: The Meta.unique_together constraint on the model
(currently set to ["mentor","student"]) causes IntegrityError on re-submitting
after cancel/decline; either change the DB/model constraint to be scoped to
active requests (e.g., add a status field partial/conditional unique index for
status in ("pending","accepted")) or update the create flow to resurrect an
existing cancelled/declined row instead of inserting; adjust model Meta and add
a schema migration to drop the unconditional unique_together and add the new
partial unique index, update the create()/request handling code path to use
resurrection logic if chosen, and add a regression test for the cancel/decline →
re-request path to verify no IntegrityError occurs.
- Around line 3257-3258: The model allows any non-negative integer for
duration_minutes; add Django validators to enforce 15–240 minutes server-side by
updating the duration_minutes field (models.PositiveIntegerField) to include
django.core.validators.MinValueValidator(15) and MaxValueValidator(240). Import
those validators at the top of web/models.py and then create a migration
(makemigrations/migrate) so the constraint is applied across all code paths that
set duration_minutes.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Around line 40-43: The Decline button in the mentorship template lacks focus
styles for keyboard users; update the button element inside the form that posts
to the respond_to_request view (the "Decline" button) to include the same focus
utility classes as the Accept button (e.g., add focus:outline-none focus:ring-2
and a red-ring class like focus:ring-red-300 or focus:ring-red-400) so it shows
a visible focus ring when tabbed to while preserving the existing color and size
classes.
In `@web/templates/mentorship/rate_session.html`:
- Around line 17-20: The radio input (name="rating") is visually hidden
(sr-only) so the visible span and star never reflect selection; add CSS rules
that target the checked and focused state of the hidden input (e.g.,
input[name="rating"]:checked + span and input[name="rating"]:checked + span + i,
and input[name="rating"]:focus + span / + i) to change color/weight so the
selected value remains highlighted after click/keyboard selection; ensure you
apply the same styling to the star element (the <i class="fas fa-star">) so both
number and star show selected state and keep the input required behavior intact.
In `@web/views.py`:
- Around line 9056-9063: parse_datetime produces a naive datetime from the HTML5
datetime-local input so comparing scheduled_at to timezone.now() raises a
TypeError; after parsing scheduled_at (the variable set by parse_datetime) wrap
it with timezone.make_aware() before performing the comparison and any saves.
Locate the block that parses scheduled_at (uses parse_datetime and
messages.error/redirect to "schedule_session") and call
timezone.make_aware(scheduled_at) (importing timezone if needed) then compare
that aware scheduled_at to timezone.now() for the future-check.
---
Duplicate comments:
In `@web/views.py`:
- Around line 8958-8969: The current parsing for experience_years and
hourly_rate uses max(...) which silently converts negative inputs to zero;
instead, parse the inputs into integers/floats in the existing try/except blocks
(remove the max(...) wrapping) and after successful parsing validate that
experience_years >= 0 and hourly_rate >= 0.0; if either is negative, call
messages.error with a clear message (e.g., "Please enter a non-negative value
for years of experience/hourly rate") and return redirect("become_mentor"). Keep
the existing ValueError/TypeError handling for invalid formats and update the
code paths around the experience_years and hourly_rate variables to use these
explicit validations.
- Around line 9064-9078: Currently malformed or too-small duration is silently
replaced with 60 and a session is created; instead, validate
request.POST["duration_minutes"] strictly (parse to int and require >=15) and
fail the request (raise/return a validation error) if invalid, and before
calling MentorshipSession.objects.create run the overlap check for both mentor
and mentorship_request.student using the same overlap-detection logic/service
your app uses (i.e., check any existing MentorshipSession for conflicting
scheduled_at/duration_minutes for mentor and student), and only call
MentorshipSession.objects.create(mentor=mentor,
student=mentorship_request.student, subject=..., scheduled_at=scheduled_at,
duration_minutes=duration, notes=notes) after both validation and overlap checks
pass.
- Around line 8904-8917: The request_mentorship flow currently allows
subject=None and become_mentor can activate a profile with zero subjects;
enforce subject as mandatory by validating request.POST.get("subject") in
request_mentorship (reject and redirect with messages.error if missing or not
found) before calling MentorshipRequest.objects.create, and update become_mentor
to block activation (or require adding subjects) when mentor.subjects.count() ==
0 — surface a clear messages.error and prevent saving/activating the profile
until at least one subject is associated.
- Around line 9093-9099: The guard currently prevents completion if
session.scheduled_at is in the future, but it should block completion until the
scheduled end time; change the check to compare the current time against the
session's scheduled end (use session.scheduled_end if that attribute exists,
otherwise compute end = session.scheduled_at +
timedelta(minutes=session.duration or session.duration_minutes)), ensuring you
use timezone-aware arithmetic with django.utils.timezone.now(); keep the rest of
the POST flow (setting status = "completed", notes, save) the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a7e6442-79e4-498b-9b9b-5c360401db57
⛔ Files ignored due to path filters (1)
web/migrations/0064_add_mentorship_models.pyis excluded by!**/migrations/**
📒 Files selected for processing (12)
web/admin.pyweb/models.pyweb/templates/mentorship/become_mentor.htmlweb/templates/mentorship/mentor_dashboard.htmlweb/templates/mentorship/mentor_list.htmlweb/templates/mentorship/mentor_profile.htmlweb/templates/mentorship/my_mentorship.htmlweb/templates/mentorship/rate_session.htmlweb/templates/mentorship/request_mentorship.htmlweb/templates/mentorship/schedule_session.htmlweb/urls.pyweb/views.py
Author
|
Additional fix:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (12)
web/templates/mentorship/mentor_list.html (2)
42-42:⚠️ Potential issue | 🟡 MinorUse descriptive avatar alt text.
The mentor avatar should expose meaningful alt text for assistive technologies.
♿ Suggested fix
- <img src="{{ mentor.user.profile.avatar.url }}" alt="" class="w-14 h-14 rounded-full object-cover"> + <img + src="{{ mentor.user.profile.avatar.url }}" + alt="{{ mentor.user.get_full_name|default:mentor.user.username }}'s avatar" + class="w-14 h-14 rounded-full object-cover" + >As per coding guidelines: "Ensure proper HTML structure and accessibility in templates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_list.html` at line 42, Replace the empty alt="" on the avatar <img> that uses mentor.user.profile.avatar.url with a descriptive, accessible alt string (e.g. "Avatar of {{ mentor.user.get_full_name }}" or fallback to "{{ mentor.user.username }}") so screen readers get meaningful context; update the template code in mentor_list.html where the <img> references mentor.user.profile.avatar.url to use that descriptive alt and include a sensible fallback if get_full_name is blank.
62-62:⚠️ Potential issue | 🟡 MinorRemove the unnecessary backslash before
$in rate rendering.This should output a normal currency prefix, not an escaped character.
💵 Suggested fix
- {% if mentor.is_free %}Free{% else %}\${{ mentor.hourly_rate }}/hr{% endif %} + {% if mentor.is_free %}Free{% else %}${{ mentor.hourly_rate }}/hr{% endif %}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_list.html` at line 62, In mentor_list.html remove the unnecessary escape before the dollar sign in the rate rendering: replace the string that outputs "\${{ mentor.hourly_rate }}/hr" with a normal "${{ mentor.hourly_rate }}/hr" so the template displays a standard currency prefix when mentor.is_free is false (refer to the conditional using mentor.is_free and mentor.hourly_rate).web/admin.py (1)
623-634: 🧹 Nitpick | 🔵 TrivialAdd
raw_id_fields/readonly_fieldson mentorship admin classes for better scaling.These models are relation-heavy; default dropdowns become slow/noisy as data grows.
⚙️ Suggested admin refinements
`@admin.register`(MentorshipRequest) class MentorshipRequestAdmin(admin.ModelAdmin): list_display = ("student", "mentor", "status", "created_at") list_filter = ("status",) search_fields = ("student__username", "mentor__user__username") + raw_id_fields = ("student", "mentor", "subject") + readonly_fields = ("created_at", "updated_at") `@admin.register`(MentorshipSession) class MentorshipSessionAdmin(admin.ModelAdmin): list_display = ("mentor", "student", "scheduled_at", "status", "rating") list_filter = ("status",) search_fields = ("mentor__user__username", "student__username") + raw_id_fields = ("mentor", "student", "subject", "request") + readonly_fields = ("created_at", "updated_at")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/admin.py` around lines 623 - 634, Both admin classes use relation-heavy fields that will slow the admin UI; update MentorshipRequestAdmin and MentorshipSessionAdmin to use raw_id_fields for FK relations and add appropriate readonly_fields for uneditable timestamps/ratings. Specifically, in MentorshipRequestAdmin add raw_id_fields = ("student", "mentor") and readonly_fields = ("created_at",) (or other audit fields), and in MentorshipSessionAdmin add raw_id_fields = ("mentor", "student") plus readonly_fields = ("created_at", "rating") or any non-editable timestamp/rating fields; keep list_display/search_fields unchanged.web/urls.py (1)
99-109:⚠️ Potential issue | 🟡 MinorApply Black formatting to the new mentorship route block.
This block is not Black-aligned with the surrounding file style (quote style + long unwrapped lines), which can fail lint/format checks.
♻️ Proposed formatting-only fix
- path('mentorship/', views.mentor_list, name='mentor_list'), - path('mentorship/become/', views.become_mentor, name='become_mentor'), - path('mentorship/dashboard/', views.mentor_dashboard, name='mentor_dashboard'), - path('mentorship/my/', views.my_mentorship, name='my_mentorship'), - path('mentorship/<int:mentor_id>/', views.mentor_profile_view, name='mentor_profile_view'), - path('mentorship/<int:mentor_id>/request/', views.request_mentorship, name='request_mentorship'), - path('mentorship/requests/<int:request_id>/respond/', views.respond_to_request, name='respond_to_request'), - path('mentorship/requests/<int:request_id>/cancel/', views.cancel_mentorship_request, name='cancel_mentorship_request'), - path('mentorship/requests/<int:request_id>/schedule/', views.schedule_session, name='schedule_session'), - path('mentorship/sessions/<int:session_id>/complete/', views.complete_session, name='complete_session'), - path('mentorship/sessions/<int:session_id>/rate/', views.rate_session, name='rate_session'), + path("mentorship/", views.mentor_list, name="mentor_list"), + path("mentorship/become/", views.become_mentor, name="become_mentor"), + path("mentorship/dashboard/", views.mentor_dashboard, name="mentor_dashboard"), + path("mentorship/my/", views.my_mentorship, name="my_mentorship"), + path("mentorship/<int:mentor_id>/", views.mentor_profile_view, name="mentor_profile_view"), + path("mentorship/<int:mentor_id>/request/", views.request_mentorship, name="request_mentorship"), + path("mentorship/requests/<int:request_id>/respond/", views.respond_to_request, name="respond_to_request"), + path( + "mentorship/requests/<int:request_id>/cancel/", + views.cancel_mentorship_request, + name="cancel_mentorship_request", + ), + path("mentorship/requests/<int:request_id>/schedule/", views.schedule_session, name="schedule_session"), + path("mentorship/sessions/<int:session_id>/complete/", views.complete_session, name="complete_session"), + path("mentorship/sessions/<int:session_id>/rate/", views.rate_session, name="rate_session"),As per coding guidelines: "
**/*.py: Format Python code with Black using 120-character line length" and "**/*.py: Fix linting errors in code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/urls.py` around lines 99 - 109, The mentorship URL block (routes registering views like mentor_list, become_mentor, mentor_dashboard, my_mentorship, mentor_profile_view, request_mentorship, respond_to_request, cancel_mentorship_request, schedule_session, complete_session, rate_session) is not formatted with Black (120-char line length) and uses inconsistent quote/wrapping; run Black with line-length=120 (or reformat the block to match surrounding quote style and wrap long lines) so each path() entry conforms to project formatting and lint checks.web/templates/mentorship/rate_session.html (1)
17-20:⚠️ Potential issue | 🟡 MinorMake rating selection persist visually and improve option naming for assistive tech.
Current styling only reacts on hover, so selected state is not obvious after click/keyboard selection.
♿ Suggested fix
- <label class="flex flex-col items-center cursor-pointer"> - <input type="radio" name="rating" value="{{ forloop.counter }}" required class="sr-only"> - <span class="text-2xl text-gray-300 dark:text-gray-600 hover:text-yellow-500 transition-colors">{{ forloop.counter }}</span> - <i class="fas fa-star text-gray-300 dark:text-gray-600 hover:text-yellow-500" aria-hidden="true"></i> + <label class="flex flex-col items-center cursor-pointer"> + <input + type="radio" + name="rating" + value="{{ forloop.counter }}" + required + class="peer sr-only" + aria-label="Rating {{ forloop.counter }}" + > + <span class="text-2xl text-gray-300 dark:text-gray-600 transition-colors peer-checked:text-yellow-500 peer-focus:text-yellow-500">{{ forloop.counter }}</span> + <i class="fas fa-star text-gray-300 dark:text-gray-600 transition-colors peer-checked:text-yellow-500 peer-focus:text-yellow-500" aria-hidden="true"></i> </label>As per coding guidelines: "Ensure proper HTML structure and accessibility in templates" and "Add hover/focus states for interactive elements".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/rate_session.html` around lines 17 - 20, The rating radios (input name="rating") only change on hover and have no persistent visual checked state or accessible naming; update the markup so the hidden radio input includes "peer" (e.g., class contains sr-only peer) and the sibling span and i use peer-checked and focus-visible utility classes to show a persistent selected state (e.g., peer-checked:text-yellow-500 and peer-focus-visible:...) and ensure keyboard focus styles are present; also add an accessible label for each option (either aria-label on the input like aria-label="{{ forloop.counter }} stars" or a visually-hidden <span> with "{{ forloop.counter }} stars") so assistive tech announces the option clearly while keeping the visual layout unchanged.web/templates/mentorship/schedule_session.html (1)
17-18:⚠️ Potential issue | 🟡 MinorPrevent past scheduling in both UI and server validation.
Please add a
minconstraint to the input for UX, and confirmschedule_sessionrejects past datetimes server-side.#!/bin/bash set -euo pipefail views_file="$(fd '^views\.py$' web | head -n1)" echo "Inspecting: ${views_file}" rg -n -C3 'def schedule_session|scheduled_at|timezone\.now|datetime' "${views_file}"Expected verification:
schedule_sessionshould compare submittedscheduled_atagainst current time and reject past values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/schedule_session.html` around lines 17 - 18, Add a client-side min constraint and server-side rejection for past datetimes: update the input element with id/name "scheduled_at" to include a min attribute set to the current datetime (so browser prevents selecting past times), and in the view function schedule_session validate the posted scheduled_at by parsing it and comparing against django.utils.timezone.now() (or timezone-aware current time) and return a validation error/HTTP 400 (or form error) if scheduled_at <= now to reject past values server-side. Ensure you reference the "scheduled_at" field name in request.POST/form and use schedule_session for the check so tests/rg search will find the validation.web/templates/mentorship/my_mentorship.html (1)
60-62:⚠️ Potential issue | 🟡 MinorRender a complete 5-star scale and include screen-reader rating text.
Current markup only shows filled stars, and assistive tech doesn’t get a clear textual rating.
♿ Suggested fix
- <span class="text-xs text-yellow-500 mt-1 inline-block"> - {% for i in "12345" %}{% if forloop.counter <= s.rating %}<i class="fas fa-star" aria-hidden="true"></i>{% endif %}{% endfor %} - </span> + <span class="text-xs text-yellow-500 mt-1 inline-block"> + <span class="sr-only">Rated {{ s.rating }} out of 5</span> + {% for i in "12345" %} + {% if forloop.counter <= s.rating %} + <i class="fas fa-star" aria-hidden="true"></i> + {% else %} + <i class="far fa-star" aria-hidden="true"></i> + {% endif %} + {% endfor %} + </span>As per coding guidelines: "Ensure proper HTML structure and accessibility in templates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/my_mentorship.html` around lines 60 - 62, The template currently only renders filled stars using the loop over "12345" and lacks accessible text; update the rating span (where s.rating is used and the forloop.counter logic appears) to always render five star icons (filled or empty based on s.rating) and add an offscreen/sr-only element that outputs the textual rating (e.g., "Rating: X out of 5") for screen readers so assistive tech receives the numeric rating while visual users see a complete 5-star scale.web/views.py (5)
9093-9099:⚠️ Potential issue | 🟠 MajorWait until the session window has ended before marking it complete.
The current guard only blocks sessions that have not started yet. A 60-minute session can still be marked complete one minute after it begins, which unlocks rating/review while the appointment is still in progress.
⏱️ Suggested fix
- if session.scheduled_at > timezone.now(): - messages.error(request, "You cannot complete a session that has not started yet.") + session_end = session.scheduled_at + timedelta(minutes=session.duration_minutes) + if session_end > timezone.now(): + messages.error(request, "You can only complete a session after it has ended.") return redirect("mentor_dashboard")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9093 - 9099, The current POST handler only prevents completing sessions that haven't started by checking session.scheduled_at, but it should also block completing a session while it's still in progress; compute the session end time (e.g., end_time = session.scheduled_at + timedelta(minutes=session.duration_minutes) or use session.ends_at if that field exists) and change the guard to if timezone.now() < end_time: messages.error(...) and return; keep using session.status = "completed", session.notes = ..., and session.save(update_fields=[...]) as before.
8850-9125: 🛠️ Refactor suggestion | 🟠 MajorPlease finish typing and documenting the new mentorship views.
These endpoints add a sizeable workflow surface, but the new views are still missing short docstrings and
HttpRequest/HttpResponseannotations, which makes this already-large module harder to maintain.As per coding guidelines
**/*.py:Use type hints in Python where appropriateandAdd docstrings to Python functions, classes, and modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8850 - 9125, Add short docstrings and type hints to each new view to match project guidelines: annotate the request parameter as django.http.HttpRequest and the return type as django.http.HttpResponse for mentor_list, mentor_profile_view, request_mentorship, my_mentorship, cancel_mentorship_request, become_mentor, mentor_dashboard, respond_to_request, schedule_session, complete_session, and rate_session; import HttpRequest and HttpResponse from django.http (or typing as needed) and add concise one-line docstrings to each function describing its purpose and key behavior plus brief param/return notes; ensure any helper variables that are returned in the context are named in the docstring and keep annotations consistent (e.g., -> HttpResponse) across these functions.
8959-8968:⚠️ Potential issue | 🟠 MajorReject negative profile values instead of silently resetting them to zero.
max(0, ...)andmax(0.0, ...)still turn tampered negative input into public0values. That hides bad input and can overwrite an existing mentor profile without feedback; negatives should be rejected just like malformed numbers.🧮 Suggested fix
try: - experience_years = max(0, int(request.POST.get("experience_years", 0))) + experience_years = int(request.POST.get("experience_years", 0)) + if experience_years < 0: + raise ValueError except (ValueError, TypeError): - messages.error(request, "Please enter a valid number for years of experience.") + messages.error(request, "Experience years must be 0 or greater.") return redirect("become_mentor") is_free = request.POST.get("is_free") == "on" try: - hourly_rate = max(0.0, float(request.POST.get("hourly_rate", 0))) + hourly_rate = float(request.POST.get("hourly_rate", 0)) + if hourly_rate < 0: + raise ValueError except (ValueError, TypeError): - messages.error(request, "Please enter a valid hourly rate.") + messages.error(request, "Hourly rate must be 0 or greater.") return redirect("become_mentor")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8959 - 8968, The code currently uses max(0, int(...)) and max(0.0, float(...)) which silently convert negative inputs to 0; change validation in the experience_years and hourly_rate handling to parse the values first (using int() and float()), then explicitly check for negative values and treat them as invalid: on negative or parse errors call messages.error with an appropriate message and return redirect("become_mentor") instead of coercing to zero. Update the blocks referencing experience_years and hourly_rate (and is_free processing remains unchanged) so negatives are rejected and never written to the mentor profile.
9056-9063:⚠️ Potential issue | 🔴 CriticalMake
scheduled_attimezone-aware before comparing it totimezone.now().When the browser posts a naive local datetime,
parse_datetime()returns a naivedatetime. Comparing that value to the awaretimezone.now()raisesTypeErrorunderUSE_TZ=True, so the view can fail before the validation message is shown.🕒 Suggested fix
scheduled_at = parse_datetime(request.POST.get("scheduled_at", "").strip()) if not scheduled_at: messages.error(request, "Please provide a valid date and time.") return redirect("schedule_session", request_id=request_id) + if timezone.is_naive(scheduled_at): + scheduled_at = timezone.make_aware(scheduled_at, timezone.get_current_timezone()) if scheduled_at <= timezone.now(): messages.error(request, "Session must be scheduled in the future.") return redirect("schedule_session", request_id=request_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9056 - 9063, The parsed scheduled_at from parse_datetime may be naive; before comparing to timezone.now() convert it to an aware datetime: check django.utils.timezone.is_naive(scheduled_at) and if so call django.utils.timezone.make_aware(scheduled_at, timezone=get_default_timezone() or settings.TIME_ZONE) so scheduled_at becomes timezone-aware (leave already-aware values unchanged), then perform the scheduled_at <= timezone.now() check and keep the same error/redirect behavior; update the block around parse_datetime, scheduled_at, and the comparison to use is_naive/make_aware to avoid TypeError under USE_TZ=True.
9064-9078:⚠️ Potential issue | 🟠 MajorServer-side scheduling rules are still too weak here.
The template already advertises a 15–240 minute range, but direct POSTs can still submit arbitrarily large durations, and there is still no overlap check for another scheduled session on either the mentor or student. That can create impossible bookings that look valid everywhere else in the workflow.
📅 Suggested fix
try: duration = int(request.POST.get("duration_minutes", 60)) - if duration < 15: + if duration < 15 or duration > 240: raise ValueError except (ValueError, TypeError): - duration = 60 + messages.error(request, "Duration must be between 15 and 240 minutes.") + return redirect("schedule_session", request_id=request_id) + new_end = scheduled_at + timedelta(minutes=duration) + overlapping = MentorshipSession.objects.filter(status="scheduled").filter( + Q(mentor=mentor) | Q(student=mentorship_request.student) + ) + if any( + existing.scheduled_at < new_end + and existing.scheduled_at + timedelta(minutes=existing.duration_minutes) > scheduled_at + for existing in overlapping + ): + messages.error(request, "This time overlaps another scheduled mentorship session.") + return redirect("schedule_session", request_id=request_id) notes = request.POST.get("notes", "").strip()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9064 - 9078, Enforce server-side duration limits and prevent overlapping bookings before calling MentorshipSession.objects.create: validate and clamp the parsed duration_minutes to the 15–240 range (or reject and set to default) instead of only lower-bounding it; compute the session end datetime as scheduled_at + timedelta(minutes=duration) and query existing MentorshipSession for the mentor and for mentorship_request.student to detect time overlaps (e.g., sessions where existing.start < new_end and existing.end > new_start), return or raise a validation error if any overlap is found, and only then call MentorshipSession.objects.create with the validated duration and notes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/models.py`:
- Around line 3267-3269: The Meta class currently only defines ordering =
["-scheduled_at"]; add a composite database index for the common query pattern
by updating the model's Meta to include an indexes list with
models.Index(fields=["status", "scheduled_at"]) so queries filtering by status
and ordering by scheduled_at use the index; locate the model's Meta (the one
that contains ordering = ["-scheduled_at"]) and add the indexes entry
referencing models.Index as shown to improve query performance for the
mentor_dashboard view.
In `@web/templates/mentorship/mentor_profile.html`:
- Line 40: The template currently escapes the dollar sign causing a literal
backslash to render; in mentor_profile.html update the conditional output for
mentor.hourly_rate so it uses an unescaped dollar sign (e.g., ${{
mentor.hourly_rate }}/hr) instead of \${{ mentor.hourly_rate }}/hr; keep the
existing {% if mentor.is_free %}Free{% else %}...{% endif %} structure and
ensure no backslash escape remains.
In `@web/views.py`:
- Around line 8897-8917: The view (request_mentorship) currently calls
MentorshipRequest.objects.create(...) but the model has unique_together =
["mentor","student"] which will raise an IntegrityError for re-requests; either
remove/change the model constraint (e.g., remove the unique_together or include
status in the uniqueness and add a migration) if you want multiple historical
rows, OR update the view to reuse the existing historical row: query
MentorshipRequest.objects.filter(mentor=mentor,
student=request.user).exclude(status__in=["pending","accepted"]).first() and if
found update its subject, message and set status="pending" (save) instead of
creating, otherwise proceed to create; alternatively wrap the create in a
transaction and catch IntegrityError to perform the update/reopen logic.
---
Duplicate comments:
In `@web/admin.py`:
- Around line 623-634: Both admin classes use relation-heavy fields that will
slow the admin UI; update MentorshipRequestAdmin and MentorshipSessionAdmin to
use raw_id_fields for FK relations and add appropriate readonly_fields for
uneditable timestamps/ratings. Specifically, in MentorshipRequestAdmin add
raw_id_fields = ("student", "mentor") and readonly_fields = ("created_at",) (or
other audit fields), and in MentorshipSessionAdmin add raw_id_fields =
("mentor", "student") plus readonly_fields = ("created_at", "rating") or any
non-editable timestamp/rating fields; keep list_display/search_fields unchanged.
In `@web/templates/mentorship/mentor_list.html`:
- Line 42: Replace the empty alt="" on the avatar <img> that uses
mentor.user.profile.avatar.url with a descriptive, accessible alt string (e.g.
"Avatar of {{ mentor.user.get_full_name }}" or fallback to "{{
mentor.user.username }}") so screen readers get meaningful context; update the
template code in mentor_list.html where the <img> references
mentor.user.profile.avatar.url to use that descriptive alt and include a
sensible fallback if get_full_name is blank.
- Line 62: In mentor_list.html remove the unnecessary escape before the dollar
sign in the rate rendering: replace the string that outputs "\${{
mentor.hourly_rate }}/hr" with a normal "${{ mentor.hourly_rate }}/hr" so the
template displays a standard currency prefix when mentor.is_free is false (refer
to the conditional using mentor.is_free and mentor.hourly_rate).
In `@web/templates/mentorship/my_mentorship.html`:
- Around line 60-62: The template currently only renders filled stars using the
loop over "12345" and lacks accessible text; update the rating span (where
s.rating is used and the forloop.counter logic appears) to always render five
star icons (filled or empty based on s.rating) and add an offscreen/sr-only
element that outputs the textual rating (e.g., "Rating: X out of 5") for screen
readers so assistive tech receives the numeric rating while visual users see a
complete 5-star scale.
In `@web/templates/mentorship/rate_session.html`:
- Around line 17-20: The rating radios (input name="rating") only change on
hover and have no persistent visual checked state or accessible naming; update
the markup so the hidden radio input includes "peer" (e.g., class contains
sr-only peer) and the sibling span and i use peer-checked and focus-visible
utility classes to show a persistent selected state (e.g.,
peer-checked:text-yellow-500 and peer-focus-visible:...) and ensure keyboard
focus styles are present; also add an accessible label for each option (either
aria-label on the input like aria-label="{{ forloop.counter }} stars" or a
visually-hidden <span> with "{{ forloop.counter }} stars") so assistive tech
announces the option clearly while keeping the visual layout unchanged.
In `@web/templates/mentorship/schedule_session.html`:
- Around line 17-18: Add a client-side min constraint and server-side rejection
for past datetimes: update the input element with id/name "scheduled_at" to
include a min attribute set to the current datetime (so browser prevents
selecting past times), and in the view function schedule_session validate the
posted scheduled_at by parsing it and comparing against
django.utils.timezone.now() (or timezone-aware current time) and return a
validation error/HTTP 400 (or form error) if scheduled_at <= now to reject past
values server-side. Ensure you reference the "scheduled_at" field name in
request.POST/form and use schedule_session for the check so tests/rg search will
find the validation.
In `@web/urls.py`:
- Around line 99-109: The mentorship URL block (routes registering views like
mentor_list, become_mentor, mentor_dashboard, my_mentorship,
mentor_profile_view, request_mentorship, respond_to_request,
cancel_mentorship_request, schedule_session, complete_session, rate_session) is
not formatted with Black (120-char line length) and uses inconsistent
quote/wrapping; run Black with line-length=120 (or reformat the block to match
surrounding quote style and wrap long lines) so each path() entry conforms to
project formatting and lint checks.
In `@web/views.py`:
- Around line 9093-9099: The current POST handler only prevents completing
sessions that haven't started by checking session.scheduled_at, but it should
also block completing a session while it's still in progress; compute the
session end time (e.g., end_time = session.scheduled_at +
timedelta(minutes=session.duration_minutes) or use session.ends_at if that field
exists) and change the guard to if timezone.now() < end_time:
messages.error(...) and return; keep using session.status = "completed",
session.notes = ..., and session.save(update_fields=[...]) as before.
- Around line 8850-9125: Add short docstrings and type hints to each new view to
match project guidelines: annotate the request parameter as
django.http.HttpRequest and the return type as django.http.HttpResponse for
mentor_list, mentor_profile_view, request_mentorship, my_mentorship,
cancel_mentorship_request, become_mentor, mentor_dashboard, respond_to_request,
schedule_session, complete_session, and rate_session; import HttpRequest and
HttpResponse from django.http (or typing as needed) and add concise one-line
docstrings to each function describing its purpose and key behavior plus brief
param/return notes; ensure any helper variables that are returned in the context
are named in the docstring and keep annotations consistent (e.g., ->
HttpResponse) across these functions.
- Around line 8959-8968: The code currently uses max(0, int(...)) and max(0.0,
float(...)) which silently convert negative inputs to 0; change validation in
the experience_years and hourly_rate handling to parse the values first (using
int() and float()), then explicitly check for negative values and treat them as
invalid: on negative or parse errors call messages.error with an appropriate
message and return redirect("become_mentor") instead of coercing to zero. Update
the blocks referencing experience_years and hourly_rate (and is_free processing
remains unchanged) so negatives are rejected and never written to the mentor
profile.
- Around line 9056-9063: The parsed scheduled_at from parse_datetime may be
naive; before comparing to timezone.now() convert it to an aware datetime: check
django.utils.timezone.is_naive(scheduled_at) and if so call
django.utils.timezone.make_aware(scheduled_at, timezone=get_default_timezone()
or settings.TIME_ZONE) so scheduled_at becomes timezone-aware (leave
already-aware values unchanged), then perform the scheduled_at <= timezone.now()
check and keep the same error/redirect behavior; update the block around
parse_datetime, scheduled_at, and the comparison to use is_naive/make_aware to
avoid TypeError under USE_TZ=True.
- Around line 9064-9078: Enforce server-side duration limits and prevent
overlapping bookings before calling MentorshipSession.objects.create: validate
and clamp the parsed duration_minutes to the 15–240 range (or reject and set to
default) instead of only lower-bounding it; compute the session end datetime as
scheduled_at + timedelta(minutes=duration) and query existing MentorshipSession
for the mentor and for mentorship_request.student to detect time overlaps (e.g.,
sessions where existing.start < new_end and existing.end > new_start), return or
raise a validation error if any overlap is found, and only then call
MentorshipSession.objects.create with the validated duration and notes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6de2a57a-a557-406e-b92a-1783780628e4
⛔ Files ignored due to path filters (1)
web/migrations/0064_add_mentorship_models.pyis excluded by!**/migrations/**
📒 Files selected for processing (12)
web/admin.pyweb/models.pyweb/templates/mentorship/become_mentor.htmlweb/templates/mentorship/mentor_dashboard.htmlweb/templates/mentorship/mentor_list.htmlweb/templates/mentorship/mentor_profile.htmlweb/templates/mentorship/my_mentorship.htmlweb/templates/mentorship/rate_session.htmlweb/templates/mentorship/request_mentorship.htmlweb/templates/mentorship/schedule_session.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/mentorship-system
branch
from
32c8cc0 to
178c312
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (12)
web/templates/mentorship/mentor_list.html (1)
61-63:⚠️ Potential issue | 🟡 MinorStill unresolved: paid mentor cards render a literal backslash.
Line 62 outputs
\$, so paid rates show with a stray\in the UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_list.html` around lines 61 - 63, The template currently renders a literal backslash because the dollar sign is escaped in the paid-rate branch; update the paid-rate output in mentor_list.html so it emits an unescaped dollar sign before mentor.hourly_rate (remove the backslash escape used before $ in the string that includes {{ mentor.hourly_rate }}), ensuring the span that uses mentor.is_free shows "${{ mentor.hourly_rate }}/hr" visually rather than "\${...}".web/templates/mentorship/rate_session.html (1)
17-20:⚠️ Potential issue | 🟡 MinorStill unresolved: the selected rating never stays highlighted.
Because the radios are visually hidden and the visible elements only change on hover, users get no persistent feedback after choosing a score.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/rate_session.html` around lines 17 - 20, The radio input is visually hidden and the visible span/i only change on hover, so selection isn't persistent; add the Tailwind "peer" utility to the input (e.g., input class includes "peer" in addition to "sr-only") and replace the hover-only classes on the sibling span and i (e.g., "hover:text-yellow-500") with peer-checked variants like "peer-checked:text-yellow-500" (and any needed dark-mode equivalent) so that when the radio (input) is checked the visible elements (the span and the i) remain highlighted; update the input and the sibling elements in the label containing the input, span, and i accordingly.web/models.py (2)
3208-3215:⚠️ Potential issue | 🟠 MajorStill unresolved: mentor card metrics trigger per-row queries.
web/templates/mentorship/mentor_list.htmlLines 51-52 call both properties inside the mentor loop, and each property always hits the ORM. The listing still scales as N+1, and rated mentors pay for two extra queries each.
3228-3238:⚠️ Potential issue | 🟠 MajorAdd database-level uniqueness constraint for active mentor/student requests to prevent race conditions.
The current application-level check at line 8897-8900 in
web/views.py(filtering for existing pending/accepted requests) cannot prevent concurrent requests from both passing the check before either writes to the database. Two simultaneous requests from the same student to the same mentor can both bypass the duplicate validation and create duplicate records.Add a conditional unique constraint on the
(mentor, student)tuple for active states ("pending"and"accepted"). This requires:
- A database constraint in the model's Meta class
- A migration to apply it to the existing table
- Error handling in the view to gracefully catch
IntegrityErrorif a duplicate is attempted during the constraint addition window- A regression test confirming the constraint prevents parallel submissions
Suggested model change
class Meta: ordering = ["-created_at"] + constraints = [ + models.UniqueConstraint( + fields=["mentor", "student"], + condition=models.Q(status__in=["pending", "accepted"]), + name="unique_active_mentorship_request", + ) + ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/models.py` around lines 3228 - 3238, The MentorshipRequest model currently allows duplicate active requests; add a conditional unique constraint in the model Meta to enforce uniqueness of the (mentor, student) pair when status is in ("pending","accepted") (use UniqueConstraint with condition referencing the status field/STATUS_CHOICES), then create and run a Django migration to apply it, update the view logic that currently filters for existing requests (in web/views.py around the existing pending/accepted check) to catch django.db.IntegrityError on save/create and return the same user-friendly response when a duplicate is attempted, and add a regression test that simulates concurrent submissions (or uses transaction.atomic with forced race) asserting that only one active request is created and the other raises IntegrityError or is handled gracefully.web/templates/mentorship/mentor_dashboard.html (2)
40-43:⚠️ Potential issue | 🟡 MinorAdd visible keyboard focus styles to the Decline button.
The Decline action is keyboard-reachable but lacks explicit focus-ring styling, unlike Accept.
♿ Minimal fix
- <button type="submit" name="action" value="decline" class="bg-red-100 hover:bg-red-200 text-red-700 dark:bg-red-900 dark:text-red-300 text-sm font-semibold px-3 py-1.5 rounded-lg transition duration-200">Decline</button> + <button type="submit" name="action" value="decline" class="bg-red-100 hover:bg-red-200 text-red-700 dark:bg-red-900 dark:text-red-300 text-sm font-semibold px-3 py-1.5 rounded-lg transition duration-200 focus:outline-none focus:ring-2 focus:ring-red-300">Decline</button>As per coding guidelines, "Add hover/focus states for interactive elements."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_dashboard.html` around lines 40 - 43, The Decline button lacks visible keyboard focus styling; update the button element in mentor_dashboard.html (the <button> inside the form with name="action" value="decline") to include focus/outline ring utility classes similar to the Accept button (for example add focus:outline-none focus:ring-2 focus:ring-red-500 focus:ring-offset-2 and matching dark:focus:ring-offset- classes) so keyboard users see a clear focus ring on tab/focus while preserving existing hover/active classes.
79-82:⚠️ Potential issue | 🟡 MinorGuard “Mark Complete” with confirmation and focus styles.
This action is irreversible in workflow terms; add a confirmation prompt and visible keyboard focus state.
🛡️ Suggested update
- <form method="post" action="{% url 'complete_session' s.id %}"> + <form method="post" action="{% url 'complete_session' s.id %}" onsubmit="return confirm('Mark this session as complete? This cannot be undone.')"> {% csrf_token %} - <button type="submit" class="text-sm bg-green-100 hover:bg-green-200 text-green-700 dark:bg-green-900 dark:text-green-300 font-semibold px-3 py-1 rounded-lg transition duration-200">Mark Complete</button> + <button type="submit" class="text-sm bg-green-100 hover:bg-green-200 text-green-700 dark:bg-green-900 dark:text-green-300 font-semibold px-3 py-1 rounded-lg transition duration-200 focus:outline-none focus:ring-2 focus:ring-green-300">Mark Complete</button> </form>As per coding guidelines, "Ensure proper HTML structure and accessibility in templates" and "Add hover/focus states for interactive elements."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_dashboard.html` around lines 79 - 82, Wrap the irreversible "Mark Complete" submit in a client-side confirmation and add visible keyboard focus styles: for the form that posts to the 'complete_session' action (the form using s.id) attach a JavaScript confirmation handler (e.g., onsubmit) that prompts the user and returns false to cancel if declined, and enhance the button by adding an accessible label/aria-label and a clear focus-visible style (outline or ring) so keyboard users see focus; ensure the markup remains a valid form submit (method="post" with {% csrf_token %}) and keep the button type="submit" while adding the confirmation and focus behavior.web/templates/mentorship/mentor_profile.html (2)
52-54:⚠️ Potential issue | 🟡 MinorExpose numeric rating text for screen readers.
The stars are visual-only (
aria-hidden="true"), so add ansr-onlyrating string next to them.♿ Suggested fix
<span class="text-yellow-500 text-xs"> + <span class="sr-only">{{ r.rating }} out of 5 stars</span> {% for i in "12345" %}{% if forloop.counter <= r.rating %}<i class="fas fa-star" aria-hidden="true"></i>{% else %}<i class="far fa-star" aria-hidden="true"></i>{% endif %}{% endfor %} </span>As per coding guidelines, "Ensure proper HTML structure and accessibility in templates."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_profile.html` around lines 52 - 54, Add an accessible, screen-reader-only numeric rating string next to the visual stars inside the same span: after the existing stars loop (the block using forloop and r.rating), output a hidden text node like "r.rating out of 5" using the sr-only class so screen readers read the numeric value (e.g., use the r.rating variable in the template to render "{{ r.rating }} out of 5" wrapped in an element with class "sr-only"); keep the visual stars aria-hidden="true" unchanged.
14-19:⚠️ Potential issue | 🟡 MinorProvide accessible avatar text alternatives.
The profile image uses empty
alt, and the fallback icon has no accessible name. Please expose the mentor name to assistive tech in both branches.♿ Suggested fix
- <img src="{{ mentor.user.profile.avatar.url }}" alt="" class="w-20 h-20 rounded-full object-cover"> + <img src="{{ mentor.user.profile.avatar.url }}" alt="{{ mentor.user.get_full_name|default:mentor.user.username }}'s profile photo" class="w-20 h-20 rounded-full object-cover"> {% else %} - <div class="w-20 h-20 rounded-full bg-teal-100 dark:bg-teal-900 flex items-center justify-center"> + <div class="w-20 h-20 rounded-full bg-teal-100 dark:bg-teal-900 flex items-center justify-center" aria-label="{{ mentor.user.get_full_name|default:mentor.user.username }}'s profile photo"> <i class="fas fa-user text-teal-500 dark:text-teal-300 text-3xl" aria-hidden="true"></i> </div>As per coding guidelines, "Ensure proper HTML structure and accessibility in templates."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/mentorship/mentor_profile.html` around lines 14 - 19, The avatar markup in mentor_profile.html currently uses an empty alt and a decorative icon with no accessible name; update both branches of the conditional that checks mentor.user.profile.avatar so the <img> alt contains the mentor's display name (e.g. use mentor.user.get_full_name or mentor.user.username) and the fallback container exposes the same name via an accessible label (for example add aria-label="Avatar of {{ mentor.user.get_full_name }}" to the div or include a visually-hidden span with the mentor's name), ensuring the check around mentor.user.profile.avatar still governs which element renders.web/views.py (4)
8904-8917:⚠️ Potential issue | 🟠 MajorRequire a mentor subject before creating the request.
A blank or tampered POST still falls through with
subject=None, which lets you persist subject-less requests and later subject-less sessions. If this workflow is meant to be subject-driven, reject missingsubject_idbefore the mentor-subject lookup.💡 Suggested fix
message = request.POST.get("message", "").strip() subject_id = request.POST.get("subject") if not message: messages.error(request, "Please include a message with your request.") return redirect("request_mentorship", mentor_id=mentor_id) - subject = None - if subject_id: - subject = mentor.subjects.filter(id=subject_id).first() - if not subject: - messages.error(request, "Please select a valid subject from the mentor profile.") - return redirect("request_mentorship", mentor_id=mentor_id) + if not subject_id: + messages.error(request, "Please choose a subject.") + return redirect("request_mentorship", mentor_id=mentor_id) + subject = mentor.subjects.filter(id=subject_id).first() + if not subject: + messages.error(request, "Please select a valid subject from the mentor profile.") + return redirect("request_mentorship", mentor_id=mentor_id) MentorshipRequest.objects.create( mentor=mentor, student=request.user, subject=subject, message=message )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8904 - 8917, The view currently allows subject-less MentorshipRequest creation when POST is tampered because subject_id can be empty; add an explicit check that request.POST.get("subject") (subject_id) is present and non-empty before doing mentor.subjects.filter(...) and creating the request: if subject_id is missing, call messages.error(request, "Please select a valid subject from the mentor profile.") and return redirect("request_mentorship", mentor_id=mentor_id); only then perform subject = mentor.subjects.filter(id=subject_id).first() and if subject is None reject similarly and avoid calling MentorshipRequest.objects.create(...).
8959-8968:⚠️ Potential issue | 🟠 MajorReject negative mentor profile numbers instead of clamping them to zero.
max(0, ...)andmax(0.0, ...)silently rewrite invalid input into public profile data. A typo like-1years or-50rate should fail validation, not save0.💡 Suggested fix
try: - experience_years = max(0, int(request.POST.get("experience_years", 0))) + experience_years = int(request.POST.get("experience_years", 0)) + if experience_years < 0: + raise ValueError except (ValueError, TypeError): messages.error(request, "Please enter a valid number for years of experience.") return redirect("become_mentor") is_free = request.POST.get("is_free") == "on" try: - hourly_rate = max(0.0, float(request.POST.get("hourly_rate", 0))) + hourly_rate = float(request.POST.get("hourly_rate", 0)) + if hourly_rate < 0: + raise ValueError except (ValueError, TypeError): messages.error(request, "Please enter a valid hourly rate.") return redirect("become_mentor")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 8959 - 8968, The code currently clamps negative inputs to zero; instead, in the parsing blocks for experience_years and hourly_rate in web/views.py, remove the max(0, ...) and max(0.0, ...) usage and validate after conversion: parse the value with int(...) / float(...) inside the existing try/except (catch ValueError/TypeError), then if the parsed value is negative call messages.error with an appropriate message and return redirect("become_mentor"); keep the existing error handling for non-numeric input. Reference the experience_years and hourly_rate parsing blocks and the redirect("become_mentor") call to locate the changes.
9076-9083:⚠️ Potential issue | 🟠 MajorPlease reject overlapping mentorship windows before saving.
This create path still never checks other scheduled sessions for either participant, so the same mentor or student can be double-booked for intersecting time ranges. Even a simple
existing.scheduled_at < new_end and existing_end > scheduled_atguard would prevent impossible bookings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9076 - 9083, The create path for MentorshipSession (MentorshipSession.objects.create) must reject overlapping sessions for the mentor or student before saving: compute the new session end as scheduled_at + duration_minutes, query MentorshipSession for any sessions for either mentor or mentorship_request.student that intersect (i.e. existing.scheduled_at < new_end AND existing_end > scheduled_at where existing_end = existing.scheduled_at + timedelta(minutes=existing.duration_minutes)), and if any are found raise/return an error (e.g., ValidationError or appropriate response) instead of calling MentorshipSession.objects.create; ensure this check runs prior to creating the object and references the mentor, student, scheduled_at and duration_minutes fields.
8850-8851: 🛠️ Refactor suggestion | 🟠 MajorPlease document and annotate the mentorship views.
This adds a large new workflow surface, but the new views are still undocumented and untyped, which makes the permission and state transitions harder to maintain.
Run this read-only check to list the remaining gaps:
#!/bin/bash python - <<'PY' import ast from pathlib import Path targets = { "mentor_list", "mentor_profile_view", "request_mentorship", "my_mentorship", "cancel_mentorship_request", "become_mentor", "mentor_dashboard", "respond_to_request", "schedule_session", "complete_session", "rate_session", } tree = ast.parse(Path("web/views.py").read_text()) for node in tree.body: if isinstance(node, ast.FunctionDef) and node.name in targets: print( f"{node.name}: " f"docstring={bool(ast.get_docstring(node))}, " f"request_annotation={bool(node.args.args and node.args.args[0].annotation)}, " f"return_annotation={node.returns is not None}" ) PYAs per coding guidelines
**/*.py:Use type hints in Python where appropriateandAdd docstrings to Python functions, classes, and modules.Also applies to: 8875-8876, 8891-8892, 8925-8926, 8939-8940, 8951-8952, 9001-9002, 9025-9026, 9046-9047, 9091-9092, 9109-9110
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/mentorship/become_mentor.html`:
- Line 21: The one-line Django template loop rendering the availability options
(the "{% for val, label in availability_choices %}...{% endfor %}" block that
checks "mentor.availability == val") must be expanded and formatted to comply
with djlint 120-char rules: break the control tags and the <option> markup onto
separate lines (opening "{% for %}" on its own line, the <option> tag indented
on the next line with the conditional "selected" expression formatted clearly,
then a closing "{% endfor %}" on its own line), then run djlint to autoformat
the file so all similar condensed blocks (including the ones at the other noted
locations) conform to the project's template style guidelines.
- Around line 33-36: Wrap the related checkbox inputs for the Subjects list in a
semantic fieldset and add a legend so screen readers treat them as a single
control group: replace the current standalone <label> + checkbox grid block with
a <fieldset> containing a <legend> (e.g., "Subjects") and keep the existing loop
that renders inputs for subjects (uses variables subjects and
mentor.subjects.all) inside that fieldset; ensure each checkbox label and input
remain unchanged so their values and checked logic (name="subjects", value="{{
s.id }}", {% if mentor and s in mentor.subjects.all %}checked{% endif %})
continue to work.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Around line 68-92: The template never renders the past_sessions context
provided by the mentor_dashboard view; add a block below the Upcoming Sessions
section that mirrors its structure to display past_sessions: check {% if
past_sessions %}, loop {% for s in past_sessions %}, and for each s show student
name (s.student.get_full_name|default:s.student.username), scheduled_at
(s.scheduled_at|date:"M d, Y H:i"), duration (s.duration_minutes) and subject
(s.subject.name if present); if empty, render a similar "No completed sessions."
placeholder; keep the same container classes and styling as the
upcoming_sessions card so layout remains consistent.
In `@web/templates/mentorship/mentor_profile.html`:
- Line 76: The Cancel Request button lacks keyboard focus styling; update the
button element (the <button> with text "Cancel Request") to include visible
focus classes so keyboard users see focus — e.g., add Tailwind focus classes
such as focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-red-500
and appropriate dark-mode offset (dark:focus:ring-offset-gray-800) or use
focus-visible variants to only show the ring for keyboard navigation; ensure the
added classes integrate with the existing text-sm text-red-500
hover:text-red-700 dark:text-red-400 classes.
In `@web/templates/mentorship/my_mentorship.html`:
- Around line 15-29: Add a conditional cancel form/button for pending mentorship
requests: when req.status == 'pending' render a small POST form (with {%
csrf_token %}) targeting the existing cancel_mentorship_request URL (e.g.
action="{% url 'cancel_mentorship_request' req.id %}") and a submit button (e.g.
"Cancel") next to the status or inside the same container; ensure the form uses
method="post" and follows the existing styling/pattern used elsewhere so
students can cancel pending requests from the my_mentorship.html view.
In `@web/templates/mentorship/schedule_session.html`:
- Around line 16-18: The datetime-local input with id/name "scheduled_at" is
timezone-naive but your app uses Django DEFAULT_TIME_ZONE (America/New_York), so
show the effective timezone next to the field (e.g., "Eastern Time (ET)") in the
schedule_session.html template and ensure the server-side form/view that
processes "scheduled_at" consistently interprets it in that same timezone;
update the template to render the timezone label adjacent to the input (using
the same settings.TIME_ZONE or a helper), and update the form clean or the view
handling scheduled_at to localize the naive datetime with Django's
timezone.get_default_timezone() (or equivalent) so saved datetimes are aware and
consistent with the displayed timezone.
In `@web/views.py`:
- Around line 9069-9074: The code silently coerces invalid or too-short
request.POST.get("duration_minutes", 60) values to duration=60; instead,
validate and surface the error: when parsing duration_minutes (the block that
sets duration from request.POST.get("duration_minutes", 60)), if it is missing,
non-numeric, or <15, add a validation error to the form/context (or return
HttpResponseBadRequest) and re-render the booking form rather than defaulting to
60; update any code that uses the duration variable afterwards (the booking
creation path) to only proceed when the validated duration is present.
- Around line 9098-9104: The current POST handler checks only
session.scheduled_at before allowing completion, which lets mentors finish
sessions immediately after start time; change the check to compare now against
the scheduled end time instead (e.g., use session.scheduled_end or compute
session.scheduled_at + timedelta(...) using the session's duration field) so you
only allow completion after the booked end; update the condition replacing
session.scheduled_at > timezone.now() with a check like scheduled_end >
timezone.now() and keep the existing messages.error, redirect, and
session.save(update_fields=["status", "notes", "updated_at"]) behavior.
---
Duplicate comments:
In `@web/models.py`:
- Around line 3228-3238: The MentorshipRequest model currently allows duplicate
active requests; add a conditional unique constraint in the model Meta to
enforce uniqueness of the (mentor, student) pair when status is in
("pending","accepted") (use UniqueConstraint with condition referencing the
status field/STATUS_CHOICES), then create and run a Django migration to apply
it, update the view logic that currently filters for existing requests (in
web/views.py around the existing pending/accepted check) to catch
django.db.IntegrityError on save/create and return the same user-friendly
response when a duplicate is attempted, and add a regression test that simulates
concurrent submissions (or uses transaction.atomic with forced race) asserting
that only one active request is created and the other raises IntegrityError or
is handled gracefully.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Around line 40-43: The Decline button lacks visible keyboard focus styling;
update the button element in mentor_dashboard.html (the <button> inside the form
with name="action" value="decline") to include focus/outline ring utility
classes similar to the Accept button (for example add focus:outline-none
focus:ring-2 focus:ring-red-500 focus:ring-offset-2 and matching
dark:focus:ring-offset- classes) so keyboard users see a clear focus ring on
tab/focus while preserving existing hover/active classes.
- Around line 79-82: Wrap the irreversible "Mark Complete" submit in a
client-side confirmation and add visible keyboard focus styles: for the form
that posts to the 'complete_session' action (the form using s.id) attach a
JavaScript confirmation handler (e.g., onsubmit) that prompts the user and
returns false to cancel if declined, and enhance the button by adding an
accessible label/aria-label and a clear focus-visible style (outline or ring) so
keyboard users see focus; ensure the markup remains a valid form submit
(method="post" with {% csrf_token %}) and keep the button type="submit" while
adding the confirmation and focus behavior.
In `@web/templates/mentorship/mentor_list.html`:
- Around line 61-63: The template currently renders a literal backslash because
the dollar sign is escaped in the paid-rate branch; update the paid-rate output
in mentor_list.html so it emits an unescaped dollar sign before
mentor.hourly_rate (remove the backslash escape used before $ in the string that
includes {{ mentor.hourly_rate }}), ensuring the span that uses mentor.is_free
shows "${{ mentor.hourly_rate }}/hr" visually rather than "\${...}".
In `@web/templates/mentorship/mentor_profile.html`:
- Around line 52-54: Add an accessible, screen-reader-only numeric rating string
next to the visual stars inside the same span: after the existing stars loop
(the block using forloop and r.rating), output a hidden text node like "r.rating
out of 5" using the sr-only class so screen readers read the numeric value
(e.g., use the r.rating variable in the template to render "{{ r.rating }} out
of 5" wrapped in an element with class "sr-only"); keep the visual stars
aria-hidden="true" unchanged.
- Around line 14-19: The avatar markup in mentor_profile.html currently uses an
empty alt and a decorative icon with no accessible name; update both branches of
the conditional that checks mentor.user.profile.avatar so the <img> alt contains
the mentor's display name (e.g. use mentor.user.get_full_name or
mentor.user.username) and the fallback container exposes the same name via an
accessible label (for example add aria-label="Avatar of {{
mentor.user.get_full_name }}" to the div or include a visually-hidden span with
the mentor's name), ensuring the check around mentor.user.profile.avatar still
governs which element renders.
In `@web/templates/mentorship/rate_session.html`:
- Around line 17-20: The radio input is visually hidden and the visible span/i
only change on hover, so selection isn't persistent; add the Tailwind "peer"
utility to the input (e.g., input class includes "peer" in addition to
"sr-only") and replace the hover-only classes on the sibling span and i (e.g.,
"hover:text-yellow-500") with peer-checked variants like
"peer-checked:text-yellow-500" (and any needed dark-mode equivalent) so that
when the radio (input) is checked the visible elements (the span and the i)
remain highlighted; update the input and the sibling elements in the label
containing the input, span, and i accordingly.
In `@web/views.py`:
- Around line 8904-8917: The view currently allows subject-less
MentorshipRequest creation when POST is tampered because subject_id can be
empty; add an explicit check that request.POST.get("subject") (subject_id) is
present and non-empty before doing mentor.subjects.filter(...) and creating the
request: if subject_id is missing, call messages.error(request, "Please select a
valid subject from the mentor profile.") and return
redirect("request_mentorship", mentor_id=mentor_id); only then perform subject =
mentor.subjects.filter(id=subject_id).first() and if subject is None reject
similarly and avoid calling MentorshipRequest.objects.create(...).
- Around line 8959-8968: The code currently clamps negative inputs to zero;
instead, in the parsing blocks for experience_years and hourly_rate in
web/views.py, remove the max(0, ...) and max(0.0, ...) usage and validate after
conversion: parse the value with int(...) / float(...) inside the existing
try/except (catch ValueError/TypeError), then if the parsed value is negative
call messages.error with an appropriate message and return
redirect("become_mentor"); keep the existing error handling for non-numeric
input. Reference the experience_years and hourly_rate parsing blocks and the
redirect("become_mentor") call to locate the changes.
- Around line 9076-9083: The create path for MentorshipSession
(MentorshipSession.objects.create) must reject overlapping sessions for the
mentor or student before saving: compute the new session end as scheduled_at +
duration_minutes, query MentorshipSession for any sessions for either mentor or
mentorship_request.student that intersect (i.e. existing.scheduled_at < new_end
AND existing_end > scheduled_at where existing_end = existing.scheduled_at +
timedelta(minutes=existing.duration_minutes)), and if any are found raise/return
an error (e.g., ValidationError or appropriate response) instead of calling
MentorshipSession.objects.create; ensure this check runs prior to creating the
object and references the mentor, student, scheduled_at and duration_minutes
fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91aed755-3003-4cbd-9109-a5db6f9ae49d
⛔ Files ignored due to path filters (1)
web/migrations/0064_add_mentorship_models.pyis excluded by!**/migrations/**
📒 Files selected for processing (12)
web/admin.pyweb/models.pyweb/templates/mentorship/become_mentor.htmlweb/templates/mentorship/mentor_dashboard.htmlweb/templates/mentorship/mentor_list.htmlweb/templates/mentorship/mentor_profile.htmlweb/templates/mentorship/my_mentorship.htmlweb/templates/mentorship/rate_session.htmlweb/templates/mentorship/request_mentorship.htmlweb/templates/mentorship/schedule_session.htmlweb/urls.pyweb/views.py
Author
|
Addressed remaining feedback:
|
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.
Adds a complete mentorship system connecting students with experienced mentors for 1-on-1 guidance.
Changes:
Features:
Purpose
Adds a full mentorship system: users can become mentors, browse and filter mentors, send/respond to mentorship requests, schedule 1:1 sessions, mark sessions complete, and rate/review completed sessions.
Key modifications
Impact / notes