Skip to content

Week 5 testing#75

Open
Purrna Srivastav (Purrnsr) wants to merge 12 commits into
Rippling:mainfrom
Purrnsr:week-5-testing
Open

Week 5 testing#75
Purrna Srivastav (Purrnsr) wants to merge 12 commits into
Rippling:mainfrom
Purrnsr:week-5-testing

Conversation

@Purrnsr

@Purrnsr Purrna Srivastav (Purrnsr) commented Mar 25, 2026

Copy link
Copy Markdown

Hi, I’ve pushed my Week 5 work on a separate branch week-5-testing and created a pull request. Could you please review it when you get time? I’ve also added integration tests and logging as part of this update.


Note

High Risk
Adds new MongoDB-backed CRUD and bulk-upload endpoints plus auto-seeding on Django startup, which impacts data integrity and runtime behavior. Also introduces csrf_exempt routes and a hardcoded Mongo connection string, increasing security/configuration risk.

Overview
Introduces a new Products/Categories API backed by MongoEngine, including product CRUD, pagination/filtering (category, price_min/max, updated_after), category CRUD, and endpoints to list/assign/remove a product’s category plus a CSV bulk upload (/products/bulk/).

Adds supporting layers (models, repositories, services), standardizes API responses via success_response/error_response, and replaces the hello/ response with a query-parameter greeting.

Wires MongoDB connection and category seeding into settings.py, adds logging, includes a one-off migration script to backfill missing categories, and adds unit/integration tests covering controllers/services and basic API flows.

Written by Cursor Bugbot for commit 2095141. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

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

created_at = DateTimeField(default=datetime.utcnow)
def save(self, *args, **kwargs):
self.updated_at = datetime.utcnow()
return super().save(*args, **kwargs)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product model missing updated_at field declaration

High Severity

The Product model never declares updated_at as a DateTimeField, unlike ProductCategory which properly declares it. The save() override sets self.updated_at as a plain Python attribute, but since Product extends Document (not DynamicDocument), MongoEngine won't persist it to MongoDB. This means to_dict() will fail with an AttributeError when reading a product from the database, and the repository's list_updated_after query on updated_at__gt will never match anything.

Additional Locations (1)
Fix in Cursor Fix in Web

message=updated["error"]
if isinstance(message, list):
message = message[0]
return error_response("VALIDATION_ERROR", updated["error"], status=400)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PUT handler passes original error instead of extracted message

Medium Severity

In the PUT handler, the code extracts updated["error"] into message and converts it from a list to its first element if needed, but then passes the original updated["error"] to error_response instead of the corrected message variable. The analogous POST handler and bulk upload handler both correctly pass message. If the error is a list, this sends the raw list as the error message instead of a string.

Fix in Cursor Fix in Web


result = ProductCategoryService.list_categories()

self.assertEqual(result, []) No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test file redefines service class, shadowing import

Medium Severity

This test file declares a local class named ProductCategoryService (line 12), which shadows the imported service from line 7. This class is a full copy of the real service, not a unittest.TestCase subclass. The two test methods (test_create_category_none_title and test_list_categories_empty) are defined as methods of this duplicate service class, so no test runner will discover or execute them. The file contains no actual runnable tests.

Fix in Cursor Fix in Web

description = StringField()

created_at = DateTimeField(default=datetime.utcnow)
updated_at = DateTimeField(default=datetime.utcnow)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProductCategory.updated_at never updates after creation

Low Severity

ProductCategory.updated_at is declared with default=datetime.utcnow but has no save() override to refresh it on updates, unlike the Product model which attempts to do this. When a category is updated via update_category, the updated_at field retains its original creation-time value. The to_dict() method exposes this stale timestamp to API consumers.

Additional Locations (1)
Fix in Cursor Fix in Web

…s for ProductService/ProductCategoryService with mocking; implemented integration tests with Django and MongoDB; refactored tests to use seed command; added negative/parameterized test cases; created seed_categories command; added regression script (run_tests.bat); configured .env; improved assertions. ~69%% coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants