GET to POST implementation and calling #4620
Conversation
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
website/views/issue.py (3)
1918-1936: Remove GET flag link in sitemap.html: in templates/sitemap.html (line 260), replace the<a href="{% url 'flag_issue' … %}">anchor with a POST-based action (form button or AJAX) so that all calls toflag_issueuse POST.
139-157: Convert the sitemap dislike link to a POST request
The<a href="{% url 'dislike_issue' 1 %}">in website/templates/sitemap.html (around line 253) still issues a GET and will 405 under@require_POST. Wrap it in a<form method="post">with{% csrf_token %}or convert it to AJAX POST so the endpoint receives a POST.
90-136: Update sitemap.html links to POST
The anchor tags inwebsite/templates/sitemap.html(around lines 246–253) still calllike_issue/dislike_issuevia GET and will 405 under@require_POST. Convert these to POST (for example using forms or AJAX).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
website/templates/includes/_like_dislike_share.html(1 hunks)website/views/issue.py(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (3)
website/templates/includes/_like_dislike_share.html (1)
195-217: LGTM! Consistent POST implementation.The bookmark handler correctly implements the POST method with CSRF token, consistent with the other handlers. The error handling was already in place.
website/views/issue.py (2)
1841-1848: LGTM! Correct POST enforcement.The
@require_POSTdecorator is correctly applied to this state-changing operation.
1851-1867: LGTM! Correct POST enforcement.The
@require_POSTdecorator is correctly applied to this state-changing operation that toggles the saved status.
| } | ||
|
|
||
| function like_unlike_handler(e) { | ||
| } function like_unlike_handler(e) { |
There was a problem hiding this comment.
Fix formatting issue.
The function definition is concatenated with the previous line's closing brace. This makes the code harder to read and maintain.
Apply this diff to fix the formatting:
- } function like_unlike_handler(e) {
+ }
+
+ function like_unlike_handler(e) {📝 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.
| } function like_unlike_handler(e) { | |
| } | |
| function like_unlike_handler(e) { |
🤖 Prompt for AI Agents
In website/templates/includes/_like_dislike_share.html around line 128, the
closing brace and the subsequent function declaration are concatenated ("}
function like_unlike_handler(e)"), making it a formatting issue; fix it by
placing the closing brace on its own line (or adding a newline) so the function
declaration starts on the next line and adjust indentation so the function
signature is aligned with other top-level functions.
| type: 'POST', | ||
| url: '/like_issue/' + issue_pk + '/', | ||
| data: {}, | ||
| data: { | ||
| 'csrfmiddlewaretoken': $('[name=csrfmiddlewaretoken]').val() | ||
| }, | ||
| success: function (data) { | ||
| window.location.reload(); | ||
| }, | ||
| error: function () { | ||
| $.notify("Some error occurred!", { | ||
| style: "custom", | ||
| className: "danger" | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify CSRF token field availability.
The code correctly uses POST and includes the CSRF token. However, ensure that a CSRF token field (<input name="csrfmiddlewaretoken">) exists in the page, either in this template, an included template, or the parent template. Without it, all AJAX requests will fail with CSRF validation errors.
Run the following script to check if the CSRF token is present in related templates:
🏁 Script executed:
#!/bin/bash
# Description: Check for CSRF token in templates
# Search for csrf_token template tag in related templates
rg -n "{% csrf_token %}" --type=html
# Search for csrfmiddlewaretoken input field
rg -n 'name=["\']csrfmiddlewaretoken["\']' --type=htmlLength of output: 160
The previous search errored due to unescaped characters. Please rerun with literal searches:
🏁 Script executed:
#!/bin/bash
# Search for CSRF token template tag
rg -nF "{% csrf_token %}" --glob "*.html"
# Search for CSRF token input field
rg -n "csrfmiddlewaretoken" --glob "*.html"Length of output: 12593
🏁 Script executed:
#!/bin/bash
# Check for csrf_token tag in the include
rg -nF "{% csrf_token %}" website/templates/includes/_like_dislike_share.html
# Check for hidden input field for CSRF token in the include
rg -n '<input[^>]*name=["'\'']csrfmiddlewaretoken["'\'']' website/templates/includes/_like_dislike_share.htmlLength of output: 0
Ensure a CSRF token input is rendered in _like_dislike_share.html.
Add {% csrf_token %} or a hidden field <input type="hidden" name="csrfmiddlewaretoken" value="{{ csrf_token }}"> in website/templates/includes/_like_dislike_share.html so $(' [name=csrfmiddlewaretoken]').val() isn’t empty.
🤖 Prompt for AI Agents
In website/templates/includes/_like_dislike_share.html around lines 132 to 146,
the AJAX expects a csrfmiddlewaretoken selector but the template does not render
a CSRF input; add a CSRF token input (either include the Django template tag {%
csrf_token %} inside the form or add a hidden input with
name="csrfmiddlewaretoken" and value="{{ csrf_token }}") in this template so $('
[name=csrfmiddlewaretoken]').val() returns the token and the POST request is
accepted.
| $('body').on('click', '#like_unlike', like_unlike_handler); | ||
|
|
||
| function dislike_handler(e){ | ||
| $('body').on('click', '#like_unlike', like_unlike_handler); function dislike_handler(e){ |
There was a problem hiding this comment.
Fix formatting issue.
The event binding is concatenated with the previous line's closing brace, followed by the function definition.
Apply this diff to fix the formatting:
- $('body').on('click', '#like_unlike', like_unlike_handler); function dislike_handler(e){
+ $('body').on('click', '#like_unlike', like_unlike_handler);
+
+ function dislike_handler(e){📝 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.
| $('body').on('click', '#like_unlike', like_unlike_handler); function dislike_handler(e){ | |
| $('body').on('click', '#like_unlike', like_unlike_handler); | |
| function dislike_handler(e){ |
🤖 Prompt for AI Agents
In website/templates/includes/_like_dislike_share.html around line 149, the
event binding statement is concatenated with the previous function's closing
brace and the next function definition (the line reads "...like_unlike_handler);
function dislike_handler(e){"); fix this by inserting a newline (and optional
semicolon if missing) between the event binding call and the start of the
dislike_handler function so the binding stands alone on its own line and the
function definition begins on the next line.
| </script> | ||
| <script> | ||
| function flag_unflag_handler(e){ | ||
| <script> function flag_unflag_handler(e){ |
There was a problem hiding this comment.
Fix formatting issue.
The script tag is concatenated with the previous line.
Apply this diff to fix the formatting:
-</script>
-<script> function flag_unflag_handler(e){
+</script>
+<script>
+ function flag_unflag_handler(e){📝 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.
| <script> function flag_unflag_handler(e){ | |
| </script> | |
| <script> | |
| function flag_unflag_handler(e){ |
🤖 Prompt for AI Agents
In website/templates/includes/_like_dislike_share.html around line 171, the
opening "<script>" tag was concatenated onto the previous line causing
formatting issues; move the "<script>" tag to its own line (insert a newline
before "<script>") so the tag starts at column 1 on line 171, preserving
existing indentation for the function that follows and ensuring proper HTML
formatting.
| </script> | ||
| <script> | ||
| function bookmark_handler(e){ | ||
| <script> function bookmark_handler(e){ |
There was a problem hiding this comment.
Fix formatting issue.
The script tag is concatenated with the previous line.
Apply this diff to fix the formatting:
-</script>
-<script> function bookmark_handler(e){
+</script>
+<script>
+ function bookmark_handler(e){📝 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.
| <script> function bookmark_handler(e){ | |
| </script> | |
| <script> | |
| function bookmark_handler(e){ |
🤖 Prompt for AI Agents
In website/templates/includes/_like_dislike_share.html around line 195 the
opening "<script>" tag is concatenated with the previous line; separate the
script tag onto its own line by inserting a newline (or line break) before
"<script> function bookmark_handler(e){" so the "<script>" starts on its own
line and the function body remains properly indented.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
website/views/issue.py (2)
90-137: Approve POST enforcement, but remove unused context variables.The
@require_POSTdecorator correctly prevents GET-based state changes. However, the context dictionary created at lines 133-136 is never used since the function returns a plainHttpResponse("Success")at line 137.Apply this diff to remove the unused code:
- total_votes = UserProfile.objects.filter(issue_upvoted=issue).count() - context["object"] = issue - context["likes"] = total_votes - context["isLiked"] = UserProfile.objects.filter(issue_upvoted=issue, user=request.user).exists() return HttpResponse("Success")Alternatively, if you need to return this data to the frontend, consider returning a JSON response:
+ total_votes = UserProfile.objects.filter(issue_upvoted=issue).count() + is_liked = UserProfile.objects.filter(issue_upvoted=issue, user=request.user).exists() - return HttpResponse("Success") + return JsonResponse({ + "status": "success", + "likes": total_votes, + "isLiked": is_liked + })
140-158: Remove unused context variables (same issue as in like_issue).Similar to
like_issue, the context dictionary at lines 154-157 is populated but never used.Apply this diff:
- total_votes = UserProfile.objects.filter(issue_downvoted=issue).count() - context["object"] = issue - context["dislikes"] = total_votes - context["isDisliked"] = UserProfile.objects.filter(issue_downvoted=issue, user=request.user).exists() return HttpResponse("Success")Or return the data as JSON if needed by the frontend:
+ total_votes = UserProfile.objects.filter(issue_downvoted=issue).count() + is_disliked = UserProfile.objects.filter(issue_downvoted=issue, user=request.user).exists() - return HttpResponse("Success") + return JsonResponse({ + "status": "success", + "dislikes": total_votes, + "isDisliked": is_disliked + })
🧹 Nitpick comments (3)
website/views/issue.py (3)
1842-1849: Consider adding error handling for database operations.The function lacks error handling for potential database issues (e.g., issue not found, user profile not found).
Consider adding error handling:
@require_POST @login_required(login_url="/accounts/login") def unsave_issue(request, issue_pk): - issue_pk = int(issue_pk) - issue = Issue.objects.get(pk=issue_pk) - userprof = UserProfile.objects.get(user=request.user) - userprof.issue_saved.remove(issue) - return HttpResponse("OK") + try: + issue_pk = int(issue_pk) + issue = get_object_or_404(Issue, pk=issue_pk) + userprof = UserProfile.objects.get(user=request.user) + userprof.issue_saved.remove(issue) + return JsonResponse({"status": "success"}) + except (ValueError, UserProfile.DoesNotExist) as e: + return JsonResponse({"status": "error", "message": str(e)}, status=400)
1852-1867: Consider error handling and standardized JSON responses.The function lacks error handling and uses inconsistent response formats ("REMOVED" vs "OK").
Consider adding error handling and returning standardized JSON:
@require_POST @login_required(login_url="/accounts/login") def save_issue(request, issue_pk): - issue_pk = int(issue_pk) - issue = Issue.objects.get(pk=issue_pk) - userprof = UserProfile.objects.get(user=request.user) - - already_saved = userprof.issue_saved.filter(pk=issue_pk).exists() - - if already_saved: - userprof.issue_saved.remove(issue) - return HttpResponse("REMOVED") - - else: - userprof.issue_saved.add(issue) - return HttpResponse("OK") + try: + issue_pk = int(issue_pk) + issue = get_object_or_404(Issue, pk=issue_pk) + userprof = UserProfile.objects.get(user=request.user) + + already_saved = userprof.issue_saved.filter(pk=issue_pk).exists() + + if already_saved: + userprof.issue_saved.remove(issue) + return JsonResponse({"status": "success", "action": "removed", "saved": False}) + else: + userprof.issue_saved.add(issue) + return JsonResponse({"status": "success", "action": "added", "saved": True}) + except (ValueError, UserProfile.DoesNotExist) as e: + return JsonResponse({"status": "error", "message": str(e)}, status=400)
1919-1936: POST enforcement looks good; consider adding error handling.The
@require_POSTdecorator correctly prevents GET-based state changes. The function returns an HTML fragment, which differs from the other functions but may be intentional if the frontend expects partial HTML updates.Consider adding error handling for database operations:
@require_POST @login_required(login_url="/accounts/login") def flag_issue(request, issue_pk): - context = {} - issue_pk = int(issue_pk) - issue = Issue.objects.get(pk=issue_pk) - userprof = UserProfile.objects.get(user=request.user) - if userprof in UserProfile.objects.filter(issue_flaged=issue): - userprof.issue_flaged.remove(issue) - else: - userprof.issue_flaged.add(issue) - issue_pk = issue.pk - - userprof.save() - total_flag_votes = UserProfile.objects.filter(issue_flaged=issue).count() - context["object"] = issue - context["flags"] = total_flag_votes - return render(request, "includes/_flags.html", context) + try: + context = {} + issue_pk = int(issue_pk) + issue = get_object_or_404(Issue, pk=issue_pk) + userprof = UserProfile.objects.get(user=request.user) + if userprof in UserProfile.objects.filter(issue_flaged=issue): + userprof.issue_flaged.remove(issue) + else: + userprof.issue_flaged.add(issue) + issue_pk = issue.pk + + userprof.save() + total_flag_votes = UserProfile.objects.filter(issue_flaged=issue).count() + context["object"] = issue + context["flags"] = total_flag_votes + return render(request, "includes/_flags.html", context) + except (ValueError, UserProfile.DoesNotExist): + return HttpResponse("Error processing flag", status=400)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
website/views/issue.py(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
|
@DonnieBLT Could you please review the PR? Specifically, do you think I should address the changes suggested by coderabbit? Those changes modify the source in ways unrelated to the security updates. |
|
Yes please update |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
website/views/issue.py (5)
90-93: LGTM! POST requirement correctly enforced.The
@require_POSTdecorator properly prevents GET-based state changes. However, thecontextdictionary initialized on line 93 is never used and can be removed.Apply this diff to remove the unused variable:
@require_POST @login_required(login_url="/accounts/login") def like_issue(request, issue_pk): - context = {} issue_pk = int(issue_pk) issue = get_object_or_404(Issue, pk=issue_pk)
136-139: LGTM! POST requirement correctly enforced.The
@require_POSTdecorator properly prevents GET-based state changes. However, thecontextdictionary initialized on line 139 is never used and can be removed.Apply this diff to remove the unused variable:
@require_POST @login_required(login_url="/accounts/login") def dislike_issue(request, issue_pk): - context = {} issue_pk = int(issue_pk) issue = get_object_or_404(Issue, pk=issue_pk)
1837-1844: Improve exception handling specificity.The try/except block catches all exceptions without logging or using the exception variable. This can hide bugs and make debugging difficult.
Consider one of these approaches:
Option 1: Log the exception for debugging
+import logging + +logger = logging.getLogger(__name__) + @require_POST @login_required(login_url="/accounts/login") def unsave_issue(request, issue_pk): try: issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) userprof.issue_saved.remove(issue) return HttpResponse("OK") except Exception as e: + logger.error(f"Error unsaving issue {issue_pk}: {e}") return HttpResponse("ERROR")Option 2: Catch specific exceptions
@require_POST @login_required(login_url="/accounts/login") def unsave_issue(request, issue_pk): try: issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) userprof.issue_saved.remove(issue) return HttpResponse("OK") - except Exception as e: + except (Issue.DoesNotExist, UserProfile.DoesNotExist, ValueError) as e: return HttpResponse("ERROR")
1850-1865: Improve exception handling specificity.The try/except block catches all exceptions without logging or using the exception variable. This can hide bugs and make debugging difficult.
Consider one of these approaches:
Option 1: Log the exception for debugging
+import logging + +logger = logging.getLogger(__name__) + @require_POST @login_required(login_url="/accounts/login") def save_issue(request, issue_pk): try: issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) already_saved = userprof.issue_saved.filter(pk=issue_pk).exists() if already_saved: userprof.issue_saved.remove(issue) return HttpResponse("REMOVED") else: userprof.issue_saved.add(issue) return HttpResponse("OK") except Exception as e: + logger.error(f"Error saving/unsaving issue {issue_pk}: {e}") return HttpResponse("ERROR")Option 2: Catch specific exceptions
@require_POST @login_required(login_url="/accounts/login") def save_issue(request, issue_pk): try: issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) already_saved = userprof.issue_saved.filter(pk=issue_pk).exists() if already_saved: userprof.issue_saved.remove(issue) return HttpResponse("REMOVED") else: userprof.issue_saved.add(issue) return HttpResponse("OK") - except Exception as e: + except (Issue.DoesNotExist, UserProfile.DoesNotExist, ValueError) as e: return HttpResponse("ERROR")
1920-1937: Improve exception handling specificity.The try/except block catches all exceptions without logging or using the exception variable. This makes debugging difficult and returns a generic error message to the user.
Consider one of these approaches:
Option 1: Log the exception for debugging
+import logging + +logger = logging.getLogger(__name__) + @require_POST @login_required(login_url="/accounts/login") def flag_issue(request, issue_pk): try: context = {} issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) if userprof in UserProfile.objects.filter(issue_flaged=issue): userprof.issue_flaged.remove(issue) else: userprof.issue_flaged.add(issue) issue_pk = issue.pk userprof.save() total_flag_votes = UserProfile.objects.filter(issue_flaged=issue).count() context["object"] = issue context["flags"] = total_flag_votes return render(request, "includes/_flags.html", context) except Exception as e: + logger.error(f"Error flagging issue {issue_pk}: {e}") return HttpResponse("Error processing flag", status=400)Option 2: Catch specific exceptions
@require_POST @login_required(login_url="/accounts/login") def flag_issue(request, issue_pk): try: context = {} issue_pk = int(issue_pk) issue = Issue.objects.get(pk=issue_pk) userprof = UserProfile.objects.get(user=request.user) if userprof in UserProfile.objects.filter(issue_flaged=issue): userprof.issue_flaged.remove(issue) else: userprof.issue_flaged.add(issue) issue_pk = issue.pk userprof.save() total_flag_votes = UserProfile.objects.filter(issue_flaged=issue).count() context["object"] = issue context["flags"] = total_flag_votes return render(request, "includes/_flags.html", context) - except Exception as e: + except (Issue.DoesNotExist, UserProfile.DoesNotExist, ValueError) as e: return HttpResponse("Error processing flag", status=400)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
website/views/issue.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.13.3)
website/views/issue.py
1843-1843: Do not catch blind exception: Exception
(BLE001)
1843-1843: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
1864-1864: Do not catch blind exception: Exception
(BLE001)
1864-1864: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
1936-1936: Do not catch blind exception: Exception
(BLE001)
1936-1936: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
website/views/issue.py (1)
90-90: CSRF protection verified on AJAX POST endpointsCsrfViewMiddleware is enabled, no @csrf_exempt on like/dislike/save/flag views, and the base templates (e.g. includes/header.html and issue.html) include a hidden
csrfmiddlewaretokeninput. All POST endpoints are CSRF-protected.
2 participants
Closes #4058
Update Issue and Activity Controllers to Use POST Method
Current Implementation
In
website/views/issue.py(GET methods)like_issue(request, issue_pk)- modifies user upvotesdislike_issue(request, issue_pk)- modifies user downvotesflag_issue(request, issue_pk)- modifies issue flagssave_issue(request, issue_pk)- adds/removes saved issuesunsave_issue(request, issue_pk)- removes saved issuesIn
website/views/organization.py(POST methods)like_activity(request, id)- modifies activity likes/approvaldislike_activity(request, id)- modifies activity dislikesapprove_activity(request, id)- changes approval statusChanges Made
In
website/views/issue.py(now POST methods)like_issue(request, issue_pk)- modifies user upvotesdislike_issue(request, issue_pk)- modifies user downvotesflag_issue(request, issue_pk)- modifies issue flagssave_issue(request, issue_pk)- adds/removes saved issuesunsave_issue(request, issue_pk)- removes saved issuesFrontend Update
website/templates/includes/_like_dislike_share.htmlto call the required APIs using POST requests.Testing
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores