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 Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Models web/models.py |
Added MentorProfile, MentorshipRequest, MentorshipSession with relations to User/Subject, availability/pricing/scheduling/status fields, validators, timestamps, ordering, and helper properties (average_rating, total_sessions). |
Admin web/admin.py |
Registered MentorProfile, MentorshipRequest, MentorshipSession via @admin.register ModelAdmin classes (list_display, list_filter, search_fields) alongside existing User admin. |
Views (logic) web/views.py |
Added ~11 login-protected views: mentor listing/filtering, mentor profile, create/cancel requests, mentor onboarding/editing, mentor dashboard, accept/decline requests, schedule/complete sessions, and rate sessions; includes validation, datetime parsing, M2M assignment, and state transitions. |
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 & Profile web/templates/mentorship/mentor_list.html, .../mentor_profile.html, .../become_mentor.html |
New templates for browsing/filtering mentors, mentor profile (reviews, availability, pricing), and creating/editing mentor profiles (subjects, bio, availability, rate). |
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 templates showing pending/accepted requests, upcoming sessions, scheduling inputs, and 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 inputs, check duplicates, 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 PR title clearly and concisely summarizes the main addition: a complete mentorship system with 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
![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:
|
ayesha1145
force-pushed
the
feat/mentorship-system
branch
from
178c312 to
d5b5c49
Compare
github-actions
bot
dismissed
their stale review
All review conversations have been resolved.
Contributor
|
This looks awesome but we are no longer going to be using this codebase, please apply this to the new @alphaonelabs/learn repo |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (14)
web/models.py (1)
3228-3232:⚠️ Potential issue | 🔴 CriticalKeep the active-request uniqueness check in the database.
The view-side
status__in=["pending", "accepted"]guard is still racy: two concurrent submits can both pass the read check and create duplicate active requests for the same mentor/student pair. A conditional unique constraint preserves re-request-after-cancel/decline while closing that TOCTOU gap. This also needs a migration.🔒 Add a partial unique constraint for active requests
class Meta: ordering = ["-created_at"] + constraints = [ + models.UniqueConstraint( + fields=["mentor", "student"], + condition=models.Q(status__in=["pending", "accepted"]), + name="unique_active_mentorship_request", + ) + ]Also applies to: 3236-3237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/models.py` around lines 3228 - 3232, Add a DB-level partial unique constraint to prevent duplicate "active" requests by the same mentor/student: update the model that defines mentor, student, status (the MentorshipRequest model) to include a Meta.constraints entry with a UniqueConstraint on ("mentor", "student") conditioned to statuses in ["pending","accepted"] (use a Q(...) condition referencing the status field and STATUS_CHOICES values), and create the corresponding migration; apply the same change for the other similar model instance referenced at lines 3236-3237 so both models enforce uniqueness at the database level.web/templates/mentorship/schedule_session.html (1)
15-18:⚠️ Potential issue | 🟠 Major
datetime-localis still ambiguous across timezones.This input posts a naive wall time, and the view now calls
make_aware()on that naive value. For users outside the app's default timezone, that silently schedules the wrong hour. Either capture the browser timezone/offset and convert server-side, or label the field with the exact timezone being used and parse it in that same zone.🤖 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 15 - 18, The scheduled_at input currently posts a naive local datetime which is then wrapped with make_aware() server-side causing incorrect hours for users outside the app timezone; fix by capturing the client timezone/offset in the form (e.g., add a hidden field like scheduled_at_tz or scheduled_at_offset populated via JS using Intl.DateTimeFormat().resolvedOptions().timeZone or new Date().getTimezoneOffset()) and send it alongside the scheduled_at value, then update the view that calls make_aware() to parse/convert the posted datetime using the provided timezone/offset (use the timezone string with pytz/zoneinfo or apply the offset before calling make_aware()) so scheduled times are interpreted in the user’s actual timezone.web/templates/mentorship/rate_session.html (1)
17-20:⚠️ Potential issue | 🟡 MinorKeep the chosen score visibly selected.
These radios are visually hidden, and only hover styles change on the visible number/star. After keyboard or mouse selection, nothing remains highlighted, so users cannot confirm what will be submitted.
♿ Preserve checked/focus feedback on the visible controls
- <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"> + <span class="text-2xl text-gray-300 dark:text-gray-600 transition-colors peer-checked:text-yellow-500 peer-focus-visible: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-visible:text-yellow-500" aria-hidden="true"></i>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/rate_session.html` around lines 17 - 20, The radio inputs are visually hidden so selected state isn't visible; add CSS selectors to style the visible elements when the hidden input is checked or focused—use the existing structure (the <input> followed by the <span> and <i>) and add rules for input:checked + span, input:checked + i and input:focus-visible + span, input:focus-visible + i to apply the same highlighted colors/classes as the hover state (e.g., yellow text/color), ensuring keyboard focus and selection remain visibly indicated; optionally add aria-checked updates on change if JS is used for other state handling.web/templates/mentorship/mentor_profile.html (3)
76-76:⚠️ Potential issue | 🟡 MinorAdd keyboard focus styling to the Cancel Request button.
Please add explicit focus classes so tab navigation remains visible.
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_profile.html` at line 76, The Cancel Request button lacks explicit keyboard focus styling; update the button element for "Cancel Request" to include focus classes (e.g., focus:outline-none, focus:ring-2, a red ring color like focus:ring-red-300 and dark:focus:ring-red-600, and focus:ring-offset-2) so tab navigation shows a visible focus state while preserving the existing text and hover classes.
15-19:⚠️ Potential issue | 🟡 MinorProvide meaningful avatar text alternatives.
The image alt is empty, and the fallback icon has no label, so assistive tech loses user context.
As per coding guidelines, "Ensure proper HTML structure and accessibility in templates."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" + > @@ - <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 }} profile placeholder" + >🤖 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 15 - 19, The avatar image and fallback icon lack accessible text; update the img element that references mentor.user.profile.avatar.url to include a descriptive alt like "Avatar of {{ mentor.user.get_full_name }}" (or mentor.user.username) and update the fallback div (the rounded bg-teal-100 / dark:bg-teal-900 element with the <i> icon) to include an accessible label—either an aria-label on the div or an offscreen/screen-reader-only span with the same descriptive text—so assistive technologies convey the mentor's identity.
52-54:⚠️ Potential issue | 🟡 MinorAdd screen-reader text for review ratings.
The star icons are hidden from assistive tech and there’s no textual rating fallback.
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, The star icons in the mentorship template are aria-hidden and lack a textual fallback; update the block that renders the stars (the for loop using "12345" and r.rating with the <i class="fas fa-star"...> / <i class="far fa-star"...> icons) to include an accessible text node for screen readers (e.g., a visually-hidden/span with class "sr-only") that states the numeric rating like "Rating: {{ r.rating }} out of 5" immediately after the icon loop, keeping the icons aria-hidden so only the sr-only text is announced.web/templates/mentorship/my_mentorship.html (2)
22-29:⚠️ Potential issue | 🟠 MajorExpose cancel action for pending mentorship requests.
Pending requests currently cannot be canceled from this page, which leaves students stuck until the mentor responds.
Suggested fix
- <span class="px-2 py-1 text-xs rounded-full + <div class="text-right"> + <span class="px-2 py-1 text-xs rounded-full {% if req.status == 'accepted' %}bg-green-100 text-green-700 dark:bg-green-900 dark:text-green-300 {% elif req.status == 'pending' %}bg-yellow-100 text-yellow-700 dark:bg-yellow-900 dark:text-yellow-300 {% elif req.status == 'declined' %}bg-red-100 text-red-700 dark:bg-red-900 dark:text-red-300 {% else %}bg-gray-100 text-gray-600 dark:bg-gray-700 dark:text-gray-400{% endif %}"> {{ req.get_status_display }} - </span> + </span> + {% if req.status == "pending" %} + <form method="post" action="{% url 'cancel_mentorship_request' req.id %}" class="mt-2"> + {% csrf_token %} + <button type="submit" class="text-sm text-red-600 hover:text-red-700 dark:text-red-400"> + Cancel request + </button> + </form> + {% endif %} + </div>🤖 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 22 - 29, Add a visible cancel action for pending mentorship requests by rendering a form/button next to the status badge when req.status == 'pending'; detect pending via the existing template variable req.status and show a small POST form (including {% csrf_token %}) that submits to the cancel endpoint (e.g. the view name handling cancel, such as mentorship_cancel or mentorship:cancel_request with req.id) or a JS-triggered DELETE/POST call, and ensure the template uses req.get_status_display unchanged while the cancel form/button has clear aria-label and confirmation to prevent accidental cancellations.
60-62:⚠️ Potential issue | 🟡 MinorMake session rating stars accessible and consistently 5-star.
The rating display is visual-only right now and only renders filled stars.
As per coding guidelines, "Ensure proper HTML structure and accessibility in templates."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 class="sr-only">{{ s.rating }} out of 5 stars</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>🤖 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 current star display only renders filled stars and is visual-only; update the loop that uses s.rating so it always renders five star elements (iterate 1..5 using the existing forloop.counter) and output a filled icon when forloop.counter <= s.rating and an outlined/empty icon otherwise; add accessibility by marking each icon aria-hidden="true" and include a single accessible label (e.g., a visually-hidden <span>) or an aria-label on the container like "Rating: X out of 5" using s.rating so screen-readers get the numeric rating.web/templates/mentorship/mentor_dashboard.html (2)
68-92:⚠️ Potential issue | 🟠 MajorRender the completed-session history section (
past_sessions).The dashboard currently stops at upcoming sessions, so mentors don’t get the completed-session history promised in the feature scope.
Suggested fix
<div> <h2 class="text-xl font-semibold text-gray-800 dark:text-white mb-4">Upcoming Sessions</h2> {% if upcoming_sessions %} <div class="space-y-3"> @@ {% else %} <div class="bg-white dark:bg-gray-800 rounded-lg shadow-lg p-8 text-center"> <p class="text-gray-500 dark:text-gray-400 text-sm">No upcoming sessions.</p> </div> {% endif %} + + <h2 class="text-xl font-semibold text-gray-800 dark:text-white mt-6 mb-4">Completed Sessions</h2> + {% if past_sessions %} + <div class="space-y-3"> + {% for s in past_sessions %} + <div class="bg-white dark:bg-gray-800 rounded-lg shadow-lg p-4"> + <div class="flex items-center justify-between mb-1"> + <p class="font-medium text-gray-900 dark:text-white">{{ s.student.get_full_name|default:s.student.username }}</p> + <span class="text-xs text-gray-500 dark:text-gray-400">{{ s.scheduled_at|date:"M d, Y H:i" }}</span> + </div> + <p class="text-xs text-gray-500 dark:text-gray-400"> + {{ s.duration_minutes }} min{% if s.subject %} - {{ s.subject.name }}{% endif %} + </p> + </div> + {% endfor %} + </div> + {% else %} + <div class="bg-white dark:bg-gray-800 rounded-lg shadow-lg p-8 text-center"> + <p class="text-gray-500 dark:text-gray-400 text-sm">No completed sessions.</p> + </div> + {% endif %} </div>🤖 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 68 - 92, The template only renders upcoming_sessions and omits the completed-session history; add a new block after the upcoming_sessions section that checks {% if past_sessions %} and iterates {% for s in past_sessions %} (matching the same structure used for upcoming_sessions) to display s.student.get_full_name|default:s.student.username, s.scheduled_at (formatted), s.duration_minutes, conditional subject name (s.subject.name), and any completed timestamp if desired, and include an {% else %} fallback that shows "No past sessions."; ensure the block uses the same classes/markup as the upcoming sessions div and places the form/buttons removed or replaced with a "View Details" or plain record display as appropriate so mentors can see completed-session history.
42-42:⚠️ Potential issue | 🟡 MinorAdd focus ring styles to the Decline button.
Decline currently has hover styles only; it should mirror Accept’s keyboard focus behavior.
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` at line 42, The Decline button (button name="action" value="decline") lacks keyboard focus styles—update its classes to mirror the Accept button’s hover/focus behavior by adding the same focus ring and offset styles used on Accept (so it receives a visible focus outline on keyboard navigation), include dark-mode focus variants as well, and ensure the transition and accessibility semantics remain consistent with the Accept button.web/views.py (4)
9069-9074:⚠️ Potential issue | 🟠 MajorReject bad duration input instead of silently coercing it to 60 minutes.
This still turns malformed or
<15input into a 60-minute session, and it never rejects values above the 240-minute cap used elsewhere in this flow. A tampered POST can therefore save a different duration than the mentor entered.💡 Suggested fix
try: - duration = int(request.POST.get("duration_minutes", 60)) - if duration < 15: - raise ValueError + duration = int(request.POST.get("duration_minutes", "")) except (ValueError, TypeError): - duration = 60 + messages.error(request, "Please enter a valid session duration.") + return redirect("schedule_session", request_id=request_id) + if duration < 15 or duration > 240: + messages.error(request, "Duration must be between 15 and 240 minutes.") + 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 9069 - 9074, The current parsing silently coerces malformed or out-of-range durations to 60; instead validate and reject bad input by parsing duration = int(request.POST.get("duration_minutes", None)), check that it is between 15 and 240 inclusive, and if parsing fails or the value is outside that range return a validation error (e.g., HTTP 400 / form error) rather than defaulting to 60; update the view logic around the duration variable and any response path that consumes it (the code that currently uses duration after the try/except) to rely on this explicit validation.
9055-9083:⚠️ Potential issue | 🟠 MajorBlock overlapping sessions before creating the booking.
The future-time guard doesn't stop this view from inserting a window that overlaps another
scheduledsession for either participant. That can double-book the mentor or the student while both sessions still look valid everywhere else in the workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9055 - 9083, Before creating the MentorshipSession, reject any booking that would overlap an existing scheduled session for either participant: compute the requested session window using scheduled_at and duration_minutes, then query MentorshipSession for the mentor and for the student for any sessions with overlapping windows (e.g., existing.scheduled_at < requested_end AND (existing.scheduled_at + existing.duration_minutes) > requested_start) and/or filter by the model's scheduled status if present; if any overlap is found, add messages.error and redirect instead of calling MentorshipSession.objects.create. Ensure you perform the overlap check with timezone-aware datetimes and consider wrapping the create in an atomic transaction to avoid race conditions.
8851-8851: 🛠️ Refactor suggestion | 🟠 MajorPlease document and type the new mentorship views.
These endpoints add a full request/scheduling workflow, but the new view entry points are still undocumented and untyped. Short docstrings plus
HttpRequest/HttpResponseannotations would make the flow much easier to follow and maintain.As per coding guidelines
**/*.py:Use type hints in Python where appropriateandAdd docstrings to Python functions, classes, and modules.Also applies to: 8876-8876, 8892-8892, 8926-8926, 8940-8940, 8952-8952, 9002-9002, 9026-9026, 9047-9047, 9092-9092, 9110-9110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` at line 8851, Add concise docstrings and Python type hints to the new mentorship view entry points: update mentor_list and the other new mentorship view functions to accept a django.http.HttpRequest and return a django.http.HttpResponse (or appropriate subclass) and include a short one- or two-sentence docstring describing the endpoint purpose, inputs, and behavior; ensure function signatures use explicit types (e.g., def mentor_list(request: HttpRequest) -> HttpResponse) and document any important side effects or expected request parameters inside the docstring so the scheduling/workflow flow is clear and maintainable.
9098-9104:⚠️ Potential issue | 🟠 MajorOnly allow completion after the booked end time.
The current guard only checks whether the session has started. A 90-minute session can be marked complete a minute after kickoff, which then unlocks rating/review before the appointment 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 timezone.now() < session_end: + messages.error(request, "You can only complete a session after it has finished.") return redirect("mentor_dashboard") session.status = "completed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 9098 - 9104, The guard only checks session.scheduled_at so mentors can mark a long session "completed" right after it starts; change the check to require the current time to be after the session end time by computing end_time = session.scheduled_at + timedelta(minutes=session.duration) (or use session.ends_at / session.scheduled_end if your model already has that field) and only allow completion when timezone.now() >= end_time; keep updating session.status, session.notes and save(update_fields=[...]) as before.
🤖 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 3195-3197: Add server-side validation to enforce hourly_rate >= 0
and the free/paid invariant: implement a model clean() that raises
ValidationError if hourly_rate is negative, if is_free is True but hourly_rate
!= 0, or if is_free is False but hourly_rate == 0 (or set a minimum >0 as
business requires); then ensure save() calls full_clean() (or normalizes values
in save()) so non-form writes can't bypass it. Apply the same clean/save changes
to the other model instance that defines hourly_rate and is_free (the duplicate
at the other block around the same lines).
In `@web/templates/mentorship/become_mentor.html`:
- Around line 19-21: The select for availability doesn't preselect the model
default when mentor is absent; update the selection logic in the availability
<select> (id="availability", name="availability") so it compares each option
value to mentor.availability with a fallback to the model default ("flexible") —
e.g. use the Jinja default filter or an explicit conditional: mark option
selected when (mentor.availability|default('flexible')) == val (or when mentor
is missing and val == "flexible") so new mentor forms preselect the actual
default.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Line 62: The "Schedule" and "Mark Complete" anchor controls lack explicit
keyboard focus-visible styles; update the anchor elements (the ones using {% url
'schedule_session' req.id %} and the "Mark Complete" link) to add accessible
focus classes such as focus:outline-none focus-visible:ring-2
focus-visible:ring-offset-2 focus-visible:ring-blue-500 (and a dark-mode variant
like focus-visible:ring-blue-300) so they show a visible focus ring when tabbed
to; make this same change for both occurrences mentioned in the template.
In `@web/templates/mentorship/my_mentorship.html`:
- Line 7: The action links "Find Mentors" (anchor with {% url 'mentor_list' %})
and the "Rate this session" link lack visible keyboard focus styles; update
these anchor elements to include accessible focus classes (e.g., Tailwind
focus:outline-none, focus:ring-2, focus:ring-offset-2, and focus:ring-blue-500
or your design system's equivalents) so they show a clear focus ring and
maintain existing hover colors, and apply the same focus-class changes to both
the "Find Mentors" and "Rate this session" anchors to satisfy the hover/focus
states guideline.
In `@web/views.py`:
- Around line 8897-8917: Add a DB-level uniqueness constraint for active
requests on the MentorshipRequest model (e.g., a UniqueConstraint on
mentor+student with a condition for status in ["pending","accepted"]) so the
invariant is enforced atomically; keep the existing pre-check in the
request_mentorship view as UX feedback but wrap the creation in a try/except
catching django.db.IntegrityError around MentorshipRequest.objects.create(...)
to show an appropriate messages.error if the DB rejects a duplicate; update
imports (IntegrityError, Q) and migrate after changing the model.
---
Duplicate comments:
In `@web/models.py`:
- Around line 3228-3232: Add a DB-level partial unique constraint to prevent
duplicate "active" requests by the same mentor/student: update the model that
defines mentor, student, status (the MentorshipRequest model) to include a
Meta.constraints entry with a UniqueConstraint on ("mentor", "student")
conditioned to statuses in ["pending","accepted"] (use a Q(...) condition
referencing the status field and STATUS_CHOICES values), and create the
corresponding migration; apply the same change for the other similar model
instance referenced at lines 3236-3237 so both models enforce uniqueness at the
database level.
In `@web/templates/mentorship/mentor_dashboard.html`:
- Around line 68-92: The template only renders upcoming_sessions and omits the
completed-session history; add a new block after the upcoming_sessions section
that checks {% if past_sessions %} and iterates {% for s in past_sessions %}
(matching the same structure used for upcoming_sessions) to display
s.student.get_full_name|default:s.student.username, s.scheduled_at (formatted),
s.duration_minutes, conditional subject name (s.subject.name), and any completed
timestamp if desired, and include an {% else %} fallback that shows "No past
sessions."; ensure the block uses the same classes/markup as the upcoming
sessions div and places the form/buttons removed or replaced with a "View
Details" or plain record display as appropriate so mentors can see
completed-session history.
- Line 42: The Decline button (button name="action" value="decline") lacks
keyboard focus styles—update its classes to mirror the Accept button’s
hover/focus behavior by adding the same focus ring and offset styles used on
Accept (so it receives a visible focus outline on keyboard navigation), include
dark-mode focus variants as well, and ensure the transition and accessibility
semantics remain consistent with the Accept button.
In `@web/templates/mentorship/mentor_profile.html`:
- Line 76: The Cancel Request button lacks explicit keyboard focus styling;
update the button element for "Cancel Request" to include focus classes (e.g.,
focus:outline-none, focus:ring-2, a red ring color like focus:ring-red-300 and
dark:focus:ring-red-600, and focus:ring-offset-2) so tab navigation shows a
visible focus state while preserving the existing text and hover classes.
- Around line 15-19: The avatar image and fallback icon lack accessible text;
update the img element that references mentor.user.profile.avatar.url to include
a descriptive alt like "Avatar of {{ mentor.user.get_full_name }}" (or
mentor.user.username) and update the fallback div (the rounded bg-teal-100 /
dark:bg-teal-900 element with the <i> icon) to include an accessible
label—either an aria-label on the div or an offscreen/screen-reader-only span
with the same descriptive text—so assistive technologies convey the mentor's
identity.
- Around line 52-54: The star icons in the mentorship template are aria-hidden
and lack a textual fallback; update the block that renders the stars (the for
loop using "12345" and r.rating with the <i class="fas fa-star"...> / <i
class="far fa-star"...> icons) to include an accessible text node for screen
readers (e.g., a visually-hidden/span with class "sr-only") that states the
numeric rating like "Rating: {{ r.rating }} out of 5" immediately after the icon
loop, keeping the icons aria-hidden so only the sr-only text is announced.
In `@web/templates/mentorship/my_mentorship.html`:
- Around line 22-29: Add a visible cancel action for pending mentorship requests
by rendering a form/button next to the status badge when req.status ==
'pending'; detect pending via the existing template variable req.status and show
a small POST form (including {% csrf_token %}) that submits to the cancel
endpoint (e.g. the view name handling cancel, such as mentorship_cancel or
mentorship:cancel_request with req.id) or a JS-triggered DELETE/POST call, and
ensure the template uses req.get_status_display unchanged while the cancel
form/button has clear aria-label and confirmation to prevent accidental
cancellations.
- Around line 60-62: The current star display only renders filled stars and is
visual-only; update the loop that uses s.rating so it always renders five star
elements (iterate 1..5 using the existing forloop.counter) and output a filled
icon when forloop.counter <= s.rating and an outlined/empty icon otherwise; add
accessibility by marking each icon aria-hidden="true" and include a single
accessible label (e.g., a visually-hidden <span>) or an aria-label on the
container like "Rating: X out of 5" using s.rating so screen-readers get the
numeric rating.
In `@web/templates/mentorship/rate_session.html`:
- Around line 17-20: The radio inputs are visually hidden so selected state
isn't visible; add CSS selectors to style the visible elements when the hidden
input is checked or focused—use the existing structure (the <input> followed by
the <span> and <i>) and add rules for input:checked + span, input:checked + i
and input:focus-visible + span, input:focus-visible + i to apply the same
highlighted colors/classes as the hover state (e.g., yellow text/color),
ensuring keyboard focus and selection remain visibly indicated; optionally add
aria-checked updates on change if JS is used for other state handling.
In `@web/templates/mentorship/schedule_session.html`:
- Around line 15-18: The scheduled_at input currently posts a naive local
datetime which is then wrapped with make_aware() server-side causing incorrect
hours for users outside the app timezone; fix by capturing the client
timezone/offset in the form (e.g., add a hidden field like scheduled_at_tz or
scheduled_at_offset populated via JS using
Intl.DateTimeFormat().resolvedOptions().timeZone or new
Date().getTimezoneOffset()) and send it alongside the scheduled_at value, then
update the view that calls make_aware() to parse/convert the posted datetime
using the provided timezone/offset (use the timezone string with pytz/zoneinfo
or apply the offset before calling make_aware()) so scheduled times are
interpreted in the user’s actual timezone.
In `@web/views.py`:
- Around line 9069-9074: The current parsing silently coerces malformed or
out-of-range durations to 60; instead validate and reject bad input by parsing
duration = int(request.POST.get("duration_minutes", None)), check that it is
between 15 and 240 inclusive, and if parsing fails or the value is outside that
range return a validation error (e.g., HTTP 400 / form error) rather than
defaulting to 60; update the view logic around the duration variable and any
response path that consumes it (the code that currently uses duration after the
try/except) to rely on this explicit validation.
- Around line 9055-9083: Before creating the MentorshipSession, reject any
booking that would overlap an existing scheduled session for either participant:
compute the requested session window using scheduled_at and duration_minutes,
then query MentorshipSession for the mentor and for the student for any sessions
with overlapping windows (e.g., existing.scheduled_at < requested_end AND
(existing.scheduled_at + existing.duration_minutes) > requested_start) and/or
filter by the model's scheduled status if present; if any overlap is found, add
messages.error and redirect instead of calling MentorshipSession.objects.create.
Ensure you perform the overlap check with timezone-aware datetimes and consider
wrapping the create in an atomic transaction to avoid race conditions.
- Line 8851: Add concise docstrings and Python type hints to the new mentorship
view entry points: update mentor_list and the other new mentorship view
functions to accept a django.http.HttpRequest and return a
django.http.HttpResponse (or appropriate subclass) and include a short one- or
two-sentence docstring describing the endpoint purpose, inputs, and behavior;
ensure function signatures use explicit types (e.g., def mentor_list(request:
HttpRequest) -> HttpResponse) and document any important side effects or
expected request parameters inside the docstring so the scheduling/workflow flow
is clear and maintainable.
- Around line 9098-9104: The guard only checks session.scheduled_at so mentors
can mark a long session "completed" right after it starts; change the check to
require the current time to be after the session end time by computing end_time
= session.scheduled_at + timedelta(minutes=session.duration) (or use
session.ends_at / session.scheduled_end if your model already has that field)
and only allow completion when timezone.now() >= end_time; keep updating
session.status, session.notes and save(update_fields=[...]) as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ee595470-4d20-49f8-8c6c-3efa6753ed35
⛔ 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
Comment on lines
+3195
to
+3197
| hourly_rate = models.DecimalField(max_digits=8, decimal_places=2, default=0.00) | ||
| is_free = models.BooleanField(default=True) | ||
| availability = models.CharField(max_length=20, choices=AVAILABILITY_CHOICES, default="flexible") |
Contributor
There was a problem hiding this comment.
Validate hourly_rate server-side and keep it consistent with is_free.
Right now non-form writes can store a negative rate, or persist a paid mentor at 0 / a free mentor with a stale non-zero rate. That breaks the free-vs-paid filter and the price shown in the mentorship UI.
💡 One way to enforce the pricing invariant
- hourly_rate = models.DecimalField(max_digits=8, decimal_places=2, default=0.00)
+ hourly_rate = models.DecimalField(
+ max_digits=8,
+ decimal_places=2,
+ default=0,
+ validators=[MinValueValidator(0)],
+ )
@@
class Meta:
ordering = ["-created_at"]
+ constraints = [
+ models.CheckConstraint(
+ check=(
+ models.Q(is_free=True, hourly_rate=0)
+ | models.Q(is_free=False, hourly_rate__gt=0)
+ ),
+ name="mentor_profile_rate_matches_free_flag",
+ )
+ ]Also applies to: 3202-3203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/models.py` around lines 3195 - 3197, Add server-side validation to
enforce hourly_rate >= 0 and the free/paid invariant: implement a model clean()
that raises ValidationError if hourly_rate is negative, if is_free is True but
hourly_rate != 0, or if is_free is False but hourly_rate == 0 (or set a minimum
>0 as business requires); then ensure save() calls full_clean() (or normalizes
values in save()) so non-form writes can't bypass it. Apply the same clean/save
changes to the other model instance that defines hourly_rate and is_free (the
duplicate at the other block around the same lines).
Comment on lines
+3257
to
+3262
| status = models.CharField(max_length=10, choices=STATUS_CHOICES, default="scheduled") | ||
| notes = models.TextField(blank=True) | ||
| rating = models.PositiveIntegerField( | ||
| null=True, blank=True, validators=[MinValueValidator(1), MaxValueValidator(5)] | ||
| ) | ||
| review = models.TextField(blank=True) |
Contributor
There was a problem hiding this comment.
Only completed sessions should be allowed to carry ratings/reviews.
Nothing here stops admin or other direct writes from attaching rating / review data to scheduled or cancelled sessions. Those rows would then feed MentorProfile.average_rating even though the session never finished.
🧩 Add a data-integrity constraint
class Meta:
ordering = ["-scheduled_at"]
+ constraints = [
+ models.CheckConstraint(
+ check=(
+ models.Q(status="completed")
+ | (models.Q(rating__isnull=True) & models.Q(review=""))
+ ),
+ name="mentorship_session_rating_requires_completed",
+ )
+ ]Also applies to: 3266-3267
Comment on lines
+19
to
+21
| <label for="availability" class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">Availability</label> | ||
| <select id="availability" name="availability" class="w-full px-4 py-2 border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-700 text-gray-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-blue-500"> | ||
| {% for val, label in availability_choices %}<option value="{{ val }}" {% if mentor.availability == val %}selected{% endif %}>{{ label }}</option>{% endfor %} |
Contributor
There was a problem hiding this comment.
New mentor profiles default to the wrong availability.
When mentor is absent, no option is marked selected, so the browser submits the first value (weekdays). The model default is "flexible", so a first-time submit stores different data than the model/API default.
🎯 Preselect the actual default on create
- {% for val, label in availability_choices %}<option value="{{ val }}" {% if mentor.availability == val %}selected{% endif %}>{{ label }}</option>{% endfor %}
+ {% for val, label in availability_choices %}
+ <option
+ value="{{ val }}"
+ {% if mentor and mentor.availability == val %}
+ selected
+ {% elif not mentor and val == "flexible" %}
+ selected
+ {% endif %}
+ >
+ {{ label }}
+ </option>
+ {% endfor %}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <label for="availability" class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">Availability</label> | |
| <select id="availability" name="availability" class="w-full px-4 py-2 border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-700 text-gray-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-blue-500"> | |
| {% for val, label in availability_choices %}<option value="{{ val }}" {% if mentor.availability == val %}selected{% endif %}>{{ label }}</option>{% endfor %} | |
| <label for="availability" class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">Availability</label> | |
| <select id="availability" name="availability" class="w-full px-4 py-2 border border-gray-300 dark:border-gray-600 rounded-lg bg-white dark:bg-gray-700 text-gray-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-blue-500"> | |
| {% for val, label in availability_choices %} | |
| <option | |
| value="{{ val }}" | |
| {% if mentor and mentor.availability == val %} | |
| selected | |
| {% elif not mentor and val == "flexible" %} | |
| selected | |
| {% endif %} | |
| > | |
| {{ label }} | |
| </option> | |
| {% endfor %} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/templates/mentorship/become_mentor.html` around lines 19 - 21, The select
for availability doesn't preselect the model default when mentor is absent;
update the selection logic in the availability <select> (id="availability",
name="availability") so it compares each option value to mentor.availability
with a fallback to the model default ("flexible") — e.g. use the Jinja default
filter or an explicit conditional: mark option selected when
(mentor.availability|default('flexible')) == val (or when mentor is missing and
val == "flexible") so new mentor forms preselect the actual default.
| <p class="font-medium text-gray-900 dark:text-white">{{ req.student.get_full_name|default:req.student.username }}</p> | ||
| {% if req.subject %}<p class="text-xs text-gray-500 dark:text-gray-400">{{ req.subject.name }}</p>{% endif %} | ||
| </div> | ||
| <a href="{% url 'schedule_session' req.id %}" class="text-sm text-blue-600 hover:text-blue-800 dark:text-blue-400 font-medium">Schedule</a> |
Contributor
There was a problem hiding this comment.
Add keyboard focus styles to Schedule and Mark Complete controls.
Both controls provide hover feedback but no explicit focus-visible styling.
As per coding guidelines, "Add hover/focus states for interactive elements."
Also applies to: 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/templates/mentorship/mentor_dashboard.html` at line 62, The "Schedule"
and "Mark Complete" anchor controls lack explicit keyboard focus-visible styles;
update the anchor elements (the ones using {% url 'schedule_session' req.id %}
and the "Mark Complete" link) to add accessible focus classes such as
focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2
focus-visible:ring-blue-500 (and a dark-mode variant like
focus-visible:ring-blue-300) so they show a visible focus ring when tabbed to;
make this same change for both occurrences mentioned in the template.
| <div class="container mx-auto px-4 py-8"> | ||
| <div class="flex items-center justify-between mb-6"> | ||
| <h1 class="text-3xl font-bold text-gray-900 dark:text-white">My Mentorship</h1> | ||
| <a href="{% url 'mentor_list' %}" class="text-blue-600 hover:text-blue-800 dark:text-blue-400 text-sm">Find Mentors</a> |
Contributor
There was a problem hiding this comment.
Add visible keyboard focus styles to action links.
Find Mentors and Rate this session have hover states but no explicit focus styling.
As per coding guidelines, "Add hover/focus states for interactive elements."
Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/templates/mentorship/my_mentorship.html` at line 7, The action links
"Find Mentors" (anchor with {% url 'mentor_list' %}) and the "Rate this session"
link lack visible keyboard focus styles; update these anchor elements to include
accessible focus classes (e.g., Tailwind focus:outline-none, focus:ring-2,
focus:ring-offset-2, and focus:ring-blue-500 or your design system's
equivalents) so they show a clear focus ring and maintain existing hover colors,
and apply the same focus-class changes to both the "Find Mentors" and "Rate this
session" anchors to satisfy the hover/focus states guideline.
Comment on lines
+8897
to
+8917
| existing = MentorshipRequest.objects.filter( | ||
| mentor=mentor, student=request.user, status__in=["pending", "accepted"] | ||
| ).first() | ||
| if existing: | ||
| messages.error(request, "You have already sent a request to this mentor.") | ||
| return redirect("mentor_profile_view", mentor_id=mentor_id) | ||
| if request.method == "POST": | ||
| 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) | ||
| MentorshipRequest.objects.create( | ||
| mentor=mentor, student=request.user, subject=subject, message=message | ||
| ) |
Contributor
There was a problem hiding this comment.
Make duplicate-request protection atomic.
A fast double-submit can still create two active requests here. existing = ...first() and MentorshipRequest.objects.create(...) are separate operations, so once historical rows became legal this view stopped enforcing the invariant by itself. Please move active-request uniqueness to the model/database layer and keep this check as UX feedback only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/views.py` around lines 8897 - 8917, Add a DB-level uniqueness constraint
for active requests on the MentorshipRequest model (e.g., a UniqueConstraint on
mentor+student with a condition for status in ["pending","accepted"]) so the
invariant is enforced atomically; keep the existing pre-check in the
request_mentorship view as UX feedback but wrap the creation in a try/except
catching django.db.IntegrityError around MentorshipRequest.objects.create(...)
to show an appropriate messages.error if the DB rejects a duplicate; update
imports (IntegrityError, Q) and migrate after changing the model.
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 / user experience
Known limitations / review notes