Feat/work review#320
Conversation
|
""" WalkthroughThis change introduces a comprehensive "Work Submission" feature set. It adds two new models: Changes
Sequence Diagram(s)sequenceDiagram
participant Student
participant WebApp
participant Teacher
participant DB
Student->>WebApp: Access 'Upload Document' page
WebApp->>DB: Fetch WorkTypes
WebApp-->>Student: Render upload form
Student->>WebApp: Submit document via form
WebApp->>DB: Validate & save WorkSubmission
WebApp-->>Student: Show success message
Teacher->>WebApp: Access 'Review Submissions' list
WebApp->>DB: Fetch all WorkSubmissions
WebApp-->>Teacher: Render submissions list
Teacher->>WebApp: View submission detail
WebApp->>DB: Fetch WorkSubmission
WebApp-->>Teacher: Render detail with feedback form
Teacher->>WebApp: Submit feedback/status update
WebApp->>DB: Update WorkSubmission
WebApp-->>Teacher: Show confirmation
WebApp-->>Student: Notify feedback received
Assessment against linked issues
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 2
🔭 Outside diff range comments (1)
web/models.py (1)
2515-2580: 🛠️ Refactor suggestionAdd server-side max file size validation.
While there's a help text specifying the 10MB limit, there's no actual validator to enforce it. Relying solely on help text and UI constraints may pose a risk of oversize files being uploaded. Consider adding a model-level or form-level check:
class GSoCProposal(models.Model): ... + def clean(self): + super().clean() + max_size_mb = 10 + if self.proposal_file and self.proposal_file.size > max_size_mb * 1024 * 1024: + raise ValidationError(f"Proposal file size must not exceed {max_size_mb} MB.")
🧹 Nitpick comments (8)
web/templates/index.html (1)
353-378: Clarify and Validate the "Initial GSoC Announcement" SectionThe updated section changes the announcement comment from GSoC Announcement to Initial GSoC Announcement and presents a clear banner inviting users to learn more about GSoC 2025. The structure, styling, and use of a fallback image (via the onerror attribute) are in line with the design language of the site.
Recommendations:
- Verify that the fallback URL in the image onerror attribute works as intended under various conditions.
- Optionally, consider adding a brief tooltip or ARIA label to the banner for better accessibility.
web/models.py (1)
2515-2580: Consider adding tests for the new model.Adding unit tests for uploading valid/invalid proposal files (e.g., invalid extension or oversize file) helps maintain robust coverage and catch regressions.
web/templates/web/gsoc/upload_proposal.html (1)
13-26: Check accessibility for color contrast and icons.Consider verifying the color contrast (e.g., blue backgrounds) and adding additional ARIA labels for icons. This promotes inclusivity for screen readers and low-vision users.
web/forms.py (1)
1675-1687: Consider a more flexible PDF content type validation.The content type validation is currently using strict equality (
==) which might not catch all valid PDF types. PDF content types can vary beyond just "application/pdf".- if not uploaded_file.content_type == "application/pdf": + if not uploaded_file.content_type.lower() in ["application/pdf", "application/x-pdf", "application/acrobat"] and not uploaded_file.content_type.lower().startswith("application/pdf"): raise forms.ValidationError("File must be a PDF document.")web/templates/web/gsoc/proposal_detail.html (1)
169-172: Avoid hardcoding dates in the timeline heading.The timeline heading has a hardcoded date which might become outdated. Consider making this dynamic based on the current GSoC cycle.
- <h3 class="text-xl font-semibold">Proposal Timeline: Till 8th April,2025</h3> + <h3 class="text-xl font-semibold">Proposal Timeline: {{ gsoc_deadline|default:"Check GSoC website for details" }}</h3>This would require passing
gsoc_deadlinein the context from the view.web/admin.py (1)
715-740: Good implementation of admin configuration for GSoC proposals.The admin configuration for the GSoCProposal model is well-structured with appropriate list displays, filters, search fields, and fieldsets. The organization of fields into logical sections improves usability.
Consider the following enhancements:
- Add
raw_id_fieldsfor foreign key fields to improve performance:class GSoCProposalAdmin(admin.ModelAdmin): list_display = ("title", "student", "organization", "year", "status", "submitted_at", "file_size_display") list_filter = ("status", "year", "organization") search_fields = ("title", "student__username", "organization", "project") readonly_fields = ("file_size", "submitted_at", "updated_at", "reviewed_at") + raw_id_fields = ("student", "reviewed_by")
- Add custom admin actions to facilitate bulk operations:
class GSoCProposalAdmin(admin.ModelAdmin): list_display = ("title", "student", "organization", "year", "status", "submitted_at", "file_size_display") list_filter = ("status", "year", "organization") search_fields = ("title", "student__username", "organization", "project") readonly_fields = ("file_size", "submitted_at", "updated_at", "reviewed_at") + actions = ["mark_as_under_review", "mark_as_reviewed"] + + def mark_as_under_review(self, request, queryset): + queryset.update(status="under_review") + mark_as_under_review.short_description = "Mark selected proposals as under review" + + def mark_as_reviewed(self, request, queryset): + queryset.update(status="reviewed", reviewed_at=timezone.now()) + mark_as_reviewed.short_description = "Mark selected proposals as reviewed"
- Add a visual status indicator for improved usability:
class GSoCProposalAdmin(admin.ModelAdmin): - list_display = ("title", "student", "organization", "year", "status", "submitted_at", "file_size_display") + list_display = ("title", "student", "organization", "year", "status_display", "submitted_at", "file_size_display") list_filter = ("status", "year", "organization") search_fields = ("title", "student__username", "organization", "project") readonly_fields = ("file_size", "submitted_at", "updated_at", "reviewed_at") + + def status_display(self, obj): + status_colors = { + "submitted": "blue", + "under_review": "orange", + "reviewed": "purple", + "rejected": "red", + "accepted": "green" + } + return format_html( + '<span style="color: {};">{}</span>', + status_colors.get(obj.status, "black"), + obj.get_status_display() + ) + status_display.short_description = "Status" + status_display.admin_order_field = "status"These suggestions would enhance the admin experience but aren't critical for functionality.
web/views.py (2)
6034-6042: Consider optimizing notifications for admin usersCreating a notification for every admin user individually could lead to performance issues if there are many teachers in the system. Consider a more scalable approach, such as:
- Creating a group notification visible to all teachers
- Using a message queue for processing notifications asynchronously
- Implementing a notification filtering system so admins can choose which notifications they receive
6103-6109: Add feedback length validationConsider adding validation for the feedback content, such as minimum/maximum length requirements, to ensure meaningful reviews.
if feedback: + # Validate feedback length + if len(feedback) < 10: + messages.error(request, "Feedback must be at least 10 characters long.") + return redirect("gsoc_proposal_detail", proposal_id=proposal_id) proposal.feedback = feedback
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
web/admin.py(2 hunks)web/forms.py(3 hunks)web/migrations/0045_gsocproposal.py(1 hunks)web/models.py(2 hunks)web/templates/index.html(2 hunks)web/templates/web/gsoc/proposal_detail.html(1 hunks)web/templates/web/gsoc/proposal_list.html(1 hunks)web/templates/web/gsoc/upload_proposal.html(1 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
web/forms.py (1)
web/models.py (17)
GSoCProposal(2515-2579)Meta(44-45)Meta(201-202)Meta(365-366)Meta(476-477)Meta(532-533)Meta(554-555)Meta(601-604)Meta(654-655)Meta(707-709)Meta(733-734)Meta(750-752)Meta(774-775)Meta(790-791)Meta(838-839)Meta(905-906)Meta(936-937)
web/admin.py (1)
web/models.py (1)
GSoCProposal(2515-2579)
web/urls.py (1)
web/views.py (3)
gsoc_proposal_list(6052-6082)upload_gsoc_proposal(6016-6048)gsoc_proposal_detail(6086-6126)
web/views.py (2)
web/forms.py (6)
GSoCProposalForm(1661-1687)save(219-257)save(566-591)save(658-672)save(1295-1308)save(1469-1484)web/models.py (14)
GSoCProposal(2515-2579)save(95-99)save(153-189)save(207-210)save(274-308)save(371-398)save(488-490)save(714-717)save(911-916)save(969-974)save(1049-1052)save(1135-1146)Notification(28-48)status(2126-2136)
🔇 Additional comments (23)
web/templates/index.html (1)
379-419: Review the New "GSoC Proposal Submission/Review" SectionThis new section effectively introduces a proposal feature with role-based conditional rendering. It displays a heading and descriptive text based on whether the user’s profile is marked as a teacher. Both the call-to-action and the overall design are consistent with the existing site aesthetics.
Points to Consider:
- Conditional Safety: The section directly uses
{% if user.profile.is_teacher %}without an explicit check for user authentication. Ensure that in contexts where a user is anonymous (if applicable), the template either provides a default profile or handles the missing attribute gracefully.- URL Consistency: Both teacher and non-teacher branches use the same URL (
gsoc_proposal_list). Confirm that this shared endpoint properly differentiates actions (review vs. upload) based on the user role.web/models.py (2)
14-14: Good import addition.Importing
FileExtensionValidator, MaxValueValidator, MinValueValidatorfacilitates better field-level validation.
2513-2513: No actionable changes.This blank line addition does not introduce any issues.
web/templates/web/gsoc/upload_proposal.html (3)
1-12: Template structure looks good.You have properly extended the base template, placed a suitable title block, and included CSRF protection. This is a solid start for the new GSoC proposal upload feature.
27-55: Thorough error handling.Displaying
non_field_errorsand per-field errors ensures clarity for users. Great job providing specific error messages.
56-74: Clear user guidance.The closing panel nicely explains the mentor review process, further enhancing user experience. No concerns here.
web/migrations/0045_gsocproposal.py (1)
1-79: Migration correctly reflects the new model.The migration properly creates the
GSoCProposaltable with relevant fields and choices. This aligns well with the model definition.web/templates/web/gsoc/proposal_list.html (6)
1-11: Well-structured template with proper inheritance.The template correctly extends the base template and conditionally sets the page title based on the user role. The responsive layout with container and flexbox is a good approach.
22-36: Good implementation of conditional UI for student vs reviewer views.The template intelligently shows an upload button only for students, creating a clean and focused interface for each user role.
38-66: Well-designed table headers with responsive considerations.The table headers are properly structured and conditionally display the "Student" column only for reviewers. The use of semantic HTML with proper styling classes follows best practices.
67-134: Great status visualization with contextual colors.The status badges using conditional rendering based on proposal status with appropriate color schemes enhances user experience and provides clear visual feedback.
135-156: Good empty state handling with user guidance.The template properly handles the case when no proposals are available, providing different messages based on the user role and offering guidance to students on how to upload their first proposal.
157-187: Well-implemented pagination with proper navigation.The pagination implementation is complete with previous/next links and current page highlighting. Good user experience consideration for navigating through multiple pages of proposals.
web/forms.py (1)
1661-1674: Form is well-structured with appropriate model fields.The GSoCProposalForm correctly inherits from ModelForm and includes the necessary fields for proposal submission. The file field is properly configured with validators and user guidance.
web/templates/web/gsoc/proposal_detail.html (5)
1-24: Well-structured template with good navigation.The template correctly extends the base template and implements breadcrumb navigation, which enhances user experience and provides clear context about the current page.
25-104: Comprehensive proposal details with well-organized layout.The proposal details card provides all necessary information in a clear, organized manner. The grid layout ensures good responsiveness across different screen sizes.
106-165: Well-implemented conditional feedback section for reviewers.The feedback section is appropriately shown only when feedback exists or for reviewers. The form for providing feedback includes all necessary elements and clear instructions.
173-252: Good implementation of timeline and tips sections.The timeline visualization with status indicators and the tips card provide valuable context to users. The visual styling is consistent with the rest of the interface.
255-269: Good use of JavaScript for enhanced interactivity.The JavaScript enhances user experience by providing visual feedback on hover for timeline items. It's properly encapsulated in the DOMContentLoaded event and follows good practices.
web/admin.py (1)
29-29: Import correctly added.The
GSoCProposalmodel is properly imported from the models module, following the project's import pattern.web/views.py (3)
6015-6048: The upload_gsoc_proposal view is well-structuredThis view properly handles file uploads, form validation, and creates appropriate notifications. The login_required decorator ensures proper authentication.
6051-6083: The gsoc_proposal_list view correctly implements role-based accessThis view appropriately differentiates between teachers (who can see all proposals) and students (who can only see their own) and provides proper pagination.
6085-6127: The gsoc_proposal_detail view implements proper access controlThe view correctly restricts access to the proposal owner and teachers, and allows teachers to provide feedback and update status while creating appropriate notifications.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/admin.py (1)
718-744: Consider adding custom admin actions for GSoC proposals
This admin class looks good. A handy enhancement could be adding admin actions (e.g., mark multiple proposals as “under_review” or “accepted”) to streamline bulk operations.web/models.py (1)
2515-2580: Enforce the stated 10MB file limit with a custom validator
The help text mentions a 10MB maximum size, but it isn’t strictly enforced in code. Consider adding a validator for file size to align with user expectations. Also, consider a dynamic default year instead of hardcoding 2025 if you want a future-proof approach.+ from django.core.exceptions import ValidationError def validate_file_size(value): if value.size > 10 * 1024 * 1024: raise ValidationError("File is too large. Max size is 10MB") class GSoCProposal(models.Model): ... - proposal_file = models.FileField( - upload_to="gsoc_proposals/", - validators=[FileExtensionValidator(allowed_extensions=["pdf"])], - help_text="Upload your GSoC proposal in PDF format (max 10MB)", - ) + proposal_file = models.FileField( + upload_to="gsoc_proposals/", + validators=[ + FileExtensionValidator(allowed_extensions=["pdf"]), + validate_file_size, + ], + help_text="Upload your GSoC proposal in PDF format (max 10MB)", + ) ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/admin.py(2 hunks)web/models.py(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/admin.py (1)
web/models.py (1)
GSoCProposal(2515-2579)
🪛 GitHub Actions: .github/workflows/test.yml
web/admin.py
[error] 1-1: black formatting check failed. Files were modified by this hook.
web/models.py
[error] 1-1: black formatting check failed. Files were modified by this hook.
🔇 Additional comments (1)
web/admin.py (1)
29-29: Importing the GSoCProposal model
No issues here. This line properly imports the newly introduced model so it can be administered.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
web/migrations/0046_gsocproposal.py (3)
32-32: Consider a dynamic default year instead of hardcoding 2025.Using a hardcoded year (2025) as the default will become outdated. A more maintainable approach would be to dynamically set the default to the current year.
- ("year", models.PositiveIntegerField(default=2025, help_text="GSoC program year")), + ("year", models.PositiveIntegerField(default=lambda: datetime.datetime.now().year, help_text="GSoC program year")),Don't forget to add
import datetimeat the top of your model file.
49-49: Consider auto-populating the file_size field.The
file_sizefield is marked as non-editable and can be null, but there's no mechanism in this migration to automatically populate it when a file is uploaded. Consider implementing this logic in your model'ssave()method.In your model definition (not in the migration), you could override the save method:
def save(self, *args, **kwargs): if self.proposal_file: self.file_size = self.proposal_file.size super().save(*args, **kwargs)
72-76: Consider adding database indexes for frequently queried fields.For improved query performance, consider adding database indexes to fields that will be frequently queried, such as
status,year, andstudent.options={ "verbose_name": "GSoC Proposal", "verbose_name_plural": "GSoC Proposals", "ordering": ["-submitted_at"], + "indexes": [ + models.Index(fields=["status"]), + models.Index(fields=["year"]), + models.Index(fields=["student"]), + ], },web/views.py (2)
6012-6014: Code organization commentary.The
# GSOC proposal viewscomment provides clarity and structure around the new GSoC functionality. It's helpful documentation for future maintainers.
6015-6049: Consider restricting uploads to students if required.The
upload_gsoc_proposalview allows all logged-in users to upload proposals, including teachers. If your use case intends for only students to submit proposals, you might want to explicitly restrict the view to non-teacher users or handle the scenario where a teacher tries to upload.+from django.core.exceptions import PermissionDenied @login_required def upload_gsoc_proposal(request): + if request.user.profile.is_teacher: + raise PermissionDenied("Teachers are not allowed to submit proposals.") ...web/models.py (1)
2563-2567: Avoid multiple lookups on save.Retrieving the old instance within
save()usingGSoCProposal.objects.get(pk=self.pk)can lead to an extra database query. Consider using a pre-save signal or a cached instance in your view logic if performance is a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/admin.py(2 hunks)web/migrations/0046_gsocproposal.py(1 hunks)web/models.py(2 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/urls.py
- web/admin.py
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (2)
web/forms.py (6)
GSoCProposalForm(1661-1687)save(219-257)save(566-591)save(658-672)save(1295-1308)save(1469-1484)web/models.py (13)
GSoCProposal(2515-2579)save(95-99)save(153-189)save(207-210)save(274-308)save(371-398)save(488-490)save(714-717)save(911-916)save(969-974)save(1049-1052)save(1135-1146)status(2126-2136)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (8)
web/views.py (5)
28-28: Looks good for pagination imports.These paginator-related imports are used appropriately later in the GSoC proposal views for paginating proposals. No issues here.
71-71: Import aligns with usage.The
GSoCProposalFormimport is consistent with its usage in the new GSoC proposal views. Thanks for keeping imports clear and localized.
120-120: Import aligns with usage.The
GSoCProposalmodel import is now referenced in the GSoC views. This import is appropriate and helps keep the model usage explicit.
6051-6082: Pagination logic is solid.The pagination logic in
gsoc_proposal_listcorrectly handlesEmptyPageandPageNotAnIntegerexceptions, ensuring a robust user experience.
6084-6126: Feedback & status workflow is well-structured.The
gsoc_proposal_detailview effectively checks user permissions and updates proposal feedback, status, reviewer, and notifications. This workflow is clear and maintainable. Great job!web/models.py (3)
14-14: Confirm necessity of imported validators.The newly included
FileExtensionValidatoris correctly used forproposal_file, butMaxValueValidatorandMinValueValidatorare already present elsewhere in the file. Confirm whether you intend to add new usage of them or if this line can be pruned for clarity.
2515-2517: Docstring is clear and concise.Your docstring adequately highlights the purpose of the
GSoCProposalmodel for storing PDF proposals submitted by students. It helps maintain clarity for future contributors.
2570-2580: Enhance user experience with status tracking tests.The property
file_size_displayandget_status_choicesare well-structured. To complete the feature, add test coverage to confirm file size is shown properly and ensure the status transitions are thoroughly validated (e.g., from “submitted” to “under_review,” etc.).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/migrations/0046_gsocproposal.py (1)
19-84: Consider inlining your validator logic within the migration.Referencing
web.models.GSoCProposal.validate_gsoc_file_sizefrom this migration can introduce fragility if the model or its validator function is ever refactored or removed. Migrations are meant to be static references to preserve schema evolution history. As a follow-up, consider inlining a “frozen” validator version within this migration to avoid breakages ifweb.modelschanges in the future.web/models.py (2)
2529-2532: Combine file extension checks with MIME-type validation for added security.Currently, only the file extension is checked via
FileExtensionValidator. Malicious files can bypass extension checks by renaming their extension to "pdf". Consider integrating a MIME-type check or a PDF library-based validation step to ensure the file is truly a valid PDF.
2562-2574: Efficiently track status changes when saving.Your approach detects when the proposal transitions to “reviewed” by comparing the old and new status, which is good. As a small optimization, you might conditionally fetch
old_instanceonly ifself.status == "reviewed", reducing an unnecessary database query when the status remains unchanged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/migrations/0046_gsocproposal.py(1 hunks)web/models.py(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/migrations/0046_gsocproposal.py (1)
web/models.py (2)
GSoCProposal(2515-2584)validate_gsoc_file_size(2529-2531)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (2)
web/models.py (2)
2540-2540: Verify the default year of 2025.Hard-coding
default=2025may cause unintentional data issues for future GSoC cycles. If you want to match the current year dynamically, consider usingtimezone.now().yearor a dedicated configuration.
2575-2582: Readable file size display.The
file_size_displaymethod provides a clear, user-friendly size representation. This is a neat addition that improves the user experience.
A1L13N
left a comment
A1L13N
left a comment
There was a problem hiding this comment.
This looks great! Can you please rename it to PDF Review instead of GSoC Proposal and make it more generic to work with any type of PDF
|
@A1L13N so, should i also remove the call to action section of gsoc proposal review, or is it fine? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
web/views.py (4)
6286-6291: Validate user role before assigning proposal ownership.Although the form validation ensures that a valid PDF is provided, it might be prudent to also verify that only valid student accounts can upload GSoC proposals, especially if you have a concept of user roles. If a user is not actually a student, assigning the proposal to them might cause downstream confusion.
6292-6309: Consider bulk creation for notifications.In a scenario with many teacher/admin users, iterating and creating notifications individually may affect performance. Using a bulk creation pattern can reduce query overhead:
-admin_users = User.objects.filter(profile__is_teacher=True) -for admin in admin_users: - Notification.objects.create( - user=admin, - title="New GSoC Proposal Submitted", - message=f"A new GSoC proposal has been submitted by {request.user.username} and needs review.", - notification_type="info", - ) +admin_users = list(User.objects.filter(profile__is_teacher=True)) +notifications = [ + Notification( + user=admin, + title="New GSoC Proposal Submitted", + message=f"A new GSoC proposal has been submitted by {request.user.username} and needs review.", + notification_type="info", + ) + for admin in admin_users +] +Notification.objects.bulk_create(notifications)
6318-6349: Optimize proposal retrieval with select_related.When loading the proposals, consider using
select_related("student")to minimize extra queries for proposal owners, especially in a paginated context. This can enhance performance by loading Student data in the same query:-if request.user.profile.is_teacher: - proposals_list = GSoCProposal.objects.all().order_by("-submitted_at") -else: - proposals_list = GSoCProposal.objects.filter(student=request.user).order_by("-submitted_at") +if request.user.profile.is_teacher: + proposals_list = ( + GSoCProposal.objects.select_related("student") + .all() + .order_by("-submitted_at") + ) +else: + proposals_list = ( + GSoCProposal.objects.select_related("student") + .filter(student=request.user) + .order_by("-submitted_at") + )
6362-6393: Handle concurrent feedback updates gracefully.When multiple reviewers submit feedback in quick succession, one reviewer’s updates might overwrite another’s. As a future improvement, consider a simple locking or version-checking mechanism to avoid overwriting feedback inadvertently.
web/models.py (3)
2519-2525: Consider adding a 'withdrawn' status if relevant.If a student withdraws a proposal after submission, having a “withdrawn” status could help track that scenario.
2534-2538: Extension-based validation is fine, but additional checks can boost security.A more robust check—such as verifying the file’s content type—could prevent malicious uploads masquerading as PDFs.
2584-2586: Utility method is optional.
get_status_choicessimply returnsSTATUS_CHOICES. You could directly refer to the class attribute, but if it aids Django form usage, it’s fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/models.py(2 hunks)web/templates/index.html(2 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/templates/index.html
- web/urls.py
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (2)
web/forms.py (6)
GSoCProposalForm(1661-1687)save(219-257)save(566-591)save(658-672)save(1295-1308)save(1469-1484)web/models.py (14)
GSoCProposal(2516-2585)save(95-99)save(153-189)save(207-210)save(274-308)save(371-398)save(488-490)save(715-718)save(912-917)save(970-975)save(1050-1053)save(1136-1147)Notification(28-48)status(2127-2137)
🪛 GitHub Actions: .github/workflows/test.yml
web/models.py
[error] 1-1: black formatting check failed. 1 file reformatted, indicating code style issues.
🔇 Additional comments (10)
web/views.py (1)
6358-6359: Double-check the access control logic.Ensuring that only the proposal’s owner or a teacher can view the proposal is a good security measure. Consider whether there are any additional roles (e.g., super admins or mentors) that should also have access.
web/models.py (9)
14-14: Reconfirm validator import and address Black formatting.This line’s validator imports were previously acknowledged in an outdated commit review, and the pipeline logs still indicate a Black formatting check failure. Please ensure you have Black installed and run it to pass the style checks.
2516-2518: Looks great as an entry point for GSoC proposals.The class name and docstring clearly convey the model’s purpose. No issues here.
2527-2529: Verify deletion behavior for student field.Currently,
on_delete=models.CASCADEwill delete the proposal if the student is removed. If preserving historical proposals is desired, you might switch toSET_NULLinstead.Would you like to keep the proposals if the user account is deleted?
2530-2533: Good approach for enforcing file size limit.This custom validator cleanly ensures a 10MB maximum file size for proposals.
2539-2545: Confirm default GSoC year.The default is set to 2025. If you intended to keep this dynamic, consider using the current year or letting users pick.
- year = models.PositiveIntegerField(default=2025, help_text="GSoC program year") + from django.utils import timezone + def current_year(): + return timezone.now().year + ... - year = models.PositiveIntegerField(default=2025, help_text="GSoC program year") + year = models.PositiveIntegerField(default=current_year, help_text="GSoC program year")
2546-2559: Tracking fields and Meta class look solid.Automatic timestamps, file size storage, and sorting by submission date all align well with typical review flows.
2560-2562: Readable string representation.Returning the student’s username and proposal title in
__str__is a helpful, descriptive choice.
2563-2575: Clarify status transitions.Currently,
reviewed_atis only set the first time status becomes “reviewed.” If you switch a proposal from “reviewed” back to “under_review,” consider whether to clear or resetreviewed_at.
2576-2583: Nice user-friendly file size display.The byte-to-megabyte conversion is concise and helpful for quick reference.
|
@coderabbitai give the list of changes that should be done in current commit to meet the requirements asked by @A1L13N |
A1L13N
left a comment
A1L13N
left a comment
There was a problem hiding this comment.
We could have another model for pdf types so you could select GSOC proposal, some other types could be; Students might submit many types of PDF papers to teachers, depending on the subject, assignment, or grade level. Here are some common ones:
-
Essays
• Literary analysis (e.g., analyzing a novel or poem)
• Argumentative essays
• Expository or informative essays
• Narrative essays -
Research Papers
• Based on independent research, with citations and references
• Often required in science, history, or social studies classes -
Lab Reports
• For science classes: includes hypothesis, procedure, results, analysis, and conclusion -
Book Reports / Reviews
• Summary, analysis, and personal reaction to a book -
Presentations
• Exported from PowerPoint, Google Slides, or Canva as PDF
• Usually for oral presentations or visual aids -
Homework Assignments
• Typed assignments compiled into a single PDF
• Scanned handwritten work -
Projects
• Interdisciplinary or creative work (posters, brochures, timelines, etc.)
• May include images, charts, and multimedia elements -
Reflections or Journals
• Personal writing on a topic or learning experience -
Portfolios
• A collection of a student’s best work over time -
Theses / Extended Essays
• For advanced levels like IB, AP, or university coursework
Want examples or templates of any of these?
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Inshamhaque I'll need to search for the specific requirements requested by @A1L13N in your pull request. Let me fetch those comments so I can provide you with a comprehensive list of changes needed. 🏁 Script executed: Length of output: 126 @Inshamhaque I couldn't find specific comments from @A1L13N in my initial search. Let me try to gather all comments and reviews from the PR to identify the requirements that were mentioned. 🏁 Script executed: Length of output: 20904 @Inshamhaque I'll need to find the specific requirements from A1L13N in order to provide you with a list of changes needed. Let me search for those review comments. 🏁 Script executed: Length of output: 46833 @Inshamhaque Based on the review comments from @A1L13N, here's a list of changes that should be done in the current commit:
Let me know if you need more specific guidance on implementing any of these changes! 🏁 Script executed: Length of output: 8597 @Inshamhaque Based on @A1L13N's feedback, here's a comprehensive list of changes needed to transform the GSoC-specific proposal review system into a more generic PDF review system:
Let me know if you need more specific code examples for any of these changes or if you have questions about implementing them! 🏁 Script executed: Length of output: 4787 @Inshamhaque Based on @A1L13N's feedback to rename and make the feature more generic, here's a detailed list of changes needed: 1. Create a New PDF Type Modelclass PDFType(models.Model):
"""Model for different types of PDF submissions."""
name = models.CharField(max_length=100)
description = models.TextField(blank=True)
icon_class = models.CharField(max_length=50, blank=True, help_text="Optional Font Awesome icon class")
class Meta:
verbose_name = "PDF Type"
verbose_name_plural = "PDF Types"
def __str__(self):
return self.name2. Rename and Update the Main Model# Change from GSoCProposal to:
class PDFSubmission(models.Model):
"""Model for storing PDF files submitted by students for review."""
STATUS_CHOICES = [
("submitted", "Submitted"),
("under_review", "Under Review"),
("reviewed", "Reviewed"),
("needs_revision", "Needs Revision"),
("approved", "Approved"),
]
title = models.CharField(max_length=200, help_text="Title of your submission")
student = models.ForeignKey(User, on_delete=models.CASCADE, related_name="pdf_submissions")
pdf_type = models.ForeignKey(PDFType, on_delete=models.PROTECT, related_name="submissions")
pdf_file = models.FileField(
upload_to="pdf_submissions/",
validators=[FileExtensionValidator(allowed_extensions=["pdf"]), validate_pdf_file_size],
help_text="Upload your PDF document (max 10MB)",
)
department = models.CharField(max_length=200, help_text="Department or subject area", blank=True)
topic = models.CharField(max_length=200, help_text="Topic or assignment name")
due_date = models.DateField(null=True, blank=True, help_text="Due date (if applicable)")
# Keep remaining fields like description, status, feedback, and tracking fields3. Update Form and Admin Classes
4. Update URLs# Change from:
path("gsoc-proposals/", views.gsoc_proposal_list, name="gsoc_proposal_list"),
# To:
path("pdf-submissions/", views.pdf_submission_list, name="pdf_submission_list"),
# Similar changes for upload and detail URLs5. Update View Functions
6. Update Templates
7. Update the Index Page Call to Action
8. Create Migration Files
Let me know if you'd like more specific code examples for any of these sections! 🏁 Script executed: Length of output: 2656 @Inshamhaque Based on @A1L13N's feedback and our analysis of the codebase, here's a detailed list of changes needed to transform the GSoC-specific proposal system into a more general PDF review system: 1. New Model for PDF TypesCreate a new model to categorize different types of PDF submissions: class PDFType(models.Model):
"""Model for different types of PDF submissions."""
name = models.CharField(max_length=100)
description = models.TextField(blank=True)
icon_class = models.CharField(max_length=50, blank=True, help_text="Font Awesome icon class")
class Meta:
verbose_name = "PDF Type"
verbose_name_plural = "PDF Types"
def __str__(self):
return self.name2. Rename and Modify the Main ModelRename def validate_pdf_file_size(file):
if file.size > 10 * 1024 * 1024: # 10MB
raise ValidationError("File size exceeds 10MB limit.")
class PDFSubmission(models.Model):
"""Model for storing PDF files submitted by students for review."""
STATUS_CHOICES = [
("submitted", "Submitted"),
("under_review", "Under Review"),
("reviewed", "Reviewed"),
("needs_revision", "Needs Revision"),
("approved", "Approved"),
]
title = models.CharField(max_length=200, help_text="Title of your submission")
student = models.ForeignKey(User, on_delete=models.CASCADE, related_name="pdf_submissions")
pdf_type = models.ForeignKey(PDFType, on_delete=models.PROTECT, related_name="submissions")
pdf_file = models.FileField(
upload_to="pdf_submissions/",
validators=[FileExtensionValidator(allowed_extensions=["pdf"]), validate_pdf_file_size],
help_text="Upload your PDF document (max 10MB)",
)
subject = models.CharField(max_length=200, help_text="Subject or department")
assignment = models.CharField(max_length=200, help_text="Assignment or topic")
due_date = models.DateField(null=True, blank=True, help_text="Due date (if applicable)")
description = models.TextField(help_text="Brief description of your submission", blank=True)
status = models.CharField(max_length=20, choices=STATUS_CHOICES, default="submitted")
feedback = models.TextField(blank=True, help_text="Feedback from reviewers")
# Tracking fields (keep the same)
file_size = models.PositiveIntegerField(editable=False, null=True)
submitted_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True)
reviewed_at = models.DateTimeField(null=True, blank=True)
reviewed_by = models.ForeignKey(
User, on_delete=models.SET_NULL, null=True, blank=True, related_name="reviewed_submissions"
)
class Meta:
ordering = ["-submitted_at"]
verbose_name = "PDF Submission"
verbose_name_plural = "PDF Submissions"
def __str__(self):
return f"{self.student.username}'s {self.pdf_type.name}: {self.title}"
# Keep the same save method and file_size_display property3. Update the Formclass PDFSubmissionForm(forms.ModelForm):
"""Form for submitting PDF documents."""
pdf_file = forms.FileField(
validators=[FileExtensionValidator(allowed_extensions=["pdf"])],
help_text="Upload your PDF document (max 10MB)",
widget=forms.FileInput(attrs={"accept": "application/pdf"}),
)
class Meta:
model = PDFSubmission
fields = ["title", "pdf_type", "subject", "assignment", "due_date", "description", "pdf_file"]4. Update Admin Classes@admin.register(PDFType)
class PDFTypeAdmin(admin.ModelAdmin):
list_display = ("name", "description")
search_fields = ("name", "description")
@admin.register(PDFSubmission)
class PDFSubmissionAdmin(admin.ModelAdmin):
list_display = ("title", "student", "pdf_type", "subject", "status", "submitted_at", "file_size_display")
list_filter = ("status", "pdf_type", "subject")
search_fields = ("title", "student__username", "subject", "assignment")
readonly_fields = ("file_size", "submitted_at", "updated_at", "reviewed_at")
fieldsets = (
(
"Submission Information",
{
"fields": (
"title",
"student",
"pdf_type",
"subject",
"assignment",
"due_date",
"description",
"pdf_file",
"file_size_display",
)
},
),
("Review Information", {"fields": ("status", "feedback", "reviewed_by", "reviewed_at")}),
("Timestamps", {"fields": ("submitted_at", "updated_at"), "classes": ("collapse",)}),
)5. Update URLspath("pdf-submissions/", views.pdf_submission_list, name="pdf_submission_list"),
path("pdf-submissions/upload/", views.upload_pdf_submission, name="upload_pdf_submission"),
path("pdf-submissions/<int:submission_id>/", views.pdf_submission_detail, name="pdf_submission_detail"),6. Update View Functionsdef pdf_submission_list(request):
"""View to see all submitted PDF documents."""
if request.user.profile.is_teacher:
submissions_list = PDFSubmission.objects.all().order_by("-submitted_at")
else:
submissions_list = PDFSubmission.objects.filter(student=request.user).order_by("-submitted_at")
# Keep pagination logic the same
context = {
"submissions": paginator,
"pdf_types": PDFType.objects.all(),
}
return render(request, "web/pdf/submission_list.html", context)
def upload_pdf_submission(request):
"""View for students to upload their PDF documents."""
if request.method == "POST":
form = PDFSubmissionForm(request.POST, request.FILES)
if form.is_valid():
submission = form.save(commit=False)
submission.student = request.user
submission.save()
# Create notification for user
Notification.objects.create(
user=request.user,
title="PDF Submission Received",
message=f"Your {submission.pdf_type.name} has been submitted successfully and is pending review.",
notification_type="success",
)
# Create notifications for teachers
# Keep the same logic
messages.success(request, "Your PDF document has been submitted successfully!")
return redirect("pdf_submission_list")
else:
form = PDFSubmissionForm()
return render(request, "web/pdf/upload_submission.html", {"form": form})
def pdf_submission_detail(request, submission_id):
"""View to see the details of a specific PDF submission and provide feedback."""
# Keep the same logic but with updated model and template references7. Rename and Update TemplatesCreate a new directory structure
8. Update Index PageReplace the current GSoC call to action with a more generic PDF submission section: <!-- PDF Submission/Review Announcement -->
<div class="bg-gradient-to-r from-blue-500 to-purple-600 rounded-lg p-6 text-white shadow-lg my-8">
<div class="flex items-center justify-between">
<div class="flex items-center space-x-4">
<div class="hidden sm:block">
<img src="{% static 'images/pdf-icon.png' %}"
alt="PDF Submissions"
class="h-16 w-16 object-contain"
onerror="this.src='https://cdn-icons-png.flaticon.com/512/337/337946.png'"
height="64"
width="64" />
</div>
<div>
{% if user.profile.is_teacher %}
<h3 class="text-xl font-bold mb-2">Review Student Documents 🔍</h3>
<p class="text-blue-100">
Review student submitted PDF documents and provide timely feedback.
</p>
{% else %}
<h3 class="text-xl font-bold mb-2">Submit Your Documents 📝</h3>
<p class="text-blue-100">
Submit your assignments, essays, reports, and other documents for review.
</p>
{% endif %}
</div>
</div>
<!-- Keep the rest of the structure but update URLs and button text -->
</div>
</div>Would you like me to provide any additional specific examples for any part of these changes? 🏁 Script executed: Length of output: 3114 1. Create New Models# web/models.py
class PDFType(models.Model):
"""Model for categorizing different types of PDF submissions."""
name = models.CharField(max_length=100)
description = models.TextField(blank=True)
icon_class = models.CharField(max_length=50, blank=True, help_text="Font Awesome icon class")
class Meta:
verbose_name = "PDF Type"
verbose_name_plural = "PDF Types"
def __str__(self):
return self.name
def validate_pdf_file_size(file):
if file.size > 10 * 1024 * 1024: # 10MB
raise ValidationError("File size exceeds 10MB limit.")
class PDFSubmission(models.Model):
"""Model for storing PDF documents submitted by students for review."""
STATUS_CHOICES = [
("submitted", "Submitted"),
("under_review", "Under Review"),
("reviewed", "Reviewed"),
("needs_revision", "Needs Revision"),
("approved", "Approved"),
]
title = models.CharField(max_length=200, help_text="Title of your document")
student = models.ForeignKey(User, on_delete=models.CASCADE, related_name="pdf_submissions")
pdf_type = models.ForeignKey(PDFType, on_delete=models.PROTECT, related_name="submissions")
pdf_file = models.FileField(
upload_to="pdf_submissions/",
validators=[FileExtensionValidator(allowed_extensions=["pdf"]), validate_pdf_file_size],
help_text="Upload your PDF document (max 10MB)",
)
subject = models.CharField(max_length=200, help_text="Subject or department")
assignment = models.CharField(max_length=200, help_text="Assignment or topic")
due_date = models.DateField(null=True, blank=True, help_text="Due date (if applicable)")
description = models.TextField(help_text="Brief description of your submission", blank=True)
status = models.CharField(max_length=20, choices=STATUS_CHOICES, default="submitted")
feedback = models.TextField(blank=True, help_text="Feedback from reviewers")
# Tracking fields
file_size = models.PositiveIntegerField(editable=False, null=True)
submitted_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True)
reviewed_at = models.DateTimeField(null=True, blank=True)
reviewed_by = models.ForeignKey(
User, on_delete=models.SET_NULL, null=True, blank=True, related_name="reviewed_submissions"
)
class Meta:
ordering = ["-submitted_at"]
verbose_name = "PDF Submission"
verbose_name_plural = "PDF Submissions"
def __str__(self):
return f"{self.student.username}'s {self.pdf_type.name}: {self.title}"
def save(self, *args, **kwargs):
# Set file size on save
if self.pdf_file:
self.file_size = self.pdf_file.size
# Set reviewed_at timestamp when status changes to "reviewed"
if self.pk:
try:
old_instance = PDFSubmission.objects.get(pk=self.pk)
if old_instance.status != "reviewed" and self.status == "reviewed":
self.reviewed_at = timezone.now()
except PDFSubmission.DoesNotExist:
pass
super().save(*args, **kwargs)
@property
def file_size_display(self):
"""Return file size in MB with two decimal places."""
if self.file_size:
return f"{self.file_size / (1024 * 1024):.2f} MB"
return "Unknown"
def get_status_choices(self):
return self.STATUS_CHOICES2. Update Form Class# web/forms.py
class PDFSubmissionForm(forms.ModelForm):
"""Form for submitting PDF documents."""
pdf_file = forms.FileField(
validators=[FileExtensionValidator(allowed_extensions=["pdf"])],
help_text="Upload your PDF document (max 10MB)",
widget=forms.FileInput(attrs={"accept": "application/pdf"}),
)
class Meta:
model = PDFSubmission
fields = ["title", "pdf_type", "subject", "assignment", "due_date", "description", "pdf_file"]
def clean_pdf_file(self):
uploaded_file = self.cleaned_data.get("pdf_file")
if uploaded_file:
# 10MB maximum file size
max_size = 10 * 1024 * 1024
if uploaded_file.size > max_size:
raise forms.ValidationError("File size must not exceed 10MB.")
# Verify file is actually a PDF
if not uploaded_file.content_type.lower() in ["application/pdf", "application/x-pdf"]:
raise forms.ValidationError("File must be a PDF document.")
return uploaded_file3. Update Admin Classes# web/admin.py
@admin.register(PDFType)
class PDFTypeAdmin(admin.ModelAdmin):
list_display = ("name", "description")
search_fields = ("name", "description")
@admin.register(PDFSubmission)
class PDFSubmissionAdmin(admin.ModelAdmin):
list_display = ("title", "student", "pdf_type", "subject", "status", "submitted_at", "file_size_display")
list_filter = ("status", "pdf_type", "subject")
search_fields = ("title", "student__username", "subject", "assignment")
readonly_fields = ("file_size", "submitted_at", "updated_at", "reviewed_at")
fieldsets = (
(
"Submission Information",
{
"fields": (
"title",
"student",
"pdf_type",
"subject",
"assignment",
"due_date",
"description",
"pdf_file",
"file_size_display",
)
},
),
("Review Information", {"fields": ("status", "feedback", "reviewed_by", "reviewed_at")}),
("Timestamps", {"fields": ("submitted_at", "updated_at"), "classes": ("collapse",)}),
)4. Update URL Patterns# web/urls.py
# Replace these URLs:
# path("gsoc-proposals/", views.gsoc_proposal_list, name="gsoc_proposal_list"),
# path("gsoc-proposals/upload/", views.upload_gsoc_proposal, name="upload_gsoc_proposal"),
# path("gsoc-proposals/ <int:proposal_id>/", views.gsoc_proposal_detail, name="gsoc_proposal_detail"),
# With these:
path("pdf-submissions/", views.pdf_submission_list, name="pdf_submission_list"),
path("pdf-submissions/upload/", views.upload_pdf_submission, name="upload_pdf_submission"),
path("pdf-submissions/<int:submission_id>/", views.pdf_submission_detail, name="pdf_submission_detail"),5. Update View Functions# web/views.py
@login_required
def upload_pdf_submission(request):
"""View for students to upload their PDF documents."""
if request.method == "POST":
form = PDFSubmissionForm(request.POST, request.FILES)
if form.is_valid():
submission = form.save(commit=False)
submission.student = request.user
submission.save()
# Create notification for user
Notification.objects.create(
user=request.user,
title="PDF Submission Received",
message=f"Your {submission.pdf_type.name} has been submitted successfully and is pending review.",
notification_type="success",
)
# Create notifications for teachers
admin_users = User.objects.filter(profile__is_teacher=True)
for admin in admin_users:
Notification.objects.create(
user=admin,
title="New PDF Submission",
message=f"A new {submission.pdf_type.name} has been submitted by {request.user.username} and needs review.",
notification_type="info",
)
messages.success(request, "Your PDF document has been submitted successfully!")
return redirect("pdf_submission_list")
else:
form = PDFSubmissionForm()
return render(request, "web/pdf/upload_submission.html", {"form": form})
@login_required
def pdf_submission_list(request):
"""View to see all submitted PDF documents."""
viewing_as = "student"
if request.user.profile.is_teacher:
viewing_as = "reviewer"
submissions_list = PDFSubmission.objects.select_related("student", "pdf_type").all().order_by("-submitted_at")
else:
submissions_list = PDFSubmission.objects.select_related("pdf_type").filter(student=request.user).order_by("-submitted_at")
# Pagination logic
paginator = Paginator(submissions_list, 10)
page = request.GET.get("page")
try:
submissions = paginator.page(page)
except PageNotAnInteger:
submissions = paginator.page(1)
except EmptyPage:
submissions = paginator.page(paginator.num_pages)
context = {
"submissions": submissions,
"viewing_as": viewing_as,
"pdf_types": PDFType.objects.all(),
}
return render(request, "web/pdf/submission_list.html", context)
@login_required
def pdf_submission_detail(request, submission_id):
"""View to see the details of a specific PDF submission and provide feedback."""
submission = get_object_or_404(PDFSubmission, pk=submission_id)
# Check if the user has access
if not (request.user == submission.student or request.user.profile.is_teacher):
raise PermissionDenied("You do not have permission to view this submission.")
if request.method == "POST" and request.user.profile.is_teacher:
feedback = request.POST.get("feedback", "")
new_status = request.POST.get("status", "")
if feedback:
submission.feedback = feedback
if new_status and new_status in dict(PDFSubmission.STATUS_CHOICES):
submission.status = new_status
submission.reviewed_by = request.user
submission.save()
# Create notification for student
Notification.objects.create(
user=submission.student,
title="Your PDF Submission Has Been Reviewed",
message=f"Your {submission.pdf_type.name} has been reviewed. Check the feedback for details.",
notification_type="info",
)
messages.success(request, "Feedback and status updated successfully.")
return redirect("pdf_submission_detail", submission_id=submission_id)
context = {
"submission": submission,
"status_choices": submission.get_status_choices(),
}
return render(request, "web/pdf/submission_detail.html", context)6. Create New TemplatesCreate a new directory 6.1 submission_list.htmlBased on the gsoc/proposal_list.html but with updated text and fields 6.2 submission_detail.htmlBased on the gsoc/proposal_detail.html but with updated text and fields 6.3 upload_submission.htmlBased on the gsoc/upload_proposal.html but with updated text and fields 7. Update Index Page<!-- PDF Submission/Review Announcement -->
<div class="bg-gradient-to-r from-blue-500 to-purple-600 rounded-lg p-6 text-white shadow-lg my-8">
<div class="flex items-center justify-between">
<div class="flex items-center space-x-4">
<div class="hidden sm:block">
<img src="{% static 'images/pdf-icon.png' %}"
alt="PDF Submissions"
class="h-16 w-16 object-contain"
onerror="this.src='https://cdn-icons-png.flaticon.com/512/337/337946.png'"
height="64"
width="64" />
</div>
<div>
{% if user.profile.is_teacher %}
<h3 class="text-xl font-bold mb-2">Review Student Documents 🔍</h3>
<p class="text-blue-100">
Review student submitted PDF documents and provide timely feedback.
</p>
{% else %}
<h3 class="text-xl font-bold mb-2">Submit Your Documents 📝</h3>
<p class="text-blue-100">
Submit your assignments, essays, reports, and other documents for review.
</p>
{% endif %}
</div>
</div>
{% if user.profile.is_teacher %}
<a href="{% url 'pdf_submission_list' %}"
class="inline-flex items-center px-6 py-2 bg-white text-blue-600 rounded-full font-semibold hover:bg-blue-50 transition-colors duration-200 whitespace-nowrap sm:inline-flex">
<span>Review Documents</span>
<i class="fas fa-clipboard-list ml-2"></i>
</a>
{% else %}
<a href="{% url 'pdf_submission_list' %}"
class="inline-flex items-center px-6 py-2 bg-white text-blue-600 rounded-full font-semibold hover:bg-blue-50 transition-colors duration-200 whitespace-nowrap sm:inline-flex">
<span>Upload Document</span>
<i class="fas fa-upload ml-2"></i>
</a>
{% endif %}
</div>
</div>8. Create MigrationsYou'll need to create migrations for these model changes. Since this is a complete rework rather than a simple rename, it might be better to:
Let me know if you need any additional details or examples for implementing these changes! |
|
@coderabbitai give the updates templates !!! |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Inshamhaque Here are the updated templates for the PDF submission system: 1. submission_list.html{% extends 'base.html' %}
{% load static %}
{% block title %}
{% if viewing_as == 'reviewer' %}
Review PDF Submissions
{% else %}
My PDF Submissions
{% endif %}
{% endblock %}
{% block content %}
<div class="container mx-auto px-4 py-8">
<div class="flex justify-between items-center mb-6">
<h1 class="text-2xl font-bold text-gray-800">
{% if viewing_as == 'reviewer' %}
Review PDF Submissions
{% else %}
My PDF Submissions
{% endif %}
</h1>
{% if not user.profile.is_teacher %}
<a href="{% url 'upload_pdf_submission' %}" class="bg-blue-600 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded">
<i class="fas fa-upload mr-2"></i>Upload New Submission
</a>
{% endif %}
</div>
{% if submissions %}
<div class="overflow-x-auto bg-white rounded-lg shadow overflow-y-auto">
<table class="min-w-full divide-y divide-gray-200">
<thead class="bg-gray-50">
<tr>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Title</th>
{% if viewing_as == 'reviewer' %}
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Student</th>
{% endif %}
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Type</th>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Subject</th>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Assignment</th>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Submitted</th>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Status</th>
<th scope="col" class="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider">Actions</th>
</tr>
</thead>
<tbody class="bg-white divide-y divide-gray-200">
{% for submission in submissions %}
<tr>
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm font-medium text-gray-900">{{ submission.title }}</div>
</td>
{% if viewing_as == 'reviewer' %}
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm text-gray-900">{{ submission.student.username }}</div>
</td>
{% endif %}
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm text-gray-900">{{ submission.pdf_type.name }}</div>
</td>
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm text-gray-900">{{ submission.subject }}</div>
</td>
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm text-gray-900">{{ submission.assignment }}</div>
</td>
<td class="px-6 py-4 whitespace-nowrap">
<div class="text-sm text-gray-500">{{ submission.submitted_at|date:"M d, Y" }}</div>
</td>
<td class="px-6 py-4 whitespace-nowrap">
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full
{% if submission.status == 'submitted' %}
bg-yellow-100 text-yellow-800
{% elif submission.status == 'under_review' %}
bg-blue-100 text-blue-800
{% elif submission.status == 'reviewed' %}
bg-indigo-100 text-indigo-800
{% elif submission.status == 'needs_revision' %}
bg-orange-100 text-orange-800
{% elif submission.status == 'approved' %}
bg-green-100 text-green-800
{% else %}
bg-gray-100 text-gray-800
{% endif %}">
{{ submission.get_status_display }}
</span>
</td>
<td class="px-6 py-4 whitespace-nowrap text-right text-sm font-medium">
<a href="{% url 'pdf_submission_detail' submission.id %}" class="text-indigo-600 hover:text-indigo-900">View</a>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
<!-- Pagination -->
{% if submissions.has_other_pages %}
<div class="flex justify-center mt-8">
<nav class="relative z-0 inline-flex rounded-md shadow-sm -space-x-px" aria-label="Pagination">
{% if submissions.has_previous %}
<a href="?page={{ submissions.previous_page_number }}" class="relative inline-flex items-center px-2 py-2 rounded-l-md border border-gray-300 bg-white text-sm font-medium text-gray-500 hover:bg-gray-50">
<span class="sr-only">Previous</span>
<i class="fas fa-chevron-left"></i>
</a>
{% endif %}
{% for i in submissions.paginator.page_range %}
{% if submissions.number == i %}
<span class="relative inline-flex items-center px-4 py-2 border border-gray-300 bg-blue-50 text-sm font-medium text-blue-600">
{{ i }}
</span>
{% else %}
<a href="?page={{ i }}" class="relative inline-flex items-center px-4 py-2 border border-gray-300 bg-white text-sm font-medium text-gray-700 hover:bg-gray-50">
{{ i }}
</a>
{% endif %}
{% endfor %}
{% if submissions.has_next %}
<a href="?page={{ submissions.next_page_number }}" class="relative inline-flex items-center px-2 py-2 rounded-r-md border border-gray-300 bg-white text-sm font-medium text-gray-500 hover:bg-gray-50">
<span class="sr-only">Next</span>
<i class="fas fa-chevron-right"></i>
</a>
{% endif %}
</nav>
</div>
{% endif %}
{% else %}
<div class="text-center py-10">
<img src="{% static 'images/empty-state.svg' %}" alt="No submissions yet" class="w-64 h-64 mx-auto opacity-40">
<h3 class="mt-4 text-xl font-medium text-gray-600">No PDF Submissions Found</h3>
{% if not user.profile.is_teacher %}
<p class="mt-2 text-gray-500">Upload your first PDF document to get started!</p>
<div class="mt-6">
<a href="{% url 'upload_pdf_submission' %}" class="bg-blue-600 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded">
<i class="fas fa-upload mr-2"></i>Upload Now
</a>
</div>
{% else %}
<p class="mt-2 text-gray-500">No submissions available for review yet.</p>
{% endif %}
</div>
{% endif %}
</div>
{% endblock %}2. submission_detail.html{% extends 'base.html' %}
{% load static %}
{% block title %}PDF Submission: {{ submission.title }}{% endblock %}
{% block content %}
<div class="container mx-auto px-4 py-8">
<!-- Breadcrumb navigation -->
<nav class="flex mb-5" aria-label="Breadcrumb">
<ol class="inline-flex items-center space-x-1 md:space-x-3">
<li class="inline-flex items-center">
<a href="{% url 'home' %}" class="inline-flex items-center text-sm text-gray-500 hover:text-gray-900">
<i class="fas fa-home mr-2"></i>
Home
</a>
</li>
<li>
<div class="flex items-center">
<i class="fas fa-chevron-right text-gray-400 mx-2"></i>
<a href="{% url 'pdf_submission_list' %}" class="text-sm text-gray-500 hover:text-gray-900">PDF Submissions</a>
</div>
</li>
<li aria-current="page">
<div class="flex items-center">
<i class="fas fa-chevron-right text-gray-400 mx-2"></i>
<span class="text-sm text-gray-500">{{ submission.title }}</span>
</div>
</li>
</ol>
</nav>
<div class="mb-8">
<div class="flex justify-between items-start">
<h1 class="text-2xl font-bold text-gray-800 mb-4">{{ submission.title }}</h1>
<div class="text-right">
<span class="px-3 py-1 inline-flex text-sm leading-5 font-semibold rounded-full
{% if submission.status == 'submitted' %}
bg-yellow-100 text-yellow-800
{% elif submission.status == 'under_review' %}
bg-blue-100 text-blue-800
{% elif submission.status == 'reviewed' %}
bg-indigo-100 text-indigo-800
{% elif submission.status == 'needs_revision' %}
bg-orange-100 text-orange-800
{% elif submission.status == 'approved' %}
bg-green-100 text-green-800
{% else %}
bg-gray-100 text-gray-800
{% endif %}">
{{ submission.get_status_display }}
</span>
</div>
</div>
</div>
<div class="grid grid-cols-1 md:grid-cols-3 gap-6">
<!-- Main content - Submission details -->
<div class="md:col-span-2">
<div class="bg-white rounded-lg shadow-md overflow-hidden mb-6">
<div class="bg-gray-50 px-4 py-3 border-b border-gray-200">
<h3 class="text-lg font-medium text-gray-900">PDF Submission Details</h3>
</div>
<div class="p-4">
<div class="grid grid-cols-1 md:grid-cols-2 gap-4 mb-4">
<div>
<p class="text-sm font-medium text-gray-500">Submission Type</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.pdf_type.name }}</p>
</div>
<div>
<p class="text-sm font-medium text-gray-500">Subject</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.subject }}</p>
</div>
<div>
<p class="text-sm font-medium text-gray-500">Assignment</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.assignment }}</p>
</div>
{% if submission.due_date %}
<div>
<p class="text-sm font-medium text-gray-500">Due Date</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.due_date|date:"F j, Y" }}</p>
</div>
{% endif %}
<div>
<p class="text-sm font-medium text-gray-500">Submitted By</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.student.username }}</p>
</div>
<div>
<p class="text-sm font-medium text-gray-500">Submitted On</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.submitted_at|date:"F j, Y" }}</p>
</div>
<div>
<p class="text-sm font-medium text-gray-500">File Size</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.file_size_display }}</p>
</div>
</div>
{% if submission.description %}
<div class="mt-4">
<p class="text-sm font-medium text-gray-500">Description</p>
<p class="mt-1 text-sm text-gray-900">{{ submission.description }}</p>
</div>
{% endif %}
<div class="mt-6 flex flex-col sm:flex-row space-y-2 sm:space-y-0 sm:space-x-2">
<a href="{{ submission.pdf_file.url }}" target="_blank" class="inline-flex items-center justify-center px-4 py-2 border border-transparent text-sm font-medium rounded-md text-white bg-blue-600 hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500">
<i class="fas fa-eye mr-2"></i>View PDF
</a>
<a href="{{ submission.pdf_file.url }}" download class="inline-flex items-center justify-center px-4 py-2 border border-transparent text-sm font-medium rounded-md text-blue-700 bg-blue-100 hover:bg-blue-200 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500">
<i class="fas fa-download mr-2"></i>Download PDF
</a>
</div>
</div>
</div>
<!-- Feedback Section -->
{% if submission.feedback or user.profile.is_teacher %}
<div class="bg-white rounded-lg shadow-md overflow-hidden mb-6">
<div class="bg-gray-50 px-4 py-3 border-b border-gray-200">
<h3 class="text-lg font-medium text-gray-900">Feedback</h3>
</div>
<div class="p-4">
{% if user.profile.is_teacher %}
<form method="post" class="space-y-4">
{% csrf_token %}
<div>
<label for="feedback" class="block text-sm font-medium text-gray-700">Provide Feedback</label>
<textarea id="feedback" name="feedback" rows="4" class="mt-1 block w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 sm:text-sm" placeholder="Enter your feedback here...">{{ submission.feedback }}</textarea>
</div>
<div>
<label for="status" class="block text-sm font-medium text-gray-700">Update Status</label>
<select id="status" name="status" class="mt-1 block w-full py-2 px-3 border border-gray-300 bg-white rounded-md shadow-sm focus:outline-none focus:ring-blue-500 focus:border-blue-500 sm:text-sm">
{% for status_code, status_label in status_choices %}
<option value="{{ status_code }}" {% if submission.status == status_code %}selected{% endif %}>{{ status_label }}</option>
{% endfor %}
</select>
</div>
<div>
<button type="submit" class="inline-flex items-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-blue-600 hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500">
<i class="fas fa-save mr-2"></i>Save Feedback
</button>
</div>
</form>
{% elif submission.feedback %}
<div class="bg-gray-50 p-4 rounded-md">
<p class="text-sm text-gray-800">{{ submission.feedback|linebreaks }}</p>
{% if submission.reviewed_at %}
<p class="text-xs text-gray-500 mt-2">Reviewed on {{ submission.reviewed_at|date:"F j, Y" }}</p>
{% endif %}
</div>
{% else %}
<p class="text-sm text-gray-500 italic">No feedback provided yet.</p>
{% endif %}
</div>
</div>
{% endif %}
</div>
<!-- Sidebar content -->
<div class="md:col-span-1">
<!-- Timeline Card -->
<div class="bg-white rounded-lg shadow-md overflow-hidden mb-6">
<div class="bg-gray-50 px-4 py-3 border-b border-gray-200">
<h3 class="text-lg font-medium text-gray-900">Timeline</h3>
</div>
<div class="p-4">
<ul class="space-y-4">
<li class="flex items-start">
<div class="flex-shrink-0">
<div class="flex items-center justify-center h-8 w-8 rounded-full bg-green-100 text-green-600">
<i class="fas fa-check"></i>
</div>
</div>
<div class="ml-4">
<p class="text-sm font-medium text-gray-900">Submitted</p>
<p class="text-xs text-gray-500">{{ submission.submitted_at|date:"F j, Y" }}</p>
</div>
</li>
<li class="flex items-start">
<div class="flex-shrink-0">
<div class="flex items-center justify-center h-8 w-8 rounded-full
{% if submission.status == 'under_review' or submission.status == 'reviewed' or submission.status == 'needs_revision' or submission.status == 'approved' %}
bg-green-100 text-green-600
{% elif submission.status == 'submitted' %}
bg-blue-100 text-blue-600 timeline-active
{% else %}
bg-gray-100 text-gray-400
{% endif %}">
<i class="fas fa-search"></i>
</div>
</div>
<div class="ml-4">
<p class="text-sm font-medium text-gray-900">Under Review</p>
{% if submission.status == 'under_review' %}
<p class="text-xs text-gray-500">Currently in progress</p>
{% endif %}
</div>
</li>
<li class="flex items-start">
<div class="flex-shrink-0">
<div class="flex items-center justify-center h-8 w-8 rounded-full
{% if submission.status == 'reviewed' or submission.status == 'needs_revision' or submission.status == 'approved' %}
bg-green-100 text-green-600
{% elif submission.status == 'under_review' %}
bg-blue-100 text-blue-600 timeline-active
{% else %}
bg-gray-100 text-gray-400
{% endif %}">
<i class="fas fa-clipboard-check"></i>
</div>
</div>
<div class="ml-4">
<p class="text-sm font-medium text-gray-900">Reviewed</p>
{% if submission.reviewed_at %}
<p class="text-xs text-gray-500">{{ submission.reviewed_at|date:"F j, Y" }}</p>
{% endif %}
</div>
</li>
<li class="flex items-start">
<div class="flex-shrink-0">
<div class="flex items-center justify-center h-8 w-8 rounded-full
{% if submission.status == 'needs_revision' %}
bg-orange-100 text-orange-600 timeline-active
{% elif submission.status == 'approved' %}
bg-green-100 text-green-600
{% else %}
bg-gray-100 text-gray-400
{% endif %}">
<i class="fas fa-{% if submission.status == 'needs_revision' %}pencil-alt{% elif submission.status == 'approved' %}check-double{% else %}clipboard-check{% endif %}"></i>
</div>
</div>
<div class="ml-4">
<p class="text-sm font-medium text-gray-900">
{% if submission.status == 'needs_revision' %}
Needs Revision
{% elif submission.status == 'approved' %}
Approved
{% else %}
Final Status
{% endif %}
</p>
{% if submission.status == 'needs_revision' or submission.status == 'approved' %}
<p class="text-xs text-gray-500">Final decision made</p>
{% endif %}
</div>
</li>
</ul>
</div>
</div>
<!-- Tips Card -->
<div class="bg-white rounded-lg shadow-md overflow-hidden">
<div class="bg-gray-50 px-4 py-3 border-b border-gray-200">
<h3 class="text-lg font-medium text-gray-900">Tips</h3>
</div>
<div class="p-4">
<ul class="space-y-2 text-sm text-gray-700">
<li class="flex items-start">
<i class="fas fa-check-circle text-green-500 mt-1 mr-2"></i>
<span>Double-check your PDF for any errors before submission.</span>
</li>
<li class="flex items-start">
<i class="fas fa-check-circle text-green-500 mt-1 mr-2"></i>
<span>Ensure your document follows any required formatting guidelines.</span>
</li>
<li class="flex items-start">
<i class="fas fa-check-circle text-green-500 mt-1 mr-2"></i>
<span>Include clear section headers to help reviewers navigate your document.</span>
</li>
<li class="flex items-start">
<i class="fas fa-check-circle text-green-500 mt-1 mr-2"></i>
<span>If you receive feedback requesting revisions, respond promptly.</span>
</li>
</ul>
</div>
</div>
</div>
</div>
</div>
{% endblock %}
{% block extra_js %}
<script>
document.addEventListener('DOMContentLoaded', function() {
const timelineItems = document.querySelectorAll('.timeline-active');
timelineItems.forEach(item => {
item.addEventListener('mouseenter', function() {
this.classList.add('bg-blue-200');
});
item.addEventListener('mouseleave', function() {
this.classList.remove('bg-blue-200');
});
});
});
</script>
{% endblock %}3. upload_submission.html{% extends 'base.html' %}
{% load static %}
{% block title %}Upload PDF Document{% endblock %}
{% block content %}
<div class="container mx-auto px-4 py-8">
<div class="max-w-2xl mx-auto">
<div class="text-center mb-8">
<h1 class="text-2xl font-bold text-gray-800">Upload PDF Document</h1>
<p class="mt-2 text-gray-600">Submit your PDF document for review by our instructors.</p>
</div>
<div class="bg-white rounded-lg shadow-md overflow-hidden">
<div class="p-6">
<form method="post" enctype="multipart/form-data" class="space-y-6">
{% csrf_token %}
{% if form.non_field_errors %}
<div class="rounded-md bg-red-50 p-4 mb-4">
<div class="flex">
<div class="flex-shrink-0">
<i class="fas fa-exclamation-circle text-red-400"></i>
</div>
<div class="ml-3">
<h3 class="text-sm font-medium text-red-800">There were errors with your submission</h3>
<div class="mt-2 text-sm text-red-700">
<ul class="list-disc pl-5 space-y-1">
{% for error in form.non_field_errors %}
<li>{{ error }}</li>
{% endfor %}
</ul>
</div>
</div>
</div>
</div>
{% endif %}
{% for field in form %}
<div class="space-y-1">
<label for="{{ field.id_for_label }}" class="block text-sm font-medium text-gray-700">
{{ field.label }}{% if field.field.required %} <span class="text-red-500">*</span>{% endif %}
</label>
{{ field }}
{% if field.errors %}
<div class="text-sm text-red-600">{{ field.errors }}</div>
{% endif %}
{% if field.help_text %}
<p class="mt-1 text-xs text-gray-500">{{ field.help_text }}</p>
{% endif %}
</div>
{% endfor %}
<div class="pt-4">
<button type="submit" class="w-full flex justify-center py-2 px-4 border border-transparent rounded-md shadow-sm text-sm font-medium text-white bg-blue-600 hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-blue-500">
Upload Document
</button>
</div>
</form>
</div>
</div>
<div class="mt-8 text-center text-sm text-gray-500">
<div class="flex items-center justify-center">
<i class="fas fa-info-circle mr-2 text-blue-500"></i>
<p>Your submission will be reviewed by our instructors who will provide feedback.</p>
</div>
</div>
</div>
</div>
{% endblock %}These templates replace all GSoC-specific terminology with more generic PDF submission terms, update field references to match the new model structure, add support for the PDF type field, and update all URL patterns to the new format. 🏁 Script executed: Length of output: 29510 1. submission_list.html{% extends "base.html" %}
{% load static %}
{% block title %}
{% if viewing_as == 'reviewer' %}
PDF Submissions for Review
{% else %}
My PDF Submissions
{% endif %}
{% endblock %}
{% block content %} <div class="container mx-auto px-4 py-8">
<div class="flex flex-col md:flex-row justify-between items-center mb-6">
<h1 class="text-3xl font-bold text-gray-900 dark:text-white mb-4 md:mb-0">
{% if viewing_as == 'reviewer' %}
PDF Submissions for Review
{% else %}
My PDF Submissions
{% endif %}
</h1>
{% if viewing_as == 'student' %}
<a href="{% url 'upload_pdf_submission' %}"
class="inline-flex items-center px-4 py-2 bg-blue-600 dark:bg-blue-700 text-white rounded-md hover:bg-blue-700 dark:hover:bg-blue-800 transition-colors duration-300">
<svg class="w-5 h-5 mr-2"
fill="none"
stroke="currentColor"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 16v1a3 3 0 003 3h10a3 3 0 003-3v-1m-4-8l-4-4m0 0L8 8m4-4v12">
</path>
</svg>
Upload New Document
</a>
{% endif %}
</div>
{% if submissions %}
<div class="bg-white dark:bg-gray-900 shadow-md rounded-lg overflow-hidden">
<div class="overflow-x-auto">
<table class="w-full">
<thead class="bg-gray-100 dark:bg-gray-800 border-b dark:border-gray-700">
<tr>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Title
</th>
{% if viewing_as == 'reviewer' %}
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Student
</th>
{% endif %}
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Type
</th>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Subject
</th>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Assignment
</th>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Submitted
</th>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Status
</th>
<th class="px-4 py-3 text-left text-xs font-medium text-gray-500 dark:text-gray-300 uppercase tracking-wider">
Actions
</th>
</tr>
</thead>
<tbody class="divide-y divide-gray-200 dark:divide-gray-700">
{% for submission in submissions %}
<tr class="hover:bg-gray-50 dark:hover:bg-gray-800 transition-colors duration-200">
<td class="px-4 py-4 whitespace-nowrap text-sm font-medium text-gray-900 dark:text-white">{{ submission.title }}</td>
{% if viewing_as == 'reviewer' %}
<td class="px-4 py-4 whitespace-nowrap text-sm text-gray-500 dark:text-gray-300">{{ submission.student.username }}</td>
{% endif %}
<td class="px-4 py-4 whitespace-nowrap text-sm text-gray-500 dark:text-gray-300">{{ submission.pdf_type.name }}</td>
<td class="px-4 py-4 whitespace-nowrap text-sm text-gray-500 dark:text-gray-300">{{ submission.subject }}</td>
<td class="px-4 py-4 whitespace-nowrap text-sm text-gray-500 dark:text-gray-300">{{ submission.assignment }}</td>
<td class="px-4 py-4 whitespace-nowrap text-sm text-gray-500 dark:text-gray-300">
{{ submission.submitted_at|date:"M d, Y" }}
</td>
<td class="px-4 py-4 whitespace-nowrap">
{% if submission.status == 'submitted' %}
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-300">
Submitted
</span>
{% elif submission.status == 'under_review' %}
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full bg-yellow-100 text-yellow-800 dark:bg-yellow-900 dark:text-yellow-300">
Under Review
</span>
{% elif submission.status == 'reviewed' %}
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-300">
Reviewed
</span>
{% elif submission.status == 'needs_revision' %}
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full bg-orange-100 text-orange-800 dark:bg-orange-900 dark:text-orange-300">
Needs Revision
</span>
{% elif submission.status == 'approved' %}
<span class="px-2 inline-flex text-xs leading-5 font-semibold rounded-full bg-purple-100 text-purple-800 dark:bg-purple-900 dark:text-purple-300">
Approved
</span>
{% endif %}
</td>
<td class="px-4 py-4 whitespace-nowrap text-sm font-medium">
<div class="flex space-x-2">
<a href="{% url 'pdf_submission_detail' submission.id %}"
class="text-blue-600 dark:text-blue-400 hover:text-blue-900 dark:hover:text-blue-300 transition-colors">
{% if viewing_as == 'reviewer' %}
Review
{% else %}
View
{% endif %}
</a>
<a href="{{ submission.pdf_file.url }}"
class="text-gray-600 dark:text-gray-300 hover:text-gray-900 dark:hover:text-white transition-colors"
target="_blank"
rel="noopener noreferrer">
<svg class="w-5 h-5"
fill="none"
stroke="currentColor"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M7 16a4 4 0 01-.88-7.903A5 5 0 1115.9 6L16 6a5 5 0 011 9.9M9 19l3 3m0 0l3-3m-3 3V10">
</path>
</svg>
</a>
</div>
</td>
</tr>
{% endfor %}
</tbody>
</table>
</div>
</div>
{% else %}
<div class="bg-blue-50 dark:bg-blue-900 border border-blue-200 dark:border-blue-800 text-blue-800 dark:text-blue-300 px-4 py-3 rounded-md shadow-md">
<div class="flex items-center">
<svg class="w-6 h-6 mr-3"
fill="none"
stroke="currentColor"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z">
</path>
</svg>
<div>
{% if viewing_as == 'reviewer' %}
No PDF submissions have been uploaded yet.
{% else %}
You haven't uploaded any PDF documents yet.
<a href="{% url 'upload_pdf_submission' %}"
class="underline hover:text-blue-600 transition-colors">Upload your first document</a>.
{% endif %}
</div>
</div>
</div>
{% endif %}
{% if submissions.paginator.num_pages > 1 %}
<div class="mt-6 flex justify-center">
<nav aria-label="Submission pagination"
class="inline-flex rounded-md shadow-sm">
{% if submissions.has_previous %}
<a href="?page={{ submissions.previous_page_number }}"
class="relative inline-flex items-center px-4 py-2 border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-800 text-sm font-medium text-gray-500 dark:text-gray-300 hover:bg-gray-50 dark:hover:bg-gray-700">
Previous
</a>
{% endif %}
{% for page_num in submissions.paginator.page_range %}
{% if page_num == submissions.number %}
<span class="relative inline-flex items-center px-4 py-2 border border-gray-300 dark:border-gray-700 bg-blue-50 dark:bg-blue-900 text-sm font-medium text-blue-600 dark:text-blue-300">
{{ page_num }}
</span>
{% else %}
<a href="?page={{ page_num }}"
class="relative inline-flex items-center px-4 py-2 border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-800 text-sm font-medium text-gray-500 dark:text-gray-300 hover:bg-gray-50 dark:hover:bg-gray-700">
{{ page_num }}
</a>
{% endif %}
{% endfor %}
{% if submissions.has_next %}
<a href="?page={{ submissions.next_page_number }}"
class="relative inline-flex items-center px-4 py-2 border border-gray-300 dark:border-gray-700 bg-white dark:bg-gray-800 text-sm font-medium text-gray-500 dark:text-gray-300 hover:bg-gray-50 dark:hover:bg-gray-700">
Next
</a>
{% endif %}
</nav>
</div>
{% endif %}
</div> {% endblock %}2. submission_detail.html{% extends "base.html" %}
{% load static %}
{% block title %}PDF Submission: {{ submission.title }}{% endblock %}
{% block content %} <div class="container mx-auto px-4 py-8 max-w-7xl">
<nav aria-label="Breadcrumb" class="mb-6">
<ol class="flex items-center space-x-2 text-sm text-gray-500 dark:text-gray-400">
<li>
<a href="{% url 'pdf_submission_list' %}"
class="hover:text-blue-600 dark:hover:text-blue-400 transition-colors">PDF Submissions</a>
</li>
<li class="flex items-center">
<svg xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4 mx-2 text-gray-500 dark:text-gray-400"
viewBox="0 0 20 20"
fill="currentColor">
<path fill-rule="evenodd" d="M7.293 14.707a1 1 0 010-1.414L10.586 10 7.293 6.707a1 1 0 011.414-1.414l4 4a1 1 0 010 1.414l-4 4a1 1 0 01-1.414 0z" clip-rule="evenodd" />
</svg>
<span class="font-medium text-gray-700 dark:text-gray-200">{{ submission.title }}</span>
</li>
</ol>
</nav>
<div class="grid md:grid-cols-3 gap-6">
<div class="md:col-span-2">
<!-- Submission Details Card -->
<div class="bg-white dark:bg-gray-800 shadow-md rounded-lg overflow-hidden mb-6">
<div class="bg-blue-600 dark:bg-blue-800 text-white px-6 py-4 flex justify-between items-center">
<h2 class="text-xl font-semibold">Submission Details</h2>
<span class="bg-blue-200 dark:bg-blue-700 text-blue-800 dark:text-blue-200 px-3 py-1 rounded-full text-sm">
{{ submission.get_status_display|capfirst }}
</span>
</div>
<div class="p-6">
<h3 class="text-lg font-bold mb-4 text-gray-800 dark:text-gray-200">{{ submission.title }}</h3>
<div class="grid md:grid-cols-2 gap-4">
<div>
<div class="space-y-2">
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Type:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.pdf_type.name }}</span>
</p>
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Subject:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.subject }}</span>
</p>
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Assignment:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.assignment }}</span>
</p>
{% if submission.due_date %}
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Due Date:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.due_date|date:"F d, Y" }}</span>
</p>
{% endif %}
</div>
</div>
<div>
<div class="space-y-2">
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Student:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.student.username }}</span>
</p>
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">Submitted:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.submitted_at|date:"F d, Y H:i" }}</span>
</p>
<p class="text-sm">
<span class="font-semibold text-gray-600 dark:text-gray-400">File Size:</span>
<span class="text-gray-800 dark:text-gray-200">{{ submission.file_size_display }}</span>
</p>
</div>
</div>
</div>
{% if submission.description %}
<div class="mt-4 bg-gray-100 dark:bg-gray-700 p-4 rounded-lg">
<h4 class="text-sm font-semibold text-gray-700 dark:text-gray-300 mb-2">Description</h4>
<p class="text-gray-600 dark:text-gray-300 text-sm">{{ submission.description|linebreaks }}</p>
</div>
{% endif %}
<div class="flex flex-wrap justify-center gap-4 mt-6">
<a href="{{ submission.pdf_file.url }}"
class="bg-blue-500 hover:bg-blue-600 dark:bg-blue-700 dark:hover:bg-blue-600 text-white px-4 py-2 rounded-md flex items-center transition-colors"
target="_blank"
rel="noopener noreferrer">
<svg class="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M4 4a2 2 0 012-2h4.586A2 2 0 0112 2.586L15.414 6A2 2 0 0116 7.414V16a2 2 0 01-2 2H6a2 2 0 01-2-2V4z" clip-rule="evenodd" />
</svg>
View PDF
</a>
<a href="{{ submission.pdf_file.url }}"
class="border border-gray-300 dark:border-gray-600 text-gray-700 dark:text-gray-300 hover:bg-gray-100 dark:hover:bg-gray-700 px-4 py-2 rounded-md flex items-center transition-colors"
download>
<svg class="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M3 17a1 1 0 011-1h12a1 1 0 110 2H4a1 1 0 01-1-1zm3.293-9.707a1 1 0 011.414 0L9 10.586V3a1 1 0 112 0v7.586l1.293-1.293a1 1 0 111.414 1.414l-3 3a1 1 0 01-1.414 0l-3-3a1 1 0 010-1.414z" clip-rule="evenodd" />
</svg>
Download
</a>
<a href="{% url 'pdf_submission_list' %}"
class="border border-gray-300 dark:border-gray-600 text-gray-700 dark:text-gray-300 hover:bg-gray-100 dark:hover:bg-gray-700 px-4 py-2 rounded-md flex items-center transition-colors">
<svg class="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M9.707 16.707a1 1 0 01-1.414 0l-6-6a1 1 0 010-1.414l6-6a1 1 0 011.414 1.414L5.414 9H17a1 1 0 110 2H5.414l4.293 4.293a1 1 0 010 1.414z" clip-rule="evenodd" />
</svg>
Back to List
</a>
</div>
</div>
</div>
<!-- Feedback Section -->
{% if submission.feedback or is_reviewer %}
<div class="bg-white dark:bg-gray-800 shadow-md rounded-lg overflow-hidden">
<div class="bg-teal-600 dark:bg-teal-800 text-white px-6 py-4">
<h3 class="text-xl font-semibold">Feedback</h3>
</div>
<div class="p-6">
{% if submission.feedback %}
<blockquote class="border-l-4 border-teal-500 pl-4 py-2 bg-gray-50 dark:bg-gray-700 italic text-gray-700 dark:text-gray-300">
<p class="mb-2">{{ submission.feedback|linebreaks }}</p>
{% if submission.reviewed_by %}
<footer class="text-right text-sm text-gray-500 dark:text-gray-400">
— Reviewed by {{ submission.reviewed_by.username }}
on {{ submission.reviewed_at|date:"F d, Y H:i" }}
</footer>
{% endif %}
</blockquote>
{% endif %}
{% if is_reviewer %}
<form method="post" class="mt-4" novalidate>
{% csrf_token %}
<div class="mb-4">
<label for="feedback"
class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-2">
Provide Feedback
</label>
<textarea name="feedback"
id="feedback"
rows="5"
class="w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-md bg-white dark:bg-gray-800 focus:ring-2 focus:ring-blue-500 focus:border-blue-500">{{ submission.feedback }}</textarea>
<p class="text-xs text-gray-500 dark:text-gray-400 mt-1">
Provide constructive and specific feedback for the submission.
</p>
</div>
<div class="mb-4">
<label for="status"
class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-2">
Update Status
</label>
<select name="status"
id="status"
class="w-full px-3 py-2 border border-gray-300 dark:border-gray-600 rounded-md bg-white dark:bg-gray-800 focus:ring-2 focus:ring-blue-500 focus:border-blue-500">
{% for status_value, status_display in submission.get_status_choices %}
<option value="{{ status_value }}"
{% if submission.status == status_value %}selected{% endif %}
class="bg-white dark:bg-gray-800 text-gray-900 dark:text-gray-200">
{{ status_display }}
</option>
{% endfor %}
</select>
<p class="text-xs text-gray-500 dark:text-gray-400 mt-1">Select the current status of the submission.</p>
</div>
<button type="submit"
class="w-full bg-blue-600 hover:bg-blue-700 dark:bg-blue-800 dark:hover:bg-blue-700 text-white py-2 rounded-md transition-colors">
Submit Feedback
</button>
</form>
{% endif %}
</div>
</div>
{% endif %}
</div>
<div class="md:col-span-1 space-y-6">
<!-- Timeline Card -->
<div class="bg-white dark:bg-gray-800 shadow-md rounded-lg overflow-hidden">
<div class="bg-gray-700 dark:bg-gray-900 text-white px-6 py-4">
<h3 class="text-xl font-semibold">Submission Timeline</h3>
</div>
<div class="divide-y divide-gray-200 dark:divide-gray-700">
<div class="px-6 py-4 flex justify-between items-center text-gray-700 dark:text-gray-300">
<div class="flex items-center">
<span class="mr-3 bg-green-500 text-white rounded-full px-2 py-1 text-xs font-bold">
1
</span>
<span class="font-medium">Submitted</span>
</div>
<small class="text-sm">
{{ submission.submitted_at|date:"M d, Y" }}
</small>
</div>
<div class="px-6 py-4 flex justify-between items-center {% if submission.status == 'submitted' %}text-gray-400 dark:text-gray-500{% else %}text-gray-700 dark:text-gray-300{% endif %}">
<div class="flex items-center">
<span class="mr-3 {% if submission.status == 'under_review' or submission.status == 'reviewed' or submission.status == 'needs_revision' or submission.status == 'approved' %}bg-green-500 {% elif submission.status == 'submitted' %}bg-yellow-500 {% else %}bg-gray-300 dark:bg-gray-600{% endif %} text-white rounded-full px-2 py-1 text-xs font-bold">
2
</span>
<span class="font-medium">Under Review</span>
</div>
<small class="text-sm">
{% if submission.status == 'under_review' %}
In Progress
{% endif %}
</small>
</div>
<div class="px-6 py-4 flex justify-between items-center {% if submission.status == 'submitted' or submission.status == 'under_review' %}text-gray-400 dark:text-gray-500{% else %}text-gray-700 dark:text-gray-300{% endif %}">
<div class="flex items-center">
<span class="mr-3 {% if submission.status == 'reviewed' or submission.status == 'needs_revision' or submission.status == 'approved' %}bg-green-500 {% elif submission.status == 'under_review' %}bg-yellow-500 {% else %}bg-gray-300 dark:bg-gray-600{% endif %} text-white rounded-full px-2 py-1 text-xs font-bold">
3
</span>
<span class="font-medium">Reviewed</span>
</div>
<small class="text-sm">
{% if submission.reviewed_at %}
{{ submission.reviewed_at|date:"M d, Y" }}
{% endif %}
</small>
</div>
<div class="px-6 py-4 flex justify-between items-center {% if submission.status != 'needs_revision' and submission.status != 'approved' %}text-gray-400 dark:text-gray-500{% else %}text-gray-700 dark:text-gray-300{% endif %}">
<div class="flex items-center">
<span class="mr-3 {% if submission.status == 'needs_revision' %}bg-orange-500 {% elif submission.status == 'approved' %}bg-green-500 {% else %}bg-gray-300 dark:bg-gray-600{% endif %} text-white rounded-full px-2 py-1 text-xs font-bold">
4
</span>
<span class="font-medium">
{% if submission.status == 'needs_revision' %}
Needs Revision
{% elif submission.status == 'approved' %}
Approved
{% else %}
Final Status
{% endif %}
</span>
</div>
<small class="text-sm">
{% if submission.status == 'needs_revision' or submission.status == 'approved' %}
Final
{% endif %}
</small>
</div>
</div>
</div>
<!-- Tips Card -->
<div class="bg-white dark:bg-gray-800 shadow-md rounded-lg overflow-hidden">
<div class="bg-teal-600 dark:bg-teal-800 text-white px-6 py-4">
<h3 class="text-xl font-semibold">Document Submission Tips</h3>
</div>
<div class="p-6 bg-gray-50 dark:bg-gray-700">
<ul class="space-y-3">
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Clear and well-organized content
</li>
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Proper formatting and styling
</li>
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Accurate citations and references
</li>
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Proofread for grammar and spelling
</li>
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Follow assignment requirements
</li>
<li class="flex items-center text-gray-700 dark:text-gray-300">
<svg class="w-5 h-5 mr-3 text-green-500"
fill="currentColor"
viewBox="0 0 20 20">
<path fill-rule="evenodd" d="M16.707 5.293a1 1 0 010 1.414l-8 8a1 1 0 01-1.414 0l-4-4a1 1 0 011.414-1.414L8 12.586l7.293-7.293a1 1 0 011.414 0z" clip-rule="evenodd" />
</svg>
Submit before deadline
</li>
</ul>
</div>
</div>
</div>
</div>
</div> {% endblock %}
{% block extra_js %} <script>
document.addEventListener('DOMContentLoaded', function() {
const timelineItems = document.querySelectorAll('[class*="timeline"] > div');
timelineItems.forEach(item => {
item.addEventListener('mouseenter', function() {
this.classList.add('bg-gray-100', 'dark:bg-gray-700');
});
item.addEventListener('mouseleave', function() {
this.classList.remove('bg-gray-100', 'dark:bg-gray-700');
});
});
});
</script> {% endblock %}3. upload_submission.html{% extends "base.html" %}
{% block title %}Upload PDF Document{% endblock %}
{% block content %} <div class="container mx-auto px-4 py-8">
<div class="max-w-2xl mx-auto">
<div class="bg-white dark:bg-gray-900 shadow-md rounded-lg overflow-hidden">
<div class="bg-blue-600 dark:bg-blue-700 text-white px-6 py-4">
<h2 class="text-xl font-semibold">Upload PDF Document</h2>
</div>
<div class="p-6">
<p class="text-gray-600 dark:text-gray-300 mb-6">
Upload your PDF document for review. Our instructors will provide constructive feedback to help you improve your work.
</p>
<form method="post" enctype="multipart/form-data" class="space-y-6">
{% csrf_token %}
{% if form.non_field_errors %}
<div class="bg-red-100 dark:bg-red-900 border border-red-400 dark:border-red-600 text-red-700 dark:text-red-300 px-4 py-3 rounded relative"
role="alert">
{% for error in form.non_field_errors %}<p>{{ error }}</p>{% endfor %}
</div>
{% endif %}
{% 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-2">
{{ field.label }}
</label>
{% if field.field.widget.input_type == 'file' %}
<input type="file"
id="{{ field.id_for_label }}"
name="{{ field.name }}"
class="block w-full text-sm text-gray-500 dark:text-gray-300 file:mr-4 file:py-2 file:px-4 file:rounded-md file:border-0 file:text-sm file:font-semibold file:bg-blue-500 file:text-white hover:file:bg-blue-600 dark:file:bg-blue-700 dark:hover:file:bg-blue-800"
accept=".pdf"
required />
{% else %}
<input type="{{ field.field.widget.input_type }}"
id="{{ field.id_for_label }}"
name="{{ field.name }}"
class="bg-white dark:bg-gray-800 text-gray-900 dark:text-white border border-gray-300 dark:border-gray-600 rounded-md p-2 focus:border-blue-500 focus:outline-none focus:ring-2 focus:ring-blue-500 w-full"
placeholder="{{ field.label }}"
{% if field.field.required %}required{% endif %} />
{% endif %}
{% if field.errors %}
<div class="mt-2 text-sm text-red-600 dark:text-red-400">
{% for error in field.errors %}<p>{{ error }}</p>{% endfor %}
</div>
{% endif %}
{% if field.help_text %}<p class="mt-2 text-sm text-gray-500 dark:text-gray-400">{{ field.help_text }}</p>{% endif %}
</div>
{% endfor %}
<div class="mt-6">
<button type="submit"
class="w-full bg-blue-600 dark:bg-blue-700 text-white py-3 px-4 rounded-md hover:bg-blue-700 dark:hover:bg-blue-800 focus:outline-none focus:ring-2 focus:ring-blue-500 dark:focus:ring-blue-600 focus:ring-offset-2 transition duration-300 ease-in-out transform hover:scale-102">
Upload Document
</button>
</div>
</form>
</div>
<div class="bg-gray-100 dark:bg-gray-800 text-gray-600 dark:text-gray-300 px-6 py-4 text-sm flex items-center">
<svg class="w-5 h-5 mr-2 text-blue-500 dark:text-blue-400"
fill="none"
stroke="currentColor"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z">
</path>
</svg>
Your document will be carefully reviewed by our instructors, who will provide constructive feedback to help you improve your work.
</div>
</div>
</div>
</div> {% endblock %}These templates have been updated to use generic PDF submission terminology and include all the necessary field updates. I've also modified the status options to match the new model's STATUS_CHOICES and updated all URL references to use the new |
|
Oops, something went wrong! Please try again later. 🐰 💔 |
1 similar comment
|
Oops, something went wrong! Please try again later. 🐰 💔 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
web/forms.py (2)
1696-1696: Fix the negation pattern in the conditional statement.The current negation pattern is less Pythonic. Use
not ininstead ofnot ... infor better readability.- if not uploaded_file.content_type.lower() in ["application/pdf", "application/x-pdf"]: + if uploaded_file.content_type.lower() not in ["application/pdf", "application/x-pdf"]:🧰 Tools
🪛 Ruff (0.8.2)
1696-1696: Test for membership should be
not inConvert to
not in(E713)
61-99: Add PDFSubmissionForm to the__all__list.The new form class needs to be added to the
__all__list to maintain consistency with the module's export pattern and ensure it's available when using wildcard imports.__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", + "PDFSubmissionForm", ]web/settings.py (1)
176-176: Update the comment to match the "optional" email verification setting.The comment still indicates that email verification is required, but the setting has been changed to "optional". This inconsistency could confuse developers about the actual behavior of the application.
-ACCOUNT_EMAIL_VERIFICATION = "optional" # Require email verification +ACCOUNT_EMAIL_VERIFICATION = "optional" # Email verification is optionalweb/templates/web/pdf/submission_list.html (1)
12-36: Well-structured responsive header with role-based controls.The header section features:
- Responsive design with Flexbox
- Clear visual hierarchy
- Appropriate conditional rendering of the upload button for students only
- Accessible SVG icon implementation
A minor suggestion would be to add an
aria-labelto the upload button for better accessibility.-<a href="{% url 'upload_pdf_submission' %}" +<a href="{% url 'upload_pdf_submission' %}" + aria-label="Upload new PDF document" class="inline-flex items-center px-4 py-2 bg-blue-600 dark:bg-blue-700 text-white rounded-md hover:bg-blue-700 dark:hover:bg-blue-800 transition-colors">web/migrations/0048_pdftype_pdfsubmission.py (1)
32-101: Comprehensive PDFSubmission model with robust validation and relationships.The PDFSubmission model is well-designed with:
- Appropriate fields for capturing all necessary metadata
- File validation for both extension and size
- Clear status tracking with predefined choices
- Proper timestamps for submission tracking
- Well-defined relationships to users and PDF types
- Sensible default ordering by submission date (newest first)
There's one potential enhancement to consider:
Consider adding a transaction-safe method to handle file size calculation during save, as the current implementation might not calculate file_size automatically. You would need to override the model's save method in your models.py file:
def save(self, *args, **kwargs): # Calculate file size if file is present and size is not set if self.pdf_file and not self.file_size: self.file_size = self.pdf_file.size super().save(*args, **kwargs)web/templates/web/pdf/submission_detail.html (1)
231-287: Helpful document submission tips with clean visual presentation.The tips card:
- Provides valuable guidance for students
- Uses consistent iconography
- Has a clean, scannable list format
- Uses appropriate text contrast for readability
Consider making these tips configurable by PDF type in the future, as different document types might benefit from different guidance.
In a future iteration, consider making these tips configurable by PDF type in the database, so that different submission types can have tailored guidance.
web/views.py (1)
6286-6323: Implementation ofupload_pdf_submissionis clean.
- The
@login_requireddecorator properly limits access to authenticated users.- The logic for saving the form and associating the PDF with the current user is straightforward.
- You effectively create two sets of notifications: one for the uploader and one for all teacher users.
- If you expect a large number of teachers in the system, consider an asynchronous approach or a bulk insert for notifications to handle performance concerns.
web/models.py (3)
14-14: Remove unused imports for cleanliness.The newly added
MaxValueValidatorandMinValueValidatoraren’t used in the changed code segments. Consider removing them to avoid confusion and maintain a clean import list.
2517-2530: Consider makingicon_classa controlled choice field.Currently,
icon_classis free text. Mismatched class names might lead to broken icons. Achoicesfield or a separate reference table would help ensure consistency, reduce typos, and improve maintainability.
2532-2535: Parameterize your file size limit.Hardcoding the 10MB limit is acceptable but less flexible. Consider storing this constant in settings (e.g.,
settings.MAX_PDF_SIZE_MB) and referencing it here to simplify maintenance and updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
web/admin.py(2 hunks)web/forms.py(3 hunks)web/migrations/0048_pdftype_pdfsubmission.py(1 hunks)web/models.py(2 hunks)web/settings.py(1 hunks)web/templates/index.html(2 hunks)web/templates/web/pdf/submission_detail.html(1 hunks)web/templates/web/pdf/submission_list.html(1 hunks)web/templates/web/pdf/upload_submission.html(1 hunks)web/urls.py(1 hunks)web/views.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/templates/index.html
- web/admin.py
🧰 Additional context used
🧬 Code Definitions (4)
web/urls.py (1)
web/views.py (3)
pdf_submission_list(6326-6353)upload_pdf_submission(6287-6322)pdf_submission_detail(6357-6401)
web/forms.py (1)
web/models.py (18)
PDFSubmission(2537-2604)PDFType(2517-2529)Meta(44-45)Meta(201-202)Meta(365-366)Meta(476-477)Meta(532-533)Meta(554-555)Meta(601-604)Meta(655-656)Meta(708-710)Meta(734-735)Meta(751-753)Meta(775-776)Meta(791-792)Meta(839-840)Meta(906-907)Meta(937-938)
web/migrations/0048_pdftype_pdfsubmission.py (1)
web/models.py (1)
validate_pdf_file_size(2532-2534)
web/views.py (2)
web/forms.py (6)
PDFSubmissionForm(1662-1698)save(220-258)save(567-592)save(659-673)save(1296-1309)save(1470-1485)web/models.py (16)
PDFSubmission(2537-2604)PDFType(2517-2529)save(95-99)save(153-189)save(207-210)save(274-308)save(371-398)save(488-490)save(715-718)save(912-917)save(970-975)save(1050-1053)save(1136-1147)Notification(28-48)status(2127-2137)get_status_choices(2603-2604)
🪛 Ruff (0.8.2)
web/forms.py
1696-1696: Test for membership should be not in
Convert to not in
(E713)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (22)
web/templates/web/pdf/upload_submission.html (1)
1-118: Well-implemented upload form with good UX considerations.The template is well-structured with appropriate error handling, clear user instructions, and good feedback mechanisms. The addition of a loading spinner during submission and dynamic file name display enhances user experience.
web/urls.py (1)
409-411:Details
❓ Verification inconclusive
The URL patterns are well-structured, but there's a naming inconsistency in the related view.
The URL patterns follow RESTful conventions and are logically organized. However, in the
pdf_submission_detailview (from the provided context), there's a naming inconsistency where the submission object is passed to the template as "proposal":context = { "proposal": submission, # Rename to match template # ... }This naming inconsistency could cause confusion. Consider updating the variable name to "submission" for consistency.
🏁 Script executed:
#!/bin/bash # Check for template references to "proposal" variable echo "Checking if templates use 'proposal' variable instead of 'submission':" fd -e html | xargs grep -l "proposal\." || echo "No templates using 'proposal.' found"Length of output: 241
Review Note: Verify the naming in the PDF submission detail view context.
The URL patterns for the new PDF submission functionality are well-structured and follow RESTful conventions. However, in the
pdf_submission_detailview the object is passed to the template with the key"proposal"rather than"submission". Although this appears inconsistent with the naming elsewhere, a grep search revealed that at least one template (web/templates/gsoc_landing_page.html) is using the"proposal"variable.Before proceeding with a rename for clarity and consistency, please verify that:
- The use of
"proposal"in this view is intentional and aligns with the expectations of all dependent templates.- Any renaming won’t inadvertently break parts of the application that rely on the current naming.
web/templates/web/pdf/submission_list.html (4)
1-11: Good template organization and conditional title handling!The template correctly extends the base template and uses conditional rendering for the title based on user role. This provides appropriate context for different user types.
37-135: Excellent responsive table implementation with role-based column visibility.The submissions table is well-structured with:
- Proper responsive design with horizontal scroll for small screens
- Appropriate column headers with semantic markup
- Conditional rendering of the Student column for reviewers only
- Clear status indicators with semantic color coding
- Concise action links with proper security attributes for external links
This implementation follows best practices for data tables in web applications.
136-158: User-friendly empty state with clear call-to-action.The empty state handling is well-implemented with:
- Distinct messaging for different user roles
- Helpful prompt for students with a direct link to upload
- Appropriate visual styling using Tailwind's utility classes
- Clear iconography to enhance the messaging
This creates a good user experience when there are no submissions to display.
159-189: Well-structured pagination with current page highlighting.The pagination implementation:
- Only displays when necessary (multiple pages exist)
- Includes proper navigation elements (previous/next and page numbers)
- Visually highlights the current page
- Uses semantic HTML and appropriate ARIA attributes
This provides a good user experience for navigating through multiple pages of submissions.
web/migrations/0048_pdftype_pdfsubmission.py (2)
1-17: Correctly structured migration file with proper imports and dependencies.The migration includes all necessary imports and correctly specifies dependencies on both the previous migration and the user model.
18-31: Well-designed PDFType model with appropriate fields and metadata.The PDFType model includes:
- Standard auto-created BigAutoField for the primary key
- Properly sized CharField for name (100 chars is appropriate)
- Optional description field for additional context
- Optional icon_class field with helpful usage hint
- Appropriate verbose name settings for admin display
This model structure provides a good foundation for categorizing different types of PDF submissions.
web/templates/web/pdf/submission_detail.html (5)
1-24: Clear template structure with proper breadcrumb navigation.The template has a well-organized structure with:
- Proper base template extension
- Appropriate title configuration
- Accessible breadcrumb navigation for easy context and navigation
- Clear visual hierarchy
This provides good user orientation and navigation capabilities.
25-110: Comprehensive and well-structured submission details card.The submission details card effectively:
- Organizes information in a logical, scannable layout
- Uses a responsive grid for different screen sizes
- Shows conditional content only when available (due date, description)
- Provides clear action buttons with appropriate security attributes
Nice work on using the
file_size_displayproperty or method, which suggests proper formatting of file sizes for human readability.
174-229: Visually effective status timeline with conditional styling.The timeline implementation:
- Clearly visualizes the submission's progress through different statuses
- Uses appropriate color coding to indicate current status
- Conditionally displays dates when available
- Visually differentiates completed, current, and future states
The implementation is complex but effective, with nested conditionals appropriately handling the various status possibilities.
292-306: Clean JavaScript implementation for interactive timeline.The JavaScript:
- Properly waits for DOM content to be loaded
- Uses modern event listeners
- Implements a subtle but effective hover interaction
- Handles both light and dark mode through appropriate class toggling
This enhances the user experience without overcomplicating the interface.
111-171:Details
❓ Verification inconclusive
Well-implemented feedback system with role-based permissions.
The feedback section:
- Appropriately displays existing feedback with reviewer attribution
- Only shows the feedback form to reviewers
- Has clearly labeled form controls with helpful guidance text
- Maintains visual consistency with the rest of the interface
Make sure your view correctly sets the
is_reviewercontext variable and populates thestatus_choicesfor the select dropdown.Verify that the view handling this template populates both the
is_reviewerboolean and thestatus_choiceslist for the status dropdown. The template references these variables but doesn't define them.
🏁 Script executed:
#!/bin/bash # Look for the view that renders this template rg -A 20 "def pdf_submission_detail" web/views.pyLength of output: 911
Action Required: Verify Context Variables Setup
The view correctly computes the
is_reviewerflag, as confirmed by the current implementation inweb/views.py. However, it's not immediately clear whether the context includesstatus_choices(typically populated fromPDFSubmission.STATUS_CHOICES) before rendering the template. Please double-check that the final context passed to the template includes a validstatus_choiceslist for the dropdown.
- Confirm that the view assigns something like
'status_choices': PDFSubmission.STATUS_CHOICESin the context.- Ensure the view returns the context containing both
is_reviewerandstatus_choices.web/views.py (8)
26-26: ImportingPermissionDeniedis appropriate here.
This import is used later to raise a 403-style exception in case of unauthorized access, ensuring a clear, built-in mechanism for permission handling.
29-29: Pagination import looks good.
UsingEmptyPage,PageNotAnInteger, andPaginatorwill help gracefully handle paginated results for user submissions.
78-78: New form import is aligned with the usage ofPDFSubmissionForm.
This properly wires up the new form for PDF submission in the view logic.
129-130: Models import for PDF submission and types.
Good job referencingPDFSubmissionandPDFTypedirectly. This will enable the new PDF submission feature to connect seamlessly with your database.
6281-6283: No functional change for these lines.
They are just comments or placeholders introducing the new PDF submission views.
6284-6285: No functional change for these blank lines.
These lines do not impact the code.
6326-6354: Well-structured listing view for PDF submissions.
- The dynamic approach to set
viewing_asbased on the user’s teacher status is a simple solution.- Pagination logic with
Paginator(PageNotAnInteger, EmptyPage)is handled properly.- Consider verifying if any staff or superuser role might require a different listing view if roles expand in the future.
6357-6402: Comprehensive detail view for reviewing PDF submissions.
- The permission check with
is_studentandis_revieweris appropriate.- Updating feedback and status in a single POST is convenient for teachers, and you properly notify the student upon review.
- For concurrent teacher reviews, consider a more granular feedback model if multiple reviewers may need to comment. Otherwise, this single feedback approach is fine for smaller teams.
web/models.py (1)
2537-2607:Details
❓ Verification inconclusive
Verify the logic for setting
reviewed_at.The code only sets
reviewed_atthe first time the status changes from anything besides"reviewed"to"reviewed". If the submission moves from"reviewed"back to another status and then again to"reviewed", the date remains unchanged. Ensure this matches your intended workflow for re-review scenarios.Please review or test whether the timestamp should reset on subsequent reviews. If you need confirmation, you can run the following script to search for any references to
reviewed_atupdates outside this model:
🏁 Script executed:
#!/bin/bash # Searching references for `reviewed_at` in the codebase to ensure no external overrides rg --case-sensitive "reviewed_at" -A 5Length of output: 3734
Action: Confirm reviewed_at behavior for repeated review transitions
The current implementation in
PDFSubmission.save()only updatesreviewed_atwhen a submission’s status changes from a non-"reviewed" state to"reviewed". This means that if a submission is moved out of"reviewed"and then set back to"reviewed", the timestamp remains unchanged.Additionally, our search in the codebase revealed that in
web/views.pythere is an explicit assignment:submission.reviewed_at = timezone.now() submission.save()This suggests that there may be scenarios where the timestamp is being set manually outside the model’s logic. Please verify that both approaches align with your intended workflow. In particular, confirm whether:
- The timestamp should record only the initial review moment, or
- It should update on every transition back to
"reviewed".Make sure the behavior is consistent across the model and view logic to avoid any unintended discrepancies.
|
@coderabbitai, kindly assist why is views.serve_work_file still not working properly and giving security issues. |
|
Tip For best results, initiate chat on the files or code changes. @Inshamhaque The def serve_work_file(request, file_path):
"""
Securely serve uploaded work files for development only.
DO NOT use this in production—let your web server handle media files.
"""
from django.http import FileResponse, Http404
from django.conf import settings
import os
import mimetypes
# Resolve symlinks and get absolute paths
media_root = os.path.realpath(settings.MEDIA_ROOT)
requested_path = os.path.realpath(os.path.join(media_root, file_path))
# Ensure the requested file is within MEDIA_ROOT (prevents path traversal)
if os.path.commonpath([requested_path, media_root]) != media_root:
raise Http404("Invalid file path")
# Restrict allowed file extensions
allowed_extensions = {"pdf", "doc", "docx", "odt", "txt", "jpg", "jpeg", "png", "gif", "bmp"}
ext = os.path.splitext(requested_path)[1][1:].lower()
if ext not in allowed_extensions:
raise Http404("File type not allowed")
if not os.path.exists(requested_path) or not os.path.isfile(requested_path):
raise Http404("File not found")
content_type, _ = mimetypes.guess_type(requested_path)
if not content_type:
content_type = 'application/octet-stream'
return FileResponse(open(requested_path, 'rb'), content_type=content_type)Key points:
If you still see security warnings:
Reminder: Let me know if you need further help or if the warning persists after this fix! ✏️ Learnings added
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
web/views.py (5)
6983-7011:⚠️ Potential issueAdd safer profile attribute checking to prevent potential errors.
The code assumes
profileandis_teacherattributes exist without verification, which could lead to attribute errors if a user's profile is not properly configured.- viewing_as = "student" - if request.user.profile.is_teacher: - viewing_as = "reviewer" + is_teacher = hasattr(request.user, 'profile') and getattr(request.user.profile, 'is_teacher', False) + if is_teacher: submissions_list = WorkSubmission.objects.select_related("student", "work_type").all().order_by("-submitted_at") else: submissions_list = ( WorkSubmission.objects.select_related("work_type").filter(student=request.user).order_by("-submitted_at") )
7005-7011: 🧹 Nitpick (assertive)Update context variable for better template clarity.
The variable name in the context is not semantically clear. Using a boolean flag would provide better clarity in the template.
context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": is_teacher, "work_types": WorkType.objects.all(), } return render(request, "web/work/submission_list.html", context)
7108-7121: 🧹 Nitpick (assertive)Add authentication and fix unused parameter in API endpoint.
The API endpoint has an unused request parameter and is missing a trailing comma in the JSON response for better maintainability.
@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style + }, } ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)🧰 Tools
🪛 Ruff (0.8.2)
7108-7108: Unused function argument:
request(ARG001)
7118-7118: Trailing comma missing
Add trailing comma
(COM812)
6941-6980: 🧹 Nitpick (assertive)Well-structured upload view with good error handling and notifications.
The upload_work_submission function is well implemented with good transaction management, form validation, and notification handling.
Consider adding a check for empty request.FILES:
if request.method == "POST": form = WorkSubmissionForm(request.POST, request.FILES) + if not request.FILES: + messages.error(request, "No file was uploaded. Please select a file to upload.") + return render(request, "web/work/upload_submission.html", {"form": form}) if form.is_valid():This would provide clearer feedback to users who might submit the form without selecting a file.
7058-7067: 🧹 Nitpick (assertive)Add type annotations and fix exception message.
This function is missing type annotations. Additionally, PermissionDenied should use the default message rather than a custom one.
@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})🧰 Tools
🪛 Ruff (0.8.2)
7058-7058: Missing return type annotation for public function
delete_work_submission(ANN201)
7058-7058: Missing type annotation for function argument
request(ANN001)
7058-7058: Missing type annotation for function argument
submission_id(ANN001)
7062-7062: Avoid specifying long messages outside the exception class
(TRY003)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/views.py
7023-7023: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7058-7058: Missing return type annotation for public function delete_work_submission
(ANN201)
7058-7058: Missing type annotation for function argument request
(ANN001)
7058-7058: Missing type annotation for function argument submission_id
(ANN001)
7062-7062: Avoid specifying long messages outside the exception class
(TRY003)
7071-7071: Missing return type annotation for public function serve_work_file
(ANN201)
7071-7071: Missing type annotation for function argument request
(ANN001)
7071-7071: Unused function argument: request
(ARG001)
7071-7071: Missing type annotation for function argument file_path
(ANN001)
7088-7088: Avoid specifying long messages outside the exception class
(TRY003)
7094-7094: Avoid specifying long messages outside the exception class
(TRY003)
7097-7097: Avoid specifying long messages outside the exception class
(TRY003)
7108-7108: Unused function argument: request
(ARG001)
7118-7118: Trailing comma missing
Add trailing comma
(COM812)
🪛 GitHub Check: CodeQL
web/views.py
[failure] 7096-7096: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 7096-7096: Uncontrolled data used in path expression
This path depends on a user-provided value.
[failure] 7103-7103: Uncontrolled data used in path expression
This path depends on a user-provided value.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (1)
web/views.py (1)
7047-7054: LGTM - Good context preparation for template rendering.The context dictionary properly includes all necessary variables for the template, with clear handling of student vs. reviewer roles.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
web/views.py (7)
7023-7023: 🧹 Nitpick (assertive)Remove unnecessary parentheses on raised exception.
The
PermissionDenied()call has unnecessary parentheses according to Python style guides.# Check if the user has access if not (is_student or is_reviewer): - raise PermissionDenied() # Use default message or define in exception class + raise PermissionDenied # Use default message🧰 Tools
🪛 Ruff (0.8.2)
7023-7023: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7005-7011: 🧹 Nitpick (assertive)Update template context variable for better semantic meaning.
The
viewing_asvariable is used for logic flow but not included in the template context in a way that matches its name. This could lead to confusion when accessing the variable in templates.context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": is_teacher, "work_types": WorkType.objects.all(), } return render(request, "web/work/submission_list.html", context)
7071-7115:⚠️ Potential issueFix security vulnerabilities in file serving and improve resource management.
The file serving function has several issues: it uses an unused parameter, has unnecessary parentheses on exceptions, and doesn't properly manage file resources. Most importantly, it needs stronger path validation to prevent path traversal attacks.
@login_required -def serve_work_file(request, file_path): +def serve_work_file(_request: HttpRequest, file_path: str) -> HttpResponse: """ Securely serve uploaded work files for development only. DO NOT use this in production—let your web server handle media files. """ import mimetypes import os from django.conf import settings from django.http import FileResponse, Http404 - # Resolve symlinks and get absolute paths - media_root = os.path.realpath(settings.MEDIA_ROOT) - requested_path = os.path.realpath(os.path.join(media_root, file_path)) + # Normalize and sanitize the file path, removing any path traversal attempts + safe_path = os.path.normpath(file_path).lstrip('/') + + # Construct the absolute file path within MEDIA_ROOT only + absolute_path = os.path.join(settings.MEDIA_ROOT, safe_path) + + # Use realpath to resolve symlinks, etc. + requested_path = os.path.realpath(absolute_path) + media_root = os.path.realpath(settings.MEDIA_ROOT) # Ensure the requested file is within MEDIA_ROOT (prevents path traversal) - if os.path.commonpath([requested_path, media_root]) != media_root: - raise Http404("Invalid file path") + if not requested_path.startswith(media_root): + raise Http404 # Restrict allowed file extensions allowed_extensions = {"pdf", "doc", "docx", "odt", "txt", "jpg", "jpeg", "png", "gif", "bmp"} ext = os.path.splitext(requested_path)[1][1:].lower() if ext not in allowed_extensions: - raise Http404("File type not allowed") + raise Http404 if not os.path.exists(requested_path) or not os.path.isfile(requested_path): - raise Http404("File not found") + raise Http404 content_type, _ = mimetypes.guess_type(requested_path) if not content_type: content_type = "application/octet-stream" - return FileResponse(open(requested_path, "rb"), content_type=content_type) + # Use with statement to ensure file is properly closed + file_handle = open(requested_path, "rb") + response = FileResponse(file_handle, content_type=content_type) + # Ensure file is closed when response is done + response.close = lambda: file_handle.close() + return response🧰 Tools
🪛 Ruff (0.8.2)
7071-7071: Missing return type annotation for public function
serve_work_file(ANN201)
7071-7071: Missing type annotation for function argument
request(ANN001)
7071-7071: Unused function argument:
request(ARG001)
7071-7071: Missing type annotation for function argument
file_path(ANN001)
7095-7095: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7101-7101: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7104-7104: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7111-7111: Use a context manager for opening files
(SIM115)
7120-7133:⚠️ Potential issueSecure the API endpoint and fix code style issues.
This API endpoint should be protected with authentication to prevent unauthorized access. Additionally, there are some code style issues like a missing trailing comma.
+@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style } ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)🧰 Tools
🪛 Ruff (0.8.2)
7120-7120: Unused function argument:
request(ARG001)
7130-7130: Trailing comma missing
Add trailing comma
(COM812)
7014-7054: 🛠️ Refactor suggestionConsider using transactions for submission updates and notifications.
When updating the submission and creating a notification, using a transaction would ensure both operations succeed or fail together, maintaining data consistency.
if request.method == "POST" and is_reviewer: feedback = request.POST.get("feedback", "") new_status = request.POST.get("status", "") + with transaction.atomic(): if feedback: submission.feedback = feedback if new_status and new_status in dict(submission.STATUS_CHOICES): submission.status = new_status submission.reviewed_by = request.user + submission.reviewed_at = timezone.now() # Add timestamp for the review submission.save() # Create notification for student Notification.objects.create( user=submission.student, title="Your Work Submission Has Been Reviewed", message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", notification_type="info", ) messages.success(request, "Feedback and status updated successfully.")🧰 Tools
🪛 Ruff (0.8.2)
7023-7023: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7058-7067:⚠️ Potential issueFix exception message handling and add type annotations.
The current code uses a custom message with
PermissionDenied, which is not recommended by Python style guides and static analyzers. Also, adding type annotations improves code clarity.@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied # Use default message for better consistency if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})🧰 Tools
🪛 Ruff (0.8.2)
7058-7058: Missing return type annotation for public function
delete_work_submission(ANN201)
7058-7058: Missing type annotation for function argument
request(ANN001)
7058-7058: Missing type annotation for function argument
submission_id(ANN001)
7062-7062: Avoid specifying long messages outside the exception class
(TRY003)
6987-6988: 💡 Verification agent🧩 Analysis chain
Add safer profile attribute checking to prevent potential AttributeError.
The code assumes the user's profile has an
is_teacherattribute without verification, which could lead to an AttributeError if the profile is missing or improperly configured.- if request.user.profile.is_teacher: + is_teacher = hasattr(request.user, 'profile') and getattr(request.user.profile, 'is_teacher', False) + if is_teacher: viewing_as = "reviewer" submissions_list = WorkSubmission.objects.select_related("student", "work_type").all().order_by("-submitted_at")
🏁 Script executed:
#!/bin/bash # Check if there are users without profiles or without is_teacher attribute python3 -c " from django.contrib.auth.models import User from web.models import Profile # Check users without profiles users_without_profiles = User.objects.filter(profile__isnull=True).count() print(f'Users without profiles: {users_without_profiles}') # Check if any profiles might be missing is_teacher attribute try: Profile._meta.get_field('is_teacher') print('is_teacher field exists in Profile model') except Exception as e: print(f'is_teacher field error: {e}') "Length of output: 630
🏁 Script executed:
#!/bin/bash # Locate Profile model definition and its fields in the web app grep -R "class Profile" -n web grep -R "is_teacher" -n web grep -R "OneToOneField" -n web/models.pyLength of output: 13319
Add safer profile attribute check to prevent AttributeError
The snippet at web/views.py:6987‑6988 currently assumes
request.user.profilealways exists. To match the safe‑access patterns used elsewhere (e.g. around line 7020), guard against a missing profile first:
- web/views.py:6987-6988
- if request.user.profile.is_teacher: + is_teacher = getattr(getattr(request.user, "profile", None), "is_teacher", False) + if is_teacher: viewing_as = "reviewer" submissions_list = WorkSubmission.objects.select_related("student", "work_type")\ - .all().order_by("-submitted_at") + .all().order_by("-submitted_at")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/views.py
7023-7023: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7058-7058: Missing return type annotation for public function delete_work_submission
(ANN201)
7058-7058: Missing type annotation for function argument request
(ANN001)
7058-7058: Missing type annotation for function argument submission_id
(ANN001)
7062-7062: Avoid specifying long messages outside the exception class
(TRY003)
7071-7071: Missing return type annotation for public function serve_work_file
(ANN201)
7071-7071: Missing type annotation for function argument request
(ANN001)
7071-7071: Unused function argument: request
(ARG001)
7071-7071: Missing type annotation for function argument file_path
(ANN001)
7095-7095: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7101-7101: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7104-7104: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7111-7111: Use a context manager for opening files
(SIM115)
7120-7120: Unused function argument: request
(ARG001)
7130-7130: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (1)
web/views.py (1)
6941-6980: Well-structured upload function with good error handling and use of transactions!The upload function is well-implemented with transaction support to ensure data consistency. Using
bulk_createfor teacher notifications is an excellent performance optimization.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
web/views.py (4)
7060-7070: 🧹 Nitpick (assertive)Add type annotations and fix permission error message.
The delete submission view correctly checks ownership and handles both GET and POST requests.
Add type annotations and fix the PermissionDenied error message according to static analysis recommendations:
@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied() # Use default message as recommended by static analysis if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})🧰 Tools
🪛 Ruff (0.8.2)
7061-7061: Missing return type annotation for public function
delete_work_submission(ANN201)
7061-7061: Missing type annotation for function argument
request(ANN001)
7061-7061: Missing type annotation for function argument
submission_id(ANN001)
7065-7065: Avoid specifying long messages outside the exception class
(TRY003)
6986-7014: 🧹 Nitpick (assertive)🛠️ Refactor suggestion
Add safer profile attribute checking to prevent potential errors.
The submission list view correctly differentiates between teachers (who see all submissions) and students (who see only their own). However, the code directly accesses
request.user.profile.is_teacherwithout checking if the profile attribute exists, which could cause an attribute error.Implement a safer check for teacher role with proper attribute validation:
- viewing_as = "student" - if request.user.profile.is_teacher: - viewing_as = "reviewer" + viewing_as = "student" + is_teacher = hasattr(request.user, 'profile') and getattr(request.user.profile, 'is_teacher', False) + if is_teacher: + viewing_as = "reviewer" submissions_list = WorkSubmission.objects.select_related("student", "work_type").all().order_by("-submitted_at")Also, consider updating the context variable to be more descriptive:
context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": is_teacher, "work_types": WorkType.objects.all(), }
7122-7136: 🛠️ Refactor suggestionAdd authentication requirement and fix JSON response formatting.
The API endpoint for work type details should require authentication and fix the trailing comma issue flagged by static analysis.
Secure the endpoint and fix formatting issues:
+@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style + }, } ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)🧰 Tools
🪛 Ruff (0.8.2)
7123-7123: Unused function argument:
request(ARG001)
7133-7133: Trailing comma missing
Add trailing comma
(COM812)
7017-7048: 🛠️ Refactor suggestionUse a transaction for updating submission feedback and sending notifications.
The submission detail view correctly checks for user permissions, but updates to the submission should be atomic with notification creation to ensure data consistency.
Add transaction support and set the reviewed_at timestamp:
if request.method == "POST" and is_reviewer: feedback = request.POST.get("feedback", "") new_status = request.POST.get("status", "") + with transaction.atomic(): + if feedback: + submission.feedback = feedback + + if new_status and new_status in dict(submission.STATUS_CHOICES): + submission.status = new_status + submission.reviewed_by = request.user + submission.reviewed_at = timezone.now() # Add timestamp for reviews + + submission.save() + + # Create notification for student + Notification.objects.create( + user=submission.student, + title="Your Work Submission Has Been Reviewed", + message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", + notification_type="info", + ) + + messages.success(request, "Feedback and status updated successfully.") - if feedback: - submission.feedback = feedback - - if new_status and new_status in dict(submission.STATUS_CHOICES): - submission.status = new_status - submission.reviewed_by = request.user - - submission.save() - - # Create notification for student - Notification.objects.create( - user=submission.student, - title="Your Work Submission Has Been Reviewed", - message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", - notification_type="info", - ) - - messages.success(request, "Feedback and status updated successfully.")🧰 Tools
🪛 Ruff (0.8.2)
7026-7026: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/views.py
7026-7026: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7061-7061: Missing return type annotation for public function delete_work_submission
(ANN201)
7061-7061: Missing type annotation for function argument request
(ANN001)
7061-7061: Missing type annotation for function argument submission_id
(ANN001)
7065-7065: Avoid specifying long messages outside the exception class
(TRY003)
7074-7074: Missing return type annotation for public function serve_work_file
(ANN201)
7074-7074: Missing type annotation for function argument request
(ANN001)
7074-7074: Unused function argument: request
(ARG001)
7074-7074: Missing type annotation for function argument file_path
(ANN001)
7098-7098: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7104-7104: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7107-7107: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7114-7114: Use a context manager for opening files
(SIM115)
7123-7123: Unused function argument: request
(ARG001)
7133-7133: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run Tests
- GitHub Check: build-and-test
🔇 Additional comments (2)
web/views.py (2)
7049-7057: Well-implemented context preparation for submission detail template.The context data for the template is properly prepared with all necessary information, including submission details, status choices, and user role flags.
7073-7118: 💡 Verification agent🧩 Analysis chain
Fix security vulnerabilities in file serving functionality.
The file serving view has potential security issues with path traversal. While there are checks in place, they could be strengthened, and resource management could be improved.
Implement a more secure file serving approach with proper resource management:
@login_required -def serve_work_file(request, file_path): +def serve_work_file(_request: HttpRequest, file_path: str) -> HttpResponse: """ Securely serve uploaded work files for development only. DO NOT use this in production—let your web server handle media files. """ import mimetypes import os from django.conf import settings from django.http import FileResponse, Http404 # Normalize and sanitize the file path, removing any path traversal attempts safe_path = os.path.normpath(file_path).lstrip('/') # Construct the absolute file path within MEDIA_ROOT only absolute_path = os.path.join(settings.MEDIA_ROOT, safe_path) # Use realpath to resolve symlinks, etc. requested_path = os.path.realpath(absolute_path) media_root = os.path.realpath(settings.MEDIA_ROOT) # Ensure the requested file is within MEDIA_ROOT (prevents path traversal) if not requested_path.startswith(media_root): - raise Http404() + raise Http404 # Restrict allowed file extensions allowed_extensions = {"pdf", "doc", "docx", "odt", "txt", "jpg", "jpeg", "png", "gif", "bmp"} ext = os.path.splitext(requested_path)[1][1:].lower() if ext not in allowed_extensions: - raise Http404() + raise Http404 if not os.path.exists(requested_path) or not os.path.isfile(requested_path): - raise Http404() + raise Http404 content_type, _ = mimetypes.guess_type(requested_path) if not content_type: content_type = "application/octet-stream" - # Use with statement to ensure file is properly closed - file_handle = open(requested_path, "rb") - response = FileResponse(file_handle, content_type=content_type) - # Ensure file is closed when response is done - response.close = lambda: file_handle.close() - return response + # Use context manager to ensure proper resource cleanup + with open(requested_path, "rb") as file_handle: + return FileResponse(file_handle, content_type=content_type)Let me run a script to check if this implementation is secure against path traversal:
🏁 Script executed:
#!/bin/bash # Test for path traversal vulnerabilities # Create a test directory structure mkdir -p test_media/allowed mkdir -p test_media/not_allowed echo "test content" > test_media/allowed/test.pdf echo "sensitive data" > test_media/not_allowed/secret.txt # Set up environment for testing export MEDIA_ROOT=$(pwd)/test_media # Test legitimate access echo "Testing legitimate access..." SAFE_PATH="allowed/test.pdf" python3 -c " import os from pathlib import Path media_root = os.environ.get('MEDIA_ROOT') safe_path = '$SAFE_PATH' absolute_path = os.path.join(media_root, safe_path) requested_path = os.path.realpath(absolute_path) print(f'Media root: {media_root}') print(f'Safe path: {safe_path}') print(f'Absolute path: {absolute_path}') print(f'Requested path: {requested_path}') print(f'Is within media root: {requested_path.startswith(os.path.realpath(media_root))}') " # Test path traversal attempt echo -e "\nTesting path traversal attempt..." TRAVERSAL_PATH="../not_allowed/secret.txt" python3 -c " import os from pathlib import Path media_root = os.environ.get('MEDIA_ROOT') traversal_path = '$TRAVERSAL_PATH' # Normalize and sanitize like in the code safe_path = os.path.normpath(traversal_path).lstrip('/') absolute_path = os.path.join(media_root, safe_path) requested_path = os.path.realpath(absolute_path) print(f'Media root: {media_root}') print(f'Traversal path: {traversal_path}') print(f'Safe path: {safe_path}') print(f'Absolute path: {absolute_path}') print(f'Requested path: {requested_path}') print(f'Is within media root: {requested_path.startswith(os.path.realpath(media_root))}') " # Clean up rm -rf test_mediaLength of output: 2143
Ensure correct resource management without prematurely closing the file
We’ve verified that the path‐normalization and realpath checks correctly block traversal attempts. However, switching to a
with open(...)context will close the file handle as soon as the block exits—before Django can stream the file—leading to errors at send time.Please revert the context manager change and keep the file open until the response is complete. For example:
- # Use context manager to ensure proper resource cleanup - with open(requested_path, "rb") as file_handle: - return FileResponse(file_handle, content_type=content_type) + # Keep the file open for streaming and close it when the response is closed + file_handle = open(requested_path, "rb") + response = FileResponse(file_handle, content_type=content_type) + response.close = file_handle.close + return response• Path traversal protection: ✅ already verified
• FileResponse resource handling: requires above adjustmentLikely an incorrect or invalid review comment.
🧰 Tools
🪛 Ruff (0.8.2)
7074-7074: Missing return type annotation for public function
serve_work_file(ANN201)
7074-7074: Missing type annotation for function argument
request(ANN001)
7074-7074: Unused function argument:
request(ARG001)
7074-7074: Missing type annotation for function argument
file_path(ANN001)
7098-7098: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7104-7104: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7107-7107: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7114-7114: Use a context manager for opening files
(SIM115)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
web/views.py (4)
6989-7018: 🧹 Nitpick (assertive)Clean implementation of submission listing with pagination.
The view properly handles roles (teachers see all submissions, students see only their own) and implements pagination with proper error handling for edge cases. Nice use of
select_relatedfor optimizing database queries.The
viewing_asvariable could be replaced with a more semantically meaningfulis_reviewerflag in the context:context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": request.user.profile.is_teacher, "work_types": WorkType.objects.all(), }
7063-7073: 🧹 Nitpick (assertive)Add type annotations to delete view function.
The delete functionality is properly implemented with ownership verification, but is missing type annotations.
Add type annotations to improve code readability and static type checking:
@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied # Use default message if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})🧰 Tools
🪛 Ruff (0.8.2)
7064-7064: Missing return type annotation for public function
delete_work_submission(ANN201)
7064-7064: Missing type annotation for function argument
request(ANN001)
7064-7064: Missing type annotation for function argument
submission_id(ANN001)
7068-7068: Avoid specifying long messages outside the exception class
(TRY003)
7021-7060: 🛠️ Refactor suggestionSubmission detail view with proper permission checks.
The view correctly checks user roles and properly restricts access based on permissions. It provides different functionality for students and reviewers.
Add transaction support to ensure database consistency when updating the submission status and creating notifications:
if request.method == "POST" and is_reviewer: feedback = request.POST.get("feedback", "") new_status = request.POST.get("status", "") + with transaction.atomic(): if feedback: submission.feedback = feedback if new_status and new_status in dict(submission.STATUS_CHOICES): submission.status = new_status submission.reviewed_by = request.user + submission.reviewed_at = timezone.now() # Add timestamp for reviews submission.save() # Create notification for student Notification.objects.create( user=submission.student, title="Your Work Submission Has Been Reviewed", message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", notification_type="info", ) messages.success(request, "Feedback and status updated successfully.") return redirect("work_submission_detail", submission_id=submission_id)Also, minor issue with the exception on line 7029:
# Check if the user has access if not (is_student or is_reviewer): - raise PermissionDenied() # Use default message or define in exception class + raise PermissionDenied # Use default message🧰 Tools
🪛 Ruff (0.8.2)
7029-7029: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7125-7139: 🧹 Nitpick (assertive)Secure and add type annotations to the API endpoint.
The API endpoint for work type details needs login protection and a trailing comma in the JSON.
Improve the API endpoint with appropriate formatting:
@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style } ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)Using
_requestinstead ofrequestindicates that the parameter is intentionally unused and removes the static analysis warning.🧰 Tools
🪛 Ruff (0.8.2)
7126-7126: Unused function argument:
request(ARG001)
7136-7136: Trailing comma missing
Add trailing comma
(COM812)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/views.py
7029-7029: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7064-7064: Missing return type annotation for public function delete_work_submission
(ANN201)
7064-7064: Missing type annotation for function argument request
(ANN001)
7064-7064: Missing type annotation for function argument submission_id
(ANN001)
7068-7068: Avoid specifying long messages outside the exception class
(TRY003)
7077-7077: Missing return type annotation for public function serve_work_file
(ANN201)
7077-7077: Missing type annotation for function argument request
(ANN001)
7077-7077: Unused function argument: request
(ARG001)
7077-7077: Missing type annotation for function argument file_path
(ANN001)
7101-7101: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7107-7107: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7110-7110: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7117-7117: Use a context manager for opening files
(SIM115)
7126-7126: Unused function argument: request
(ARG001)
7136-7136: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
web/views.py (4)
7063-7073: 🧹 Nitpick (assertive)Add type annotations and fix PermissionDenied exception message.
This function is missing type annotations and uses a message in the PermissionDenied exception that should be handled differently.
@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied # Use default message if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})🧰 Tools
🪛 Ruff (0.8.2)
7064-7064: Missing return type annotation for public function
delete_work_submission(ANN201)
7064-7064: Missing type annotation for function argument
request(ANN001)
7064-7064: Missing type annotation for function argument
submission_id(ANN001)
7068-7068: Avoid specifying long messages outside the exception class
(TRY003)
7125-7139:⚠️ Potential issueSecure API endpoint and fix code style issues.
This API endpoint is missing authentication and has style issues flagged by static analysis:
+@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style } ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)🧰 Tools
🪛 Ruff (0.8.2)
7126-7126: Unused function argument:
request(ARG001)
7136-7136: Trailing comma missing
Add trailing comma
(COM812)
6989-7017: 🛠️ Refactor suggestionAdd safer profile attribute checking to prevent potential errors.
The code assumes
profileandis_teacherattributes exist without verification, which could lead to attribute errors if a user doesn't have a properly configured profile.- if request.user.profile.is_teacher: + is_teacher = hasattr(request.user, 'profile') and getattr(request.user.profile, 'is_teacher', False) + if is_teacher: submissions_list = WorkSubmission.objects.select_related("student", "work_type").all().order_by("-submitted_at") else: submissions_list = ( WorkSubmission.objects.select_related("work_type").filter(student=request.user).order_by("-submitted_at") )Also, consider updating the template context variable for better clarity:
context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": is_teacher, "work_types": WorkType.objects.all(), }
7040-7060: 🛠️ Refactor suggestionAdd transaction support for submission updates and add reviewed_at timestamp.
When updating the submission and creating a notification, it's important to wrap these operations in a transaction to ensure data consistency.
if request.method == "POST" and is_reviewer: feedback = request.POST.get("feedback", "") new_status = request.POST.get("status", "") + with transaction.atomic(): if feedback: submission.feedback = feedback if new_status and new_status in dict(submission.STATUS_CHOICES): submission.status = new_status submission.reviewed_by = request.user + submission.reviewed_at = timezone.now() # Add timestamp for reviews submission.save() # Create notification for student Notification.objects.create( user=submission.student, title="Your Work Submission Has Been Reviewed", message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", notification_type="info", ) messages.success(request, "Feedback and status updated successfully.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
web/views.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
web/views.py
7029-7029: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7064-7064: Missing return type annotation for public function delete_work_submission
(ANN201)
7064-7064: Missing type annotation for function argument request
(ANN001)
7064-7064: Missing type annotation for function argument submission_id
(ANN001)
7068-7068: Avoid specifying long messages outside the exception class
(TRY003)
7077-7077: Missing return type annotation for public function serve_work_file
(ANN201)
7077-7077: Missing type annotation for function argument request
(ANN001)
7077-7077: Unused function argument: request
(ARG001)
7077-7077: Missing type annotation for function argument file_path
(ANN001)
7101-7101: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7107-7107: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7110-7110: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7117-7117: Use a context manager for opening files
(SIM115)
7126-7126: Unused function argument: request
(ARG001)
7136-7136: Trailing comma missing
Add trailing comma
(COM812)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Run Tests
🔇 Additional comments (2)
web/views.py (2)
6941-6986: Well-structured submission view with transaction support and meaningful notifications.The implementation follows good practices with atomic transaction handling for data consistency, creates appropriate notifications for both students and teachers, and uses
bulk_createfor optimizing teacher notifications.I appreciate the comprehensive error handling with contextual error messages for different scenarios.
7020-7039: 🛠️ Refactor suggestionImplement safer permission checking and use
transaction.atomic()for data consistency.The permission check implementation is good but can be made safer, and a transaction should be used when updating submissions and creating notifications.
# Determine user role is_student = request.user == submission.student - is_reviewer = hasattr(request.user, "profile") and getattr(request.user.profile, "is_teacher", False) + is_reviewer = hasattr(request.user, "profile") and getattr(request.user.profile, "is_teacher", False) # Check if the user has access if not (is_student or is_reviewer): - raise PermissionDenied() # Use default message or define in exception class + raise PermissionDenied # Remove unnecessary parenthesesLikely an incorrect or invalid review comment.
🧰 Tools
🪛 Ruff (0.8.2)
7029-7029: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
|
@A1L13N the previous suggestion that coderabbit is giving is causing security issue and that's why I am not committing that changes. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (19)
web/templates/index.html (1)
321-325: 🧹 Nitpick (assertive)Direct students to the upload form instead of the listing page.
For students, it would be more efficient to link directly to the upload form rather than making them navigate through the listing page first.
- <a href="{% url 'work_submission_list' %}" + <a href="{% url 'upload_work_submission' %}" class="inline-flex items-center px-6 py-2 bg-white text-blue-600 rounded-full font-semibold hover:bg-blue-50 transition-colors duration-200 whitespace-nowrap sm:inline-flex"> <span>Upload Document</span> <i class="fas fa-upload ml-2"></i> </a>web/forms.py (3)
1884-1887: 🧹 Nitpick (assertive)Add trailing commas to improve code style.
Missing trailing commas in the attribute dictionary can lead to inconsistent formatting and make future changes more error-prone.
"class": "bg-white dark:bg-gray-800 text-gray-900 dark:text-white border border-gray-300 " "dark:border-gray-600 rounded-md p-2 focus:border-blue-500 focus:outline-none focus:ring-2 " - "focus:ring-blue-500 w-full" + "focus:ring-blue-500 w-full", }🧰 Tools
🪛 Ruff (0.8.2)
1887-1887: Trailing comma missing
Add trailing comma
(COM812)
1901-1918: 🧹 Nitpick (assertive)Dynamic form field update based on selected work type is well implemented but needs type annotations.
The
__init__method effectively updates the file input'sacceptattribute and help text based on the selected work type, providing a better user experience with clear feedback on allowed file types and sizes.Add type annotations and handle potential errors more gracefully:
- def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) # Set dynamic attributes based on the selected work type if "work_type" in self.data: try: work_type_id = int(self.data.get("work_type")) work_type = WorkType.objects.get(pk=work_type_id) allowed_extensions = work_type.get_allowed_extensions_list() accept_types = ",".join([f".{ext}" for ext in allowed_extensions]) self.fields["work_file"].widget.attrs["accept"] = accept_types self.fields["work_file"].help_text = ( f"Upload your document (max {work_type.max_file_size_mb}MB, " f"allowed types: {', '.join(allowed_extensions)})" ) - except (ValueError, WorkType.DoesNotExist): + except (ValueError, WorkType.DoesNotExist) as e: + logger.warning(f"Failed to set dynamic attributes for work type: {e}") passDon't forget to add the import for
Anyfrom thetypingmodule at the top of the file:from typing import Any🧰 Tools
🪛 Ruff (0.8.2)
1901-1901: Missing return type annotation for special method
__init__Add return type annotation:
None(ANN204)
1901-1901: Missing type annotation for
*args(ANN002)
1901-1901: Missing type annotation for
**kwargs(ANN003)
1919-1940: 🧹 Nitpick (assertive)Improve validation error handling in clean_work_file method.
The validation logic is good, but the error messages should be defined as class constants for better maintainability, and type annotations should be added.
+ # Error messages + FILE_SIZE_ERROR = "File size must not exceed {size}MB." + FILE_TYPE_ERROR = "File type '.{ext}' is not allowed. Allowed types: {allowed_types}" + WORK_TYPE_REQUIRED_ERROR = "Please select a work type" - def clean_work_file(self): + def clean_work_file(self) -> Any: uploaded_file = self.cleaned_data.get("work_file") work_type = self.cleaned_data.get("work_type") if uploaded_file: if work_type: # Check file size max_size = work_type.max_file_size_mb * 1024 * 1024 if uploaded_file.size > max_size: - raise forms.ValidationError(f"File size must not exceed {work_type.max_file_size_mb}MB.") + raise forms.ValidationError(self.FILE_SIZE_ERROR.format(size=work_type.max_file_size_mb)) # Verify file extension ext = uploaded_file.name.split(".")[-1].lower() allowed_extensions = work_type.get_allowed_extensions_list() if ext not in allowed_extensions: raise forms.ValidationError( - f"File type '.{ext}' is not allowed. Allowed types: {', '.join(allowed_extensions)}" + self.FILE_TYPE_ERROR.format(ext=ext, allowed_types=', '.join(allowed_extensions)) ) else: - raise forms.ValidationError("Please select a work type") + raise forms.ValidationError(self.WORK_TYPE_REQUIRED_ERROR) return uploaded_fileDon't forget to add the import for
Anyfrom thetypingmodule if you haven't already.🧰 Tools
🪛 Ruff (0.8.2)
1919-1919: Missing return type annotation for private function
clean_work_file(ANN202)
1928-1928: Avoid specifying long messages outside the exception class
(TRY003)
1934-1936: Avoid specifying long messages outside the exception class
(TRY003)
1935-1935: Trailing comma missing
Add trailing comma
(COM812)
1938-1938: Avoid specifying long messages outside the exception class
(TRY003)
web/urls.py (2)
449-455: 💡 Verification agent🧩 Analysis chain
Direct media serving is correctly restricted to DEBUG mode.
The implementation uses a lambda to conditionally serve files only in development mode, which is a good approach. When
settings.DEBUGis False, it returns a 404 response, preventing access in production.Ensure the
serve_work_fileview function also has internal checks for DEBUG mode for defense in depth:
🏁 Script executed:
#!/bin/bash # Check if serve_work_file has an internal DEBUG check rg -A 5 "def serve_work_file" web/views.py | rg "settings.DEBUG"Length of output: 64
🏁 Script executed:
#!/bin/bash # Display the implementation of serve_work_file for review rg -n "def serve_work_file" web/views.py -A 20Length of output: 934
Enforce DEBUG check inside
serve_work_fileWhile the URL route is restricted to
DEBUGmode, the view itself currently lacks an internal guard. Add a defensive check at the top ofserve_work_fileto prevent accidental exposure in production.• File:
web/views.py
• Function:serve_work_file(request, file_path)(around line 7097)Suggested change:
def serve_work_file(request, file_path): + from django.conf import settings + from django.http import Http404 + + if not settings.DEBUG: + # Prevent serving media in production + raise Http404("Not found") """ Securely serve uploaded work files for development only.
445-458: 🧹 Nitpick (assertive)Work review URL patterns are well-structured, secure, and mostly complete.
The URL patterns for the work review system follow Django's conventions with consistent naming. The direct media serving is properly restricted to development mode using a lambda with
settings.DEBUGcheck, which is a good security practice. The delete functionality requested in PR comments has been implemented correctly.For a complete CRUD implementation, consider adding an update endpoint:
path("work-submissions/<int:submission_id>/", views.work_submission_detail, name="work_submission_detail"), +path("work-submissions/<int:submission_id>/update/", views.update_work_submission, name="update_work_submission"), path("work-submissions/<int:submission_id>/delete/", views.delete_work_submission, name="delete_work_submission"),web/views.py (6)
7010-7037: 🛠️ Refactor suggestionReplace deprecated approach to checking user role and update context variable.
The implementation is functional, but there are two issues to address for improved robustness and clarity.
- Replace direct profile attribute access with safer attribute checking to prevent potential AttributeError:
- viewing_as = "student" - if request.user.profile.is_teacher: - viewing_as = "reviewer" + is_teacher = hasattr(request.user, 'profile') and getattr(request.user.profile, 'is_teacher', False) + if is_teacher: submissions_list = WorkSubmission.objects.select_related("student", "work_type").all().order_by("-submitted_at")
- Update the context variable for better semantic meaning:
context = { "submissions": submissions, - "viewing_as": viewing_as, + "is_reviewer": is_teacher, "work_types": WorkType.objects.all(), }This change improves code robustness by safely handling the case where a user might not have a profile, and makes the template context more intuitive.
7084-7093: 🧹 Nitpick (assertive)Add type annotations and fix PermissionDenied exception usage.
The deletion view misses type annotations and uses unnecessary parentheses and a long message with the PermissionDenied exception.
Add type annotations and fix the exception style:
@login_required -def delete_work_submission(request, submission_id): +def delete_work_submission(request: HttpRequest, submission_id: int) -> HttpResponse: """Allow the owner to delete their work submission.""" submission = get_object_or_404(WorkSubmission, pk=submission_id) if request.user != submission.student: - raise PermissionDenied("You do not have permission to delete this submission.") + raise PermissionDenied # Use default message if request.method == "POST": submission.delete() messages.success(request, "Your submission has been deleted.") return redirect("work_submission_list") return render(request, "web/work/confirm_delete.html", {"submission": submission})This improves code readability with type annotations and follows Django's recommended style for raising exceptions.
🧰 Tools
🪛 Ruff (0.8.2)
7084-7084: Missing return type annotation for public function
delete_work_submission(ANN201)
7084-7084: Missing type annotation for function argument
request(ANN001)
7084-7084: Missing type annotation for function argument
submission_id(ANN001)
7088-7088: Avoid specifying long messages outside the exception class
(TRY003)
7146-7159:⚠️ Potential issueAdd login requirement and fix static analysis issues.
The work type API endpoint is missing authentication and has style issues.
Add login protection and address style issues:
+@login_required -def work_type_detail_api(request: HttpRequest, work_type_id: int) -> JsonResponse: +def work_type_detail_api(_request: HttpRequest, work_type_id: int) -> JsonResponse: """API to get work type details for dynamic form updates.""" try: work_type = WorkType.objects.get(pk=work_type_id) return JsonResponse( { "id": work_type.id, "name": work_type.name, "allowed_file_types": work_type.allowed_file_types, "max_file_size_mb": work_type.max_file_size_mb, + # Add trailing comma for better code style + }, ) except WorkType.DoesNotExist: return JsonResponse({"error": "Work type not found"}, status=404)This adds authentication protection to prevent unauthorized access to work type details and fixes the identified style issues, making the code more consistent.
🧰 Tools
🪛 Ruff (0.8.2)
7146-7146: Unused function argument:
request(ARG001)
7156-7156: Trailing comma missing
Add trailing comma
(COM812)
6961-7006: 🧹 Nitpick (assertive)Well-structured view with good transaction handling and error feedback.
The
upload_work_submissionfunction follows best practices by using transaction.atomic() to ensure data consistency, provides good user feedback, and correctly notifies all relevant parties. The error handling for form validation is comprehensive.Consider enhancing the form validation message with field-specific error details:
if form.errors: if hasattr(form, "non_field_errors") and form.non_field_errors(): messages.error(request, form.non_field_errors()[0]) else: - # Include the specific field errors in the message - error_msg = "Please fix the following errors: " - error_fields = ", ".join(form.errors.keys()) - messages.error(request, f"{error_msg}{error_fields}") + # Include the specific field errors with their messages in a more user-friendly format + error_details = "; ".join([f"{field}: {', '.join(errors)}" for field, errors in form.errors.items()]) + messages.error(request, f"Please fix the following errors: {error_details}")
7040-7073: 🛠️ Refactor suggestionAdd transaction support for submission reviews and status updates.
The submission detail view handles permission checks correctly, but lacks transaction support when updating a submission and creating notifications, which could lead to data inconsistency if one operation fails.
Wrap the updates in a transaction to ensure atomicity:
if request.method == "POST" and is_reviewer: feedback = request.POST.get("feedback", "") new_status = request.POST.get("status", "") + with transaction.atomic(): if feedback: submission.feedback = feedback if new_status and new_status in dict(submission.STATUS_CHOICES): submission.status = new_status submission.reviewed_by = request.user + submission.reviewed_at = timezone.now() # Add timestamp for reviews submission.save() # Create notification for student Notification.objects.create( user=submission.student, title="Your Work Submission Has Been Reviewed", message=f"Your {submission.work_type.name} has been reviewed. Check the feedback for details.", notification_type="info", ) messages.success(request, "Feedback and status updated successfully.")This change ensures that if any part of the update process fails, the entire operation is rolled back, maintaining data consistency. It also adds a timestamp to record when the review was performed.
🧰 Tools
🪛 Ruff (0.8.2)
7049-7049: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7097-7141:⚠️ Potential issueFix file serving security issues and improve resource management.
The file serving view has good security checks for path traversal, but has several issues that should be addressed for better security and resource handling.
Improve the file serving code with these changes:
@login_required -def serve_work_file(request, file_path): +def serve_work_file(_request: HttpRequest, file_path: str) -> HttpResponse: """ Securely serve uploaded work files for development only. DO NOT use this in production—let your web server handle media files. """ import mimetypes import os from django.conf import settings from django.http import FileResponse, Http404 # Normalize and sanitize the file path, removing any path traversal attempts safe_path = os.path.normpath(file_path).lstrip("/") # Construct the absolute file path within MEDIA_ROOT only absolute_path = os.path.join(settings.MEDIA_ROOT, safe_path) # Use realpath to resolve symlinks, etc. requested_path = os.path.realpath(absolute_path) media_root = os.path.realpath(settings.MEDIA_ROOT) # Ensure the requested file is within MEDIA_ROOT (prevents path traversal) - if not requested_path.startswith(media_root): - raise Http404() + if not os.path.commonpath([requested_path, media_root]) == media_root: + raise Http404 # Remove unnecessary parentheses # Restrict allowed file extensions allowed_extensions = {"pdf", "doc", "docx", "odt", "txt", "jpg", "jpeg", "png", "gif", "bmp"} ext = os.path.splitext(requested_path)[1][1:].lower() if ext not in allowed_extensions: - raise Http404() + raise Http404 # Remove unnecessary parentheses if not os.path.exists(requested_path) or not os.path.isfile(requested_path): - raise Http404() + raise Http404 # Remove unnecessary parentheses content_type, _ = mimetypes.guess_type(requested_path) if not content_type: content_type = "application/octet-stream" - # Use with statement to ensure file is properly closed - file_handle = open(requested_path, "rb") - response = FileResponse(file_handle, content_type=content_type) - # Ensure file is closed when response is done - response.close = lambda: file_handle.close() + # Use context manager to ensure file is properly closed + with open(requested_path, "rb") as file_handle: + response = FileResponse(file_handle, content_type=content_type) return responseThese changes:
- Mark the unused request parameter with an underscore
- Add type annotations for better code clarity
- Use
os.path.commonpathfor more reliable path comparison- Remove unnecessary parentheses from exceptions
- Use a context manager to properly handle file resources
🧰 Tools
🪛 Ruff (0.8.2)
7097-7097: Missing return type annotation for public function
serve_work_file(ANN201)
7097-7097: Missing type annotation for function argument
request(ANN001)
7097-7097: Unused function argument:
request(ARG001)
7097-7097: Missing type annotation for function argument
file_path(ANN001)
7121-7121: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7127-7127: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7130-7130: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7137-7137: Use a context manager for opening files
(SIM115)
web/models.py (7)
2768-2775: 🧹 Nitpick (assertive)Add return type annotation to file_size_display property.
@property - def file_size_display(self): + def file_size_display(self) -> str: """Return file size in MB with two decimal places.""" if self.file_size: return f"{self.file_size / (1024 * 1024):.2f} MB" return "Unknown"🧰 Tools
🪛 Ruff (0.8.2)
2770-2770: Missing return type annotation for public function
file_size_displayAdd return type annotation:
str(ANN201)
2793-2795: 🧹 Nitpick (assertive)Add return type annotation to get_status_choices method.
- def get_status_choices(self): + def get_status_choices(self) -> list[tuple[str, str]]: return self.STATUS_CHOICES🧰 Tools
🪛 Ruff (0.8.2)
2793-2793: Missing return type annotation for public function
get_status_choices(ANN201)
2776-2781: 🧹 Nitpick (assertive)Add return type annotation to file_extension property.
@property - def file_extension(self): + def file_extension(self) -> str: """Return the file extension of the work file.""" if self.work_file: return self.work_file.name.split(".")[-1].lower() return ""🧰 Tools
🪛 Ruff (0.8.2)
2777-2777: Missing return type annotation for public function
file_extension(ANN201)
2698-2708: 🧹 Nitpick (assertive)Annotate STATUS_CHOICES with ClassVar.
Use typing.ClassVar for class-level attributes like STATUS_CHOICES to improve type safety:
+from typing import ClassVar, Any class WorkSubmission(models.Model): """Model for storing work submissions for review.""" - STATUS_CHOICES = [ + STATUS_CHOICES: ClassVar[list[tuple[str, str]]] = [ ("submitted", "Submitted"), ("under_review", "Under Review"), ("reviewed", "Reviewed"), ("needs_revision", "Needs Revision"), ("approved", "Approved"), ("rejected", "Rejected"), ]🧰 Tools
🪛 Ruff (0.8.2)
2701-2708: Mutable class attributes should be annotated with
typing.ClassVar(RUF012)
2691-2696: 🧹 Nitpick (assertive)Add type annotations to the file size validation function.
The function correctly validates file size against a configurable threshold but would benefit from type annotations:
-def validate_file_size(file, max_size_mb=10): +def validate_file_size(file: models.FileField, max_size_mb: int = 10) -> None: """Validate that the file doesn't exceed the maximum size.""" max_size = max_size_mb * 1024 * 1024 # Convert MB to bytes if file.size > max_size: raise ValidationError(f"File size exceeds {max_size_mb}MB limit.")🧰 Tools
🪛 Ruff (0.8.2)
2691-2691: Missing return type annotation for public function
validate_file_sizeAdd return type annotation:
None(ANN201)
2691-2691: Missing type annotation for function argument
file(ANN001)
2691-2691: Missing type annotation for function argument
max_size_mb(ANN001)
2695-2695: Avoid specifying long messages outside the exception class
(TRY003)
2782-2792: 🧹 Nitpick (assertive)Enhance file type detection with additional formats.
The file type detection methods could be more comprehensive by including additional common file formats:
@property def is_image(self) -> bool: """Check if the file is an image.""" - return self.file_extension in ["jpg", "jpeg", "png", "gif", "bmp"] + return self.file_extension in ["jpg", "jpeg", "png", "gif", "bmp", "svg", "webp", "tiff"] @property def is_document(self) -> bool: """Check if the file is a document.""" - return self.file_extension in ["pdf", "doc", "docx", "odt", "txt", "rtf", "md"] + return self.file_extension in ["pdf", "doc", "docx", "odt", "txt", "rtf", "md", "tex", "ppt", "pptx", "xls", "xlsx"]
2730-2731: 🧹 Nitpick (assertive)Add trailing comma for consistent formatting.
reviewed_by = models.ForeignKey( User, on_delete=models.SET_NULL, null=True, blank=True, related_name="reviewed_submissions" + , )🧰 Tools
🪛 Ruff (0.8.2)
2730-2730: Trailing comma missing
Add trailing comma
(COM812)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
web/forms.py(2 hunks)web/models.py(1 hunks)web/templates/index.html(1 hunks)web/urls.py(2 hunks)web/views.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/urls.py (1)
web/views.py (6)
work_submission_list(7010-7037)upload_work_submission(6961-7006)work_submission_detail(7041-7080)serve_work_file(7097-7141)delete_work_submission(7084-7093)work_type_detail_api(7146-7159)
web/forms.py (1)
web/models.py (19)
WorkSubmission(2698-2794)WorkType(2665-2688)Meta(45-46)Meta(206-207)Meta(398-399)Meta(544-545)Meta(600-601)Meta(622-623)Meta(676-679)Meta(730-731)Meta(783-785)Meta(834-835)Meta(874-876)Meta(898-899)Meta(917-918)Meta(972-973)Meta(1039-1040)Meta(1070-1071)get_allowed_extensions_list(2686-2688)
🪛 Ruff (0.8.2)
web/forms.py
1887-1887: Trailing comma missing
Add trailing comma
(COM812)
1899-1899: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
1901-1901: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
1901-1901: Missing type annotation for *args
(ANN002)
1901-1901: Missing type annotation for **kwargs
(ANN003)
1919-1919: Missing return type annotation for private function clean_work_file
(ANN202)
1928-1928: Avoid specifying long messages outside the exception class
(TRY003)
1934-1936: Avoid specifying long messages outside the exception class
(TRY003)
1935-1935: Trailing comma missing
Add trailing comma
(COM812)
1938-1938: Avoid specifying long messages outside the exception class
(TRY003)
web/views.py
7049-7049: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7084-7084: Missing return type annotation for public function delete_work_submission
(ANN201)
7084-7084: Missing type annotation for function argument request
(ANN001)
7084-7084: Missing type annotation for function argument submission_id
(ANN001)
7088-7088: Avoid specifying long messages outside the exception class
(TRY003)
7097-7097: Missing return type annotation for public function serve_work_file
(ANN201)
7097-7097: Missing type annotation for function argument request
(ANN001)
7097-7097: Unused function argument: request
(ARG001)
7097-7097: Missing type annotation for function argument file_path
(ANN001)
7121-7121: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7127-7127: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7130-7130: Unnecessary parentheses on raised exception
Remove unnecessary parentheses
(RSE102)
7137-7137: Use a context manager for opening files
(SIM115)
7146-7146: Unused function argument: request
(ARG001)
7156-7156: Trailing comma missing
Add trailing comma
(COM812)
web/models.py
2691-2691: Missing return type annotation for public function validate_file_size
Add return type annotation: None
(ANN201)
2691-2691: Missing type annotation for function argument file
(ANN001)
2691-2691: Missing type annotation for function argument max_size_mb
(ANN001)
2695-2695: Avoid specifying long messages outside the exception class
(TRY003)
2701-2708: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2730-2730: Trailing comma missing
Add trailing comma
(COM812)
2734-2734: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2741-2741: Missing type annotation for *args
(ANN002)
2741-2741: Missing type annotation for **kwargs
(ANN003)
2751-2753: Avoid specifying long messages outside the exception class
(TRY003)
2752-2752: Trailing comma missing
Add trailing comma
(COM812)
2770-2770: Missing return type annotation for public function file_size_display
Add return type annotation: str
(ANN201)
2777-2777: Missing return type annotation for public function file_extension
(ANN201)
2793-2793: Missing return type annotation for public function get_status_choices
(ANN201)
🔇 Additional comments (3)
web/urls.py (1)
6-6: Import addition supports the conditional file serving implementation.The HttpResponseNotFound import is correctly added to support the conditional file serving implementation that follows Django's best practices for development-only functionality.
web/models.py (2)
2738-2741: Add return type annotation to__str__method.Add proper return type annotation for consistency and type safety:
- def __str__(self) -> str: + def __str__(self) -> str: return f"{self.student.username}'s {self.work_type.name}: {self.title}"🧰 Tools
🪛 Ruff (0.8.2)
2741-2741: Missing type annotation for
*args(ANN002)
2741-2741: Missing type annotation for
**kwargs(ANN003)
2665-2689: 🧹 Nitpick (assertive)Well-designed WorkType model with clear purpose.
The WorkType model provides a flexible way to categorize work submissions with configurable file type restrictions. Consider adding type annotations to the
get_allowed_extensions_listmethod:- def get_allowed_extensions_list(self): + def get_allowed_extensions_list(self) -> list[str]: """Return list of allowed file extensions.""" return [ext.strip() for ext in self.allowed_file_types.split(",")]Likely an incorrect or invalid review comment.
| <!-- PDF Submission/Review Announcement --> | ||
| <div class="bg-gradient-to-r from-blue-500 to-purple-600 rounded-lg p-6 text-white shadow-lg my-8 relative"> | ||
| <!-- GSOC Proposal CTA --> | ||
| <div class="absolute top-0 right-0 m-2 bg-yellow-400 text-blue-900 px-3 py-1 rounded-full text-xs font-bold uppercase tracking-wider transform rotate-6 shadow-lg"> | ||
| GSOC Proposals Open! | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
The announcement title and badge need to be updated to reflect the generalized work submission feature.
The section is titled "PDF Submission/Review Announcement" but the feature has been generalized to support multiple file types beyond PDFs. Additionally, the hardcoded "GSOC Proposals Open!" badge might become outdated.
Consider updating the terminology and making the badge content dynamic:
-<!-- PDF Submission/Review Announcement -->
+<!-- Work Submission/Review Announcement -->
...
- <div class="absolute top-0 right-0 m-2 bg-yellow-400 text-blue-900 px-3 py-1 rounded-full text-xs font-bold uppercase tracking-wider transform rotate-6 shadow-lg">
- GSOC Proposals Open!
+ <div class="absolute top-0 right-0 m-2 bg-yellow-400 text-blue-900 px-3 py-1 rounded-full text-xs font-bold uppercase tracking-wider transform rotate-6 shadow-lg">
+ Work Submissions Open!
</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.
| <!-- PDF Submission/Review Announcement --> | |
| <div class="bg-gradient-to-r from-blue-500 to-purple-600 rounded-lg p-6 text-white shadow-lg my-8 relative"> | |
| <!-- GSOC Proposal CTA --> | |
| <div class="absolute top-0 right-0 m-2 bg-yellow-400 text-blue-900 px-3 py-1 rounded-full text-xs font-bold uppercase tracking-wider transform rotate-6 shadow-lg"> | |
| GSOC Proposals Open! | |
| </div> | |
| <!-- Work Submission/Review Announcement --> | |
| <div class="bg-gradient-to-r from-blue-500 to-purple-600 rounded-lg p-6 text-white shadow-lg my-8 relative"> | |
| <!-- GSOC Proposal CTA --> | |
| <div class="absolute top-0 right-0 m-2 bg-yellow-400 text-blue-900 px-3 py-1 rounded-full text-xs font-bold uppercase tracking-wider transform rotate-6 shadow-lg"> | |
| Work Submissions Open! | |
| </div> |
| def clean_work_file(self): | ||
| uploaded_file = self.cleaned_data.get("work_file") | ||
| work_type = self.cleaned_data.get("work_type") | ||
|
|
||
| if uploaded_file: | ||
| if work_type: | ||
| # Check file size | ||
| max_size = work_type.max_file_size_mb * 1024 * 1024 | ||
| if uploaded_file.size > max_size: | ||
| raise forms.ValidationError(f"File size must not exceed {work_type.max_file_size_mb}MB.") | ||
|
|
||
| # Verify file extension | ||
| ext = uploaded_file.name.split(".")[-1].lower() | ||
| allowed_extensions = work_type.get_allowed_extensions_list() | ||
| if ext not in allowed_extensions: | ||
| raise forms.ValidationError( | ||
| f"File type '.{ext}' is not allowed. Allowed types: {', '.join(allowed_extensions)}" | ||
| ) | ||
| else: | ||
| raise forms.ValidationError("Please select a work type") | ||
|
|
||
| return uploaded_file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Note: Redundant validation in model and form.
The form's validation in clean_work_file duplicates some of the validation logic already present in the WorkSubmission.save() method, which also validates file extensions and size.
Consider whether this duplicate validation is intentional for better user experience (early validation in the form) or if it could be consolidated. If kept in both places, ensure they stay in sync when requirements change.
🏁 Script executed:
#!/bin/bash
# Check if the model also implements similar validation
# Look for file extension validation in the model
echo "Looking for file extension validation in WorkSubmission model..."
grep -A 5 "ext not in allowed_extensions" $(find . -name "models.py")
# Look for file size validation in the model
echo "Looking for file size validation in WorkSubmission model..."
grep -A 2 "validate_file_size" $(find . -name "models.py")Length of output: 1022
Unify File Validation Logic to Prevent Duplication
There's duplication between the form-level checks in web/forms.py:clean_work_file and the model‐level validators in web/models.py (validate_file_size and the extension check in WorkSubmission.save). While form‐level validation gives faster user feedback, maintaining two copies of the same logic risks them getting out of sync.
Recommendations:
- Extract shared validation into a single utility or reuse the model validators in the form:
- Call
validate_file_size(uploaded_file, work_type.max_file_size_mb)instead of re‑implementing size checks. - Move extension checks into a helper (e.g.
validate_file_extension(name, allowed_extensions)) and invoke it from both form and model.
- Call
- If you prefer to keep separate layers, add comments in each location pointing to the "source of truth" to ensure updates remain consistent.
Locations to update:
- web/forms.py (around
clean_work_file, lines ~1919–1940) - web/models.py (the
validate_file_sizefunction and the inline extension check inWorkSubmission.save)
🧰 Tools
🪛 Ruff (0.8.2)
1919-1919: Missing return type annotation for private function clean_work_file
(ANN202)
1928-1928: Avoid specifying long messages outside the exception class
(TRY003)
1934-1936: Avoid specifying long messages outside the exception class
(TRY003)
1935-1935: Trailing comma missing
Add trailing comma
(COM812)
1938-1938: Avoid specifying long messages outside the exception class
(TRY003)
| class WorkType(models.Model): | ||
| """Model for categorizing different types of work submissions.""" | ||
|
|
||
| name = models.CharField(max_length=100) | ||
| description = models.TextField(blank=True) | ||
| icon_class = models.CharField(max_length=50, blank=True, help_text="Font Awesome icon class") | ||
| # New fields for file type support | ||
| allowed_file_types = models.CharField( | ||
| max_length=200, | ||
| default="pdf", | ||
| help_text="Comma-separated list of allowed file extensions (e.g., 'pdf,doc,docx')", | ||
| ) | ||
| max_file_size_mb = models.PositiveIntegerField(default=10, help_text="Maximum file size in MB") | ||
|
|
||
| class Meta: | ||
| verbose_name = "Work Type" | ||
| verbose_name_plural = "Work Types" | ||
|
|
||
| def __str__(self) -> str: | ||
| return self.name | ||
|
|
||
| def get_allowed_extensions_list(self) -> list[str]: | ||
| """Return list of allowed file extensions.""" | ||
| return [ext.strip() for ext in self.allowed_file_types.split(",")] | ||
|
|
||
|
|
||
| def validate_file_size(file, max_size_mb=10): | ||
| """Validate that the file doesn't exceed the maximum size.""" | ||
| max_size = max_size_mb * 1024 * 1024 # Convert MB to bytes | ||
| if file.size > max_size: | ||
| raise ValidationError(f"File size exceeds {max_size_mb}MB limit.") | ||
|
|
||
|
|
||
| class WorkSubmission(models.Model): | ||
| """Model for storing work submissions for review.""" | ||
|
|
||
| STATUS_CHOICES = [ | ||
| ("submitted", "Submitted"), | ||
| ("under_review", "Under Review"), | ||
| ("reviewed", "Reviewed"), | ||
| ("needs_revision", "Needs Revision"), | ||
| ("approved", "Approved"), | ||
| ("rejected", "Rejected"), | ||
| ] | ||
|
|
||
| title = models.CharField(max_length=200, help_text="Title of your document") | ||
| student = models.ForeignKey(User, on_delete=models.CASCADE, related_name="work_submissions") | ||
| work_type = models.ForeignKey(WorkType, on_delete=models.PROTECT, related_name="submissions") | ||
| work_file = models.FileField( | ||
| upload_to="work_submissions/", | ||
| help_text="Upload your document (file type and size restrictions apply)", | ||
| ) | ||
| subject = models.CharField(max_length=200, help_text="Subject or department") | ||
| assignment = models.CharField(max_length=200, help_text="Assignment or topic") | ||
| due_date = models.DateField(null=True, blank=True, help_text="Due date (if applicable)") | ||
| description = models.TextField(help_text="Brief description of your submission", blank=True) | ||
| status = models.CharField(max_length=20, choices=STATUS_CHOICES, default="submitted") | ||
| feedback = models.TextField(blank=True, help_text="Feedback from reviewers") | ||
|
|
||
| # Tracking fields | ||
| file_size = models.PositiveIntegerField(editable=False, default=0) | ||
| submitted_at = models.DateTimeField(auto_now_add=True) | ||
| updated_at = models.DateTimeField(auto_now=True) | ||
| reviewed_at = models.DateTimeField(null=True, blank=True) | ||
| reviewed_by = models.ForeignKey( | ||
| User, on_delete=models.SET_NULL, null=True, blank=True, related_name="reviewed_submissions" | ||
| ) | ||
|
|
||
| class Meta: | ||
| ordering = ["-submitted_at"] | ||
| verbose_name = "Work Submission" | ||
| verbose_name_plural = "Work Submissions" | ||
|
|
||
| def __str__(self) -> str: | ||
| return f"{self.student.username}'s {self.work_type.name}: {self.title}" | ||
|
|
||
| def save(self, *args, **kwargs) -> None: | ||
| # Set file size on save | ||
| if self.work_file: | ||
| self.file_size = self.work_file.size | ||
|
|
||
| # Validate file extension | ||
| if self.work_type: | ||
| ext = self.work_file.name.split(".")[-1].lower() | ||
| allowed_extensions = self.work_type.get_allowed_extensions_list() | ||
| if ext not in allowed_extensions: | ||
| raise ValidationError( | ||
| f"File type '.{ext}' is not allowed. Allowed types: {', '.join(allowed_extensions)}" | ||
| ) | ||
|
|
||
| # Validate file size | ||
| validate_file_size(self.work_file, self.work_type.max_file_size_mb) | ||
|
|
||
| # Set reviewed_at timestamp when status changes to "reviewed" | ||
| if self.pk: | ||
| try: | ||
| old_instance = WorkSubmission.objects.get(pk=self.pk) | ||
| if old_instance.status != "reviewed" and self.status == "reviewed": | ||
| self.reviewed_at = timezone.now() | ||
| except WorkSubmission.DoesNotExist: | ||
| pass | ||
|
|
||
| super().save(*args, **kwargs) | ||
|
|
||
| @property | ||
| def file_size_display(self): | ||
| """Return file size in MB with two decimal places.""" | ||
| if self.file_size: | ||
| return f"{self.file_size / (1024 * 1024):.2f} MB" | ||
| return "Unknown" | ||
|
|
||
| @property | ||
| def file_extension(self): | ||
| """Return the file extension of the work file.""" | ||
| if self.work_file: | ||
| return self.work_file.name.split(".")[-1].lower() | ||
| return "" | ||
|
|
||
| @property | ||
| def is_image(self) -> bool: | ||
| """Check if the file is an image.""" | ||
| return self.file_extension in ["jpg", "jpeg", "png", "gif", "bmp"] | ||
|
|
||
| @property | ||
| def is_document(self) -> bool: | ||
| """Check if the file is a document.""" | ||
| return self.file_extension in ["pdf", "doc", "docx", "odt", "txt", "rtf", "md"] | ||
|
|
||
| def get_status_choices(self): | ||
| return self.STATUS_CHOICES |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding unit tests for these new models.
The models look well-implemented, but there appear to be no tests to verify their functionality, especially around file validation and status transitions.
Would you like me to generate sample unit tests for:
- File size validation
- File extension validation
- Status transitions and timestamp updates
- File size display formatting
🧰 Tools
🪛 Ruff (0.8.2)
2691-2691: Missing return type annotation for public function validate_file_size
Add return type annotation: None
(ANN201)
2691-2691: Missing type annotation for function argument file
(ANN001)
2691-2691: Missing type annotation for function argument max_size_mb
(ANN001)
2695-2695: Avoid specifying long messages outside the exception class
(TRY003)
2701-2708: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2730-2730: Trailing comma missing
Add trailing comma
(COM812)
2734-2734: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
2741-2741: Missing type annotation for *args
(ANN002)
2741-2741: Missing type annotation for **kwargs
(ANN003)
2751-2753: Avoid specifying long messages outside the exception class
(TRY003)
2752-2752: Trailing comma missing
Add trailing comma
(COM812)
2770-2770: Missing return type annotation for public function file_size_display
Add return type annotation: str
(ANN201)
2777-2777: Missing return type annotation for public function file_extension
(ANN201)
2793-2793: Missing return type annotation for public function get_status_choices
(ANN201)
| def save(self, *args, **kwargs) -> None: | ||
| # Set file size on save | ||
| if self.work_file: | ||
| self.file_size = self.work_file.size | ||
|
|
||
| # Validate file extension | ||
| if self.work_type: | ||
| ext = self.work_file.name.split(".")[-1].lower() | ||
| allowed_extensions = self.work_type.get_allowed_extensions_list() | ||
| if ext not in allowed_extensions: | ||
| raise ValidationError( | ||
| f"File type '.{ext}' is not allowed. Allowed types: {', '.join(allowed_extensions)}" | ||
| ) | ||
|
|
||
| # Validate file size | ||
| validate_file_size(self.work_file, self.work_type.max_file_size_mb) | ||
|
|
||
| # Set reviewed_at timestamp when status changes to "reviewed" | ||
| if self.pk: | ||
| try: | ||
| old_instance = WorkSubmission.objects.get(pk=self.pk) | ||
| if old_instance.status != "reviewed" and self.status == "reviewed": | ||
| self.reviewed_at = timezone.now() | ||
| except WorkSubmission.DoesNotExist: | ||
| pass | ||
|
|
||
| super().save(*args, **kwargs) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve the save method with better validation flow.
Consider moving validation logic to the clean() method and adding proper type annotations:
- def save(self, *args, **kwargs) -> None:
+ def save(self, *args: Any, **kwargs: Any) -> None:
# Set file size on save
if self.work_file:
self.file_size = self.work_file.size
# Validate file extension
if self.work_type:
ext = self.work_file.name.split(".")[-1].lower()
allowed_extensions = self.work_type.get_allowed_extensions_list()
if ext not in allowed_extensions:
raise ValidationError(
f"File type '.{ext}' is not allowed. Allowed types: {', '.join(allowed_extensions)}"
)
# Validate file size
validate_file_size(self.work_file, self.work_type.max_file_size_mb)Consider expanding the status tracking to include all final statuses:
# Set reviewed_at timestamp when status changes to "reviewed"
if self.pk:
try:
old_instance = WorkSubmission.objects.get(pk=self.pk)
- if old_instance.status != "reviewed" and self.status == "reviewed":
+ if old_instance.status not in ["reviewed", "approved", "rejected"] and self.status in ["reviewed", "approved", "rejected"]:
self.reviewed_at = timezone.now()
except WorkSubmission.DoesNotExist:
passCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.8.2)
2741-2741: Missing type annotation for *args
(ANN002)
2741-2741: Missing type annotation for **kwargs
(ANN003)
2751-2753: Avoid specifying long messages outside the exception class
(TRY003)
2752-2752: Trailing comma missing
Add trailing comma
(COM812)
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive work submission and review system that allows students to upload documents and teachers to provide feedback. The system includes configurable work types with file type restrictions, role-based access controls, and notification workflows.
- Adds new models
WorkTypeandWorkSubmissionwith comprehensive validation and metadata tracking - Implements views for uploading, listing, viewing, and managing work submissions with role-based permissions
- Creates responsive HTML templates with dark mode support and file preview capabilities
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/views.py | Core view logic for work submission workflows including upload, list, detail, delete, and file serving |
| web/urls.py | URL routing for work submission endpoints and secure file serving |
| web/models.py | Data models for WorkType and WorkSubmission with validation and file handling |
| web/forms.py | Django form for work submission with dynamic validation based on work type |
| web/admin.py | Admin interface configuration for managing work types and submissions |
| web/migrations/0059_worktype_worksubmission.py | Database migration creating the new models |
| web/templates/web/work/upload_submission.html | Upload form template with responsive design |
| web/templates/web/work/submission_list.html | Listing template with pagination and role-based views |
| web/templates/web/work/submission_detail.html | Detail view with file preview and feedback forms |
| web/templates/index.html | Homepage announcements for the new feature |
| web/templates/base.html | Navigation menu additions for work submission access |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def upload_work_submission(request: HttpRequest) -> HttpResponse: | ||
| """View for students to upload their work documents.""" | ||
| if request.method == "POST": |
There was a problem hiding this comment.
The function lacks a @login_required decorator. Based on the pattern used in other views in this file and the fact that it accesses request.user, this view should require authentication.
| def upload_work_submission(request: HttpRequest) -> HttpResponse: | |
| """View for students to upload their work documents.""" | |
| if request.method == "POST": | |
| @login_required | |
| def upload_work_submission(request: HttpRequest) -> HttpResponse: | |
| """View for students to upload their work documents.""" |
| # Use with statement to ensure file is properly closed | ||
| file_handle = open(requested_path, "rb") | ||
| response = FileResponse(file_handle, content_type=content_type) | ||
| # Ensure file is closed when response is done | ||
| response.close = lambda: file_handle.close() |
There was a problem hiding this comment.
File handle should be opened using a context manager (with statement) to ensure proper resource cleanup. The current approach of manually assigning a close function is not standard and may not guarantee file closure in all scenarios.
| # Use with statement to ensure file is properly closed | |
| file_handle = open(requested_path, "rb") | |
| response = FileResponse(file_handle, content_type=content_type) | |
| # Ensure file is closed when response is done | |
| response.close = lambda: file_handle.close() | |
| # FileResponse will handle closing the file when the response is closed | |
| file_handle = open(requested_path, "rb") | |
| response = FileResponse(file_handle, content_type=content_type) |
| lambda request, file_path: ( | ||
| views.serve_work_file(request, file_path) if settings.DEBUG else HttpResponseNotFound() | ||
| ), |
There was a problem hiding this comment.
Using a lambda function in URL patterns makes debugging more difficult and reduces code readability. Consider creating a proper view function that handles the DEBUG check internally.
| def validate_file_size(file, max_size_mb=10): | ||
| """Validate that the file doesn't exceed the maximum size.""" | ||
| max_size = max_size_mb * 1024 * 1024 # Convert MB to bytes | ||
| if file.size > max_size: | ||
| raise ValidationError(f"File size exceeds {max_size_mb}MB limit.") | ||
|
|
||
|
|
||
| class WorkSubmission(models.Model): | ||
| """Model for storing work submissions for review.""" | ||
|
|
There was a problem hiding this comment.
[nitpick] This standalone validation function is only used within the WorkSubmission model. Consider moving it as a method within the WorkSubmission class or making it a static method to improve code organization and maintainability.
| def validate_file_size(file, max_size_mb=10): | |
| """Validate that the file doesn't exceed the maximum size.""" | |
| max_size = max_size_mb * 1024 * 1024 # Convert MB to bytes | |
| if file.size > max_size: | |
| raise ValidationError(f"File size exceeds {max_size_mb}MB limit.") | |
| class WorkSubmission(models.Model): | |
| """Model for storing work submissions for review.""" | |
| class WorkSubmission(models.Model): | |
| """Model for storing work submissions for review.""" | |
| @staticmethod | |
| def validate_file_size(file, max_size_mb=10): | |
| """Validate that the file doesn't exceed the maximum size.""" | |
| max_size = max_size_mb * 1024 * 1024 # Convert MB to bytes | |
| if file.size > max_size: | |
| raise ValidationError(f"File size exceeds {max_size_mb}MB limit.") |
Added Work review feature for students :-
Screencast.from.2025-04-15.15-31-56.webm
closes #229
Summary by CodeRabbit
New Features
Style
Chores