Classroom Crowdfunding Feature #346
Conversation
WalkthroughThis pull request introduces a classroom crowdfunding feature by adding new models, forms, views, and templates. It includes the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
✨ 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: 5
🧹 Nitpick comments (7)
web/templates/crowdfunding_create.html (1)
1-31: Template Structure & djLint Compliance
The template is well structured, clearly extending the base and defining distinct blocks for title and content. It correctly includes a CSRF token and iterates over the form fields to render them with associated error messages.However, note that the pipeline indicates a djLint linting failure. Please run djLint’s auto-fix (or review its suggestions) to ensure the template formatting conforms to the project’s style guidelines.
🧰 Tools
🪛 GitHub Actions: .github/workflows/test.yml
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
web/templates/crowdfunding_detail.html (1)
1-53: Campaign Detail Display & Donation Form
This new template effectively presents detailed information for a crowdfunding campaign. It conditionally displays the campaign image and video pitch (if available) and shows key details such as the funding goal and amount raised. The donation form is set up correctly with CSRF protection and an appropriate input for the donation amount.A couple of points for consideration:
- The use of the
json_scriptfilter within a<pre>tag for displaying the itemized budget is unconventional. Verify that this is the intended output format; typically,json_scriptis used to safely pass JSON data to JavaScript rather than for direct display.- Ensure that any external video URLs used in the iframe are sanitized and validated on the backend to prevent potential security issues.
Overall, the template is concise and well-organized.
🧰 Tools
🪛 GitHub Actions: .github/workflows/test.yml
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
web/forms.py (2)
59-97: Add CampaignForm to the all list.The newly added CampaignForm class should be included in the all list to make it importable from this module.
__all__ = [ "UserRegistrationForm", "ProfileForm", "ChallengeSubmissionForm", "CourseCreationForm", "CourseForm", "PeerChallengeForm", "PeerChallengeInvitationForm", "SessionForm", "ReviewForm", "CourseMaterialForm", "TeacherSignupForm", "ProfileUpdateForm", "CustomLoginForm", "LearnForm", "TeachForm", "InviteStudentForm", "ForumCategoryForm", "ForumTopicForm", "BlogPostForm", "MessageTeacherForm", "FeedbackForm", "GoodsForm", "StorefrontForm", "EducationalVideoForm", "ProgressTrackerForm", "SuccessStoryForm", "TeamGoalForm", "TeamInviteForm", "MemeForm", "QuizForm", "QuizQuestionForm", "QuizOptionFormSet", "TakeQuizForm", "AvatarForm", "GradeableLinkForm", "LinkGradeForm", "AwardAchievementForm", + "CampaignForm", ]
289-293: Add URL validation for video_url field.The video_url field should be validated to ensure it's a proper URL format and potentially check if it's from supported video platforms (YouTube, Vimeo, etc.).
def clean_video_url(self): video_url = self.cleaned_data.get("video_url") if video_url: # Similar to the validation in EducationalVideoForm youtube_pattern = r"^(https?://)?(www\.)?(youtube\.com/watch\?v=|youtu\.be/)[a-zA-Z0-9_-]{11}.*$" vimeo_pattern = r"^(https?://)?(www\.)?vimeo\.com/[0-9]{8,}.*$" if not (re.match(youtube_pattern, video_url) or re.match(vimeo_pattern, video_url)): raise forms.ValidationError("Please enter a valid YouTube or Vimeo URL") return video_urlweb/models.py (2)
28-73: Consider validation and concurrency improvements for crowdfunding.The
Campaignmodel lacks validation to prevent invalid or negativefunding_goaland might experience race conditions when multiple donations occur concurrently. Adding a minimum value validator tofunding_goalhelps ensure non-negative campaign targets. Also, wrappingupdate_amount_raisedlogic in a database transaction or using an F-expression can mitigate race conditions.Example diff to require a positive funding goal (applied to line 38):
-from django.core.validators import MaxValueValidator, MinValueValidator +from django.core.validators import MaxValueValidator, MinValueValidator funding_goal = models.DecimalField( - max_digits=10, decimal_places=2, help_text="Target funding amount in dollars" + max_digits=10, + decimal_places=2, + validators=[MinValueValidator(1.00)], + help_text="Target funding amount in dollars" )
74-88: Enforce a minimum donation amount.Consider preventing zero or negative donations by using a
MinValueValidatoron theamountfield.Example diff (applied to line 84):
-from django.core.validators import MaxValueValidator, MinValueValidator +from django.core.validators import MaxValueValidator, MinValueValidator amount = models.DecimalField( max_digits=10, decimal_places=2, + validators=[MinValueValidator(0.01)], help_text="Donation amount in dollars" )web/urls.py (1)
41-43: Strengthen access control on the create route.Although the view checks
request.user.profile.is_teacher, applying a decorator (e.g.,login_required) or a custom teacher-only decorator at the URL level can promptly prevent unauthorized/unauthenticated access before the view logic runs.Example approach:
from django.contrib.auth.decorators import login_required, user_passes_test def teacher_required(user): return user.is_authenticated and user.profile.is_teacher urlpatterns += i18n_patterns( # ... path("crowdfunding/create/", login_required(user_passes_test(teacher_required)(views.crowdfunding_create)), name="crowdfunding_create"), # ... )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
web/forms.py(2 hunks)web/models.py(1 hunks)web/templates/crowdfunding_create.html(1 hunks)web/templates/crowdfunding_detail.html(1 hunks)web/templates/crowdfunding_list.html(1 hunks)web/templates/dashboard/teacher.html(1 hunks)web/templates/index.html(1 hunks)web/urls.py(1 hunks)web/views.py(3 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
web/models.py (1)
web/forms.py (16)
Meta(260-299)Meta(351-357)Meta(361-390)Meta(436-474)Meta(485-522)Meta(550-556)Meta(560-584)Meta(678-680)Meta(783-803)Meta(826-838)Meta(844-860)Meta(944-981)Meta(987-998)Meta(1015-1020)Meta(1086-1117)Meta(1123-1144)
web/urls.py (1)
web/views.py (3)
crowdfunding_list(1232-1238)crowdfunding_create(1241-1262)crowdfunding_detail(1223-1229)
🪛 GitHub Actions: .github/workflows/test.yml
web/templates/crowdfunding_create.html
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
web/templates/crowdfunding_list.html
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
web/templates/crowdfunding_detail.html
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (7)
web/templates/dashboard/teacher.html (1)
240-246: New Crowdfunding Button Integration
The newly added section (lines 240–246) adds a clear, well-styled "Create Crowdfunding Campaigns" button. It is placed appropriately within the “else” block when no storefront exists, maintaining UI consistency.Ensure that the URL tag
{% url 'crowdfunding_create' %}correctly matches the URL configuration in yourweb/urls.py.web/templates/crowdfunding_list.html (1)
1-30: Active Campaigns Listing Template
This new template cleanly displays active crowdfunding campaigns. The use of conditional logic to check for available campaigns and the grid layout for iterating over campaign objects is effective. The alt attributes for images (using the campaign title) enhance accessibility.Overall, the markup is clear and consistent with typical Django template standards.
🧰 Tools
🪛 GitHub Actions: .github/workflows/test.yml
[error] 1-1: djLint linting for Django failed. Files were modified by this hook.
web/templates/index.html (1)
677-684: Call-to-Action Button for Crowdfunding Campaigns
The addition of the "View Crowdfunding Campaigns" button (lines 677–684) integrates smoothly into the index page. The styling mirrors that of other call-to-action buttons, and it uses the Django URL tag to link to the campaigns list.Please verify that the URL name
'crowdfunding_list'is correctly configured in your URLs module.web/forms.py (1)
17-17: Correct import of new Campaign model.The Campaign model is correctly imported from the models module for use in the new CampaignForm.
web/views.py (3)
63-63: LGTM: Added import for CampaignFormThis import is correctly placed within the forms import group and is needed for the new crowdfunding functionality.
104-104: LGTM: Added import for Campaign modelThis import is correctly placed within the models import group and is required for the new crowdfunding features.
1232-1239: LGTM: Crowdfunding list implementationThe list function correctly filters for approved and live campaigns, ordered by most recent first. This implementation is appropriate and follows the pattern used in other list views throughout the codebase.
| attrs={ | ||
| "placeholder": "Optional JSON field for itemized budget breakdown", | ||
| "class": "appearance-none block w-full px-3 py-2 border border-gray-300 rounded-md placeholder-gray-400 focus:outline-none", | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding JSON validation for itemized_budget field.
Since itemized_budget is described as a JSON field, it would be helpful to add validation to ensure it contains valid JSON when provided. This can prevent errors when processing the data later.
def clean_itemized_budget(self):
itemized_budget = self.cleaned_data.get("itemized_budget")
if itemized_budget:
try:
import json
json.loads(itemized_budget)
except json.JSONDecodeError:
raise forms.ValidationError("Please enter valid JSON for the itemized budget")
return itemized_budget
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
web/models.py (3)
38-41: Consider adding validation for monetary fields.While the model structure is sound, monetary fields should have validation to ensure positive values.
- funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") - amount_raised = models.DecimalField( - max_digits=10, decimal_places=2, default=0, help_text="Total funds raised so far" - ) + funding_goal = models.DecimalField( + max_digits=10, + decimal_places=2, + validators=[MinValueValidator(0.01)], + help_text="Target funding amount in dollars" + ) + amount_raised = models.DecimalField( + max_digits=10, + decimal_places=2, + default=0, + validators=[MinValueValidator(0)], + help_text="Total funds raised so far" + )
83-84: Consider adding notifications when donations are received.Teachers would likely want to be notified when they receive donations for their campaigns.
You could implement this by modifying the CampaignDonation model's save method:
def save(self, *args, **kwargs): is_new = self.pk is None super().save(*args, **kwargs) if is_new: # Update campaign amount self.campaign.update_amount_raised() # Create notification for the teacher Notification.objects.create( user=self.campaign.teacher, title="New Donation Received", message=f"{self.donor_name} donated ${self.amount} to your campaign: {self.campaign.title}", notification_type="success" )
28-89: Ensure the models are registered with Django admin.The Campaign and CampaignDonation models should be registered with the Django admin site for easy management.
Would you like me to generate the admin registration code for these models?
web/views.py (2)
1223-1233: Consider returning a 404 or 403 instead of redirecting.When the campaign is not publicly accessible, you might prefer returning a
403 Forbiddenor a404 Not Foundfor better REST semantics rather than redirecting users to the list page. This approach clarifies to the user (or API client) that the campaign is unavailable rather than simply sending them elsewhere.if not (campaign.approved and campaign.live) and not ( request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff) ): - messages.error(request, "This campaign is not available.") - return redirect("crowdfunding_list") + return HttpResponseForbidden("You do not have permission to access this campaign.")
1235-1242: Add pagination support for listing campaigns.If the list of active campaigns grows large, you may wish to implement pagination or infinite scroll to improve performance and user experience.
def crowdfunding_list(request): campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") + # Example pagination snippet + from django.core.paginator import Paginator + paginator = Paginator(campaigns, 10) + page_number = request.GET.get("page") + page_obj = paginator.get_page(page_number) + return render(request, "crowdfunding_list.html", {"campaigns": page_obj, "is_paginated": True})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/forms.py(2 hunks)web/models.py(1 hunks)web/views.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/forms.py
🧰 Additional context used
🧬 Code Definitions (1)
web/models.py (1)
web/forms.py (21)
Meta(260-295)Meta(357-363)Meta(367-396)Meta(442-480)Meta(491-528)Meta(556-562)Meta(566-590)Meta(684-686)Meta(789-809)Meta(832-844)Meta(850-866)Meta(950-987)Meta(993-1004)Meta(1021-1026)Meta(1092-1123)Meta(1129-1150)save(218-256)save(614-639)save(706-720)save(1343-1356)save(1517-1532)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (7)
web/models.py (3)
28-72: Well-structured Campaign model with comprehensive attributes.The Campaign model effectively captures all essential aspects of a crowdfunding campaign: basic information (title, description), financial details (funding_goal, amount_raised), supporting media (video_url, image), and administrative controls (approved, live).
I especially like the update_amount_raised method for recalculating the total raised amount from related donations.
47-48: Consider default approval flow for campaigns.Both
approvedandlivedefault to True, which means campaigns go live immediately without review. Depending on your moderation requirements, you might want to setapproved=Falseas the default and implement an approval workflow.Is this the intended behavior? If moderation is required before campaigns go public, consider changing the defaults.
74-89: Well-designed CampaignDonation model for tracking contributions.The CampaignDonation model captures all essential donation information and establishes the correct relationship with the Campaign model. The string representation method provides a clear description of each donation record.
web/views.py (4)
63-63: ImportingCampaignFormlooks correct.No issues spotted with this addition.
104-104: ImportingCampaignis valid.This aligns with the crowdfunding feature and is consistent with the usage in subsequent functions.
2170-2171: Confirm the intent of commenting out these decorators.By commenting out
@login_requiredand@teacher_required, theteacher_dashboardview can be accessed by any user, potentially exposing teacher-only functionality to unauthorized users. Ensure this is intentional or revert if access should remain restricted.Do you want to restore these decorators to maintain teacher-only access?
1243-1265: Ensure the approval workflow is correctly handled.Here, the code auto-approves and activates the campaign (
campaign.approved = Trueandcampaign.live = True), bypassing a potential moderation or admin-approval workflow. Confirm that this is desired, as it could allow unreviewed campaigns to become public.Would you like to introduce an extra check or a review step before making campaigns live?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
web/models.py (6)
28-71: Campaign model is well-structured but lacks validation.The Campaign model is well-designed with appropriate fields for a crowdfunding campaign. However, it doesn't include validation for financial fields.
Consider adding validation to ensure
funding_goalis positive:from django.core.validators import MinValueValidator class Campaign(models.Model): # ... - funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") + funding_goal = models.DecimalField( + max_digits=10, + decimal_places=2, + validators=[MinValueValidator(0.01)], + help_text="Target funding amount in dollars" + )
28-71: Consider adding indexing for better performance.The model doesn't define any custom indexes which could impact performance as the number of campaigns grows.
Add appropriate indexes for common query patterns:
class Campaign(models.Model): # ... class Meta: ordering = ["-created_at"] verbose_name = "Crowdfunding Campaign" verbose_name_plural = "Crowdfunding Campaigns" + indexes = [ + models.Index(fields=['teacher']), + models.Index(fields=['approved', 'live']), + models.Index(fields=['-created_at']), + ]
74-88: CampaignDonation model needs validation for donation amount.The CampaignDonation model is missing validation to ensure the donation amount is positive.
Add validation to ensure
amountis positive:class CampaignDonation(models.Model): # ... - amount = models.DecimalField(max_digits=10, decimal_places=2, help_text="Donation amount in dollars") + amount = models.DecimalField( + max_digits=10, + decimal_places=2, + validators=[MinValueValidator(0.01)], + help_text="Donation amount in dollars" + )
74-88: Consider adding a donor privacy option.For donor privacy, it might be good to add an option for anonymous donations.
class CampaignDonation(models.Model): # ... donor_name = models.CharField(max_length=255, help_text="Name of the donor") donor_email = models.EmailField(help_text="Email address of the donor") + is_anonymous = models.BooleanField(default=False, help_text="Whether the donation should be displayed as anonymous") amount = models.DecimalField(max_digits=10, decimal_places=2, help_text="Donation amount in dollars") donated_at = models.DateTimeField(auto_now_add=True) def __str__(self): + donor_display = "Anonymous" if self.is_anonymous else self.donor_name - return f"{self.donor_name} donated ${self.amount} to {self.campaign.title}" + return f"{donor_display} donated ${self.amount} to {self.campaign.title}"
28-71: Consider adding progress tracking functionality.The model tracks
funding_goalandamount_raisedbut doesn't provide a method to calculate progress percentage.Add a property method to calculate the funding progress percentage:
class Campaign(models.Model): # ...existing code... + @property + def progress_percentage(self): + """ + Returns the percentage of the funding goal that has been raised. + """ + if self.funding_goal == 0: + return 0 + return min(100, int((self.amount_raised / self.funding_goal) * 100))
28-71: Missing campaign end date functionality.The model doesn't track when a campaign should end, which is a common feature for crowdfunding campaigns.
Add an end date field and a property to check if the campaign is active:
class Campaign(models.Model): # ... live = models.BooleanField(default=True, help_text="Indicates whether the campaign is live and accepting donations") created_at = models.DateTimeField(auto_now_add=True) + end_date = models.DateTimeField(null=True, blank=True, help_text="Date when the campaign ends") # ... + @property + def is_active(self): + """ + Returns whether the campaign is currently active. + """ + now = timezone.now() + return ( + self.approved and + self.live and + (not self.end_date or self.end_date > now) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/models.py(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/models.py (1)
web/forms.py (16)
Meta(260-295)Meta(357-363)Meta(367-396)Meta(442-480)Meta(491-528)Meta(556-562)Meta(566-590)Meta(684-686)Meta(789-809)Meta(832-844)Meta(850-866)Meta(950-987)Meta(993-1004)Meta(1021-1026)Meta(1092-1123)Meta(1129-1150)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (3)
web/models.py (3)
65-71: Excellent implementation of the update_amount_raised method.The method efficiently uses aggregation to calculate the total amount raised from donations.
2790-2798: Good use of Django signals to keep donation totals in sync.The signal handlers ensure that donation totals are automatically updated whenever donations are created, modified, or deleted.
33-35:Details
❓ Verification inconclusive
Potential performance concern with related_name choice.
The
related_name="campaigns"could conflict with other models if they also use the same related name for their relations to User.Run the following to check for any other models with the same related name:
🏁 Script executed:
#!/bin/bash # Check for other models that might have a related_name="campaigns" on a ForeignKey to User rg "models\.ForeignKey.*User.*related_name=\"campaigns\"" --type pyLength of output: 67
Attention: Verify
related_nameUniquenessAfter running a search for occurrences of
related_name="campaigns"across the codebase, no instances beyond the one inweb/models.py(Lines 33–35) were found. However, because the initial script output was low-quality (i.e. no output despite expecting to find potential conflicts), please manually verify that no other models are inadvertently using the same reverse relation name on theUsermodel. Consistent uniqueness of reverse relation names is essential to avoid potential performance or lookup conflicts as the codebase grows.
- Confirm that no additional models are using a conflicting
related_name="campaigns".- Ensure that any future additions linking to
Useralso follow a unique naming convention.
|
feedback given and ready for you @debadityaroy99 |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (1)
web/forms.py (6)
CampaignForm(259-306)save(218-256)save(614-639)save(706-720)save(1343-1356)save(1517-1532)
🪛 Ruff (0.8.2)
web/views.py
1247-1247: Missing return type annotation for public function crowdfunding_detail
(ANN201)
1247-1247: Missing type annotation for function argument request
(ANN001)
1247-1247: Missing type annotation for function argument campaign_id
(ANN001)
1259-1259: Missing return type annotation for public function crowdfunding_list
(ANN201)
1259-1259: Missing type annotation for function argument request
(ANN001)
1269-1269: Missing return type annotation for public function crowdfunding_create
(ANN201)
1269-1269: Missing type annotation for function argument request
(ANN001)
1284-1284: Unnecessary else after return statement
Remove unnecessary else
(RET505)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (4)
web/views.py (4)
1247-1257: Proper access control implementation - looks good!The detail view correctly implements access control to ensure that only approved and live campaigns are visible to regular users, while campaign creators or admins can see their own unapproved campaigns. The error message and redirect path are appropriate.
🧰 Tools
🪛 Ruff (0.8.2)
1247-1247: Missing return type annotation for public function
crowdfunding_detail(ANN201)
1247-1247: Missing type annotation for function argument
request(ANN001)
1247-1247: Missing type annotation for function argument
campaign_id(ANN001)
1259-1266: Well-structured campaign listing function.The function effectively retrieves and displays all active campaigns that are both approved and live. The ordering by creation date (newest first) provides a good default sorting.
🧰 Tools
🪛 Ruff (0.8.2)
1259-1259: Missing return type annotation for public function
crowdfunding_list(ANN201)
1259-1259: Missing type annotation for function argument
request(ANN001)
63-63: Correct form import added.Adding the import for
CampaignFormmaintains the alphabetical ordering of imports.
104-104: Correct model import added.Adding the import for
Campaignmaintains the alphabetical ordering of imports.
| # @login_required | ||
| # @teacher_required | ||
| def teacher_dashboard(request): |
There was a problem hiding this comment.
Security issue: Teacher dashboard protection removed.
The @login_required and @teacher_required decorators have been commented out from the teacher_dashboard function. This removes authentication and authorization protection, potentially allowing any user (even unauthenticated ones) to access the teacher dashboard.
Apply this diff to restore the security measures:
-# @login_required
-# @teacher_required
+@login_required
+@teacher_required
def teacher_dashboard(request):📝 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.
| # @login_required | |
| # @teacher_required | |
| def teacher_dashboard(request): | |
| @login_required | |
| @teacher_required | |
| def teacher_dashboard(request): |
🧰 Tools
🪛 Ruff (0.8.2)
2196-2196: Missing return type annotation for public function teacher_dashboard
(ANN201)
2196-2196: Missing type annotation for function argument request
(ANN001)
|
@coderabbitai code the stripe payment for the crowdfunding feature |
|
Tip For best results, initiate chat on the files or code changes. Code Analysis ToolsTool Analysis ResultsCodeRabbit ran static analysis tools on your files and found the following: Details🪛 Ruff (0.8.2)web/forms.py262-262: Mutable class attributes should be annotated with (RUF012) 263-295: Mutable class attributes should be annotated with (RUF012) 266-266: Trailing comma missing Add trailing comma (COM812) 267-267: Trailing comma missing Add trailing comma (COM812) 272-272: Trailing comma missing Add trailing comma (COM812) 277-277: Trailing comma missing Add trailing comma (COM812) 281-281: Trailing comma missing Add trailing comma (COM812) 282-282: Trailing comma missing Add trailing comma (COM812) 288-288: Trailing comma missing Add trailing comma (COM812) 293-293: Trailing comma missing Add trailing comma (COM812) 296-300: Mutable class attributes should be annotated with (RUF012) 302-302: Missing return type annotation for private function (ANN202) 305-305: Avoid specifying long messages outside the exception class (TRY003) web/models.py34-34: Trailing comma missing Add trailing comma (COM812) 40-40: Trailing comma missing Add trailing comma (COM812) 43-43: Trailing comma missing Add trailing comma (COM812) 45-45: Avoid using (DJ001) 52-52: Mutable class attributes should be annotated with (RUF012) 56-56: Missing return type annotation for special method Add return type annotation: (ANN204) 59-59: Missing return type annotation for public function (ANN201) 65-65: Missing return type annotation for public function Add return type annotation: (ANN201) 80-80: Trailing comma missing Add trailing comma (COM812) 87-87: Missing return type annotation for special method Add return type annotation: (ANN204) 2791-2791: Missing return type annotation for public function Add return type annotation: (ANN201) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for (ANN003) 2791-2791: Unused function argument: (ARG001) 2796-2796: Missing return type annotation for public function Add return type annotation: (ANN201) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Unused function argument: (ARG001) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Missing type annotation for (ANN003) 2796-2796: Unused function argument: (ARG001) web/views.py1247-1247: Missing return type annotation for public function (ANN201) 1247-1247: Missing type annotation for function argument (ANN001) 1247-1247: Missing type annotation for function argument (ANN001) 1259-1259: Missing return type annotation for public function (ANN201) 1259-1259: Missing type annotation for function argument (ANN001) 1269-1269: Missing return type annotation for public function (ANN201) 1269-1269: Missing type annotation for function argument (ANN001) 1284-1284: Unnecessary Remove unnecessary (RET505) Files with Issuesweb/forms.py🪛 Ruff (0.8.2)262-262: Mutable class attributes should be annotated with (RUF012) 263-295: Mutable class attributes should be annotated with (RUF012) 266-266: Trailing comma missing Add trailing comma (COM812) 267-267: Trailing comma missing Add trailing comma (COM812) 272-272: Trailing comma missing Add trailing comma (COM812) 277-277: Trailing comma missing Add trailing comma (COM812) 281-281: Trailing comma missing Add trailing comma (COM812) 282-282: Trailing comma missing Add trailing comma (COM812) 288-288: Trailing comma missing Add trailing comma (COM812) 293-293: Trailing comma missing Add trailing comma (COM812) 296-300: Mutable class attributes should be annotated with (RUF012) 302-302: Missing return type annotation for private function (ANN202) 305-305: Avoid specifying long messages outside the exception class (TRY003) web/models.py🪛 Ruff (0.8.2)34-34: Trailing comma missing Add trailing comma (COM812) 40-40: Trailing comma missing Add trailing comma (COM812) 43-43: Trailing comma missing Add trailing comma (COM812) 45-45: Avoid using (DJ001) 52-52: Mutable class attributes should be annotated with (RUF012) 56-56: Missing return type annotation for special method Add return type annotation: (ANN204) 59-59: Missing return type annotation for public function (ANN201) 65-65: Missing return type annotation for public function Add return type annotation: (ANN201) 80-80: Trailing comma missing Add trailing comma (COM812) 87-87: Missing return type annotation for special method Add return type annotation: (ANN204) 2791-2791: Missing return type annotation for public function Add return type annotation: (ANN201) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for (ANN003) 2791-2791: Unused function argument: (ARG001) 2796-2796: Missing return type annotation for public function Add return type annotation: (ANN201) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Unused function argument: (ARG001) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Missing type annotation for (ANN003) 2796-2796: Unused function argument: (ARG001) web/views.py🪛 Ruff (0.8.2)1247-1247: Missing return type annotation for public function (ANN201) 1247-1247: Missing type annotation for function argument (ANN001) 1247-1247: Missing type annotation for function argument (ANN001) 1259-1259: Missing return type annotation for public function (ANN201) 1259-1259: Missing type annotation for function argument (ANN001) 1269-1269: Missing return type annotation for public function (ANN201) 1269-1269: Missing type annotation for function argument (ANN001) 1284-1284: Unnecessary Remove unnecessary (RET505) Next StepsBased on these results, you can:
|
|
Tip The multi-step agents feature is in experimental mode. A summary of the edits CodeRabbit can apply:
A summary of the context that CodeRabbit is considering across the codebase:
Code Analysis ToolsTool Analysis ResultsCodeRabbit ran static analysis tools on your files and found the following: Details🪛 Ruff (0.8.2)web/forms.py262-262: Mutable class attributes should be annotated with (RUF012) 263-295: Mutable class attributes should be annotated with (RUF012) 266-266: Trailing comma missing Add trailing comma (COM812) 267-267: Trailing comma missing Add trailing comma (COM812) 272-272: Trailing comma missing Add trailing comma (COM812) 277-277: Trailing comma missing Add trailing comma (COM812) 281-281: Trailing comma missing Add trailing comma (COM812) 282-282: Trailing comma missing Add trailing comma (COM812) 288-288: Trailing comma missing Add trailing comma (COM812) 293-293: Trailing comma missing Add trailing comma (COM812) 296-300: Mutable class attributes should be annotated with (RUF012) 302-302: Missing return type annotation for private function (ANN202) 305-305: Avoid specifying long messages outside the exception class (TRY003) web/models.py34-34: Trailing comma missing Add trailing comma (COM812) 40-40: Trailing comma missing Add trailing comma (COM812) 43-43: Trailing comma missing Add trailing comma (COM812) 45-45: Avoid using (DJ001) 52-52: Mutable class attributes should be annotated with (RUF012) 56-56: Missing return type annotation for special method Add return type annotation: (ANN204) 59-59: Missing return type annotation for public function (ANN201) 65-65: Missing return type annotation for public function Add return type annotation: (ANN201) 80-80: Trailing comma missing Add trailing comma (COM812) 87-87: Missing return type annotation for special method Add return type annotation: (ANN204) 2791-2791: Missing return type annotation for public function Add return type annotation: (ANN201) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for (ANN003) 2791-2791: Unused function argument: (ARG001) 2796-2796: Missing return type annotation for public function Add return type annotation: (ANN201) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Unused function argument: (ARG001) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Missing type annotation for (ANN003) 2796-2796: Unused function argument: (ARG001) web/views.py1247-1247: Missing return type annotation for public function (ANN201) 1247-1247: Missing type annotation for function argument (ANN001) 1247-1247: Missing type annotation for function argument (ANN001) 1259-1259: Missing return type annotation for public function (ANN201) 1259-1259: Missing type annotation for function argument (ANN001) 1269-1269: Missing return type annotation for public function (ANN201) 1269-1269: Missing type annotation for function argument (ANN001) 1284-1284: Unnecessary Remove unnecessary (RET505) Files with Issuesweb/forms.py🪛 Ruff (0.8.2)262-262: Mutable class attributes should be annotated with (RUF012) 263-295: Mutable class attributes should be annotated with (RUF012) 266-266: Trailing comma missing Add trailing comma (COM812) 267-267: Trailing comma missing Add trailing comma (COM812) 272-272: Trailing comma missing Add trailing comma (COM812) 277-277: Trailing comma missing Add trailing comma (COM812) 281-281: Trailing comma missing Add trailing comma (COM812) 282-282: Trailing comma missing Add trailing comma (COM812) 288-288: Trailing comma missing Add trailing comma (COM812) 293-293: Trailing comma missing Add trailing comma (COM812) 296-300: Mutable class attributes should be annotated with (RUF012) 302-302: Missing return type annotation for private function (ANN202) 305-305: Avoid specifying long messages outside the exception class (TRY003) web/models.py🪛 Ruff (0.8.2)34-34: Trailing comma missing Add trailing comma (COM812) 40-40: Trailing comma missing Add trailing comma (COM812) 43-43: Trailing comma missing Add trailing comma (COM812) 45-45: Avoid using (DJ001) 52-52: Mutable class attributes should be annotated with (RUF012) 56-56: Missing return type annotation for special method Add return type annotation: (ANN204) 59-59: Missing return type annotation for public function (ANN201) 65-65: Missing return type annotation for public function Add return type annotation: (ANN201) 80-80: Trailing comma missing Add trailing comma (COM812) 87-87: Missing return type annotation for special method Add return type annotation: (ANN204) 2791-2791: Missing return type annotation for public function Add return type annotation: (ANN201) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Missing type annotation for function argument (ANN001) 2791-2791: Unused function argument: (ARG001) 2791-2791: Missing type annotation for (ANN003) 2791-2791: Unused function argument: (ARG001) 2796-2796: Missing return type annotation for public function Add return type annotation: (ANN201) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Unused function argument: (ARG001) 2796-2796: Missing type annotation for function argument (ANN001) 2796-2796: Missing type annotation for (ANN003) 2796-2796: Unused function argument: (ARG001) web/views.py🪛 Ruff (0.8.2)1247-1247: Missing return type annotation for public function (ANN201) 1247-1247: Missing type annotation for function argument (ANN001) 1247-1247: Missing type annotation for function argument (ANN001) 1259-1259: Missing return type annotation for public function (ANN201) 1259-1259: Missing type annotation for function argument (ANN001) 1269-1269: Missing return type annotation for public function (ANN201) 1269-1269: Missing type annotation for function argument (ANN001) 1284-1284: Unnecessary Remove unnecessary (RET505) Next StepsBased on these results, you can:
|
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
web/urls.py (1)
41-61: 💡 Verification agent🧩 Analysis chain
Consider adding URL patterns for donation success and cancellation.
The crowdfunding feature includes a redirect to "donation_success" in the template, but there doesn't appear to be a specific URL pattern for handling successful crowdfunding donations.
Let's check if separate success/cancel routes are needed for crowdfunding donations:
🏁 Script executed:
#!/bin/bash # Check if donation_success view has handling for campaign donations # Look for donation_success view implementation rg -A 10 "def donation_success" web/views.py # Check for references to crowdfunding in donation success handling rg -A 3 "donation_success.*crowdfund" web/views.pyLength of output: 480
Action Required: Separate Endpoints for Crowdfunding Donation Flows
The current URL patterns provide generic donation endpoints:
/donate/success/→ handled bydonation_success/donate/cancel/→ handled bydonation_cancelHowever, the crowdfunding flow (via templates and the crowdfunding donation intent at
/crowdfunding/<int:campaign_id>/donate/) redirects to the generic donation success endpoint. Thedonation_successview implementation confirms it handles only generic donation logic (no crowdfunding-specific logic is present).Recommendations:
- Dedicated Crowdfunding Endpoints: Consider adding specific URL patterns (e.g.,
/crowdfunding/<int:campaign_id>/donate/success/and/crowdfunding/<int:campaign_id>/donate/cancel/) with accompanying view logic that can handle any nuances required for crowdfunding donations.- Alternative Approach: If you intend to use the generic donation endpoints for both donation types, update the
donation_success(and potentiallydonation_cancel) view to correctly differentiate and handle crowdfunding donations versus regular donations.
♻️ Duplicate comments (3)
web/views.py (3)
2294-2296:⚠️ Potential issueSecurity issue: Teacher dashboard protection removed
The
@login_requiredand@teacher_requireddecorators have been commented out from theteacher_dashboardfunction. This removes authentication and authorization protection, potentially allowing any user (even unauthenticated ones) to access the teacher dashboard.Apply this diff to restore the security measures:
-# @login_required -# @teacher_required +@login_required +@teacher_required def teacher_dashboard(request):🧰 Tools
🪛 Ruff (0.8.2)
2296-2296: Missing return type annotation for public function
teacher_dashboard(ANN201)
2296-2296: Missing type annotation for function argument
request(ANN001)
1359-1366: 🧹 Nitpick (assertive)Add type annotations to
crowdfunding_listThe function implementation is straightforward and correct, but requires type annotations.
-def crowdfunding_list(request): +def crowdfunding_list(request: HttpRequest) -> HttpResponse: """ Display all active crowdfunding campaigns. Active campaigns are those that are approved and live. """ campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") return render(request, "crowdfunding_list.html", {"campaigns": campaigns})🧰 Tools
🪛 Ruff (0.8.2)
1359-1359: Missing return type annotation for public function
crowdfunding_list(ANN201)
1359-1359: Missing type annotation for function argument
request(ANN001)
1367-1389: 🧹 Nitpick (assertive)Add type annotations and remove unnecessary
elseafterreturnincrowdfunding_createThe implementation of the campaign creation function is good, with proper authentication, authorization, and message handling. However, it needs type annotations and has an unnecessary
elseblock after areturnstatement.Here's how to improve the function:
@login_required @teacher_required -def crowdfunding_create(request): +def crowdfunding_create(request: HttpRequest) -> HttpResponse: """ View to allow teachers to create a new crowdfunding campaign. """ if request.method == "POST": form = CampaignForm(request.POST, request.FILES) if form.is_valid(): campaign = form.save(commit=False) campaign.teacher = request.user # Set initial approval status (set to False if admin approval is required) campaign.approved = True # or False if approval workflow is needed campaign.live = True # or False if campaigns shouldn't be live immediately campaign.save() messages.success(request, "Your crowdfunding campaign has been created successfully!") return redirect("crowdfunding_detail", campaign_id=campaign.id) - else: - messages.error(request, "Please correct the errors below.") + messages.error(request, "Please correct the errors below.") else: form = CampaignForm() return render(request, "crowdfunding_create.html", {"form": form})You might also want to consider adding validation checks for the form and a more informative error message that points out specific fields that have issues.
🧰 Tools
🪛 Ruff (0.8.2)
1369-1369: Missing return type annotation for public function
crowdfunding_create(ANN201)
1369-1369: Missing type annotation for function argument
request(ANN001)
1384-1384: Unnecessary
elseafterreturnstatementRemove unnecessary
else(RET505)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
web/templates/crowdfunding_detail.html(1 hunks)web/urls.py(2 hunks)web/views.py(6 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/urls.py (1)
web/views.py (4)
crowdfunding_list(1359-1365)crowdfunding_create(1369-1388)crowdfunding_detail(1346-1356)create_donation_intent(1174-1206)
🪛 GitHub Actions: .github/workflows/test.yml
web/templates/crowdfunding_detail.html
[error] 1-1: Trailing whitespace found and removed by the trailing-whitespace hook.
web/views.py
[error] 1-1: Trailing whitespace found and removed by the trailing-whitespace hook.
🪛 Ruff (0.8.2)
web/views.py
1174-1174: Missing return type annotation for public function create_donation_intent
(ANN201)
1174-1174: Missing type annotation for function argument request
(ANN001)
1174-1174: Missing type annotation for function argument campaign_id
(ANN001)
1202-1202: Do not catch blind exception: Exception
(BLE001)
1255-1255: Missing return type annotation for public function handle_successful_donation
Add return type annotation: None
(ANN201)
1255-1255: Missing type annotation for function argument payment_intent
(ANN001)
1275-1275: Local variable donation is assigned to but never used
Remove assignment to unused variable donation
(F841)
1291-1291: Do not catch blind exception: Exception
(BLE001)
1294-1294: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1294-1294: Logging statement uses f-string
(G004)
1297-1297: Missing return type annotation for public function handle_failed_donation
Add return type annotation: None
(ANN201)
1297-1297: Missing type annotation for function argument payment_intent
(ANN001)
1307-1307: Logging statement uses f-string
(G004)
1308-1308: Do not catch blind exception: Exception
(BLE001)
1310-1310: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1310-1310: Logging statement uses f-string
(G004)
1346-1346: Missing return type annotation for public function crowdfunding_detail
(ANN201)
1346-1346: Missing type annotation for function argument request
(ANN001)
1346-1346: Missing type annotation for function argument campaign_id
(ANN001)
1354-1354: Trailing comma missing
Add trailing comma
(COM812)
1359-1359: Missing return type annotation for public function crowdfunding_list
(ANN201)
1359-1359: Missing type annotation for function argument request
(ANN001)
1369-1369: Missing return type annotation for public function crowdfunding_create
(ANN201)
1369-1369: Missing type annotation for function argument request
(ANN001)
1384-1384: Unnecessary else after return statement
Remove unnecessary else
(RET505)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (2)
web/views.py (1)
1330-1340: Good implementation of payment intent handling in webhookThe integration of donation-specific handling in the webhook is well implemented. The code correctly checks if the payment intent is for a course or a donation campaign.
web/urls.py (1)
41-43: The crowdfunding URL structure looks good.The URL patterns for the crowdfunding list, creation, and detail views are well-organized and follow the application's URL naming conventions.
| def handle_failed_donation(payment_intent): | ||
| """ | ||
| Handle a failed donation payment. | ||
| You can log the failure or notify the donor that the payment did not go through. | ||
| """ | ||
| try: | ||
| campaign_id = payment_intent.metadata.get("campaign_id") | ||
| user_id = payment_intent.metadata.get("user_id") | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| logger.warning(f"Donation payment failed for campaign {campaign_id} and user {user_id}.") | ||
| except Exception as e: | ||
| logger = logging.getLogger(__name__) | ||
| logger.error(f"Error handling failed donation: {e}") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type annotations and improve error handling in handle_failed_donation
Similar to the previous function, this handler needs some improvements:
- Add type annotations
- Improve exception handling
- Use
logging.exceptioninstead oflogging.error
Here's the improved implementation:
-def handle_failed_donation(payment_intent):
+def handle_failed_donation(payment_intent) -> None:
"""
Handle a failed donation payment.
You can log the failure or notify the donor that the payment did not go through.
"""
try:
campaign_id = payment_intent.metadata.get("campaign_id")
user_id = payment_intent.metadata.get("user_id")
logger = logging.getLogger(__name__)
- logger.warning(f"Donation payment failed for campaign {campaign_id} and user {user_id}.")
+ logger.warning("Donation payment failed for campaign %s and user %s", campaign_id, user_id)
except Exception as e:
logger = logging.getLogger(__name__)
- logger.error(f"Error handling failed donation: {e}")
+ logger.exception("Error handling failed donation")📝 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.
| def handle_failed_donation(payment_intent): | |
| """ | |
| Handle a failed donation payment. | |
| You can log the failure or notify the donor that the payment did not go through. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| logger = logging.getLogger(__name__) | |
| logger.warning(f"Donation payment failed for campaign {campaign_id} and user {user_id}.") | |
| except Exception as e: | |
| logger = logging.getLogger(__name__) | |
| logger.error(f"Error handling failed donation: {e}") | |
| def handle_failed_donation(payment_intent) -> None: | |
| """ | |
| Handle a failed donation payment. | |
| You can log the failure or notify the donor that the payment did not go through. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| logger = logging.getLogger(__name__) | |
| logger.warning("Donation payment failed for campaign %s and user %s", campaign_id, user_id) | |
| except Exception as e: | |
| logger = logging.getLogger(__name__) | |
| logger.exception("Error handling failed donation") |
🧰 Tools
🪛 Ruff (0.8.2)
1297-1297: Missing return type annotation for public function handle_failed_donation
Add return type annotation: None
(ANN201)
1297-1297: Missing type annotation for function argument payment_intent
(ANN001)
1307-1307: Logging statement uses f-string
(G004)
1308-1308: Do not catch blind exception: Exception
(BLE001)
1310-1310: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1310-1310: Logging statement uses f-string
(G004)
| def crowdfunding_detail(request, campaign_id): | ||
| campaign = get_object_or_404(Campaign, id=campaign_id) | ||
| # Check if campaign is approved and live or if user has special access | ||
| if not (campaign.approved and campaign.live) and not (request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff)): | ||
| messages.error(request, "This campaign is not available.") | ||
| return redirect("crowdfunding_list") | ||
| context = { | ||
| "campaign": campaign, | ||
| "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY | ||
| } | ||
| return render(request, "crowdfunding_detail.html", context) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type annotations and fix missing trailing comma in crowdfunding_detail
This function is well-structured and includes proper permission checks. It needs type annotations and has a missing trailing comma in the context dictionary.
Here's how to improve it:
-def crowdfunding_detail(request, campaign_id):
+def crowdfunding_detail(request: HttpRequest, campaign_id: int) -> HttpResponse:
campaign = get_object_or_404(Campaign, id=campaign_id)
# Check if campaign is approved and live or if user has special access
if not (campaign.approved and campaign.live) and not (request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff)):
messages.error(request, "This campaign is not available.")
return redirect("crowdfunding_list")
context = {
"campaign": campaign,
- "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY
+ "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY,
}
return render(request, "crowdfunding_detail.html", context)📝 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.
| def crowdfunding_detail(request, campaign_id): | |
| campaign = get_object_or_404(Campaign, id=campaign_id) | |
| # Check if campaign is approved and live or if user has special access | |
| if not (campaign.approved and campaign.live) and not (request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff)): | |
| messages.error(request, "This campaign is not available.") | |
| return redirect("crowdfunding_list") | |
| context = { | |
| "campaign": campaign, | |
| "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY | |
| } | |
| return render(request, "crowdfunding_detail.html", context) | |
| def crowdfunding_detail(request: HttpRequest, campaign_id: int) -> HttpResponse: | |
| campaign = get_object_or_404(Campaign, id=campaign_id) | |
| # Check if campaign is approved and live or if user has special access | |
| if not (campaign.approved and campaign.live) and not (request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff)): | |
| messages.error(request, "This campaign is not available.") | |
| return redirect("crowdfunding_list") | |
| context = { | |
| "campaign": campaign, | |
| "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY, | |
| } | |
| return render(request, "crowdfunding_detail.html", context) |
🧰 Tools
🪛 Ruff (0.8.2)
1346-1346: Missing return type annotation for public function crowdfunding_detail
(ANN201)
1346-1346: Missing type annotation for function argument request
(ANN001)
1346-1346: Missing type annotation for function argument campaign_id
(ANN001)
1354-1354: Trailing comma missing
Add trailing comma
(COM812)
| def handle_successful_donation(payment_intent): | ||
| """ | ||
| Handle a successful donation payment. | ||
| Create a Donation record and update the campaign's amount raised. | ||
| """ | ||
| try: | ||
| campaign_id = payment_intent.metadata.get("campaign_id") | ||
| user_id = payment_intent.metadata.get("user_id") | ||
| if not campaign_id or not user_id: | ||
| # Not a donation payment; you might log this or ignore. | ||
| return | ||
|
|
||
| # Convert amount from cents to dollars. | ||
| donation_amount = Decimal(payment_intent.amount_received) / 100 | ||
|
|
||
| # Retrieve campaign and user. | ||
| campaign = Campaign.objects.get(id=campaign_id) | ||
| user = User.objects.get(id=user_id) | ||
|
|
||
| # Create the donation record. | ||
| donation = Donation.objects.create( | ||
| campaign=campaign, | ||
| donor_name=user.get_full_name() or user.username, | ||
| donor_email=user.email, | ||
| amount=donation_amount, | ||
| status="completed", # or set appropriate status if your Donation model uses one | ||
| stripe_payment_intent_id=payment_intent.id, | ||
| ) | ||
|
|
||
| # Update the campaign's amount raised. | ||
| campaign.update_amount_raised() | ||
|
|
||
| # Optionally, you can send a notification email to the donor or campaign teacher here. | ||
| # For example: | ||
| # send_donation_confirmation_email(user, campaign, donation) | ||
|
|
||
| except Exception as e: | ||
| # Log error if needed. | ||
| logger = logging.getLogger(__name__) | ||
| logger.error(f"Error handling successful donation: {e}") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type annotations and improve error handling in handle_successful_donation
The function contains good functionality for handling successful donations but needs some refinements:
- Add type annotations
- Use more specific exception handling
- The created
donationvariable is assigned but never used - Use
logging.exceptioninstead oflogging.errorfor better stack traces
Here's an improved implementation:
-def handle_successful_donation(payment_intent):
+def handle_successful_donation(payment_intent) -> None:
"""
Handle a successful donation payment.
Create a Donation record and update the campaign's amount raised.
"""
try:
campaign_id = payment_intent.metadata.get("campaign_id")
user_id = payment_intent.metadata.get("user_id")
if not campaign_id or not user_id:
# Not a donation payment; you might log this or ignore.
return
# Convert amount from cents to dollars.
donation_amount = Decimal(payment_intent.amount_received) / 100
# Retrieve campaign and user.
campaign = Campaign.objects.get(id=campaign_id)
user = User.objects.get(id=user_id)
# Create the donation record.
- donation = Donation.objects.create(
+ Donation.objects.create(
campaign=campaign,
donor_name=user.get_full_name() or user.username,
donor_email=user.email,
amount=donation_amount,
status="completed", # or set appropriate status if your Donation model uses one
stripe_payment_intent_id=payment_intent.id,
)
# Update the campaign's amount raised.
campaign.update_amount_raised()
# Optionally, you can send a notification email to the donor or campaign teacher here.
# For example:
# send_donation_confirmation_email(user, campaign, donation)
- except Exception as e:
+ except (Campaign.DoesNotExist, User.DoesNotExist) as e:
# Log error if needed.
logger = logging.getLogger(__name__)
- logger.error(f"Error handling successful donation: {e}")
+ logger.error(f"Campaign or User not found: {e}")
+ except Exception as e:
+ # Log error if needed.
+ logger = logging.getLogger(__name__)
+ logger.exception("Error handling successful donation")📝 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.
| def handle_successful_donation(payment_intent): | |
| """ | |
| Handle a successful donation payment. | |
| Create a Donation record and update the campaign's amount raised. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| if not campaign_id or not user_id: | |
| # Not a donation payment; you might log this or ignore. | |
| return | |
| # Convert amount from cents to dollars. | |
| donation_amount = Decimal(payment_intent.amount_received) / 100 | |
| # Retrieve campaign and user. | |
| campaign = Campaign.objects.get(id=campaign_id) | |
| user = User.objects.get(id=user_id) | |
| # Create the donation record. | |
| donation = Donation.objects.create( | |
| campaign=campaign, | |
| donor_name=user.get_full_name() or user.username, | |
| donor_email=user.email, | |
| amount=donation_amount, | |
| status="completed", # or set appropriate status if your Donation model uses one | |
| stripe_payment_intent_id=payment_intent.id, | |
| ) | |
| # Update the campaign's amount raised. | |
| campaign.update_amount_raised() | |
| # Optionally, you can send a notification email to the donor or campaign teacher here. | |
| # For example: | |
| # send_donation_confirmation_email(user, campaign, donation) | |
| except Exception as e: | |
| # Log error if needed. | |
| logger = logging.getLogger(__name__) | |
| logger.error(f"Error handling successful donation: {e}") | |
| def handle_successful_donation(payment_intent) -> None: | |
| """ | |
| Handle a successful donation payment. | |
| Create a Donation record and update the campaign's amount raised. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| if not campaign_id or not user_id: | |
| # Not a donation payment; you might log this or ignore. | |
| return | |
| # Convert amount from cents to dollars. | |
| donation_amount = Decimal(payment_intent.amount_received) / 100 | |
| # Retrieve campaign and user. | |
| campaign = Campaign.objects.get(id=campaign_id) | |
| user = User.objects.get(id=user_id) | |
| # Create the donation record. | |
| Donation.objects.create( | |
| campaign=campaign, | |
| donor_name=user.get_full_name() or user.username, | |
| donor_email=user.email, | |
| amount=donation_amount, | |
| status="completed", # or set appropriate status if your Donation model uses one | |
| stripe_payment_intent_id=payment_intent.id, | |
| ) | |
| # Update the campaign's amount raised. | |
| campaign.update_amount_raised() | |
| # Optionally, you can send a notification email to the donor or campaign teacher here. | |
| # For example: | |
| # send_donation_confirmation_email(user, campaign, donation) | |
| except (Campaign.DoesNotExist, User.DoesNotExist) as e: | |
| # Log error if needed. | |
| logger = logging.getLogger(__name__) | |
| logger.error(f"Campaign or User not found: {e}") | |
| except Exception as e: | |
| # Log error if needed. | |
| logger = logging.getLogger(__name__) | |
| logger.exception("Error handling successful donation") |
🧰 Tools
🪛 Ruff (0.8.2)
1255-1255: Missing return type annotation for public function handle_successful_donation
Add return type annotation: None
(ANN201)
1255-1255: Missing type annotation for function argument payment_intent
(ANN001)
1275-1275: Local variable donation is assigned to but never used
Remove assignment to unused variable donation
(F841)
1291-1291: Do not catch blind exception: Exception
(BLE001)
1294-1294: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1294-1294: Logging statement uses f-string
(G004)
| <input type="number" id="amount" name="amount" step="0.01" min="1" required | ||
| class="border rounded p-2 w-full mb-4"> |
There was a problem hiding this comment.
Fix trailing whitespace issue.
There's a trailing whitespace at the end of line 46 that needs to be removed, as indicated by the pipeline failure.
Apply this diff to fix the whitespace issue:
- <input type="number" id="amount" name="amount" step="0.01" min="1" required
+ <input type="number" id="amount" name="amount" step="0.01" min="1" required
class="border rounded p-2 w-full mb-4">📝 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.
| <input type="number" id="amount" name="amount" step="0.01" min="1" required | |
| class="border rounded p-2 w-full mb-4"> | |
| <input type="number" id="amount" name="amount" step="0.01" min="1" required | |
| class="border rounded p-2 w-full mb-4"> |
| // Initialize Stripe with your publishable key. | ||
| var stripe = Stripe("{{ STRIPE_PUBLISHABLE_KEY }}"); | ||
| var elements = stripe.elements(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace hardcoded Stripe initialization with environment variable check.
The Stripe initialization is using the publishable key directly from template context, but there's no visible verification that the key exists before initialization.
Add a check before initializing Stripe:
// Initialize Stripe with your publishable key.
- var stripe = Stripe("{{ STRIPE_PUBLISHABLE_KEY }}");
+ {% if STRIPE_PUBLISHABLE_KEY %}
+ var stripe = Stripe("{{ STRIPE_PUBLISHABLE_KEY }}");
+ {% else %}
+ console.error("Missing Stripe publishable key");
+ alert("Payment system configuration error. Please contact support.");
+ {% endif %}
var elements = stripe.elements();Committable suggestion skipped: line range outside the PR's diff.
| <div class="aspect-w-16 aspect-h-9"> | ||
| <iframe src="{{ campaign.video_url }}" frameborder="0" allowfullscreen></iframe> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Secure the iframe source to prevent potential XSS attacks.
The video URL is being inserted directly into the iframe src attribute without validation, which could potentially be a security risk.
It's important to validate that the video URL comes from trusted sources like YouTube or Vimeo:
🏁 Script executed:
#!/bin/bash
# Check if any sanitization for video URLs is implemented
# Search for video URL sanitization in the views
rg -A 3 "video_url" web/views.py
# Check the Campaign model for any validators on video_url field
rg -A 3 "video_url.*Field" web/models.pyLength of output: 899
Action Required: Validate video URL sources to prevent XSS risks.
- The current implementation relies on Django’s built-in URLField validator, which checks the URL format but does not restrict the source to trusted domains.
- There is no additional sanitization or domain verification (e.g., ensuring the URL is from YouTube or Vimeo) either in the views or in the Campaign model.
- Please implement a whitelist check (or similar mechanism) to explicitly allow only URLs from trusted video providers before rendering the iframe.
| path("blog/create/", views.create_blog_post, name="create_blog_post"), | ||
| path("blog/tag/<str:tag>/", views.blog_tag, name="blog_tag"), | ||
| path("blog/<slug:slug>/", views.blog_detail, name="blog_detail"), | ||
| path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"), |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider relocating the donation intent URL to group with other crowdfunding URLs.
The URL for creating a donation intent is separated from the other crowdfunding URLs, which might make maintenance more difficult.
Consider moving this URL to be grouped with the other crowdfunding URLs for better organization:
- path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"),Move it to line 44 after the other crowdfunding URLs:
+ path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"),📝 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.
| path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"), | |
| # Example snippet from web/urls.py | |
| urlpatterns = [ | |
| # ... other URL patterns ... | |
| # Crowdfunding URLs grouped together for better organization | |
| path("crowdfunding/<int:campaign_id>/details/", views.campaign_details, name="campaign_details"), | |
| path("crowdfunding/<int:campaign_id>/stats/", views.campaign_stats, name="campaign_stats"), | |
| + path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"), | |
| # ... other URLs that were originally placed later in the file ... | |
| ] |
|
|
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
web/forms.py (1)
59-96: 🛠️ Refactor suggestionUpdate
__all__to include the newCampaignForm.The
CampaignFormclass is not included in the__all__list, which means it won't be exported when importing * from this module.__all__ = [ "UserRegistrationForm", "ProfileForm", "ChallengeSubmissionForm", "CourseCreationForm", "CourseForm", "PeerChallengeForm", "PeerChallengeInvitationForm", "SessionForm", "ReviewForm", "CourseMaterialForm", "TeacherSignupForm", "ProfileUpdateForm", "CustomLoginForm", "LearnForm", "TeachForm", "InviteStudentForm", "ForumCategoryForm", "ForumTopicForm", "BlogPostForm", "MessageTeacherForm", "FeedbackForm", "GoodsForm", "StorefrontForm", "EducationalVideoForm", "ProgressTrackerForm", "SuccessStoryForm", "TeamGoalForm", "TeamInviteForm", "MemeForm", "QuizForm", "QuizQuestionForm", "QuizOptionFormSet", "TakeQuizForm", "AvatarForm", "GradeableLinkForm", "LinkGradeForm", "AwardAchievementForm", + "CampaignForm", ]🧰 Tools
🪛 Ruff (0.8.2)
59-97:
__all__is not sortedApply an isort-style sorting to
__all__(RUF022)
♻️ Duplicate comments (12)
web/templates/crowdfunding_detail.html (5)
26-26:⚠️ Potential issueSecurity vulnerability: Validate video URL source.
The iframe source is directly using the campaign's video URL without validation, which could lead to XSS attacks. Implement proper URL validation to ensure only trusted video providers are allowed.
Replace the current implementation with a solution that validates the video source:
- <iframe src="{{ campaign.video_url }}" frameborder="0" allowfullscreen></iframe> + {% if "youtube.com" in campaign.video_url or "youtu.be" in campaign.video_url or "vimeo.com" in campaign.video_url %} + <iframe src="{{ campaign.video_url }}" frameborder="0" allowfullscreen class="w-full aspect-video"></iframe> + {% else %} + <p class="text-red-500">Invalid video source. Only YouTube and Vimeo are supported.</p> + {% endif %}
32-34:⚠️ Potential issueFix itemized budget display.
The current implementation uses json_script but doesn't properly display the formatted budget. This will cause the raw JSON to be shown instead of a formatted display.
Apply the suggested fix for properly parsing and displaying the JSON budget:
- <pre class="bg-gray-100 p-2 rounded">{{ campaign.itemized_budget|json_script:"itemizedBudget" }}</pre> + <pre class="bg-gray-100 p-2 rounded" id="budget-display">Loading budget...</pre> + {{ campaign.itemized_budget|json_script:"itemizedBudget" }} + + <script> + document.addEventListener('DOMContentLoaded', function() { + try { + const budgetJson = JSON.parse(document.getElementById('itemizedBudget').textContent); + let formattedBudget = ''; + for (const [item, amount] of Object.entries(budgetJson)) { + formattedBudget += `${item}: $${amount}\n`; + } + document.getElementById('budget-display').textContent = formattedBudget || 'No budget items available'; + } catch (e) { + console.error('Error parsing budget:', e); + document.getElementById('budget-display').textContent = 'Error displaying budget'; + } + }); + </script>
83-85: 🛠️ Refactor suggestionAdd error handling for missing Stripe key.
The Stripe initialization doesn't check if the publishable key exists before initializing.
Add a check before initializing Stripe:
- var stripe = Stripe("{{ STRIPE_PUBLISHABLE_KEY }}"); - var elements = stripe.elements(); + {% if STRIPE_PUBLISHABLE_KEY %} + var stripe = Stripe("{{ STRIPE_PUBLISHABLE_KEY }}"); + var elements = stripe.elements(); + {% else %} + console.error("Missing Stripe publishable key"); + document.getElementById('card-element').innerHTML = '<div class="p-4 bg-red-100 text-red-700 rounded">Payment system configuration error. Please contact support.</div>'; + document.getElementById('donate-button').disabled = true; + {% endif %}
112-116: 🛠️ Refactor suggestionEnhance donation amount validation.
The current validation only checks if the amount is positive, but it would be better to also validate against unreasonably large amounts and add additional user feedback.
Improve the validation:
- if (!donationAmount || donationAmount <= 0) { - alert("Please enter a valid donation amount."); - return; - } + if (!donationAmount || donationAmount <= 0) { + alert("Please enter a positive donation amount."); + return; + } + if (donationAmount > 10000) { + if (!confirm("Are you sure you want to donate $" + donationAmount + "? For donations over $10,000, please contact us directly.")) { + return; + } + }
143-149: 🧹 Nitpick (assertive)Add loading indicator during payment processing.
The payment confirmation process doesn't show any loading state, which could lead to users thinking the payment failed if processing takes time.
Add a loading state:
}).then(function(result) { + // Show loading state + const donateButton = document.getElementById('donate-button'); + donateButton.disabled = true; + donateButton.innerHTML = '<span class="spinner-border spinner-border-sm" role="status" aria-hidden="true"></span> Processing...'; + if (result.error) { document.getElementById('card-errors').textContent = result.error.message; + // Reset button state + donateButton.disabled = false; + donateButton.innerHTML = 'Donate'; } else if (result.paymentIntent && result.paymentIntent.status === 'succeeded') { window.location.href = "{% url 'donation_success' %}"; }web/views.py (7)
2297-2299:⚠️ Potential issueSecurity issue: Teacher dashboard protection removed.
The
@login_requiredand@teacher_requireddecorators have been commented out from theteacher_dashboardfunction. This removes authentication and authorization protection, potentially allowing any user (even unauthenticated ones) to access the teacher dashboard.Apply this diff to restore the security measures:
-# @login_required -# @teacher_required +@login_required +@teacher_required def teacher_dashboard(request):🧰 Tools
🪛 Ruff (0.8.2)
2299-2299: Missing return type annotation for public function
teacher_dashboard(ANN201)
2299-2299: Missing type annotation for function argument
request(ANN001)
1348-1358: 🧹 Nitpick (assertive)Add type annotations and fix missing trailing comma in
crowdfunding_detail.This function needs type annotations and a minor fix for the trailing comma in the context dictionary.
-def crowdfunding_detail(request, campaign_id): +def crowdfunding_detail(request: HttpRequest, campaign_id: int) -> HttpResponse: campaign = get_object_or_404(Campaign, id=campaign_id) # Check if campaign is approved and live or if user has special access if not (campaign.approved and campaign.live) and not ( request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff) ): messages.error(request, "This campaign is not available.") return redirect("crowdfunding_list") - context = {"campaign": campaign, "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY} + context = {"campaign": campaign, "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY,} return render(request, "crowdfunding_detail.html", context)🧰 Tools
🪛 Ruff (0.8.2)
1348-1348: Missing return type annotation for public function
crowdfunding_detail(ANN201)
1348-1348: Missing type annotation for function argument
request(ANN001)
1348-1348: Missing type annotation for function argument
campaign_id(ANN001)
1360-1367: 🧹 Nitpick (assertive)Add type annotations to
crowdfunding_list.This function implementation is good but needs type annotations.
-def crowdfunding_list(request): +def crowdfunding_list(request: HttpRequest) -> HttpResponse: """ Display all active crowdfunding campaigns. Active campaigns are those that are approved and live. """ campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") return render(request, "crowdfunding_list.html", {"campaigns": campaigns})🧰 Tools
🪛 Ruff (0.8.2)
1360-1360: Missing return type annotation for public function
crowdfunding_list(ANN201)
1360-1360: Missing type annotation for function argument
request(ANN001)
1300-1314: 🧹 Nitpick (assertive)Improve error handling and logging in
handle_failed_donation.The current implementation uses f-strings in logging (which is not recommended) and catches all exceptions broadly.
-def handle_failed_donation(payment_intent): +def handle_failed_donation(payment_intent) -> None: """ Handle a failed donation payment. You can log the failure or notify the donor that the payment did not go through. """ try: campaign_id = payment_intent.metadata.get("campaign_id") user_id = payment_intent.metadata.get("user_id") logger = logging.getLogger(__name__) - logger.warning(f"Donation payment failed for campaign {campaign_id} and user {user_id}.") + logger.warning("Donation payment failed for campaign %s and user %s", campaign_id, user_id) except Exception as e: logger = logging.getLogger(__name__) - logger.error(f"Error handling failed donation: {e}") + logger.exception("Error handling failed donation")🧰 Tools
🪛 Ruff (0.8.2)
1300-1300: Missing return type annotation for public function
handle_failed_donationAdd return type annotation:
None(ANN201)
1300-1300: Missing type annotation for function argument
payment_intent(ANN001)
1310-1310: Logging statement uses f-string
(G004)
1311-1311: Do not catch blind exception:
Exception(BLE001)
1313-1313: Use
logging.exceptioninstead oflogging.errorReplace with
exception(TRY400)
1313-1313: Logging statement uses f-string
(G004)
1174-1208: 🛠️ Refactor suggestionAdd type annotations and improve error handling in
create_donation_intent.The function implementation is good overall, but it should include proper type annotations and more specific error handling for better maintainability and robustness.
Apply these improvements:
@csrf_exempt # For simplicity; in production, configure CSRF properly. -def create_donation_intent(request, campaign_id): +def create_donation_intent(request: HttpRequest, campaign_id: int) -> JsonResponse: """ Create a Stripe PaymentIntent for donating to a crowdfunding campaign. """ # Get the campaign campaign = get_object_or_404(Campaign, id=campaign_id) # For simplicity, we assume the donation amount is sent via POST data as a float (in dollars) if request.method == "POST": try: data = json.loads(request.body) donation_amount = float(data.get("amount", 0)) if donation_amount <= 0: return JsonResponse({"error": "Invalid donation amount."}, status=400) # Convert dollars to cents for Stripe amount_cents = int(donation_amount * 100) # Create a PaymentIntent intent = stripe.PaymentIntent.create( amount=amount_cents, currency="usd", metadata={ "campaign_id": campaign.id, "user_id": request.user.id, }, ) return JsonResponse({"clientSecret": intent.client_secret}) - except Exception as e: + except (ValueError, json.JSONDecodeError) as e: + return JsonResponse({"error": f"Invalid input: {str(e)}"}, status=400) + except stripe.error.StripeError as e: return JsonResponse({"error": str(e)}, status=400) else: # For non-POST requests, redirect back or render a page. return JsonResponse({"error": "POST method required"}, status=405)🧰 Tools
🪛 Ruff (0.8.2)
1175-1175: Missing return type annotation for public function
create_donation_intent(ANN201)
1175-1175: Missing type annotation for function argument
request(ANN001)
1175-1175: Missing type annotation for function argument
campaign_id(ANN001)
1203-1203: Do not catch blind exception:
Exception(BLE001)
1369-1391: 🧹 Nitpick (assertive)Add type annotations and remove unnecessary else in
crowdfunding_create.This function has good access control and form handling but needs type annotations and has an unnecessary else clause after a return statement.
@login_required @teacher_required -def crowdfunding_create(request): +def crowdfunding_create(request: HttpRequest) -> HttpResponse: """ View to allow teachers to create a new crowdfunding campaign. """ if request.method == "POST": form = CampaignForm(request.POST, request.FILES) if form.is_valid(): campaign = form.save(commit=False) campaign.teacher = request.user # Set initial approval status (set to False if admin approval is required) campaign.approved = True # or False if approval workflow is needed campaign.live = True # or False if campaigns shouldn't be live immediately campaign.save() messages.success(request, "Your crowdfunding campaign has been created successfully!") return redirect("crowdfunding_detail", campaign_id=campaign.id) - else: - messages.error(request, "Please correct the errors below.") + messages.error(request, "Please correct the errors below.") else: form = CampaignForm() return render(request, "crowdfunding_create.html", {"form": form})🧰 Tools
🪛 Ruff (0.8.2)
1371-1371: Missing return type annotation for public function
crowdfunding_create(ANN201)
1371-1371: Missing type annotation for function argument
request(ANN001)
1386-1386: Unnecessary
elseafterreturnstatementRemove unnecessary
else(RET505)
1258-1298:⚠️ Potential issueFix the incomplete implementation of
handle_successful_donation.This function has significant parts of the implementation commented out, which will prevent successful processing of donations. Additionally, the error handling can be improved.
Uncomment and fix the donation creation code, and improve error handling:
-def handle_successful_donation(payment_intent): +def handle_successful_donation(payment_intent) -> None: """ Handle a successful donation payment. Create a Donation record and update the campaign's amount raised. """ try: campaign_id = payment_intent.metadata.get("campaign_id") user_id = payment_intent.metadata.get("user_id") if not campaign_id or not user_id: # Not a donation payment; you might log this or ignore. return # Convert amount from cents to dollars. - # donation_amount = Decimal(payment_intent.amount_received) / 100 + donation_amount = Decimal(payment_intent.amount_received) / 100 # Retrieve campaign and user. campaign = Campaign.objects.get(id=campaign_id) - # user = User.objects.get(id=user_id) + user = User.objects.get(id=user_id) - # # Create the donation record. - # donation = Donation.objects.create( - # campaign=campaign, - # donor_name=user.get_full_name() or user.username, - # donor_email=user.email, - # amount=donation_amount, - # status="completed", # or set appropriate status if your Donation model uses one - # stripe_payment_intent_id=payment_intent.id, - # ) + # Create the donation record. + donation = Donation.objects.create( + campaign=campaign, + donor_name=user.get_full_name() or user.username, + donor_email=user.email, + amount=donation_amount, + status="completed", # or set appropriate status if your Donation model uses one + stripe_payment_intent_id=payment_intent.id, + ) # Update the campaign's amount raised. campaign.update_amount_raised() # Optionally, you can send a notification email to the donor or campaign teacher here. # For example: # send_donation_confirmation_email(user, campaign, donation) - except Exception as e: + except (Campaign.DoesNotExist, User.DoesNotExist) as e: # Log error if needed. logger = logging.getLogger(__name__) - logger.error(f"Error handling successful donation: {e}") + logger.error("Campaign or User not found: %s", e) + except Exception as e: + # Log error if needed. + logger = logging.getLogger(__name__) + logger.exception("Error handling successful donation")🧰 Tools
🪛 Ruff (0.8.2)
1258-1258: Missing return type annotation for public function
handle_successful_donationAdd return type annotation:
None(ANN201)
1258-1258: Missing type annotation for function argument
payment_intent(ANN001)
1294-1294: Do not catch blind exception:
Exception(BLE001)
1297-1297: Use
logging.exceptioninstead oflogging.errorReplace with
exception(TRY400)
1297-1297: Logging statement uses f-string
(G004)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
web/forms.py(2 hunks)web/models.py(3 hunks)web/templates/crowdfunding_create.html(1 hunks)web/templates/crowdfunding_detail.html(1 hunks)web/templates/crowdfunding_list.html(1 hunks)web/views.py(6 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
web/forms.py (2)
web/models.py (17)
Campaign(28-69)Meta(49-52)Meta(105-106)Meta(263-264)Meta(427-428)Meta(538-539)Meta(594-595)Meta(616-617)Meta(663-666)Meta(717-718)Meta(770-772)Meta(796-797)Meta(813-815)Meta(837-838)Meta(853-854)Meta(901-902)Meta(968-969)web/widgets.py (4)
TailwindInput(5-16)TailwindTextarea(19-31)TailwindNumberInput(48-59)TailwindFileInput(89-101)
web/views.py (2)
web/forms.py (6)
CampaignForm(259-301)save(218-256)save(610-635)save(702-716)save(1339-1352)save(1513-1528)web/models.py (13)
Campaign(28-69)update_amount_raised(63-69)save(157-161)save(215-251)save(269-272)save(336-370)save(433-460)save(550-552)save(777-780)save(974-979)save(1032-1037)save(1112-1115)save(1198-1209)
🪛 Ruff (0.8.2)
web/forms.py
262-262: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
263-289: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
268-268: Trailing comma missing
Add trailing comma
(COM812)
273-273: Trailing comma missing
Add trailing comma
(COM812)
276-276: Trailing comma missing
Add trailing comma
(COM812)
282-282: Trailing comma missing
Add trailing comma
(COM812)
287-287: Trailing comma missing
Add trailing comma
(COM812)
291-295: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
297-297: Missing return type annotation for private function clean_funding_goal
(ANN202)
300-300: Avoid specifying long messages outside the exception class
(TRY003)
web/models.py
34-34: Trailing comma missing
Add trailing comma
(COM812)
40-40: Trailing comma missing
Add trailing comma
(COM812)
50-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
54-54: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
57-57: Missing return type annotation for public function get_absolute_url
(ANN201)
63-63: Missing return type annotation for public function update_amount_raised
Add return type annotation: None
(ANN201)
78-78: Trailing comma missing
Add trailing comma
(COM812)
85-85: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
2790-2790: Missing return type annotation for public function update_campaign_amount_on_donation
Add return type annotation: None
(ANN201)
2790-2790: Missing type annotation for function argument sender
(ANN001)
2790-2790: Unused function argument: sender
(ARG001)
2790-2790: Missing type annotation for function argument instance
(ANN001)
2790-2790: Missing type annotation for function argument created
(ANN001)
2790-2790: Unused function argument: created
(ARG001)
2790-2790: Missing type annotation for **kwargs
(ANN003)
2790-2790: Unused function argument: kwargs
(ARG001)
2796-2796: Missing return type annotation for public function update_campaign_amount_on_donation_delete
Add return type annotation: None
(ANN201)
2796-2796: Missing type annotation for function argument sender
(ANN001)
2796-2796: Unused function argument: sender
(ARG001)
2796-2796: Missing type annotation for function argument instance
(ANN001)
2796-2796: Missing type annotation for **kwargs
(ANN003)
2796-2796: Unused function argument: kwargs
(ARG001)
web/views.py
1175-1175: Missing return type annotation for public function create_donation_intent
(ANN201)
1175-1175: Missing type annotation for function argument request
(ANN001)
1175-1175: Missing type annotation for function argument campaign_id
(ANN001)
1203-1203: Do not catch blind exception: Exception
(BLE001)
1258-1258: Missing return type annotation for public function handle_successful_donation
Add return type annotation: None
(ANN201)
1258-1258: Missing type annotation for function argument payment_intent
(ANN001)
1294-1294: Do not catch blind exception: Exception
(BLE001)
1297-1297: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1297-1297: Logging statement uses f-string
(G004)
1300-1300: Missing return type annotation for public function handle_failed_donation
Add return type annotation: None
(ANN201)
1300-1300: Missing type annotation for function argument payment_intent
(ANN001)
1310-1310: Logging statement uses f-string
(G004)
1311-1311: Do not catch blind exception: Exception
(BLE001)
1313-1313: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1313-1313: Logging statement uses f-string
(G004)
1348-1348: Missing return type annotation for public function crowdfunding_detail
(ANN201)
1348-1348: Missing type annotation for function argument request
(ANN001)
1348-1348: Missing type annotation for function argument campaign_id
(ANN001)
1360-1360: Missing return type annotation for public function crowdfunding_list
(ANN201)
1360-1360: Missing type annotation for function argument request
(ANN001)
1371-1371: Missing return type annotation for public function crowdfunding_create
(ANN201)
1371-1371: Missing type annotation for function argument request
(ANN001)
1386-1386: Unnecessary else after return statement
Remove unnecessary else
(RET505)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (4)
web/models.py (3)
16-16: Looks good for signal imports.
No issues found with importingpost_deleteandpost_saveto facilitate signals.
72-86: Verify PII considerations for storing donor data.
Storingdonor_emailanddonor_namefields may require privacy and data-retention policies. Please ensure compliance with relevant regulations and your application’s privacy statements.🧰 Tools
🪛 Ruff (0.8.2)
78-78: Trailing comma missing
Add trailing comma
(COM812)
85-85: Missing return type annotation for special method
__str__Add return type annotation:
str(ANN204)
2787-2789: Decorator usage for signal registration looks fine.
The@receiver(post_save, sender=CampaignDonation)pattern is correct and aligns well with Django’s recommended approach.web/views.py (1)
1333-1343: Integration with webhook handler looks good.The modifications to the webhook handler to check for donation-related events and route them to the appropriate handlers are well implemented.
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | ||
| {% csrf_token %} | ||
| <div class="space-y-4"> | ||
| {% for field in form %} | ||
| {{ field }} | ||
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | ||
| {% endfor %} | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Update form structure to improve accessibility.
The form lacks accessibility features such as proper labeling and field descriptions. Consider adding labels and help text to provide better guidance to users, especially for complex fields like the itemized budget.
<form method="post" enctype="multipart/form-data" class="space-y-6">
{% csrf_token %}
<div class="space-y-4">
{% for field in form %}
- {{ field }}
+ <div class="mb-4">
+ <label for="{{ field.id_for_label }}" class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">
+ {{ field.label }}
+ </label>
+ {{ field }}
+ {% if field.help_text %}<p class="mt-1 text-xs text-gray-500 dark:text-gray-400">{{ field.help_text }}</p>{% endif %}
+ </div>
{% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %}
{% endfor %}
</div>📝 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.
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | |
| {% csrf_token %} | |
| <div class="space-y-4"> | |
| {% for field in form %} | |
| {{ field }} | |
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | |
| {% endfor %} | |
| </div> | |
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | |
| {% csrf_token %} | |
| <div class="space-y-4"> | |
| {% for field in form %} | |
| <div class="mb-4"> | |
| <label for="{{ field.id_for_label }}" class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1"> | |
| {{ field.label }} | |
| </label> | |
| {{ field }} | |
| {% if field.help_text %}<p class="mt-1 text-xs text-gray-500 dark:text-gray-400">{{ field.help_text }}</p>{% endif %} | |
| </div> | |
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | |
| {% endfor %} | |
| </div> |
| <div class="container mx-auto p-9"> | ||
| <div class="max-w-3xl mx-auto bg-white dark:bg-gray-800 shadow-md rounded-lg p-8"> | ||
| <h1 class="text-3xl font-bold mb-8 text-gray-800 dark:text-gray-100 text-center">Create Crowdfunding Campaign</h1> | ||
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | ||
| {% csrf_token %} | ||
| <div class="space-y-4"> | ||
| {% for field in form %} | ||
| {{ field }} | ||
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | ||
| {% endfor %} | ||
| </div> | ||
| <div> | ||
| <button type="submit" | ||
| class="w-full bg-green-500 hover:bg-green-600 text-white font-semibold px-4 py-3 rounded-lg transition duration-200"> | ||
| Create Campaign | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add campaign preview functionality.
Consider adding a preview functionality to let teachers see how their campaign will appear before submitting it. This would improve the user experience and help ensure campaigns look as intended.
<div class="container mx-auto p-9">
<div class="max-w-3xl mx-auto bg-white dark:bg-gray-800 shadow-md rounded-lg p-8">
<h1 class="text-3xl font-bold mb-8 text-gray-800 dark:text-gray-100 text-center">Create Crowdfunding Campaign</h1>
+ <div class="flex justify-end mb-4">
+ <button type="button" id="preview-button" class="px-4 py-2 bg-blue-500 hover:bg-blue-600 text-white rounded-lg transition duration-200">
+ Preview Campaign
+ </button>
+ </div>
<form method="post" enctype="multipart/form-data" class="space-y-6">
{% csrf_token %}
<div class="space-y-4">
{% for field in form %}
{{ field }}
{% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %}
{% endfor %}
</div>
<div>
<button type="submit"
class="w-full bg-green-500 hover:bg-green-600 text-white font-semibold px-4 py-3 rounded-lg transition duration-200">
Create Campaign
</button>
</div>
</form>
</div>
+ <!-- Preview Modal -->
+ <div id="preview-modal" class="hidden fixed inset-0 bg-black bg-opacity-50 z-50 flex items-center justify-center">
+ <div class="bg-white dark:bg-gray-800 rounded-lg max-w-3xl w-full max-h-[90vh] overflow-y-auto p-6">
+ <div class="flex justify-between items-center mb-4">
+ <h2 class="text-2xl font-bold">Campaign Preview</h2>
+ <button id="close-preview" class="text-gray-500 hover:text-gray-700">×</button>
+ </div>
+ <div id="preview-content"></div>
+ </div>
+ </div>
</div>
+
+ <script>
+ document.getElementById('preview-button').addEventListener('click', function() {
+ const form = document.querySelector('form');
+ const formData = new FormData(form);
+ const previewContent = document.getElementById('preview-content');
+
+ // Basic preview of campaign details
+ let title = formData.get('title') || 'Campaign Title';
+ let description = formData.get('description') || 'Campaign Description';
+ let fundingGoal = formData.get('funding_goal') || '0';
+
+ // Create preview HTML
+ previewContent.innerHTML = `
+ <div class="space-y-4">
+ <h3 class="text-xl font-bold">${title}</h3>
+ <p>${description}</p>
+ <div class="mt-2">
+ <strong>Goal:</strong> $${fundingGoal}
+ <br />
+ <strong>Raised:</strong> $0
+ </div>
+ </div>
+ `;
+
+ // Show the modal
+ document.getElementById('preview-modal').classList.remove('hidden');
+ });
+
+ document.getElementById('close-preview').addEventListener('click', function() {
+ document.getElementById('preview-modal').classList.add('hidden');
+ });
+ </script>📝 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.
| <div class="container mx-auto p-9"> | |
| <div class="max-w-3xl mx-auto bg-white dark:bg-gray-800 shadow-md rounded-lg p-8"> | |
| <h1 class="text-3xl font-bold mb-8 text-gray-800 dark:text-gray-100 text-center">Create Crowdfunding Campaign</h1> | |
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | |
| {% csrf_token %} | |
| <div class="space-y-4"> | |
| {% for field in form %} | |
| {{ field }} | |
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | |
| {% endfor %} | |
| </div> | |
| <div> | |
| <button type="submit" | |
| class="w-full bg-green-500 hover:bg-green-600 text-white font-semibold px-4 py-3 rounded-lg transition duration-200"> | |
| Create Campaign | |
| </button> | |
| </div> | |
| </form> | |
| </div> | |
| </div> | |
| <div class="container mx-auto p-9"> | |
| <div class="max-w-3xl mx-auto bg-white dark:bg-gray-800 shadow-md rounded-lg p-8"> | |
| <h1 class="text-3xl font-bold mb-8 text-gray-800 dark:text-gray-100 text-center">Create Crowdfunding Campaign</h1> | |
| <div class="flex justify-end mb-4"> | |
| <button type="button" id="preview-button" class="px-4 py-2 bg-blue-500 hover:bg-blue-600 text-white rounded-lg transition duration-200"> | |
| Preview Campaign | |
| </button> | |
| </div> | |
| <form method="post" enctype="multipart/form-data" class="space-y-6"> | |
| {% csrf_token %} | |
| <div class="space-y-4"> | |
| {% for field in form %} | |
| {{ field }} | |
| {% for error in field.errors %}<p class="mt-1 text-xs text-red-500">{{ error }}</p>{% endfor %} | |
| {% endfor %} | |
| </div> | |
| <div> | |
| <button type="submit" | |
| class="w-full bg-green-500 hover:bg-green-600 text-white font-semibold px-4 py-3 rounded-lg transition duration-200"> | |
| Create Campaign | |
| </button> | |
| </div> | |
| </form> | |
| </div> | |
| <!-- Preview Modal --> | |
| <div id="preview-modal" class="hidden fixed inset-0 bg-black bg-opacity-50 z-50 flex items-center justify-center"> | |
| <div class="bg-white dark:bg-gray-800 rounded-lg max-w-3xl w-full max-h-[90vh] overflow-y-auto p-6"> | |
| <div class="flex justify-between items-center mb-4"> | |
| <h2 class="text-2xl font-bold">Campaign Preview</h2> | |
| <button id="close-preview" class="text-gray-500 hover:text-gray-700">×</button> | |
| </div> | |
| <div id="preview-content"></div> | |
| </div> | |
| </div> | |
| </div> | |
| <script> | |
| document.getElementById('preview-button').addEventListener('click', function() { | |
| const form = document.querySelector('form'); | |
| const formData = new FormData(form); | |
| const previewContent = document.getElementById('preview-content'); | |
| // Basic preview of campaign details | |
| let title = formData.get('title') || 'Campaign Title'; | |
| let description = formData.get('description') || 'Campaign Description'; | |
| let fundingGoal = formData.get('funding_goal') || '0'; | |
| // Create preview HTML | |
| previewContent.innerHTML = ` | |
| <div class="space-y-4"> | |
| <h3 class="text-xl font-bold">${title}</h3> | |
| <p>${description}</p> | |
| <div class="mt-2"> | |
| <strong>Goal:</strong> $${fundingGoal} | |
| <br /> | |
| <strong>Raised:</strong> $0 | |
| </div> | |
| </div> | |
| `; | |
| // Show the modal | |
| document.getElementById('preview-modal').classList.remove('hidden'); | |
| }); | |
| document.getElementById('close-preview').addEventListener('click', function() { | |
| document.getElementById('preview-modal').classList.add('hidden'); | |
| }); | |
| </script> |
| {% endfor %} | ||
| </div> | ||
| {% else %} | ||
| <p>No active crowdfunding campaigns at this time.</p> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance empty state experience.
The current empty state message is very plain. Consider enhancing it with a more engaging message, possibly an illustration, and a clear call-to-action for teachers to create a campaign.
- <p>No active crowdfunding campaigns at this time.</p>
+ <div class="text-center py-10">
+ <div class="mx-auto w-24 h-24 bg-gray-200 rounded-full flex items-center justify-center mb-4">
+ <svg xmlns="http://www.w3.org/2000/svg" class="h-12 w-12 text-gray-400" fill="none" viewBox="0 0 24 24" stroke="currentColor">
+ <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4m0 4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z" />
+ </svg>
+ </div>
+ <h2 class="text-xl font-semibold mb-2">No active campaigns yet</h2>
+ <p class="text-gray-600 mb-6">Be the first to create a crowdfunding campaign for your classroom project!</p>
+ {% if request.user.profile.is_teacher %}
+ <a href="{% url 'crowdfunding_create' %}" class="inline-block bg-green-600 hover:bg-green-700 text-white px-6 py-3 rounded-lg font-semibold">
+ Create a Campaign
+ </a>
+ {% endif %}
+ </div>📝 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.
| <p>No active crowdfunding campaigns at this time.</p> | |
| <div class="text-center py-10"> | |
| <div class="mx-auto w-24 h-24 bg-gray-200 rounded-full flex items-center justify-center mb-4"> | |
| <svg xmlns="http://www.w3.org/2000/svg" class="h-12 w-12 text-gray-400" fill="none" viewBox="0 0 24 24" stroke="currentColor"> | |
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 8v4m0 4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z" /> | |
| </svg> | |
| </div> | |
| <h2 class="text-xl font-semibold mb-2">No active campaigns yet</h2> | |
| <p class="text-gray-600 mb-6">Be the first to create a crowdfunding campaign for your classroom project!</p> | |
| {% if request.user.profile.is_teacher %} | |
| <a href="{% url 'crowdfunding_create' %}" class="inline-block bg-green-600 hover:bg-green-700 text-white px-6 py-3 rounded-lg font-semibold"> | |
| Create a Campaign | |
| </a> | |
| {% endif %} | |
| </div> |
| <div class="mt-2"> | ||
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | ||
| <br /> | ||
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add progress bar for funding visualization.
The current funding display only shows the goal and amount raised as text. Adding a visual progress bar would make it easier for users to quickly understand campaign progress.
<div class="mt-2">
<strong>Goal:</strong> ${{ campaign.funding_goal }}
<br />
<strong>Raised:</strong> ${{ campaign.amount_raised }}
+ <div class="mt-2 w-full bg-gray-200 rounded-full h-2.5 dark:bg-gray-700">
+ <div class="bg-blue-600 h-2.5 rounded-full" style="width: {% widthratio campaign.amount_raised campaign.funding_goal 100 %}%"></div>
+ </div>
+ <p class="text-xs text-gray-500 mt-1">{% widthratio campaign.amount_raised campaign.funding_goal 100 %}% funded</p>
</div>📝 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.
| <div class="mt-2"> | |
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | |
| <br /> | |
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | |
| </div> | |
| <div class="mt-2"> | |
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | |
| <br /> | |
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | |
| <div class="mt-2 w-full bg-gray-200 rounded-full h-2.5 dark:bg-gray-700"> | |
| <div class="bg-blue-600 h-2.5 rounded-full" style="width: {% widthratio campaign.amount_raised campaign.funding_goal 100 %}%"></div> | |
| </div> | |
| <p class="text-xs text-gray-500 mt-1">{% widthratio campaign.amount_raised campaign.funding_goal 100 %}% funded</p> | |
| </div> |
| <div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> | ||
| {% for campaign in campaigns %} | ||
| <div class="border rounded-lg shadow-lg p-4"> | ||
| {% if campaign.image %} | ||
| <img src="{{ campaign.image.url }}" | ||
| alt="{{ campaign.title }}" | ||
| class="w-full h-48 object-cover rounded" /> | ||
| {% endif %} | ||
| <h2 class="mt-3 text-xl font-semibold">{{ campaign.title }}</h2> | ||
| <p class="mt-1 text-gray-700">{{ campaign.description|truncatewords:20 }}</p> | ||
| <div class="mt-2"> | ||
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | ||
| <br /> | ||
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | ||
| </div> | ||
| <a href="{% url 'crowdfunding_detail' campaign.id %}" | ||
| class="mt-4 inline-block bg-blue-600 hover:bg-blue-700 text-white px-4 py-2 rounded">View Details</a> | ||
| </div> | ||
| {% endfor %} | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add filtering and sorting functionality for campaigns.
Currently, the list view displays all campaigns without any filtering or sorting options. Consider adding controls to filter by category, sort by various criteria (newest, most funded, etc.), and search by title or description.
<div class="container mx-auto p-6">
<h1 class="text-3xl font-bold mb-6">Active Crowdfunding Campaigns</h1>
+ <div class="mb-6 flex flex-col md:flex-row gap-4 justify-between">
+ <div class="flex gap-2 items-center">
+ <input type="text" id="search-input" placeholder="Search campaigns..."
+ class="border rounded-lg px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500" />
+ <button id="search-button" class="bg-blue-600 hover:bg-blue-700 text-white px-4 py-2 rounded-lg">
+ Search
+ </button>
+ </div>
+ <div class="flex gap-4">
+ <select id="sort-campaigns" class="border rounded-lg px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500">
+ <option value="newest">Newest</option>
+ <option value="most-funded">Most Funded</option>
+ <option value="closest-to-goal">Closest to Goal</option>
+ </select>
+ </div>
+ </div>
{% if campaigns %}
<div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6">
{% for campaign in campaigns %}
<div class="border rounded-lg shadow-lg p-4">
{% if campaign.image %}
<img src="{{ campaign.image.url }}"
alt="{{ campaign.title }}"
class="w-full h-48 object-cover rounded" />
{% endif %}📝 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.
| <div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> | |
| {% for campaign in campaigns %} | |
| <div class="border rounded-lg shadow-lg p-4"> | |
| {% if campaign.image %} | |
| <img src="{{ campaign.image.url }}" | |
| alt="{{ campaign.title }}" | |
| class="w-full h-48 object-cover rounded" /> | |
| {% endif %} | |
| <h2 class="mt-3 text-xl font-semibold">{{ campaign.title }}</h2> | |
| <p class="mt-1 text-gray-700">{{ campaign.description|truncatewords:20 }}</p> | |
| <div class="mt-2"> | |
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | |
| <br /> | |
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | |
| </div> | |
| <a href="{% url 'crowdfunding_detail' campaign.id %}" | |
| class="mt-4 inline-block bg-blue-600 hover:bg-blue-700 text-white px-4 py-2 rounded">View Details</a> | |
| </div> | |
| {% endfor %} | |
| </div> | |
| <div class="container mx-auto p-6"> | |
| <h1 class="text-3xl font-bold mb-6">Active Crowdfunding Campaigns</h1> | |
| <div class="mb-6 flex flex-col md:flex-row gap-4 justify-between"> | |
| <div class="flex gap-2 items-center"> | |
| <input type="text" id="search-input" placeholder="Search campaigns..." | |
| class="border rounded-lg px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500" /> | |
| <button id="search-button" class="bg-blue-600 hover:bg-blue-700 text-white px-4 py-2 rounded-lg"> | |
| Search | |
| </button> | |
| </div> | |
| <div class="flex gap-4"> | |
| <select id="sort-campaigns" class="border rounded-lg px-3 py-2 focus:outline-none focus:ring-2 focus:ring-blue-500"> | |
| <option value="newest">Newest</option> | |
| <option value="most-funded">Most Funded</option> | |
| <option value="closest-to-goal">Closest to Goal</option> | |
| </select> | |
| </div> | |
| </div> | |
| {% if campaigns %} | |
| <div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> | |
| {% for campaign in campaigns %} | |
| <div class="border rounded-lg shadow-lg p-4"> | |
| {% if campaign.image %} | |
| <img src="{{ campaign.image.url }}" | |
| alt="{{ campaign.title }}" | |
| class="w-full h-48 object-cover rounded" /> | |
| {% endif %} | |
| <h2 class="mt-3 text-xl font-semibold">{{ campaign.title }}</h2> | |
| <p class="mt-1 text-gray-700">{{ campaign.description|truncatewords:20 }}</p> | |
| <div class="mt-2"> | |
| <strong>Goal:</strong> ${{ campaign.funding_goal }} | |
| <br /> | |
| <strong>Raised:</strong> ${{ campaign.amount_raised }} | |
| </div> | |
| <a href="{% url 'crowdfunding_detail' campaign.id %}" | |
| class="mt-4 inline-block bg-blue-600 hover:bg-blue-700 text-white px-4 py-2 rounded"> | |
| View Details | |
| </a> | |
| </div> | |
| {% endfor %} | |
| </div> | |
| {% endif %} | |
| </div> |
| <div class="mt-6"> | ||
| <h2 class="text-xl font-semibold mb-2">Donate to this Campaign</h2> | ||
| <form id="donation-form"> | ||
| {% csrf_token %} | ||
| <input type="hidden" id="campaign_id" value="{{ campaign.id }}" /> | ||
| <label for="amount" class="block mb-1 text-gray-700">Donation Amount ($):</label> | ||
| <input type="number" | ||
| id="amount" | ||
| name="amount" | ||
| step="0.01" | ||
| min="1" | ||
| required | ||
| class="border rounded p-2 w-full mb-4" /> | ||
| <!-- Stripe Card Element Container --> | ||
| <div id="card-element" class="mb-4"></div> | ||
| <!-- Display errors from Stripe Element --> | ||
| <div id="card-errors" class="text-red-500 text-sm mb-4"></div> | ||
| <button type="button" | ||
| id="donate-button" | ||
| class="w-full bg-blue-600 hover:bg-blue-700 text-white font-semibold px-4 py-2 rounded"> | ||
| Donate | ||
| </button> | ||
| </form> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add donation tiers for better user experience.
Instead of having users enter a free-form amount, consider adding donation tiers with predefined amounts to simplify the donation process. This can increase conversion rates by reducing decision fatigue.
<!-- Donation Section -->
<div class="mt-6">
<h2 class="text-xl font-semibold mb-2">Donate to this Campaign</h2>
<form id="donation-form">
{% csrf_token %}
<input type="hidden" id="campaign_id" value="{{ campaign.id }}" />
- <label for="amount" class="block mb-1 text-gray-700">Donation Amount ($):</label>
- <input type="number"
- id="amount"
- name="amount"
- step="0.01"
- min="1"
- required
- class="border rounded p-2 w-full mb-4" />
+ <div class="mb-4">
+ <label class="block mb-1 text-gray-700">Select donation amount:</label>
+ <div class="grid grid-cols-2 md:grid-cols-4 gap-2 mb-2">
+ <button type="button" class="donation-tier p-2 border rounded hover:bg-blue-50 focus:ring-2 focus:ring-blue-500" data-amount="5">$5</button>
+ <button type="button" class="donation-tier p-2 border rounded hover:bg-blue-50 focus:ring-2 focus:ring-blue-500" data-amount="10">$10</button>
+ <button type="button" class="donation-tier p-2 border rounded hover:bg-blue-50 focus:ring-2 focus:ring-blue-500" data-amount="25">$25</button>
+ <button type="button" class="donation-tier p-2 border rounded hover:bg-blue-50 focus:ring-2 focus:ring-blue-500" data-amount="50">$50</button>
+ </div>
+ <div class="flex items-center">
+ <label for="amount" class="inline-block mr-2 text-gray-700">Custom amount:</label>
+ <input type="number"
+ id="amount"
+ name="amount"
+ step="0.01"
+ min="1"
+ required
+ class="border rounded p-2 w-full" />
+ </div>
+ </div>
<!-- Stripe Card Element Container -->
<div id="card-element" class="mb-4"></div>
<!-- Display errors from Stripe Element -->
<div id="card-errors" class="text-red-500 text-sm mb-4"></div>
<button type="button"
id="donate-button"
class="w-full bg-blue-600 hover:bg-blue-700 text-white font-semibold px-4 py-2 rounded">
Donate
</button>
</form>
</div>Add the necessary JavaScript to handle the donation tiers:
// Add this to your script section
document.addEventListener('DOMContentLoaded', function() {
const amountInput = document.getElementById('amount');
const tiers = document.querySelectorAll('.donation-tier');
tiers.forEach(button => {
button.addEventListener('click', function() {
// Remove active class from all buttons
tiers.forEach(btn => btn.classList.remove('bg-blue-100', 'font-semibold'));
// Add active class to clicked button
this.classList.add('bg-blue-100', 'font-semibold');
// Set amount value
amountInput.value = this.getAttribute('data-amount');
});
});
});| def update_campaign_amount_on_donation(sender, instance, created, **kwargs): | ||
| """Update campaign amount_raised when a donation is saved.""" | ||
| instance.campaign.update_amount_raised() | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Rename unused parameters or remove them to avoid confusion.
The sender, created, and **kwargs parameters are unused in update_campaign_amount_on_donation. Consider renaming them with a leading underscore or removing them if unneeded. Also add a return type annotation for clarity.
@receiver(post_save, sender=CampaignDonation)
-def update_campaign_amount_on_donation(sender, instance, created, **kwargs):
+def update_campaign_amount_on_donation(_sender, instance, _created, **_kwargs) -> None:
"""Update campaign amount_raised when a donation is saved."""
instance.campaign.update_amount_raised()📝 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.
| def update_campaign_amount_on_donation(sender, instance, created, **kwargs): | |
| """Update campaign amount_raised when a donation is saved.""" | |
| instance.campaign.update_amount_raised() | |
| @receiver(post_save, sender=CampaignDonation) | |
| def update_campaign_amount_on_donation(_sender, instance, _created, **_kwargs) -> None: | |
| """Update campaign amount_raised when a donation is saved.""" | |
| instance.campaign.update_amount_raised() |
🧰 Tools
🪛 Ruff (0.8.2)
2790-2790: Missing return type annotation for public function update_campaign_amount_on_donation
Add return type annotation: None
(ANN201)
2790-2790: Missing type annotation for function argument sender
(ANN001)
2790-2790: Unused function argument: sender
(ARG001)
2790-2790: Missing type annotation for function argument instance
(ANN001)
2790-2790: Missing type annotation for function argument created
(ANN001)
2790-2790: Unused function argument: created
(ARG001)
2790-2790: Missing type annotation for **kwargs
(ANN003)
2790-2790: Unused function argument: kwargs
(ARG001)
| @receiver(post_delete, sender=CampaignDonation) | ||
| def update_campaign_amount_on_donation_delete(sender, instance, **kwargs): | ||
| """Update campaign amount_raised when a donation is deleted.""" | ||
| instance.campaign.update_amount_raised() |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Apply the same cleanup for the delete signal handler.
Similar to the previous function, you can add a return type annotation and rename unused parameters.
@receiver(post_delete, sender=CampaignDonation)
-def update_campaign_amount_on_donation_delete(sender, instance, **kwargs):
+def update_campaign_amount_on_donation_delete(_sender, instance, **_kwargs) -> None:
"""Update campaign amount_raised when a donation is deleted."""
instance.campaign.update_amount_raised()📝 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.
| @receiver(post_delete, sender=CampaignDonation) | |
| def update_campaign_amount_on_donation_delete(sender, instance, **kwargs): | |
| """Update campaign amount_raised when a donation is deleted.""" | |
| instance.campaign.update_amount_raised() | |
| @receiver(post_delete, sender=CampaignDonation) | |
| def update_campaign_amount_on_donation_delete(_sender, instance, **_kwargs) -> None: | |
| """Update campaign amount_raised when a donation is deleted.""" | |
| instance.campaign.update_amount_raised() |
🧰 Tools
🪛 Ruff (0.8.2)
2796-2796: Missing return type annotation for public function update_campaign_amount_on_donation_delete
Add return type annotation: None
(ANN201)
2796-2796: Missing type annotation for function argument sender
(ANN001)
2796-2796: Unused function argument: sender
(ARG001)
2796-2796: Missing type annotation for function argument instance
(ANN001)
2796-2796: Missing type annotation for **kwargs
(ANN003)
2796-2796: Unused function argument: kwargs
(ARG001)
| class Campaign(models.Model): | ||
| """ | ||
| Model representing a crowdfunding campaign created by a teacher. | ||
| """ | ||
|
|
||
| teacher = models.ForeignKey( | ||
| User, on_delete=models.CASCADE, related_name="campaigns", help_text="Teacher who created the campaign." | ||
| ) | ||
| title = models.CharField(max_length=255, help_text="Campaign title") | ||
| description = models.TextField(help_text="Detailed campaign description") | ||
| funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") | ||
| amount_raised = models.DecimalField( | ||
| max_digits=10, decimal_places=2, default=0, help_text="Total funds raised so far" | ||
| ) | ||
| itemized_budget = models.JSONField(blank=True, help_text="Optional JSON field for itemized budget breakdown") | ||
| video_url = models.URLField(blank=True, help_text="Optional video pitch URL for the campaign") | ||
| image = models.ImageField(upload_to="campaign_images/", blank=True, help_text="Optional campaign image") | ||
| approved = models.BooleanField(default=True, help_text="Indicates whether the campaign is approved by moderators") | ||
| live = models.BooleanField(default=True, help_text="Indicates whether the campaign is live and accepting donations") | ||
| created_at = models.DateTimeField(auto_now_add=True) | ||
|
|
||
| class Meta: | ||
| ordering = ["-created_at"] | ||
| verbose_name = "Crowdfunding Campaign" | ||
| verbose_name_plural = "Crowdfunding Campaigns" | ||
|
|
||
| def __str__(self): | ||
| return self.title | ||
|
|
||
| def get_absolute_url(self): | ||
| """ | ||
| Returns the URL to access a particular campaign's detail view. | ||
| """ | ||
| return reverse("crowdfunding_detail", kwargs={"campaign_id": self.id}) | ||
|
|
||
| def update_amount_raised(self): | ||
| """ | ||
| Recalculate and update the total amount raised from related donations. | ||
| """ | ||
| total = self.donations.aggregate(total=models.Sum("amount"))["total"] or 0 | ||
| self.amount_raised = total | ||
| self.save() | ||
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider minor enhancements for concurrency and type annotations.
- In
update_amount_raised(), regularly recalculating might introduce overhead for high-volume donations. You could use an atomic approach (e.g.,F()) to incrementamount_raiseddirectly in the donation signals, which is often more performant and concurrency-safe. - According to static analysis, adding return type annotations (e.g.,
-> None) to methods like__str__,get_absolute_url, andupdate_amount_raisedcan improve code clarity.
class Campaign(models.Model):
...
- def __str__(self):
+ def __str__(self) -> str:
return self.title
- def get_absolute_url(self):
+ def get_absolute_url(self) -> str:
return reverse("crowdfunding_detail", kwargs={"campaign_id": self.id})
- def update_amount_raised(self):
+ def update_amount_raised(self) -> None:
total = self.donations.aggregate(total=models.Sum("amount"))["total"] or 0
self.amount_raised = total
self.save()📝 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.
| class Campaign(models.Model): | |
| """ | |
| Model representing a crowdfunding campaign created by a teacher. | |
| """ | |
| teacher = models.ForeignKey( | |
| User, on_delete=models.CASCADE, related_name="campaigns", help_text="Teacher who created the campaign." | |
| ) | |
| title = models.CharField(max_length=255, help_text="Campaign title") | |
| description = models.TextField(help_text="Detailed campaign description") | |
| funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") | |
| amount_raised = models.DecimalField( | |
| max_digits=10, decimal_places=2, default=0, help_text="Total funds raised so far" | |
| ) | |
| itemized_budget = models.JSONField(blank=True, help_text="Optional JSON field for itemized budget breakdown") | |
| video_url = models.URLField(blank=True, help_text="Optional video pitch URL for the campaign") | |
| image = models.ImageField(upload_to="campaign_images/", blank=True, help_text="Optional campaign image") | |
| approved = models.BooleanField(default=True, help_text="Indicates whether the campaign is approved by moderators") | |
| live = models.BooleanField(default=True, help_text="Indicates whether the campaign is live and accepting donations") | |
| created_at = models.DateTimeField(auto_now_add=True) | |
| class Meta: | |
| ordering = ["-created_at"] | |
| verbose_name = "Crowdfunding Campaign" | |
| verbose_name_plural = "Crowdfunding Campaigns" | |
| def __str__(self): | |
| return self.title | |
| def get_absolute_url(self): | |
| """ | |
| Returns the URL to access a particular campaign's detail view. | |
| """ | |
| return reverse("crowdfunding_detail", kwargs={"campaign_id": self.id}) | |
| def update_amount_raised(self): | |
| """ | |
| Recalculate and update the total amount raised from related donations. | |
| """ | |
| total = self.donations.aggregate(total=models.Sum("amount"))["total"] or 0 | |
| self.amount_raised = total | |
| self.save() | |
| class Campaign(models.Model): | |
| """ | |
| Model representing a crowdfunding campaign created by a teacher. | |
| """ | |
| teacher = models.ForeignKey( | |
| User, on_delete=models.CASCADE, related_name="campaigns", help_text="Teacher who created the campaign." | |
| ) | |
| title = models.CharField(max_length=255, help_text="Campaign title") | |
| description = models.TextField(help_text="Detailed campaign description") | |
| funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") | |
| amount_raised = models.DecimalField( | |
| max_digits=10, decimal_places=2, default=0, help_text="Total funds raised so far" | |
| ) | |
| itemized_budget = models.JSONField(blank=True, help_text="Optional JSON field for itemized budget breakdown") | |
| video_url = models.URLField(blank=True, help_text="Optional video pitch URL for the campaign") | |
| image = models.ImageField(upload_to="campaign_images/", blank=True, help_text="Optional campaign image") | |
| approved = models.BooleanField(default=True, help_text="Indicates whether the campaign is approved by moderators") | |
| live = models.BooleanField(default=True, help_text="Indicates whether the campaign is live and accepting donations") | |
| created_at = models.DateTimeField(auto_now_add=True) | |
| class Meta: | |
| ordering = ["-created_at"] | |
| verbose_name = "Crowdfunding Campaign" | |
| verbose_name_plural = "Crowdfunding Campaigns" | |
| def __str__(self) -> str: | |
| return self.title | |
| def get_absolute_url(self) -> str: | |
| """ | |
| Returns the URL to access a particular campaign's detail view. | |
| """ | |
| return reverse("crowdfunding_detail", kwargs={"campaign_id": self.id}) | |
| def update_amount_raised(self) -> None: | |
| """ | |
| Recalculate and update the total amount raised from related donations. | |
| """ | |
| total = self.donations.aggregate(total=models.Sum("amount"))["total"] or 0 | |
| self.amount_raised = total | |
| self.save() |
🧰 Tools
🪛 Ruff (0.8.2)
34-34: Trailing comma missing
Add trailing comma
(COM812)
40-40: Trailing comma missing
Add trailing comma
(COM812)
50-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
54-54: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
57-57: Missing return type annotation for public function get_absolute_url
(ANN201)
63-63: Missing return type annotation for public function update_amount_raised
Add return type annotation: None
(ANN201)
A1L13N
left a comment
A1L13N
left a comment
There was a problem hiding this comment.
Please see feedback and make sure tests are passing
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
web/forms.py (1)
61-99: 🛠️ Refactor suggestionThe CampaignForm needs to be added to the
__all__list.The new
CampaignFormclass should be added to the__all__list at the top of the file to make it available for importing from other modules.__all__ = [ "UserRegistrationForm", "ProfileForm", "ChallengeSubmissionForm", "CourseCreationForm", "CourseForm", "PeerChallengeForm", "PeerChallengeInvitationForm", "SessionForm", "ReviewForm", "CourseMaterialForm", "TeacherSignupForm", "ProfileUpdateForm", "CustomLoginForm", "LearnForm", "TeachForm", "InviteStudentForm", "ForumCategoryForm", "ForumTopicForm", "BlogPostForm", "MessageTeacherForm", "FeedbackForm", "GoodsForm", "StorefrontForm", "EducationalVideoForm", "ProgressTrackerForm", "SuccessStoryForm", "TeamGoalForm", "TeamInviteForm", "MemeForm", "QuizForm", "QuizQuestionForm", "QuizOptionFormSet", "TakeQuizForm", "AvatarForm", "GradeableLinkForm", "LinkGradeForm", "AwardAchievementForm", + "CampaignForm", ]🧰 Tools
🪛 Ruff (0.8.2)
61-99:
__all__is not sortedApply an isort-style sorting to
__all__(RUF022)
♻️ Duplicate comments (7)
web/forms.py (3)
323-323: 🛠️ Refactor suggestionAdd JSON validation for itemized_budget field.
Since
itemized_budgetis described as a JSON field, validation should be added to ensure it contains valid JSON when provided.def clean_itemized_budget(self) -> dict: """Validate that itemized_budget contains valid JSON.""" itemized_budget = self.cleaned_data.get("itemized_budget") if itemized_budget: try: import json json_data = json.loads(itemized_budget) if not isinstance(json_data, dict): raise forms.ValidationError("Budget must be a JSON object with key-value pairs.") except json.JSONDecodeError: raise forms.ValidationError("Please enter valid JSON for the itemized budget.") return itemized_budget
318-322: 🧹 Nitpick (assertive)Add return type annotation and improve error message.
The
clean_funding_goalmethod is missing a return type annotation, and the error message could be improved by adding a period at the end for consistency.- def clean_funding_goal(self): + def clean_funding_goal(self) -> 'decimal.Decimal': funding_goal = self.cleaned_data.get("funding_goal") if funding_goal <= 0: - raise forms.ValidationError("Funding goal must be greater than zero") + raise forms.ValidationError("Funding goal must be greater than zero.") return funding_goal🧰 Tools
🪛 Ruff (0.8.2)
318-318: Missing return type annotation for private function
clean_funding_goal(ANN202)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
280-310: 🛠️ Refactor suggestionFix mutable class attributes and add trailing commas.
The form's
widgetsdictionary is a mutable class attribute that should be annotated withtyping.ClassVar. Also, trailing commas are missing at the end of multiple dictionary entries.+from typing import ClassVar, Dict, Any class CampaignForm(forms.ModelForm): class Meta: model = Campaign fields = ["title", "description", "funding_goal", "itemized_budget", "video_url", "image"] - widgets = { + widgets: ClassVar[Dict[str, Any]] = { "title": TailwindInput(attrs={"placeholder": "Campaign Title"}), "description": TailwindTextarea( attrs={ - "placeholder": "Describe your campaign in detail", + "placeholder": "Describe your campaign in detail", } ), "funding_goal": TailwindNumberInput( attrs={ - "placeholder": "Target funding amount in dollars", + "placeholder": "Target funding amount in dollars", } ), "itemized_budget": TailwindTextarea( - attrs={"placeholder": "Optional JSON field for itemized budget breakdown"} + attrs={"placeholder": "Optional JSON field for itemized budget breakdown"}, ), "video_url": TailwindInput( attrs={ "type": "url", - "placeholder": "Optional video pitch URL", + "placeholder": "Optional video pitch URL", } ), "image": TailwindFileInput( attrs={ - "accept": "image/*", + "accept": "image/*", } ), }🧰 Tools
🪛 Ruff (0.8.2)
283-283: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
284-310: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
289-289: Trailing comma missing
Add trailing comma
(COM812)
294-294: Trailing comma missing
Add trailing comma
(COM812)
297-297: Trailing comma missing
Add trailing comma
(COM812)
303-303: Trailing comma missing
Add trailing comma
(COM812)
308-308: Trailing comma missing
Add trailing comma
(COM812)
web/views.py (4)
2344-2346:⚠️ Potential issueSecurity issue: Teacher dashboard protection removed.
The
@login_requiredand@teacher_requireddecorators are commented out, removing authentication and authorization requirements. This is a significant security risk that could allow any unauthenticated user to access the teacher dashboard.Restore the security decorators to protect this view:
-# @login_required -# @teacher_required +@login_required +@teacher_required def teacher_dashboard(request):🧰 Tools
🪛 Ruff (0.8.2)
2346-2346: Missing return type annotation for public function
teacher_dashboard(ANN201)
2346-2346: Missing type annotation for function argument
request(ANN001)
1395-1404: 🧹 Nitpick (assertive)Add type annotations and fix missing trailing comma in
crowdfunding_detail.The function contains good access control checks but needs a few minor improvements:
-def crowdfunding_detail(request, campaign_id): +def crowdfunding_detail(request: HttpRequest, campaign_id: int) -> HttpResponse: campaign = get_object_or_404(Campaign, id=campaign_id) # Check if campaign is approved and live or if user has special access if not (campaign.approved and campaign.live) and not ( request.user.is_authenticated and (campaign.teacher == request.user or request.user.is_staff) ): messages.error(request, "This campaign is not available.") return redirect("crowdfunding_list") - context = {"campaign": campaign, "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY} + context = {"campaign": campaign, "STRIPE_PUBLISHABLE_KEY": settings.STRIPE_PUBLISHABLE_KEY,} return render(request, "crowdfunding_detail.html", context)🧰 Tools
🪛 Ruff (0.8.2)
1395-1395: Missing return type annotation for public function
crowdfunding_detail(ANN201)
1395-1395: Missing type annotation for function argument
request(ANN001)
1395-1395: Missing type annotation for function argument
campaign_id(ANN001)
1347-1361: 🧹 Nitpick (assertive)Improve error handling and add type annotations in
handle_failed_donation.The function needs type annotations and better logging practices:
-def handle_failed_donation(payment_intent): +def handle_failed_donation(payment_intent) -> None: """ Handle a failed donation payment. You can log the failure or notify the donor that the payment did not go through. """ try: campaign_id = payment_intent.metadata.get("campaign_id") user_id = payment_intent.metadata.get("user_id") logger = logging.getLogger(__name__) - logger.warning(f"Donation payment failed for campaign {campaign_id} and user {user_id}.") + logger.warning("Donation payment failed for campaign %s and user %s", campaign_id, user_id) except Exception as e: logger = logging.getLogger(__name__) - logger.error(f"Error handling failed donation: {e}") + logger.exception("Error handling failed donation")🧰 Tools
🪛 Ruff (0.8.2)
1347-1347: Missing return type annotation for public function
handle_failed_donationAdd return type annotation:
None(ANN201)
1347-1347: Missing type annotation for function argument
payment_intent(ANN001)
1357-1357: Logging statement uses f-string
(G004)
1358-1358: Do not catch blind exception:
Exception(BLE001)
1360-1360: Use
logging.exceptioninstead oflogging.errorReplace with
exception(TRY400)
1360-1360: Logging statement uses f-string
(G004)
1221-1254: 🛠️ Refactor suggestionAdd type annotations and improve error handling in
create_donation_intent.This function is well-structured for creating Stripe payment intents, but could use several improvements:
- Add type annotations for better code maintainability
- Use more specific exception handling instead of catching all exceptions
- Verify user authentication before accessing
request.user.idHere's a safer implementation with appropriate type annotations:
@csrf_exempt # For simplicity; in production, configure CSRF properly. -def create_donation_intent(request, campaign_id): +def create_donation_intent(request: HttpRequest, campaign_id: int) -> JsonResponse: """ Create a Stripe PaymentIntent for donating to a crowdfunding campaign. """ # Get the campaign campaign = get_object_or_404(Campaign, id=campaign_id) # For simplicity, we assume the donation amount is sent via POST data as a float (in dollars) if request.method == "POST": try: data = json.loads(request.body) donation_amount = float(data.get("amount", 0)) if donation_amount <= 0: return JsonResponse({"error": "Invalid donation amount."}, status=400) # Convert dollars to cents for Stripe amount_cents = int(donation_amount * 100) # Create a PaymentIntent intent = stripe.PaymentIntent.create( amount=amount_cents, currency="usd", metadata={ "campaign_id": campaign.id, - "user_id": request.user.id, + "user_id": request.user.id if request.user.is_authenticated else None, }, ) return JsonResponse({"clientSecret": intent.client_secret}) - except Exception as e: + except json.JSONDecodeError: + return JsonResponse({"error": "Invalid JSON in request body"}, status=400) + except ValueError: + return JsonResponse({"error": "Invalid donation amount format"}, status=400) + except stripe.error.StripeError as e: return JsonResponse({"error": str(e)}, status=400) + except Exception as e: + logger.exception("Unexpected error in create_donation_intent") + return JsonResponse({"error": "An unexpected error occurred"}, status=500) else: # For non-POST requests, redirect back or render a page. return JsonResponse({"error": "POST method required"}, status=405)🧰 Tools
🪛 Ruff (0.8.2)
1222-1222: Missing return type annotation for public function
create_donation_intent(ANN201)
1222-1222: Missing type annotation for function argument
request(ANN001)
1222-1222: Missing type annotation for function argument
campaign_id(ANN001)
1250-1250: Do not catch blind exception:
Exception(BLE001)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
web/forms.py(2 hunks)web/models.py(3 hunks)web/urls.py(2 hunks)web/views.py(6 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
web/forms.py (2)
web/models.py (17)
Campaign(28-69)Meta(49-52)Meta(105-106)Meta(263-264)Meta(455-456)Meta(601-602)Meta(657-658)Meta(679-680)Meta(726-729)Meta(780-781)Meta(833-835)Meta(859-860)Meta(876-878)Meta(900-901)Meta(916-917)Meta(964-965)Meta(1031-1032)web/widgets.py (4)
TailwindInput(5-16)TailwindTextarea(19-31)TailwindNumberInput(48-59)TailwindFileInput(89-101)
web/urls.py (1)
web/views.py (4)
crowdfunding_list(1407-1413)crowdfunding_create(1418-1437)crowdfunding_detail(1395-1404)create_donation_intent(1222-1254)
web/models.py (1)
web/forms.py (21)
Meta(281-310)Meta(374-380)Meta(384-413)Meta(459-497)Meta(508-547)Meta(575-581)Meta(585-609)Meta(703-705)Meta(808-828)Meta(851-863)Meta(869-885)Meta(969-1006)Meta(1012-1023)Meta(1040-1045)Meta(1111-1142)Meta(1148-1169)save(239-277)save(633-658)save(725-739)save(1362-1375)save(1536-1551)
🪛 Ruff (0.8.2)
web/forms.py
283-283: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
284-310: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
289-289: Trailing comma missing
Add trailing comma
(COM812)
294-294: Trailing comma missing
Add trailing comma
(COM812)
297-297: Trailing comma missing
Add trailing comma
(COM812)
303-303: Trailing comma missing
Add trailing comma
(COM812)
308-308: Trailing comma missing
Add trailing comma
(COM812)
312-316: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
318-318: Missing return type annotation for private function clean_funding_goal
(ANN202)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
web/models.py
34-34: Trailing comma missing
Add trailing comma
(COM812)
40-40: Trailing comma missing
Add trailing comma
(COM812)
50-50: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
54-54: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
57-57: Missing return type annotation for public function get_absolute_url
(ANN201)
63-63: Missing return type annotation for public function update_amount_raised
Add return type annotation: None
(ANN201)
78-78: Trailing comma missing
Add trailing comma
(COM812)
85-85: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
2854-2854: Missing return type annotation for public function update_campaign_amount_on_donation
Add return type annotation: None
(ANN201)
2854-2854: Missing type annotation for function argument sender
(ANN001)
2854-2854: Unused function argument: sender
(ARG001)
2854-2854: Missing type annotation for function argument instance
(ANN001)
2854-2854: Missing type annotation for function argument created
(ANN001)
2854-2854: Unused function argument: created
(ARG001)
2854-2854: Missing type annotation for **kwargs
(ANN003)
2854-2854: Unused function argument: kwargs
(ARG001)
2860-2860: Missing return type annotation for public function update_campaign_amount_on_donation_delete
Add return type annotation: None
(ANN201)
2860-2860: Missing type annotation for function argument sender
(ANN001)
2860-2860: Unused function argument: sender
(ARG001)
2860-2860: Missing type annotation for function argument instance
(ANN001)
2860-2860: Missing type annotation for **kwargs
(ANN003)
2860-2860: Unused function argument: kwargs
(ARG001)
web/views.py
1222-1222: Missing return type annotation for public function create_donation_intent
(ANN201)
1222-1222: Missing type annotation for function argument request
(ANN001)
1222-1222: Missing type annotation for function argument campaign_id
(ANN001)
1250-1250: Do not catch blind exception: Exception
(BLE001)
1305-1305: Missing return type annotation for public function handle_successful_donation
Add return type annotation: None
(ANN201)
1305-1305: Missing type annotation for function argument payment_intent
(ANN001)
1341-1341: Do not catch blind exception: Exception
(BLE001)
1344-1344: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1344-1344: Logging statement uses f-string
(G004)
1347-1347: Missing return type annotation for public function handle_failed_donation
Add return type annotation: None
(ANN201)
1347-1347: Missing type annotation for function argument payment_intent
(ANN001)
1357-1357: Logging statement uses f-string
(G004)
1358-1358: Do not catch blind exception: Exception
(BLE001)
1360-1360: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1360-1360: Logging statement uses f-string
(G004)
1395-1395: Missing return type annotation for public function crowdfunding_detail
(ANN201)
1395-1395: Missing type annotation for function argument request
(ANN001)
1395-1395: Missing type annotation for function argument campaign_id
(ANN001)
1407-1407: Missing return type annotation for public function crowdfunding_list
(ANN201)
1407-1407: Missing type annotation for function argument request
(ANN001)
1418-1418: Missing return type annotation for public function crowdfunding_create
(ANN201)
1418-1418: Missing type annotation for function argument request
(ANN001)
1433-1433: Unnecessary else after return statement
Remove unnecessary else
(RET505)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (7)
web/models.py (4)
16-16: No issues with the new signals import.
Clean addition to prepare for the post_save and post_delete handlers.
72-87: No major issues inCampaignDonation.
This model effectively tracks donor details and amounts. The separation from the globalDonationmodel makes sense if they serve different business cases, though any shared logic (e.g., user references or reward processing) could be consolidated if desired.🧰 Tools
🪛 Ruff (0.8.2)
78-78: Trailing comma missing
Add trailing comma
(COM812)
85-85: Missing return type annotation for special method
__str__Add return type annotation:
str(ANN204)
2853-2858: Add type annotations and rename unused parameters for clarity.
The same suggestion was previously raised for signals. Reiterating for completeness:🧰 Tools
🪛 Ruff (0.8.2)
2854-2854: Missing return type annotation for public function
update_campaign_amount_on_donationAdd return type annotation:
None(ANN201)
2854-2854: Missing type annotation for function argument
sender(ANN001)
2854-2854: Unused function argument:
sender(ARG001)
2854-2854: Missing type annotation for function argument
instance(ANN001)
2854-2854: Missing type annotation for function argument
created(ANN001)
2854-2854: Unused function argument:
created(ARG001)
2854-2854: Missing type annotation for
**kwargs(ANN003)
2854-2854: Unused function argument:
kwargs(ARG001)
2859-2862: Apply type annotations and rename unused parameters in the delete signal as well.
Mirrors prior feedback for uniformity with the post_save signal.🧰 Tools
🪛 Ruff (0.8.2)
2860-2860: Missing return type annotation for public function
update_campaign_amount_on_donation_deleteAdd return type annotation:
None(ANN201)
2860-2860: Missing type annotation for function argument
sender(ANN001)
2860-2860: Unused function argument:
sender(ARG001)
2860-2860: Missing type annotation for function argument
instance(ANN001)
2860-2860: Missing type annotation for
**kwargs(ANN003)
2860-2860: Unused function argument:
kwargs(ARG001)
web/urls.py (2)
41-43: URL patterns for crowdfunding look good!The URL patterns for listing, creating, and viewing campaigns follow good naming conventions and URL structure. These endpoints properly map to their corresponding view functions in views.py.
61-61: Consider relocating the donation intent URL to group with other crowdfunding URLs.The URL for creating a donation intent is separated from the other crowdfunding URLs, which might make maintenance more difficult.
Consider moving this URL to be grouped with the other crowdfunding URLs for better organization:
- path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"),Move it to line 44 after the other crowdfunding URLs:
+ path("crowdfunding/<int:campaign_id>/donate/", views.create_donation_intent, name="create_donation_intent"),web/views.py (1)
1378-1391: Well-implemented payment intent handling in webhook.The implementation for different payment intent types is clear and well-structured. Good job branching the logic to handle both course and campaign payment intents.
| if funding_goal <= 0: | ||
| raise forms.ValidationError("Funding goal must be greater than zero") | ||
| return funding_goal | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add URL validation for video_url field.
The video_url field should have validation to ensure it contains a valid URL from trusted video sources like YouTube or Vimeo.
def clean_video_url(self) -> str:
"""Validate that video URL is from a trusted source."""
video_url = self.cleaned_data.get("video_url")
if video_url:
import re
# Check for YouTube or Vimeo URLs
youtube_pattern = r"^(https?://)?(www\.)?(youtube\.com/watch\?v=|youtu\.be/)[a-zA-Z0-9_-]{11}.*$"
vimeo_pattern = r"^(https?://)?(www\.)?vimeo\.com/[0-9]{8,}.*$"
if not (re.match(youtube_pattern, video_url) or re.match(vimeo_pattern, video_url)):
raise forms.ValidationError("Please enter a valid YouTube or Vimeo URL.")
return video_url| help_texts = { | ||
| "itemized_budget": "Enter a JSON object with budget items and costs", | ||
| "video_url": "Link to a video that explains your campaign", | ||
| "image": "Upload an image for your campaign (max size: 5MB)", | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Move help_texts inside the Meta class and add ClassVar annotation.
For consistency with Django's form convention, the help_texts dictionary should be moved inside the Meta class and annotated with typing.ClassVar.
class CampaignForm(forms.ModelForm):
class Meta:
model = Campaign
fields = ["title", "description", "funding_goal", "itemized_budget", "video_url", "image"]
widgets: ClassVar[Dict[str, Any]] = {
# ... widget definitions ...
}
+ help_texts: ClassVar[Dict[str, str]] = {
+ "itemized_budget": "Enter a JSON object with budget items and costs",
+ "video_url": "Link to a video that explains your campaign",
+ "image": "Upload an image for your campaign (max size: 5MB)",
+ }
- help_texts = {
- "itemized_budget": "Enter a JSON object with budget items and costs",
- "video_url": "Link to a video that explains your campaign",
- "image": "Upload an image for your campaign (max size: 5MB)",
- }📝 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.
| help_texts = { | |
| "itemized_budget": "Enter a JSON object with budget items and costs", | |
| "video_url": "Link to a video that explains your campaign", | |
| "image": "Upload an image for your campaign (max size: 5MB)", | |
| } | |
| from typing import ClassVar, Dict, Any | |
| from django import forms | |
| class CampaignForm(forms.ModelForm): | |
| class Meta: | |
| model = Campaign | |
| fields = ["title", "description", "funding_goal", "itemized_budget", "video_url", "image"] | |
| widgets: ClassVar[Dict[str, Any]] = { | |
| # ... widget definitions ... | |
| } | |
| help_texts: ClassVar[Dict[str, str]] = { | |
| "itemized_budget": "Enter a JSON object with budget items and costs", | |
| "video_url": "Link to a video that explains your campaign", | |
| "image": "Upload an image for your campaign (max size: 5MB)", | |
| } | |
| # Removed the previous class-level help_texts attribute | |
| # (if present outside the Meta class) |
🧰 Tools
🪛 Ruff (0.8.2)
312-316: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
| class Campaign(models.Model): | ||
| """ | ||
| Model representing a crowdfunding campaign created by a teacher. | ||
| """ | ||
|
|
||
| teacher = models.ForeignKey( | ||
| User, on_delete=models.CASCADE, related_name="campaigns", help_text="Teacher who created the campaign." | ||
| ) | ||
| title = models.CharField(max_length=255, help_text="Campaign title") | ||
| description = models.TextField(help_text="Detailed campaign description") | ||
| funding_goal = models.DecimalField(max_digits=10, decimal_places=2, help_text="Target funding amount in dollars") | ||
| amount_raised = models.DecimalField( | ||
| max_digits=10, decimal_places=2, default=0, help_text="Total funds raised so far" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using an atomic update strategy for amount_raised.
Currently, update_amount_raised() recalculates the total from all donations, which may cause concurrency issues under high load if donations are saved in quick succession. Although recalculating might be acceptable for smaller volumes, an atomic approach or incremental updates could enhance performance and consistency for larger-scale scenarios.
🧰 Tools
🪛 Ruff (0.8.2)
34-34: Trailing comma missing
Add trailing comma
(COM812)
40-40: Trailing comma missing
Add trailing comma
(COM812)
| def __str__(self): | ||
| return self.title | ||
|
|
||
| def get_absolute_url(self): | ||
| """ | ||
| Returns the URL to access a particular campaign's detail view. | ||
| """ | ||
| return reverse("crowdfunding_detail", kwargs={"campaign_id": self.id}) | ||
|
|
||
| def update_amount_raised(self): | ||
| """ | ||
| Recalculate and update the total amount raised from related donations. | ||
| """ | ||
| total = self.donations.aggregate(total=models.Sum("amount"))["total"] or 0 | ||
| self.amount_raised = total | ||
| self.save() | ||
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add return type annotations to improve clarity.
The functions __str__, get_absolute_url, and update_amount_raised lack return type annotations. Adding them (e.g., -> str or -> None) can help meet static analysis recommendations and make the code more comprehensible.
🧰 Tools
🪛 Ruff (0.8.2)
54-54: Missing return type annotation for special method __str__
Add return type annotation: str
(ANN204)
57-57: Missing return type annotation for public function get_absolute_url
(ANN201)
63-63: Missing return type annotation for public function update_amount_raised
Add return type annotation: None
(ANN201)
| def crowdfunding_list(request): | ||
| """ | ||
| Display all active crowdfunding campaigns. | ||
| Active campaigns are those that are approved and live. | ||
| """ | ||
| campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") | ||
| return render(request, "crowdfunding_list.html", {"campaigns": campaigns}) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type annotations to crowdfunding_list.
The function is well-implemented but needs type annotations for better code maintainability.
-def crowdfunding_list(request):
+def crowdfunding_list(request: HttpRequest) -> HttpResponse:
"""
Display all active crowdfunding campaigns.
Active campaigns are those that are approved and live.
"""
campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at")
return render(request, "crowdfunding_list.html", {"campaigns": campaigns})📝 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.
| def crowdfunding_list(request): | |
| """ | |
| Display all active crowdfunding campaigns. | |
| Active campaigns are those that are approved and live. | |
| """ | |
| campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") | |
| return render(request, "crowdfunding_list.html", {"campaigns": campaigns}) | |
| def crowdfunding_list(request: HttpRequest) -> HttpResponse: | |
| """ | |
| Display all active crowdfunding campaigns. | |
| Active campaigns are those that are approved and live. | |
| """ | |
| campaigns = Campaign.objects.filter(approved=True, live=True).order_by("-created_at") | |
| return render(request, "crowdfunding_list.html", {"campaigns": campaigns}) |
🧰 Tools
🪛 Ruff (0.8.2)
1407-1407: Missing return type annotation for public function crowdfunding_list
(ANN201)
1407-1407: Missing type annotation for function argument request
(ANN001)
| @login_required | ||
| @teacher_required | ||
| def crowdfunding_create(request): | ||
| """ | ||
| View to allow teachers to create a new crowdfunding campaign. | ||
| """ | ||
| if request.method == "POST": | ||
| form = CampaignForm(request.POST, request.FILES) | ||
| if form.is_valid(): | ||
| campaign = form.save(commit=False) | ||
| campaign.teacher = request.user | ||
| # Set initial approval status (set to False if admin approval is required) | ||
| campaign.approved = True # or False if approval workflow is needed | ||
| campaign.live = True # or False if campaigns shouldn't be live immediately | ||
| campaign.save() | ||
| messages.success(request, "Your crowdfunding campaign has been created successfully!") | ||
| return redirect("crowdfunding_detail", campaign_id=campaign.id) | ||
| else: | ||
| messages.error(request, "Please correct the errors below.") | ||
| else: | ||
| form = CampaignForm() | ||
| return render(request, "crowdfunding_create.html", {"form": form}) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve implementation of crowdfunding_create with type annotations and fix unnecessary else.
The function is well-implemented with proper decorators and error handling, but could use some improvements:
@login_required
@teacher_required
-def crowdfunding_create(request):
+def crowdfunding_create(request: HttpRequest) -> HttpResponse:
"""
View to allow teachers to create a new crowdfunding campaign.
"""
if request.method == "POST":
form = CampaignForm(request.POST, request.FILES)
if form.is_valid():
campaign = form.save(commit=False)
campaign.teacher = request.user
# Set initial approval status (set to False if admin approval is required)
campaign.approved = True # or False if approval workflow is needed
campaign.live = True # or False if campaigns shouldn't be live immediately
campaign.save()
messages.success(request, "Your crowdfunding campaign has been created successfully!")
return redirect("crowdfunding_detail", campaign_id=campaign.id)
- else:
- messages.error(request, "Please correct the errors below.")
+ messages.error(request, "Please correct the errors below.")
else:
form = CampaignForm()
return render(request, "crowdfunding_create.html", {"form": form})The else after the return statement is unnecessary since if the form is valid, the function will return before reaching the line where the error message is added.
📝 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.
| @login_required | |
| @teacher_required | |
| def crowdfunding_create(request): | |
| """ | |
| View to allow teachers to create a new crowdfunding campaign. | |
| """ | |
| if request.method == "POST": | |
| form = CampaignForm(request.POST, request.FILES) | |
| if form.is_valid(): | |
| campaign = form.save(commit=False) | |
| campaign.teacher = request.user | |
| # Set initial approval status (set to False if admin approval is required) | |
| campaign.approved = True # or False if approval workflow is needed | |
| campaign.live = True # or False if campaigns shouldn't be live immediately | |
| campaign.save() | |
| messages.success(request, "Your crowdfunding campaign has been created successfully!") | |
| return redirect("crowdfunding_detail", campaign_id=campaign.id) | |
| else: | |
| messages.error(request, "Please correct the errors below.") | |
| else: | |
| form = CampaignForm() | |
| return render(request, "crowdfunding_create.html", {"form": form}) | |
| @login_required | |
| @teacher_required | |
| def crowdfunding_create(request: HttpRequest) -> HttpResponse: | |
| """ | |
| View to allow teachers to create a new crowdfunding campaign. | |
| """ | |
| if request.method == "POST": | |
| form = CampaignForm(request.POST, request.FILES) | |
| if form.is_valid(): | |
| campaign = form.save(commit=False) | |
| campaign.teacher = request.user | |
| # Set initial approval status (set to False if admin approval is required) | |
| campaign.approved = True # or False if approval workflow is needed | |
| campaign.live = True # or False if campaigns shouldn't be live immediately | |
| campaign.save() | |
| messages.success(request, "Your crowdfunding campaign has been created successfully!") | |
| return redirect("crowdfunding_detail", campaign_id=campaign.id) | |
| messages.error(request, "Please correct the errors below.") | |
| else: | |
| form = CampaignForm() | |
| return render(request, "crowdfunding_create.html", {"form": form}) |
🧰 Tools
🪛 Ruff (0.8.2)
1418-1418: Missing return type annotation for public function crowdfunding_create
(ANN201)
1418-1418: Missing type annotation for function argument request
(ANN001)
1433-1433: Unnecessary else after return statement
Remove unnecessary else
(RET505)
| def handle_successful_donation(payment_intent): | ||
| """ | ||
| Handle a successful donation payment. | ||
| Create a Donation record and update the campaign's amount raised. | ||
| """ | ||
| try: | ||
| campaign_id = payment_intent.metadata.get("campaign_id") | ||
| user_id = payment_intent.metadata.get("user_id") | ||
| if not campaign_id or not user_id: | ||
| # Not a donation payment; you might log this or ignore. | ||
| return | ||
|
|
||
| # Convert amount from cents to dollars. | ||
| # donation_amount = Decimal(payment_intent.amount_received) / 100 | ||
|
|
||
| # Retrieve campaign and user. | ||
| campaign = Campaign.objects.get(id=campaign_id) | ||
| # user = User.objects.get(id=user_id) | ||
|
|
||
| # # Create the donation record. | ||
| # donation = Donation.objects.create( | ||
| # campaign=campaign, | ||
| # donor_name=user.get_full_name() or user.username, | ||
| # donor_email=user.email, | ||
| # amount=donation_amount, | ||
| # status="completed", # or set appropriate status if your Donation model uses one | ||
| # stripe_payment_intent_id=payment_intent.id, | ||
| # ) | ||
|
|
||
| # Update the campaign's amount raised. | ||
| campaign.update_amount_raised() | ||
|
|
||
| # Optionally, you can send a notification email to the donor or campaign teacher here. | ||
| # For example: | ||
| # send_donation_confirmation_email(user, campaign, donation) | ||
|
|
||
| except Exception as e: | ||
| # Log error if needed. | ||
| logger = logging.getLogger(__name__) | ||
| logger.error(f"Error handling successful donation: {e}") | ||
|
|
There was a problem hiding this comment.
Fix incomplete implementation in handle_successful_donation.
The implementation has several issues:
- Contains commented out code that appears to be part of the implementation
- Missing type annotations
- Uses generic exception handling and f-string in logging
The function has incomplete/commented out code for creating donation records, which needs to be implemented:
-def handle_successful_donation(payment_intent):
+def handle_successful_donation(payment_intent) -> None:
"""
Handle a successful donation payment.
Create a Donation record and update the campaign's amount raised.
"""
try:
campaign_id = payment_intent.metadata.get("campaign_id")
user_id = payment_intent.metadata.get("user_id")
if not campaign_id or not user_id:
# Not a donation payment; you might log this or ignore.
return
# Convert amount from cents to dollars.
- # donation_amount = Decimal(payment_intent.amount_received) / 100
+ donation_amount = Decimal(payment_intent.amount_received) / 100
# Retrieve campaign and user.
campaign = Campaign.objects.get(id=campaign_id)
- # user = User.objects.get(id=user_id)
+ user = User.objects.get(id=user_id) if user_id else None
- # # Create the donation record.
- # donation = Donation.objects.create(
- # campaign=campaign,
- # donor_name=user.get_full_name() or user.username,
- # donor_email=user.email,
- # amount=donation_amount,
- # status="completed", # or set appropriate status if your Donation model uses one
- # stripe_payment_intent_id=payment_intent.id,
- # )
+ # Create the donation record.
+ donation = CampaignDonation.objects.create(
+ campaign=campaign,
+ donor_name=user.get_full_name() or user.username if user else "Anonymous",
+ donor_email=user.email if user else None,
+ amount=donation_amount,
+ status="completed",
+ stripe_payment_intent_id=payment_intent.id,
+ )
# Update the campaign's amount raised.
campaign.update_amount_raised()
# Optionally, you can send a notification email to the donor or campaign teacher here.
# For example:
# send_donation_confirmation_email(user, campaign, donation)
- except Exception as e:
+ except (Campaign.DoesNotExist, User.DoesNotExist) as e:
+ logger.error("Campaign or User not found: %s", str(e))
+ except Exception as e:
# Log error if needed.
logger = logging.getLogger(__name__)
- logger.error(f"Error handling successful donation: {e}")
+ logger.exception("Error handling successful donation")The model referenced in the code should be CampaignDonation based on the context, rather than Donation. Make sure this model exists and is properly imported.
📝 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.
| def handle_successful_donation(payment_intent): | |
| """ | |
| Handle a successful donation payment. | |
| Create a Donation record and update the campaign's amount raised. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| if not campaign_id or not user_id: | |
| # Not a donation payment; you might log this or ignore. | |
| return | |
| # Convert amount from cents to dollars. | |
| # donation_amount = Decimal(payment_intent.amount_received) / 100 | |
| # Retrieve campaign and user. | |
| campaign = Campaign.objects.get(id=campaign_id) | |
| # user = User.objects.get(id=user_id) | |
| # # Create the donation record. | |
| # donation = Donation.objects.create( | |
| # campaign=campaign, | |
| # donor_name=user.get_full_name() or user.username, | |
| # donor_email=user.email, | |
| # amount=donation_amount, | |
| # status="completed", # or set appropriate status if your Donation model uses one | |
| # stripe_payment_intent_id=payment_intent.id, | |
| # ) | |
| # Update the campaign's amount raised. | |
| campaign.update_amount_raised() | |
| # Optionally, you can send a notification email to the donor or campaign teacher here. | |
| # For example: | |
| # send_donation_confirmation_email(user, campaign, donation) | |
| except Exception as e: | |
| # Log error if needed. | |
| logger = logging.getLogger(__name__) | |
| logger.error(f"Error handling successful donation: {e}") | |
| def handle_successful_donation(payment_intent) -> None: | |
| """ | |
| Handle a successful donation payment. | |
| Create a Donation record and update the campaign's amount raised. | |
| """ | |
| try: | |
| campaign_id = payment_intent.metadata.get("campaign_id") | |
| user_id = payment_intent.metadata.get("user_id") | |
| if not campaign_id or not user_id: | |
| # Not a donation payment; you might log this or ignore. | |
| return | |
| # Convert amount from cents to dollars. | |
| donation_amount = Decimal(payment_intent.amount_received) / 100 | |
| # Retrieve campaign and user. | |
| campaign = Campaign.objects.get(id=campaign_id) | |
| user = User.objects.get(id=user_id) if user_id else None | |
| # Create the donation record. | |
| donation = CampaignDonation.objects.create( | |
| campaign=campaign, | |
| donor_name=user.get_full_name() or user.username if user else "Anonymous", | |
| donor_email=user.email if user else None, | |
| amount=donation_amount, | |
| status="completed", | |
| stripe_payment_intent_id=payment_intent.id, | |
| ) | |
| # Update the campaign's amount raised. | |
| campaign.update_amount_raised() | |
| # Optionally, you can send a notification email to the donor or campaign teacher here. | |
| # For example: | |
| # send_donation_confirmation_email(user, campaign, donation) | |
| except (Campaign.DoesNotExist, User.DoesNotExist) as e: | |
| logger.error("Campaign or User not found: %s", str(e)) | |
| except Exception as e: | |
| # Log error if needed. | |
| logger = logging.getLogger(__name__) | |
| logger.exception("Error handling successful donation") |
🧰 Tools
🪛 Ruff (0.8.2)
1305-1305: Missing return type annotation for public function handle_successful_donation
Add return type annotation: None
(ANN201)
1305-1305: Missing type annotation for function argument payment_intent
(ANN001)
1341-1341: Do not catch blind exception: Exception
(BLE001)
1344-1344: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1344-1344: Logging statement uses f-string
(G004)
|
changes were requested 3 weeks ago and no further changes have been made |
2 participants
Fixes #306
feat(crowdfunding): add Classroom Crowdfunding feature
Teachers Create a Campaign
1)Title, description, images, funding goal
-Itemized budget and purpose
2)Campaign Goes Live
-Featured on Alpha One Labs’ Crowdfund page
Summary by CodeRabbit