Feature/week 06#90
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.
| <h2 class="title">${product.name}</h2> | ||
| <p class="description">Brand: ${product.brand}</p> | ||
| <div class="price">$${product.price}</div> | ||
| `; |
There was a problem hiding this comment.
XSS vulnerability via unsanitized innerHTML injection
High Severity
API response values (product.name, product.brand, product.price) are interpolated directly into innerHTML without any sanitization. Since the backend API accepts arbitrary strings for product names and brands (via both the POST endpoint and bulk_upload_products), an attacker can store a malicious payload like <img src=x onerror=alert(1)> as a product name, creating a stored XSS vulnerability that executes when any user loads the page. Using textContent or DOM creation methods instead of innerHTML would avoid this.
Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.
| category_id=row.get('category_id') | ||
| ) | ||
|
|
||
| return JsonResponse({"status": "Bulk upload complete"}) No newline at end of file |
There was a problem hiding this comment.
Bulk upload lacks method check and error handling
Medium Severity
bulk_upload_products has two issues: it doesn't verify request.method == 'POST', so any HTTP method (GET, PUT, DELETE) triggers CSV processing; and it lacks error handling around the loop, so if create_product raises (e.g., invalid category_id), previously created products remain committed while the endpoint returns a 500 error. This causes silent partial writes with no indication of which rows succeeded.
Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.
| return JsonResponse({"status": "Success", "message": f"{product.name} removed from category"}) | ||
| return JsonResponse({"error": "Product not found"}, status=404) | ||
| except Exception as e: | ||
| return JsonResponse({"error": str(e)}) |
There was a problem hiding this comment.
View returns None for unsupported HTTP methods
Low Severity
api_for_a_product has no fallback return at the end. When an unsupported HTTP method (e.g., PATCH) is sent with a product_id, execution falls through all conditionals and the function implicitly returns None, causing Django to raise a ValueError and return a 500 error. The same issue applies to api_for_a_category. Both views lack a default return for unmatched methods.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.
| return JsonResponse({"status": "Success", "product": product.name, "category_id": str(product.category.id)}) | ||
| return JsonResponse({"error": "Product not found"}, status=404) | ||
| except Exception as e: | ||
| return JsonResponse({"error": str(e)}) |
There was a problem hiding this comment.
PUT errors return 200 instead of 404
Medium Severity
add_product_to_category raises ValueError for not-found products/categories, unlike the other service methods (e.g., remove_product_from_category) which return None. The view's PUT handler expects a None return to trigger the 404 response on line 125, but the ValueError is caught by the except block on line 126, which returns a 200 status (no status code specified). This means "product not found" and "category not found" errors on PUT always return HTTP 200 instead of a proper error status, making the 404 on line 125 unreachable dead code.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.
| # objects() is a special funciton written in the original document class from which ProductCategory is inherited | ||
| # we can use it without intantiating the class | ||
| def get_category_by_id(self, category_id): | ||
| return ProductCategory.objects(id=category_id).first() |
There was a problem hiding this comment.
We can create a repo layer for model queries.
Like in repo file expose a function get_product_category_by_id(..)
| return ProductCategory.objects(id=category_id).first() | ||
|
|
||
| def delete_category(self,category_id): | ||
| category = ProductCategory.objects(id=category_id).first() |
| class ProductService(): | ||
|
|
||
| def get_all_products(self): | ||
| return Product.objects() |
There was a problem hiding this comment.
same for all model related queries we can expose the methods for responsibility segregation
| def add_product_to_category(self, product_id, category_id): | ||
| product = Product.objects(id=product_id).first() | ||
| if not product: | ||
| raise ValueError("product not found") |
There was a problem hiding this comment.
Always try to return Id also along with error message like ( same for all exceptions)
product not found with id = "123456789"
There was a problem hiding this comment.
updated code to put id when method fails
| @csrf_exempt | ||
| def api_for_a_category(request, category_id=None): | ||
|
|
||
| if request.method == 'GET': |
There was a problem hiding this comment.
As GET, POST, PUT, DELETE will be used at multiple views, we can define a constant for these rather than hardcoding
There was a problem hiding this comment.
made a new file with all the http constants and all the error codes
| if request.method == 'GET': | ||
| if category_id: | ||
| try: | ||
| c = category_service.get_category_by_id(category_id) |
There was a problem hiding this comment.
we can always try using meaningful variable names, like category
There was a problem hiding this comment.
changed c and p to meaningful names
| success = category_service.delete_category(category_id) | ||
| if success: | ||
| return JsonResponse({"status": "deleted"}) | ||
| return JsonResponse({"error": "category not found"}, status=404) |
There was a problem hiding this comment.
as "category not found" error message is used at multiple places, can be defined in constants file.
Also we can define a map of Status code -> Error message.
| try: | ||
| p = product_service.get_product_by_id(product_id) | ||
| if p: | ||
| return JsonResponse({ |
There was a problem hiding this comment.
Rather than manually creating key -> Values, we can define a DTO/Model to return as a response along with serializer, it will reduce the manual effort.
| return JsonResponse({"title": c.title, "description": c.description}) | ||
| return JsonResponse({"error": "Category not found"}, status=404) | ||
| except Exception as e: | ||
| return JsonResponse({"error": str(e)}) |
There was a problem hiding this comment.
By default the status code will be 200, if its a error we can send 4XX, 5XX based on error.
| from .models import ProductCategory, Product | ||
| from .repositories import category_repo, product_repo | ||
|
|
||
| class CategoryService: |
There was a problem hiding this comment.
seperate file for each service.
| from .models import ProductCategory, Product | ||
|
|
||
| class CategoryRepository: | ||
| def get_by_id(self, category_id): |
There was a problem hiding this comment.
same here we can create seperate files for both
| @@ -0,0 +1,31 @@ | |||
| const container = document.getElementById('product-container'); | |||
|
|
|||
| fetch('http://127.0.0.1:8000/api/products/') | |||
There was a problem hiding this comment.
url should not be hard coded in same file


Completed basic and advanced Week 06 tasks.
Note
Medium Risk
Adds new HTTP endpoints for CRUD and bulk ingestion and connects to MongoDB via
mongoengine, which can impact data integrity and security (notably@csrf_exempthandlers) if deployed beyond local dev.Overview
Adds a new
week_04Django project that connects to MongoDB and exposes JSON APIs to create/read/update/delete product categories and create/list products, including endpoints to assign/remove a product’s category and aproducts/bulk-upload/CSV ingestion route.Updates the existing Django
helloendpoint to return a JSON greeting using anamequery param, and adds a simpleweek_06static page that fetches/api/products/and renders product tiles. Frontend housekeeping includes switchingApp.tsxstyling toApp.scssand pinning Yarn viapackageManagerinpackage.json.Reviewed by Cursor Bugbot for commit 6ab44c4. Configure here.