Feat/forum voting#505
Conversation
WalkthroughThe pull request introduces a new voting functionality for forum topics and replies. It adds a migration that creates a database model ( Changes
Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
web/migrations/0056_forumvote.py(1 hunks)web/models.py(3 hunks)web/templates/base.html(1 hunks)web/templates/web/forum/category.html(1 hunks)web/templates/web/forum/topic.html(7 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
web/urls.py (1)
web/views.py (2)
topic_vote(7134-7169)reply_vote(7173-7209)
web/views.py (1)
web/models.py (6)
ForumVote(2867-2899)ForumReply(832-870)delete(467-473)user_vote(815-823)user_vote(855-863)ForumTopic(789-829)
🪛 Ruff (0.8.2)
web/models.py
803-803: Missing return type annotation for public function upvote_count
(ANN201)
807-807: Missing return type annotation for public function downvote_count
(ANN201)
811-811: Missing return type annotation for public function vote_score
(ANN201)
815-815: Missing return type annotation for public function user_vote
(ANN201)
815-815: Missing type annotation for function argument user
(ANN001)
821-821: Consider moving this statement to an else block
(TRY300)
843-843: Missing return type annotation for public function upvote_count
(ANN201)
847-847: Missing return type annotation for public function downvote_count
(ANN201)
851-851: Missing return type annotation for public function vote_score
(ANN201)
855-855: Missing return type annotation for public function user_vote
(ANN201)
855-855: Missing type annotation for function argument user
(ANN001)
861-861: Consider moving this statement to an else block
(TRY300)
2870-2873: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2883-2883: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2884-2892: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2891-2891: Trailing comma missing
Add trailing comma
(COM812)
2894-2894: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
2897-2897: Unnecessary elif after return statement
Remove unnecessary elif
(RET505)
web/migrations/0056_forumvote.py
10-13: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
15-66: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
61-61: Trailing comma missing
Add trailing comma
(COM812)
web/views.py
2899-2899: Unnecessary else after return statement
Remove unnecessary else
(RET505)
7134-7134: Missing return type annotation for public function topic_vote
(ANN201)
7134-7134: Missing type annotation for function argument request
(ANN001)
7134-7134: Missing type annotation for function argument pk
(ANN001)
7150-7150: Trailing comma missing
Add trailing comma
(COM812)
7173-7173: Missing return type annotation for public function reply_vote
(ANN201)
7173-7173: Missing type annotation for function argument request
(ANN001)
7173-7173: Missing type annotation for function argument pk
(ANN001)
7188-7188: Trailing comma missing
Add trailing comma
(COM812)
7208-7208: try-except block with duplicate exception ForumReply.DoesNotExist
(B025)
7212-7212: Missing return type annotation for public function topic_detail
(ANN201)
7212-7212: Missing type annotation for function argument request
(ANN001)
7212-7212: Missing type annotation for function argument pk
(ANN001)
7228-7228: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (7)
web/templates/web/forum/category.html (1)
138-148: Good implementation of voting metrics display!The voting metrics display is well-integrated into the topic information section using proper semantic HTML and consistent styling with the rest of the UI.
web/urls.py (1)
218-219: Well-designed URL patterns for voting functionality.The new URL patterns for forum voting follow consistent naming conventions and properly use RESTful design principles for the voting actions.
web/templates/base.html (1)
154-154: Good practice for exposing authentication state to client-side code.Adding the
user-authenticatedclass to the body element is a clean way to make authentication state available to JavaScript without exposing sensitive information.web/templates/web/forum/topic.html (3)
6-7: Appropriate template imports for the new functionality.The addition of the
dict_filterslibrary and static files import properly supports the voting feature implementation.
134-210: Excellent voting UI implementation with strong accessibility.The topic voting UI is well-structured with proper accessibility attributes (aria-label, aria-pressed), clear visual feedback, and separation of concerns. The SVG icons and color-coded vote score provide excellent visual cues.
299-321: Well-designed vote statistics for replies.The vote statistics display for replies is appropriately sized and styled, maintaining visual consistency with the topic statistics while being properly scaled for reply elements.
web/migrations/0056_forumvote.py (1)
10-66: False positive about mutable class attributes.The static analysis tool flags the class attributes (e.g.,
dependenciesoroperations) as mutable and suggests usingtyping.ClassVar. In Django migrations, these attributes are conventionally defined as is and pose no real issue. You may safely ignore this warning unless your project guidelines specifically require type annotations for migrations.🧰 Tools
🪛 Ruff (0.8.2)
10-13: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
15-66: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
61-61: Trailing comma missing
Add trailing comma
(COM812)
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
web/views.py (1)
7209-7237: 🧹 Nitpick (assertive)
topic_detailchanges: add missing type hints and handle trailing comma.
- Missing annotations:
def topic_detail(request: HttpRequest, pk: int) -> HttpResponse: ...- Trailing comma: The dict at line 7228 could include a trailing comma after the last key-value pair if following project style.
- These changes correctly retrieve
user_topic_voteanduser_reply_voteswith minimal overhead. Good job ensuring the code checksrequest.user.is_authenticatedfirst.🧰 Tools
🪛 Ruff (0.8.2)
7212-7212: Missing return type annotation for public function
topic_detail(ANN201)
7212-7212: Missing type annotation for function argument
request(ANN001)
7212-7212: Missing type annotation for function argument
pk(ANN001)
7228-7228: Trailing comma missing
Add trailing comma
(COM812)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/templates/web/forum/topic.html(7 hunks)web/views.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (1)
web/models.py (6)
ForumVote(2867-2899)ForumReply(832-870)delete(467-473)user_vote(815-823)user_vote(855-863)ForumTopic(789-829)
🪛 Ruff (0.8.2)
web/views.py
2899-2899: Unnecessary else after return statement
Remove unnecessary else
(RET505)
7134-7134: Missing return type annotation for public function topic_vote
(ANN201)
7134-7134: Missing type annotation for function argument request
(ANN001)
7134-7134: Missing type annotation for function argument pk
(ANN001)
7150-7150: Trailing comma missing
Add trailing comma
(COM812)
7173-7173: Missing return type annotation for public function reply_vote
(ANN201)
7173-7173: Missing type annotation for function argument request
(ANN001)
7173-7173: Missing type annotation for function argument pk
(ANN001)
7188-7188: Trailing comma missing
Add trailing comma
(COM812)
7208-7208: try-except block with duplicate exception ForumReply.DoesNotExist
(B025)
7212-7212: Missing return type annotation for public function topic_detail
(ANN201)
7212-7212: Missing type annotation for function argument request
(ANN001)
7212-7212: Missing type annotation for function argument pk
(ANN001)
7228-7228: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (12)
web/templates/web/forum/topic.html (10)
6-8: Load Template Filters and Static ResourcesThe addition of
{% load dict_filters %}and{% load static %}ensures that custom template filters (e.g., for dictionary lookups) and static file handling are available. This is essential for the subsequent use of filters likeget_itemin the template.
134-152: Accessible Topic Upvote ButtonThe upvote button for topics now includes descriptive attributes:
aria-label="Upvote this topic"clarifies the purpose for screen readers.aria-pressed="{% if user_topic_vote == 'up' %}true{% else %}false{% endif %}"dynamically reflects the vote state.The SVG icon styling conditionally applies the
text-teal-500class when active. This approach improves both accessibility and visual feedback.
153-171: Consistent Vote Score Display and Downvote ButtonThe vote score is presented with conditional styling based on its value, and the downvote button mirrors the accessibility features of the upvote button. Using clear, conditional classes (e.g.,
text-red-500when the user has downvoted) ensures a cohesive user experience.
176-210: Clear Voting Statistics and Conditional Delete OptionThe block displaying vote statistics (upvotes and downvotes) and the conditional delete form for the topic is well-organized. The separation of concerns—displaying vote counts and offering deletion only to the author—improves both clarity and security.
223-257: Enhanced Accessibility for Reply VotingThe reply voting buttons are updated to include
aria-labelandaria-pressedattributes. This, together with the use of the customget_itemfilter to determine the vote state, significantly improves accessibility and consistency with the topic voting buttons.
303-325: Clear Presentation of Reply Vote StatisticsThe section that displays reply vote counts uses concise SVG icons and a clear layout. Conditional styling on vote counts (using classes like
text-teal-500for positive scores) ensures that users receive immediate visual feedback on the reply’s popularity.
375-383: Efficient CSRF Token ExtractionThe
getCsrfTokenfunction is succinct and reliably retrieves the CSRF token from the document. This encapsulation improves maintainability for secure AJAX operations.
385-424: Robust AJAX Vote HandlingThe
handleVotefunction efficiently manages vote submissions by:
- Preventing the default event action.
- Checking if the user is authenticated (and redirecting if not).
- Determining the correct URL dynamically based on whether the vote is for a topic or a reply.
- Initiating a fetch POST request with proper headers and body data.
The promise chain handles the response by updating the UI via
updateVoteUI. This refactoring removes redundant code and enhances the overall user experience.
430-458: Dynamic UI Update After VotingThe
updateVoteUIfunction effectively updates the vote score and adjusts button highlights based on the user's vote. The targeted DOM queries (e.g., selecting the vote score element and the associated SVGs) ensure that only the necessary parts of the UI are updated. This approach minimizes layout thrashing and maintains a responsive interface.
460-463: Efficient Event Binding for Vote ButtonsAttaching the
handleVoteevent listener to all elements with the.vote-btnclass is a clean and efficient approach. This ensures consistent behavior across both topic and reply voting without duplicating code.web/views.py (2)
131-131: Added import forForumVote: Looks good.The addition of
ForumVotein the import list appears correct and needed for the voting features below. No issues here.
1843-1887: POST handling for replies, voting, and deletion: Implementation approved.All conditionals and permissions checks look consistent with the existing forum logic. The checks for
reply.author == request.userandtopic.author == request.userproperly control deletion actions. The approach of retrieving user votes later in the code (user_topic_vote,user_reply_votes) is cleanly separated from POST actions. Overall, these changes align well with existing patterns.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/migrations/0056_forumvote.py (1)
53-62: Consider adding a trailing comma for consistency.According to the static analysis hint (COM812), adding a trailing comma after the
name="vote_topic_xor_reply"argument in theCheckConstraintcan improve consistency. This change is purely stylistic and does not affect functionality.models.CheckConstraint( condition=models.Q( models.Q(("reply__isnull", True), ("topic__isnull", False)), models.Q(("reply__isnull", False), ("topic__isnull", True)), _connector="OR", ), - name="vote_topic_xor_reply" + name="vote_topic_xor_reply", )🧰 Tools
🪛 Ruff (0.8.2)
61-61: Trailing comma missing
Add trailing comma
(COM812)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/migrations/0056_forumvote.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/migrations/0056_forumvote.py
10-13: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
15-66: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
61-61: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run Tests
🔇 Additional comments (1)
web/migrations/0056_forumvote.py (1)
16-65: The ForumVote model design looks good.The ForumVote model implementation correctly:
- Uses appropriate field types and relationships
- Enforces constraints to ensure a vote belongs to either a topic OR a reply (not both)
- Prevents duplicate votes from the same user with unique_together constraints
- Tracks creation and update timestamps automatically
This provides a solid foundation for the forum voting feature.
🧰 Tools
🪛 Ruff (0.8.2)
61-61: Trailing comma missing
Add trailing comma
(COM812)
|
@coderabbitai can u assess why the docker build and run is failing and how to fix it ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7458 @Inshamhaque To diagnose why your Docker build is failing, I need to examine your .env.sample file and any error logs you're receiving. However, based on the Dockerfile analysis, I can identify several likely issues:
To fix these issues, I recommend:
Would you like me to help create a proper docker-compose.yml file and startup script to address these issues? |
|
@coderabbitai examine the .env.sample and here are the logs from docker build and run tests kindly give the possible fixes and errors |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3651 @Inshamhaque I've examined your Docker configuration and identified several issues that are likely causing the "No such container: education-app" error. Here's what's happening and how to fix it: Root Issues
Solution Steps
&&
Let me know if you'd like me to elaborate on any of these solutions or if you need help implementing them! |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 23819 @Inshamhaque Based on the files I've examined, I can identify why you're getting the "No such container: education-app" error. Here are the problems and solutions: Issues Identified
Solutions1. Create a docker-compose.yml file:version: '3.8'
services:
web:
build: .
container_name: education-app
ports:
- "8000:8000"
environment:
- DATABASE_URL=sqlite:///db.sqlite3
- DEBUG=True
volumes:
- ./:/app
command:
>
bash -c "python manage.py migrate &&
python manage.py create_test_data &&
python manage.py createsuperuser --noinput &&
python manage.py runserver 0.0.0.0:8000"2. Modify your Dockerfile:# Python base image
FROM python:3.10-slim
# Set working directory
WORKDIR /app
# Install dependencies
RUN apt-get update && apt-get install -y \
curl \
&& rm -rf /var/lib/apt/lists/*
# Copy requirements and install Python dependencies
COPY requirements.txt .
RUN python -m pip install --upgrade pip && \
python -m pip install -r requirements.txt
# Copy project files
COPY . .
# Create necessary directories for static files
RUN mkdir -p /app/static /app/staticfiles
# Create and configure environment variables
COPY .env.sample .env
# Collect static files
RUN python manage.py collectstatic --noinput
# Set superuser credentials as environment variables
ENV DJANGO_SUPERUSER_USERNAME=admin
ENV DJANGO_SUPERUSER_EMAIL=admin@example.com
ENV DJANGO_SUPERUSER_PASSWORD=adminpassword
# Expose port
EXPOSE 8000
# Start the server (migrations will be run by docker-compose)
CMD ["python", "manage.py", "runserver", "0.0.0.0:8000"]3. Build and run commands:# Build and start containers
docker-compose up --build
# If you want to run without docker-compose:
docker build -t education-website:latest .
docker run -p 8000:8000 --name education-app education-website:latest4. For running commands inside the container:# Make sure to use the correct container name
docker exec -it education-app python manage.py shellThis approach fixes the issue by:
Would you like me to explain any part of this solution in more detail? |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (13)
web/migrations/0056_forumvote.py (2)
1-1: 🧹 Nitpick (assertive)Remove “test” suffix from the migration comment.
This appears to be a debugging artifact. For consistency, clean up the auto-generated heading to avoid confusion in production migrations.-# Generated by Django 5.1.6 on 2025-04-08 09:41 test +# Generated by Django 5.1.6 on 2025-04-08 09:41
53-62: 🧹 Nitpick (assertive)Add a trailing comma to comply with style checks.
The static analysis hint (COM812) recommends adding a trailing comma in the check constraint for cleaner diffs and style consistency.name="vote_topic_xor_reply" +name="vote_topic_xor_reply",🧰 Tools
🪛 Ruff (0.8.2)
61-61: Trailing comma missing
Add trailing comma
(COM812)
web/models.py (4)
802-824: Add type annotations toForumTopicmethods for clarity.The voting methods look functionally correct, but would benefit from type annotations to improve code readability and maintainability. Also, consider using an
elseblock in the exception handling for better structure.-def upvote_count(self): +def upvote_count(self) -> int: """Return the number of upvotes for this topic.""" return self.votes.filter(vote_type="up").count() -def downvote_count(self): +def downvote_count(self) -> int: """Return the number of downvotes for this topic.""" return self.votes.filter(vote_type="down").count() -def vote_score(self): +def vote_score(self) -> int: """Return the total vote score (upvotes - downvotes).""" return self.upvote_count() - self.downvote_count() -def user_vote(self, user): +def user_vote(self, user: User) -> Optional[str]: """Return the user's vote type for this topic, or None if not voted.""" if not user.is_authenticated: return None try: vote = self.votes.get(user=user) return vote.vote_type except ForumVote.DoesNotExist: - return None + pass + return None🧰 Tools
🪛 Ruff (0.8.2)
803-803: Missing return type annotation for public function
upvote_count(ANN201)
807-807: Missing return type annotation for public function
downvote_count(ANN201)
811-811: Missing return type annotation for public function
vote_score(ANN201)
815-815: Missing return type annotation for public function
user_vote(ANN201)
815-815: Missing type annotation for function argument
user(ANN001)
821-821: Consider moving this statement to an
elseblock(TRY300)
842-864: Add type annotations toForumReplymethods for consistency.Similarly, adding type hints (e.g., return types for
upvote_count,downvote_count,vote_score, etc.) can help maintain consistency and clarity across bothForumTopicandForumReplyclasses.-def upvote_count(self): +def upvote_count(self) -> int: """Return the number of upvotes for this reply.""" return self.votes.filter(vote_type="up").count() -def downvote_count(self): +def downvote_count(self) -> int: """Return the number of downvotes for this reply.""" return self.votes.filter(vote_type="down").count() -def vote_score(self): +def vote_score(self) -> int: """Return the total vote score (upvotes - downvotes).""" return self.upvote_count() - self.downvote_count() -def user_vote(self, user): +def user_vote(self, user: User) -> Optional[str]: """Return the user's vote type for this reply, or None if not voted.""" if not user.is_authenticated: return None try: vote = self.votes.get(user=user) return vote.vote_type except ForumVote.DoesNotExist: - return None + pass + return None🧰 Tools
🪛 Ruff (0.8.2)
843-843: Missing return type annotation for public function
upvote_count(ANN201)
847-847: Missing return type annotation for public function
downvote_count(ANN201)
851-851: Missing return type annotation for public function
vote_score(ANN201)
855-855: Missing return type annotation for public function
user_vote(ANN201)
855-855: Missing type annotation for function argument
user(ANN001)
861-861: Consider moving this statement to an
elseblock(TRY300)
2884-2892: Add a trailing comma to match Django style conventions.As in your migration file, Django often prefers a trailing comma for multi-line lists or constraints. This is purely stylistic and won't affect behavior, but it keeps your code consistent with Django's recommended style.
constraints = [ models.CheckConstraint( check=( models.Q(topic__isnull=False, reply__isnull=True) | models.Q(topic__isnull=True, reply__isnull=False) ), - name="vote_topic_xor_reply" + name="vote_topic_xor_reply", ) ]🧰 Tools
🪛 Ruff (0.8.2)
2884-2892: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
2891-2891: Trailing comma missing
Add trailing comma
(COM812)
2894-2901: Refine__str__method structure for simplicity.The static analysis flags an "Unnecessary
elifafter return statement" (RET505). Functionally, this code is fine. However, you could simplify flow by using separateifchecks instead ofelif. If you prefer to keep theif/elif, you can safely ignore this minor style nitpick. Additionally, consider adding a return type annotation.-def __str__(self): +def __str__(self) -> str: if self.topic: return f"{self.user.username} {self.vote_type}voted topic #{self.topic.id}" - elif self.reply: + if self.reply: return f"{self.user.username} {self.vote_type}voted reply #{self.reply.id}" return f"{self.user.username} cast a vote"🧰 Tools
🪛 Ruff (0.8.2)
2894-2894: Missing return type annotation for special method
__str__Add return type annotation:
str(ANN204)
2897-2897: Unnecessary
elifafterreturnstatementRemove unnecessary
elif(RET505)
web/views.py (7)
7209-7237: 🧹 Nitpick (assertive)Add type annotations to
topic_detail.The
topic_detailfunction is missing type annotations, similar to the other new functions.-def topic_detail(request, pk): +from django.http import HttpRequest, HttpResponse + +def topic_detail(request: HttpRequest, pk: int) -> HttpResponse: topic = get_object_or_404(ForumTopic, pk=pk)Also, add a trailing comma to the
values_listcall:reply_votes = ForumVote.objects.filter(reply__topic=topic, user=request.user).values_list( - "reply_id", "vote_type" + "reply_id", + "vote_type", )🧰 Tools
🪛 Ruff (0.8.2)
7209-7209: Missing return type annotation for public function
topic_detail(ANN201)
7209-7209: Missing type annotation for function argument
request(ANN001)
7209-7209: Missing type annotation for function argument
pk(ANN001)
7225-7225: Trailing comma missing
Add trailing comma
(COM812)
7133-7170: 🧹 Nitpick (assertive)Add type annotations and trailing commas to the topic_vote function.
The function implementation looks good, but could benefit from type annotations and style improvements.
-def topic_vote(request, pk): +from django.http import HttpRequest, HttpResponse +from typing import Union + +def topic_vote(request: HttpRequest, pk: int) -> Union[HttpResponse, JsonResponse]: """Handle voting on a topic.""" if request.method != "POST": return JsonResponse({"error": "Only POST method allowed"}, status=405) try: topic = ForumTopic.objects.get(pk=pk) vote_type = request.POST.get("vote_type") if vote_type not in ["up", "down"]: # For form submissions, redirect back with an error message if needed messages.error(request, "Invalid vote type") return redirect("topic_vote", pk=topic.id) # Check if user already voted on this topic vote, created = ForumVote.objects.get_or_create( - user=request.user, topic=topic, defaults={"vote_type": vote_type} + user=request.user, + topic=topic, + defaults={"vote_type": vote_type}, )🧰 Tools
🪛 Ruff (0.8.2)
7134-7134: Missing return type annotation for public function
topic_vote(ANN201)
7134-7134: Missing type annotation for function argument
request(ANN001)
7134-7134: Missing type annotation for function argument
pk(ANN001)
7150-7150: Trailing comma missing
Add trailing comma
(COM812)
7140-7142: 🧹 Nitpick (assertive)Consider preventing authors from voting on their own topic.
Currently, any authenticated user (including topic authors) can vote. If you want to exclude self-voting or impose other constraints (like one vote per user), add checks here.
try: topic = ForumTopic.objects.get(pk=pk) vote_type = request.POST.get("vote_type") + + # Prevent authors from voting on their own topics + if topic.author == request.user: + messages.error(request, "You cannot vote on your own topic") + return redirect("forum_topic", category_slug=topic.category.slug, topic_id=topic.id)
7166-7166:⚠️ Potential issuePotential issue with
"view_topic"route.You're redirecting to
"topic_vote"with just apkparameter, but this doesn't match the URL patterns seen in the error handling below, which use"forum_topic"with bothcategory_slugandtopic_id. This may cause redirect failures.Consider fixing the redirect target:
- return redirect("topic_vote", pk=topic.id) + return redirect("forum_topic", category_slug=topic.category.slug, topic_id=topic.id)
7173-7207: 🧹 Nitpick (assertive)Apply the same refinements as
topic_votetoreply_vote.The
reply_votefunction needs the same improvements as thetopic_votefunction.-def reply_vote(request, pk): +from django.http import HttpRequest, HttpResponse +from typing import Union + +def reply_vote(request: HttpRequest, pk: int) -> Union[HttpResponse, JsonResponse]: """Handle voting on a reply.""" if request.method != "POST": return JsonResponse({"error": "Only POST method allowed"}, status=405) try: reply = ForumReply.objects.get(pk=pk) vote_type = request.POST.get("vote_type") if vote_type not in ["up", "down"]: messages.error(request, "Invalid vote type") return redirect("forum_topic", category_slug=reply.topic.category.slug, topic_id=reply.topic.id) # Check if user already voted on this reply vote, created = ForumVote.objects.get_or_create( - user=request.user, reply=reply, defaults={"vote_type": vote_type} + user=request.user, + reply=reply, + defaults={"vote_type": vote_type}, )Also consider preventing authors from voting on their own replies, similar to the suggestion for topics.
🧰 Tools
🪛 Ruff (0.8.2)
7173-7173: Missing return type annotation for public function
reply_vote(ANN201)
7173-7173: Missing type annotation for function argument
request(ANN001)
7173-7173: Missing type annotation for function argument
pk(ANN001)
7188-7188: Trailing comma missing
Add trailing comma
(COM812)
7208-7208:⚠️ Potential issueDuplicate exception clause may hide errors.
This is a duplicate
exceptblock forForumReply.DoesNotExist. The second occurrence (at line 7208) is never reached because the first one (at line 7205) always catches the exception first.Remove the second exception block or merge them:
except ForumReply.DoesNotExist: messages.error(request, "Reply not found") return redirect("forum_categories") - except ForumReply.DoesNotExist: - return JsonResponse({"error": "Reply not found"}, status=404)
2893-2900: 🧹 Nitpick (assertive)Replace or remove debugging print statement.
Line 2900 uses a plain
print(form.errors)for debugging. Consider removing this line or using a logger call; relying onif form.is_valid(): category = form.save(commit=False) if not category.slug: category.slug = slugify(category.name) category.save() messages.success(request, f"Forum category '{category.name}' created successfully!") return redirect("forum_category", slug=category.slug) - else: - print(form.errors) + # Error details are automatically passed to the template🧰 Tools
🪛 Ruff (0.8.2)
2899-2899: Unnecessary
elseafterreturnstatementRemove unnecessary
else(RET505)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
web/migrations/0056_forumvote.py(1 hunks)web/models.py(3 hunks)web/templates/base.html(0 hunks)web/templates/web/forum/category.html(1 hunks)web/templates/web/forum/topic.html(7 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
💤 Files with no reviewable changes (1)
- web/templates/base.html
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/urls.py (1)
web/views.py (2)
topic_vote(7134-7169)reply_vote(7173-7206)
web/views.py (1)
web/models.py (6)
ForumVote(2867-2899)ForumReply(832-870)delete(467-473)user_vote(815-823)user_vote(855-863)ForumTopic(789-829)
🪛 Ruff (0.8.2)
web/migrations/0056_forumvote.py
10-13: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
15-66: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
61-61: Trailing comma missing
Add trailing comma
(COM812)
web/models.py
803-803: Missing return type annotation for public function upvote_count
(ANN201)
807-807: Missing return type annotation for public function downvote_count
(ANN201)
811-811: Missing return type annotation for public function vote_score
(ANN201)
815-815: Missing return type annotation for public function user_vote
(ANN201)
815-815: Missing type annotation for function argument user
(ANN001)
821-821: Consider moving this statement to an else block
(TRY300)
843-843: Missing return type annotation for public function upvote_count
(ANN201)
847-847: Missing return type annotation for public function downvote_count
(ANN201)
851-851: Missing return type annotation for public function vote_score
(ANN201)
855-855: Missing return type annotation for public function user_vote
(ANN201)
855-855: Missing type annotation for function argument user
(ANN001)
861-861: Consider moving this statement to an else block
(TRY300)
2870-2873: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2883-2883: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2884-2892: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2891-2891: Trailing comma missing
Add trailing comma
(COM812)
2894-2894: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
2897-2897: Unnecessary elif after return statement
Remove unnecessary elif
(RET505)
web/views.py
2899-2899: Unnecessary else after return statement
Remove unnecessary else
(RET505)
7134-7134: Missing return type annotation for public function topic_vote
(ANN201)
7134-7134: Missing type annotation for function argument request
(ANN001)
7134-7134: Missing type annotation for function argument pk
(ANN001)
7150-7150: Trailing comma missing
Add trailing comma
(COM812)
7173-7173: Missing return type annotation for public function reply_vote
(ANN201)
7173-7173: Missing type annotation for function argument request
(ANN001)
7173-7173: Missing type annotation for function argument pk
(ANN001)
7188-7188: Trailing comma missing
Add trailing comma
(COM812)
7209-7209: Missing return type annotation for public function topic_detail
(ANN201)
7209-7209: Missing type annotation for function argument request
(ANN001)
7209-7209: Missing type annotation for function argument pk
(ANN001)
7225-7225: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (9)
web/templates/web/forum/category.html (1)
138-148: Highlight the voting metrics for clarity.
By displaying both the upvote and downvote counts, you're providing immediate visibility into topic engagement. The implementation is straightforward and aids quick scanning. Nice addition!web/urls.py (1)
218-219: Confirm URL patterns remain consistent.
Addingtopic_voteandreply_voteroutes is straightforward. Make sure these new endpoints conform to existing forum URL naming conventions (e.g.,forum_topicvs.topic_vote). If consistent, this looks good.web/templates/web/forum/topic.html (4)
6-8: Imports for additional template filters.
Loadingdict_filtershere is a nice way to Condition-check user votes ({% if user_reply_votes|get_item:reply.id ... %}). This keeps your templates more readable.
134-176: Topic voting layout provides clarity.
The side-by-side arrangement of upvote, score, and downvote is intuitive. The usage ofaria-pressedfor accessibility is commendable. Great job ensuring screen readers can identify the state of each button!
177-210: Stats block complements the voting UI.
Displaying upvote and downvote counts directly under the topic fosters transparency. Keeping the textual “upvotes” & “downvotes” clarifies the numbers for all audiences.
303-325: Clear separation of reply vote statistics.
Well done separating direct numeric counts (upvote/downvote) from the reply’s main content. This improves scannability and keeps the interaction area distinct.web/models.py (1)
2867-2883: The ForumVote model looks well-structured.The model correctly implements the voting functionality with appropriate fields and constraints. The unique_together constraint ensures a user can only have one vote per topic or reply, and the check constraint ensures a vote is associated with either a topic or a reply (but not both).
For completeness, you could add type annotations to the class attributes:
- VOTE_TYPES = [ + VOTE_TYPES: typing.ClassVar[list[tuple[str, str]]] = [ ("up", "Upvote"), ("down", "Downvote"), ]🧰 Tools
🪛 Ruff (0.8.2)
2870-2873: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
2883-2883: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
web/views.py (2)
131-131: Appropriate model import added.The
ForumVotemodel is correctly imported from the models module to support the new voting functionality.
1843-1887: Good enhancement to the forum topic view with user voting data.The forum topic view now properly includes voting information by fetching the user's votes on the topic and its replies. This contextual data enables the template to display voting state appropriately.
| {% for reply in replies %} | ||
| <div class="bg-white dark:bg-gray-800 rounded-lg shadow-sm p-4 lg:p-5"> | ||
| <div class="flex"> | ||
| <!-- Voting for Replies --> | ||
| <div class="flex flex-col items-center mr-3 bg-gray-100 dark:bg-gray-700 rounded-md p-1 pb-0"> | ||
| <!-- Upvote button --> | ||
| <button class="vote-btn p-1 hover:text-teal-500 transition-colors duration-200 focus:outline-none" | ||
| data-reply="{{ reply.id }}" | ||
| data-vote-type="up" | ||
| aria-label="Upvote this reply" | ||
| aria-pressed="{% if user_reply_votes|get_item:reply.id == 'up' %}true{% else %}false{% endif %}"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" | ||
| class="w-5 h-5 {% if user_reply_votes|get_item:reply.id == 'up' %}text-teal-500{% endif %}" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 15l7-7 7 7" /> | ||
| </svg> | ||
| </button> | ||
| <!-- Vote score --> | ||
| <span class="vote-score text-xs font-bold my-1 {% if reply.vote_score > 0 %}text-teal-500{% elif reply.vote_score < 0 %}text-red-500{% else %}text-gray-500 dark:text-gray-400{% endif %}"> | ||
| {{ reply.vote_score }} | ||
| </span> | ||
| <!-- Downvote button --> | ||
| <button class="vote-btn p-1 hover:text-red-500 transition-colors duration-200 focus:outline-none" | ||
| data-reply="{{ reply.id }}" | ||
| data-vote-type="down" | ||
| aria-label="Downvote this reply" | ||
| aria-pressed="{% if user_reply_votes|get_item:reply.id == 'down' %}true{% else %}false{% endif %}"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" | ||
| class="w-5 h-5 {% if user_reply_votes|get_item:reply.id == 'down' %}text-red-500{% endif %}" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7" /> | ||
| </svg> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reply voting duplicates the topic approach effectively.
Reusing the same pattern for replies is consistent. It may be worthwhile to consider a shared partial template if the voting UI continues to grow or if you plan to add more engagement features.
| <script> | ||
| document.addEventListener("DOMContentLoaded", function() { | ||
| // CSRF token extractor | ||
| function getCsrfToken() { | ||
| const csrfInput = document.querySelector("[name=csrfmiddlewaretoken]"); | ||
| return csrfInput ? csrfInput.value : ''; | ||
| } | ||
|
|
||
| const loginUrl = "{% url 'account_signup' %}"; | ||
|
|
||
| // Event handler for vote buttons | ||
| function handleVote(e) { | ||
| e.preventDefault(); | ||
|
|
||
| // Redirect to login if user is not authenticated | ||
| if (!document.body.classList.contains("user-authenticated")) { | ||
| window.location.href = loginUrl; | ||
| return; | ||
| } | ||
|
|
||
| const button = e.currentTarget; | ||
| const voteType = button.dataset.voteType; | ||
| const csrftoken = getCsrfToken(); | ||
| const isTopicVote = button.hasAttribute('data-topic'); | ||
|
|
||
| let url; | ||
| if (isTopicVote) { | ||
| const topicId = button.dataset.topic; | ||
| url = "{% url 'topic_vote' topic.id %}"; | ||
| } else { | ||
| const replyId = button.dataset.reply; | ||
| url = "{% url 'reply_vote' 0 %}".replace('0', replyId); | ||
| } | ||
|
|
||
| fetch(url, { | ||
| method: "POST", | ||
| headers: { | ||
| "X-CSRFToken": csrftoken, | ||
| "Content-Type": "application/x-www-form-urlencoded" | ||
| }, | ||
| body: `vote_type=${voteType}` | ||
| }) | ||
| .then(response => { | ||
| if (response.ok) return response.json(); | ||
| throw new Error("Vote failed"); | ||
| }) | ||
| .then(data => { | ||
| updateVoteUI(button, data); | ||
| }) | ||
| .catch(error => { | ||
| console.error(error); | ||
| location.reload(); // Fallback | ||
| }); | ||
| } | ||
|
|
||
| // Update the UI after voting | ||
| function updateVoteUI(button, data) { | ||
| const container = button.closest('.flex-col'); // Find vote container | ||
| const scoreEl = container.querySelector('.vote-score'); | ||
| const upBtn = container.querySelector('button[data-vote-type="up"]'); | ||
| const downBtn = container.querySelector('button[data-vote-type="down"]'); | ||
|
|
||
| // Update score and color | ||
| scoreEl.textContent = data.vote_score; | ||
| scoreEl.className = "vote-score text-sm font-bold my-1"; | ||
| if (data.vote_score > 0) { | ||
| scoreEl.classList.add("text-teal-500"); | ||
| } else if (data.vote_score < 0) { | ||
| scoreEl.classList.add("text-red-500"); | ||
| } else { | ||
| scoreEl.classList.add("text-gray-500", "dark:text-gray-400"); | ||
| } | ||
|
|
||
| // Reset icon colors | ||
| upBtn.querySelector("svg").classList.remove("text-teal-500"); | ||
| downBtn.querySelector("svg").classList.remove("text-red-500"); | ||
|
|
||
| // Toggle button highlight | ||
| if (data.user_vote === "up") { | ||
| upBtn.querySelector("svg").classList.add("text-teal-500"); | ||
| } else if (data.user_vote === "down") { | ||
| downBtn.querySelector("svg").classList.add("text-red-500"); | ||
| } | ||
| } | ||
|
|
||
| // Attach to all vote buttons | ||
| document.querySelectorAll(".vote-btn").forEach(btn => { | ||
| btn.addEventListener("click", handleVote); | ||
| }); | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
AJAX vote logic improves user experience.
- Using
fetchto postvote_typeand updating the UI without reload is excellent. - Consider a more user-friendly error handling strategy than
location.reload()for smoother UX. - Overall, the
handleVoteandupdateVoteUIfunctions are well-structured and easy to maintain.
| @login_required | ||
| def topic_vote(request, pk): | ||
| """Handle voting on a topic.""" | ||
| if request.method != "POST": | ||
| return JsonResponse({"error": "Only POST method allowed"}, status=405) | ||
|
|
||
| try: | ||
| topic = ForumTopic.objects.get(pk=pk) | ||
| vote_type = request.POST.get("vote_type") | ||
|
|
||
| if vote_type not in ["up", "down"]: | ||
| # For form submissions, redirect back with an error message if needed | ||
| messages.error(request, "Invalid vote type") | ||
| return redirect("topic_vote", pk=topic.id) | ||
|
|
||
| # Check if user already voted on this topic | ||
| vote, created = ForumVote.objects.get_or_create( | ||
| user=request.user, topic=topic, defaults={"vote_type": vote_type} | ||
| ) | ||
|
|
||
| if not created: | ||
| # User already voted, check if they're changing their vote | ||
| if vote.vote_type == vote_type: | ||
| # Same vote type, so remove the vote | ||
| vote.delete() | ||
| else: | ||
| # Different vote type, so update the vote | ||
| vote.vote_type = vote_type | ||
| vote.save() | ||
|
|
||
| # After processing the vote, redirect back to the topic page | ||
| return redirect("forum_topic", category_slug=topic.category.slug, topic_id=topic.id) | ||
|
|
||
| except ForumTopic.DoesNotExist: | ||
| # Handle case when topic doesn't exist | ||
| messages.error(request, "Topic not found") | ||
| return redirect("forum_categories") | ||
|
|
||
|
|
||
| @login_required | ||
| def reply_vote(request, pk): | ||
| """Handle voting on a reply.""" | ||
| if request.method != "POST": | ||
| return JsonResponse({"error": "Only POST method allowed"}, status=405) | ||
|
|
||
| try: | ||
| reply = ForumReply.objects.get(pk=pk) | ||
| vote_type = request.POST.get("vote_type") | ||
|
|
||
| if vote_type not in ["up", "down"]: | ||
| messages.error(request, "Invalid vote type") | ||
| return redirect("forum_topic", category_slug=reply.topic.category.slug, topic_id=reply.topic.id) | ||
|
|
||
| # Check if user already voted on this reply | ||
| vote, created = ForumVote.objects.get_or_create( | ||
| user=request.user, reply=reply, defaults={"vote_type": vote_type} | ||
| ) | ||
|
|
||
| if not created: | ||
| # User already voted, check if they're changing their vote | ||
| if vote.vote_type == vote_type: | ||
| # Same vote type, so remove the vote | ||
| vote.delete() | ||
| else: | ||
| # Different vote type, so update the vote | ||
| vote.vote_type = vote_type | ||
| vote.save() | ||
|
|
||
| # After processing the vote, redirect back to the topic page | ||
| return redirect("forum_topic", category_slug=reply.topic.category.slug, topic_id=reply.topic.id) | ||
|
|
||
| except ForumReply.DoesNotExist: | ||
| messages.error(request, "Reply not found") | ||
| return redirect("forum_categories") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider refactoring the vote handling functions to reduce duplication.
The topic_vote and reply_vote functions contain nearly identical logic. Consider refactoring them into a single, more generic function to handle both cases.
You could create a helper function that takes the vote target (topic or reply) and performs the common vote handling logic:
def handle_vote(request, vote_target, target_type):
"""
Handle voting on a forum entity (topic or reply).
Args:
request: The HTTP request
vote_target: The target object (topic or reply)
target_type: String 'topic' or 'reply'
Returns:
HttpResponse: Redirect to the appropriate page
"""
vote_type = request.POST.get("vote_type")
if vote_type not in ["up", "down"]:
messages.error(request, "Invalid vote type")
return get_redirect_response(vote_target)
# Prevent self-voting
if vote_target.author == request.user:
messages.error(request, f"You cannot vote on your own {target_type}")
return get_redirect_response(vote_target)
# Create vote kwargs based on target type
vote_kwargs = {
"user": request.user,
target_type: vote_target,
"defaults": {"vote_type": vote_type}
}
# Process vote
vote, created = ForumVote.objects.get_or_create(**vote_kwargs)
if not created:
if vote.vote_type == vote_type:
vote.delete()
else:
vote.vote_type = vote_type
vote.save()
return get_redirect_response(vote_target)
def get_redirect_response(vote_target):
"""Generate the appropriate redirect response based on the vote target."""
if hasattr(vote_target, 'topic'): # It's a reply
return redirect("forum_topic",
category_slug=vote_target.topic.category.slug,
topic_id=vote_target.topic.id)
else: # It's a topic
return redirect("forum_topic",
category_slug=vote_target.category.slug,
topic_id=vote_target.id)Then your view functions would be much simpler:
@login_required
def topic_vote(request: HttpRequest, pk: int) -> HttpResponse:
"""Handle voting on a topic."""
if request.method != "POST":
return JsonResponse({"error": "Only POST method allowed"}, status=405)
try:
topic = ForumTopic.objects.get(pk=pk)
return handle_vote(request, topic, "topic")
except ForumTopic.DoesNotExist:
messages.error(request, "Topic not found")
return redirect("forum_categories")
@login_required
def reply_vote(request: HttpRequest, pk: int) -> HttpResponse:
"""Handle voting on a reply."""
if request.method != "POST":
return JsonResponse({"error": "Only POST method allowed"}, status=405)
try:
reply = ForumReply.objects.get(pk=pk)
return handle_vote(request, reply, "reply")
except ForumReply.DoesNotExist:
messages.error(request, "Reply not found")
return redirect("forum_categories")🧰 Tools
🪛 Ruff (0.8.2)
7134-7134: Missing return type annotation for public function topic_vote
(ANN201)
7134-7134: Missing type annotation for function argument request
(ANN001)
7134-7134: Missing type annotation for function argument pk
(ANN001)
7150-7150: Trailing comma missing
Add trailing comma
(COM812)
7173-7173: Missing return type annotation for public function reply_vote
(ANN201)
7173-7173: Missing type annotation for function argument request
(ANN001)
7173-7173: Missing type annotation for function argument pk
(ANN001)
7188-7188: Trailing comma missing
Add trailing comma
(COM812)
| user_reply_votes = {} | ||
| if request.user.is_authenticated: | ||
| reply_votes = ForumVote.objects.filter(reply__topic=topic, user=request.user).values_list( | ||
| "reply_id", "vote_type" | ||
| ) | ||
| user_reply_votes = dict(reply_votes) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optimize database query with prefetch_related.
The current implementation makes a separate database query for vote information. Consider using prefetch_related to optimize this and reduce the number of database queries.
# Get user votes on replies
user_reply_votes = {}
if request.user.is_authenticated:
- reply_votes = ForumVote.objects.filter(reply__topic=topic, user=request.user).values_list(
- "reply_id", "vote_type"
- )
- user_reply_votes = dict(reply_votes)
+ # More efficient query using prefetch_related
+ votes = ForumVote.objects.filter(
+ user=request.user,
+ ).filter(
+ Q(topic=topic) | Q(reply__topic=topic)
+ ).select_related('reply')
+
+ # Process votes
+ user_topic_vote = next((v.vote_type for v in votes if v.topic_id == topic.id), None)
+ user_reply_votes = {v.reply_id: v.vote_type for v in votes if v.reply_id is not None}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.8.2)
7225-7225: Trailing comma missing
Add trailing comma
(COM812)
2 participants
added forum voting feature
closes #464
supported screenshots :-
comprehensive version of #489
Summary by CodeRabbit