Fix anonymous user crash in learn form#1017
Fix anonymous user crash in learn form#1017kritikamangal4 wants to merge 2 commits intoalphaonelabs:mainfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Waiting Room Creator Assignment web/views.py |
Modified learn() to set waiting_room.creator only if request.user.is_authenticated, removing unconditional assignment for anonymous requests. |
Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~3 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% 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 title directly aligns with the changeset: it identifies fixing a crash for anonymous users in the learn form, which matches the code change that now conditionally sets the creator only for authenticated users. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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.
👀 Peer Review RequiredHi @kritikamangal4! 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! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/views.py (1)
1209-1218:⚠️ Potential issue | 🟠 MajorHandle
creator=Noneall the way through the waiting-room flow.This avoids assigning
AnonymousUser, but it also creates waiting rooms with no creator.web/notifications.py:195-202still sends a creator notification unconditionally, so anonymous submissions can still fail later when the room is fulfilled. Please harden that path before shipping this hotfix.
🤖 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/views.py`:
- Around line 1209-1210: Fix the indentation so the assignment sits inside the
if block (ensure "waiting_room.creator = request.user" is indented under the "if
request.user.is_authenticated:" check) and then make notification logic
null-safe: in notify_waiting_room_fulfilled (and any call sites using
waiting_room.creator) add a guard to skip send_notification if
waiting_room.creator is None (e.g., only call
send_notification(waiting_room.creator, ...) when waiting_room.creator is not
None and not in waiting_room.participants.all()); alternatively decide to reject
anonymous waiting room creation in the same view by returning an error when
request.user is not authenticated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cf109392-3f2b-4e8d-bbfa-4398dda4aec1
📒 Files selected for processing (1)
web/views.py
web/views.py
Outdated
| if request.user.is_authenticated: | ||
| waiting_room.creator = request.user |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
path = Path("web/views.py")
source = path.read_text()
try:
compile(source, str(path), "exec")
print("Syntax OK")
except SyntaxError as exc:
print(f"SyntaxError: {exc.msg} at line {exc.lineno}, column {exc.offset}")
start = max(1, exc.lineno - 2)
end = exc.lineno + 2
for no, line in enumerate(source.splitlines(), start=1):
if start <= no <= end:
prefix = ">>" if no == exc.lineno else " "
print(f"{prefix} {no}: {line}")
PYRepository: alphaonelabs/website
Length of output: 425
🏁 Script executed:
# Check web/notifications.py for the notify_waiting_room_fulfilled function
fd -t f "notifications.py" | xargs head -n 250 | tail -n 100Repository: alphaonelabs/website
Length of output: 3565
🏁 Script executed:
# Search for usages of waiting_room.creator throughout the codebase
rg "waiting_room\.creator" --type py -B 2 -A 2Repository: alphaonelabs/website
Length of output: 1436
Fix the indentation and handle anonymous waiting room creators.
The code has two issues:
-
Syntax error at Line 1210: The assignment is not indented under the
ifblock, preventing the file from importing. -
Null-handling in notifications: If an anonymous user creates a waiting room,
waiting_room.creatorwill beNone. However,notify_waiting_room_fulfilled()inweb/notifications.pylater assumeswaiting_room.creatorexists when executing:if waiting_room.creator not in waiting_room.participants.all(): send_notification(waiting_room.creator, notification_data)
This will raise an error for anonymous waiting rooms.
Syntax fix
if request.user.is_authenticated:
- waiting_room.creator = request.user
+ waiting_room.creator = request.userConsider whether anonymous users should create waiting rooms at all, or add a null check in the notification logic before accessing waiting_room.creator.
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 1209-1210: Expected an indented block after if statement
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/views.py` around lines 1209 - 1210, Fix the indentation so the assignment
sits inside the if block (ensure "waiting_room.creator = request.user" is
indented under the "if request.user.is_authenticated:" check) and then make
notification logic null-safe: in notify_waiting_room_fulfilled (and any call
sites using waiting_room.creator) add a guard to skip send_notification if
waiting_room.creator is None (e.g., only call
send_notification(waiting_room.creator, ...) when waiting_room.creator is not
None and not in waiting_room.participants.all()); alternatively decide to reject
anonymous waiting room creation in the same view by returning an error when
request.user is not authenticated.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/views.py (1)
1209-1210:⚠️ Potential issue | 🟠 MajorGuard all
creatordereferences now that this can beNone.This avoids assigning
AnonymousUser, but it also means anonymous rooms now persist withcreator=None. Any fulfillment/notification path that still does membership checks or sends directly towaiting_room.creatorcan now fail later instead of here.Verify the remaining creator-dependent paths before merging. Expect every dereference/send path to guard
waiting_room.creatorfirst.#!/bin/bash rg -n -C3 --type=py '\bwaiting_room\.creator\b' web🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 1209 - 1210, The assignment now allows waiting_room.creator to be None (when request.user is not authenticated), so audit and guard every use of waiting_room.creator to avoid AttributeError / sending to None/AnonymousUser: locate all references to waiting_room.creator (search for "waiting_room.creator") and update code paths that send/notify/check membership (e.g., any calls like waiting_room.creator.email, .send, .notify, .is_member, or permission checks) to first check if waiting_room.creator is not None and authenticated before dereferencing; if None, route to the anonymous/fallback flow (skip send, use room-level broadcast, or log and enqueue for later) and add/adjust unit tests to cover both creator=None and creator=User cases. Ensure any template contexts or serializers that access waiting_room.creator also handle None safely.
🤖 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/views.py`:
- Around line 1209-1210: waiting_room_detail() currently adds 1 unconditionally
for the creator which inflates counts for anonymous rooms; update the logic that
computes participant/occupancy counts (the code path around waiting_room_detail
and the waiting_room.creator field set by the view where you see "if
request.user.is_authenticated: waiting_room.creator = request.user") to only
include the creator in any +1 adjustment when waiting_room.creator is not None
(or request.user.is_authenticated is True), and ensure any display/count methods
that reference waiting_room.creator perform a null check before incrementing.
---
Duplicate comments:
In `@web/views.py`:
- Around line 1209-1210: The assignment now allows waiting_room.creator to be
None (when request.user is not authenticated), so audit and guard every use of
waiting_room.creator to avoid AttributeError / sending to None/AnonymousUser:
locate all references to waiting_room.creator (search for
"waiting_room.creator") and update code paths that send/notify/check membership
(e.g., any calls like waiting_room.creator.email, .send, .notify, .is_member, or
permission checks) to first check if waiting_room.creator is not None and
authenticated before dereferencing; if None, route to the anonymous/fallback
flow (skip send, use room-level broadcast, or log and enqueue for later) and
add/adjust unit tests to cover both creator=None and creator=User cases. Ensure
any template contexts or serializers that access waiting_room.creator also
handle None safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5c648ec-4d83-4e3a-82d5-8ca53604bd94
📒 Files selected for processing (1)
web/views.py
| if request.user.is_authenticated: | ||
| waiting_room.creator = request.user |
There was a problem hiding this comment.
Adjust creator-inclusive counts for anonymous rooms.
Making creator optional also breaks waiting_room_detail(): it currently adds 1 unconditionally for the creator, so anonymous rooms will show an inflated participant count.
Suggested follow-up
- "participant_count": waiting_room.participants.count() + 1, # Add 1 to include the creator
+ "participant_count": waiting_room.participants.count()
+ + (1 if waiting_room.creator_id else 0), # Include creator only when present🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/views.py` around lines 1209 - 1210, waiting_room_detail() currently adds
1 unconditionally for the creator which inflates counts for anonymous rooms;
update the logic that computes participant/occupancy counts (the code path
around waiting_room_detail and the waiting_room.creator field set by the view
where you see "if request.user.is_authenticated: waiting_room.creator =
request.user") to only include the creator in any +1 adjustment when
waiting_room.creator is not None (or request.user.is_authenticated is True), and
ensure any display/count methods that reference waiting_room.creator perform a
null check before incrementing.
|
Hi maintainers 👋 |
|
@kritikamangal4 please go through the coderabbit comment |
2 participants
Related issues
Fixes #<ISSUE_NUMBER>
Checklist
Summary
This PR fixes a crash occurring when anonymous users submit the learn form to create a waiting room.
Changes
In web/views.py, the learn(request) flow now assigns waiting_room.creator only when the requester is authenticated:
Before:
waiting_room.creator = request.user
After:
if request.user.is_authenticated:
waiting_room.creator = request.user
This prevents assigning an AnonymousUser to the creator field.
Impact
Testing Notes
The PR description includes the repository template checklist but no completed items; pre-commit checks and manual tests have not been confirmed in the PR.