-
Notifications
You must be signed in to change notification settings - Fork 42
Add Robust Unit and Integration Tests for Product APITest complete suite #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b8f15b7
9e56b7d
f4dac96
6499ec1
a8ed77a
f15a23b
45fdf38
57cf729
30915f0
ceca2a2
4211960
1cb7dc1
9d7f838
3d02250
ee3a543
08fb0b6
31da1df
3778c39
7880e14
1ad91b3
e2dbed7
cb9f7bc
b92bb43
97e3a5b
53a90af
df6a639
ce852b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,39 @@ | ||
| from django.contrib import admin | ||
| from django.urls import path | ||
| from django.urls import path, include | ||
| from django.http import HttpResponse | ||
| from django.http import JsonResponse | ||
|
|
||
| # def hello_world(request): | ||
| # return HttpResponse("Hello, world! This is our interneers-lab Django server.") | ||
|
|
||
| # urlpatterns = [ | ||
| # path('admin/', admin.site.urls), | ||
| # path('hello/', hello_world), | ||
| # ] | ||
|
|
||
|
|
||
| def home(request): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove these from urls.py.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I’ve removed the unnecessary logic from urls.py as it’s no longer needed. |
||
| return HttpResponse("Hi") | ||
|
|
||
|
|
||
| def hello(request): | ||
| return HttpResponse("Hello, world! This is our interneers-lab Django server, First change made, function name changed from i.e hello_world to hello") | ||
|
|
||
| def hello_name(request): | ||
| """ | ||
| A simple view that returns 'Hello, {name}' in JSON format. | ||
| Uses a query parameter named 'name'. | ||
| """ | ||
| # Get 'name' from the query string, default to 'World' if missing | ||
| name = request.GET.get("name", "World") | ||
| return JsonResponse({"message": f"Hello, {name}!"}) | ||
|
|
||
| def hello_world(request): | ||
| return HttpResponse("Hello, world! This is our interneers-lab Django server.") | ||
|
|
||
| urlpatterns = [ | ||
| path('admin/', admin.site.urls), | ||
| path('hello/', hello_world), | ||
| path('hello/', hello), | ||
| path('', home), | ||
| path('hello_name/', hello_name), #test this using -> hello_name/?name=Vedanshi | ||
| #tested with few more apis like http://127.0.0.1:8000/hello_name/?name=John%20Doe!@# , etc | ||
| path('api/', include('products.urls')), | ||
| ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,11 @@ services: | |
| image: mongo:latest | ||
| container_name: interneers_lab_mongodb | ||
| ports: | ||
| - '27018:27017' | ||
| - '27017:27017' | ||
| environment: | ||
| MONGO_INITDB_ROOT_USERNAME: root | ||
| MONGO_INITDB_ROOT_PASSWORD: example | ||
| command: ["mongod", "--auth"] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! Glad the setup is on point! |
||
| volumes: | ||
| - mongodb_data:/data/db | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| from django.contrib import admin | ||
| from .models import Product | ||
|
|
||
| # admin.site.register(Product) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove comments which no longer is need. Applicable everywhere.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ve removed the unnecessary comments to keep the code clean and more readable. Let me know if there's anything else I can improve! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| from django.apps import AppConfig | ||
|
|
||
|
|
||
| class ProductsConfig(AppConfig): | ||
| name = 'products' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| from rest_framework.response import Response | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checkout File naming convention - https://realpython.com/python-pep8/#how-to-choose-names
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I’ll make sure to rename the file according to PEP 8 conventions for better readability and consistency. |
||
| from rest_framework.exceptions import APIException, NotFound | ||
| from rest_framework import status | ||
|
|
||
| class ProductNotFound(NotFound): | ||
| default_detail = {"error": "Product not found"} | ||
| default_code = "not_found" | ||
|
|
||
| def handle_exception(exception): | ||
| print("Exception Caught:", type(exception), exception) | ||
|
|
||
| if isinstance(exception, ProductNotFound): | ||
| return Response(exception.default_detail, status=status.HTTP_404_NOT_FOUND) | ||
|
|
||
| if isinstance(exception, APIException): | ||
| return Response({"error": str(exception)}, status=exception.status_code) | ||
|
|
||
| return Response({"error": "An unexpected error occurred"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import mongoengine | ||
| import datetime | ||
|
|
||
| class ProductCategory(mongoengine.Document): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can import all the necessary Classes from monogengine directly.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ve updated the imports to directly include only the necessary classes from mongoengine in both CategoryModel and ProductModel |
||
| title = mongoengine.StringField(max_length=100, required=True, unique=True) | ||
| description = mongoengine.StringField(required=False) | ||
| created_at = mongoengine.DateTimeField(default=lambda: datetime.datetime.now(datetime.UTC)) | ||
| updated_at = mongoengine.DateTimeField(default=lambda: datetime.datetime.now(datetime.UTC)) | ||
|
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since all of the models would need created_at and updated_at. We can create a base class for setting them up and inherit them wherever you need.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ve created a TimestampedDocument base class to handle the created_at and updated_at fields, and now Product and ProductCategory inherits from it. I’ve updated the save() method to use datetime.utcnow() for consistency with the TimestampedDocument base class. |
||
|
|
||
|
|
||
| meta = {'collection': 'ProductCategory' , 'db_alias': 'default'} | ||
|
|
||
| def save(self, *args, **kwargs): | ||
| self.updated_at = datetime.datetime.now(datetime.UTC) | ||
| return super(ProductCategory, self).save(*args, **kwargs) | ||
|
|
||
|
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great that you have overriden the method to update updated at field.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I've explored MongoEngine signals and implemented the pre_save signal to automatically update the updated_at field. |
||
| def __str__(self): | ||
| return self.title | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import mongoengine | ||
| import datetime | ||
| from .CategoryModel import ProductCategory | ||
|
|
||
| class Product(mongoengine.Document): | ||
| name = mongoengine.StringField(max_length=255, required=True) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should title be unique?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the title field should be unique, as each product category needs to have a distinct identifier. Ensuring uniqueness will help prevent any potential issues with having multiple categories with the same name, which could lead to confusion or errors in the system. |
||
| description = mongoengine.StringField(required=False) | ||
| brand = mongoengine.StringField(max_length=100, required=True) | ||
| category = mongoengine.ListField( | ||
| mongoengine.ReferenceField(ProductCategory, reverse_delete_rule=mongoengine.PULL) | ||
| ) #Use PULL when you want to remove references to a deleted document without deleting dependent documents. | ||
| price = mongoengine.DecimalField(precision=2, required=True) | ||
| quantity = mongoengine.IntField(default=0, min_value=0) | ||
| created_at = mongoengine.DateTimeField(default=lambda: datetime.datetime.now(datetime.UTC)) | ||
| updated_at = mongoengine.DateTimeField(default=lambda: datetime.datetime.now(datetime.UTC)) | ||
|
|
||
| meta = {'collection': 'Product' , 'db_alias': 'default'} | ||
|
|
||
| def save(self, *args, **kwargs): | ||
| self.updated_at = datetime.datetime.now(datetime.UTC) | ||
| return super(Product, self).save(*args, **kwargs) | ||
|
|
||
| def __str__(self): | ||
| return self.name | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
Comment on lines
+26
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And linter to fix these issues
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion. I will use linter (pylint) to identify and fix any issues, including removing unnecessary imports, formatting corrections, and adding docstrings where necessary. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| from ..models.CategoryModel import ProductCategory | ||
| from ..models.ProductModel import Product | ||
|
|
||
| class CategoryRepository: | ||
| @staticmethod | ||
| def create(title, description=None): | ||
| if ProductCategory.objects.filter(title=title).first(): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Get() instead of filter here.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ve updated both the create and update functions as per your suggestions. I replaced filter() with get() in the create function for better performance and applied proper exception handling in the update function. |
||
| raise ValueError("Category with this title already exists.") | ||
| category = ProductCategory.objects.create(title=title, description=description) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you check
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have checked and confirmed that ProductCategory(title=title, description=description) is successfully creating the document as expected. |
||
| return category | ||
|
|
||
| @staticmethod | ||
| def get_category_by_title(title): | ||
| return ProductCategory.objects.filter(title__iexact=title).first() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets store only upper/lowecase for category
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your suggestion. At present, the case-insensitive approach is being used and several existing functions depend on it, and they have been thoroughly tested. Transitioning to a case-sensitive implementation would require substantial changes and additional testing. For now, I would prefer to maintain the current approach, but I am open to revisiting this if necessary in the future. |
||
|
|
||
|
|
||
| @staticmethod | ||
| def getCategoryById(category_id): | ||
| try: | ||
| return ProductCategory.objects.get(id = category_id) | ||
|
|
||
| except ProductCategory.DoesNotExist: | ||
| return None | ||
|
|
||
|
|
||
| @staticmethod | ||
| def get_products_by_category(category): | ||
| return Product.objects.filter(category=category) | ||
|
|
||
| @staticmethod | ||
| def get_all(): | ||
| return ProductCategory.objects.all() | ||
|
|
||
| @staticmethod | ||
| def update(title, new_title=None, new_description=None): | ||
| category = ProductCategory.objects.filter(title=title).first() | ||
| if not category: | ||
| raise ValueError("Category not found.") | ||
|
|
||
| if new_title and ProductCategory.objects.filter(title=new_title).exists(): | ||
| raise ValueError("Category with this new title already exists.") | ||
| if new_title: | ||
| category.title = new_title | ||
|
|
||
| if new_description is not None: | ||
| category.description = new_description | ||
|
|
||
| category.save() | ||
| return category | ||
|
|
||
| @staticmethod | ||
| def delete(title): | ||
| category = ProductCategory.objects.filter(title=title).first() | ||
| if not category: | ||
| raise ValueError("Category not found.") | ||
| category.delete() | ||
| return True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| from ..models.ProductModel import Product | ||
| from bson import ObjectId | ||
| from bson.errors import InvalidId | ||
|
|
||
| class ProductRepository: | ||
|
|
||
| @staticmethod | ||
| def createProd(prod_details): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add types in function definition.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated the functions to include type hints as per your recommendation. This will improve the readability of the code and help with understanding the expected inputs and outputs for each function. |
||
| prod_details["price"] = float(prod_details["price"]) | ||
| prod = Product(**prod_details) | ||
| prod.save() | ||
| return prod | ||
|
|
||
|
|
||
| @staticmethod | ||
| def getAllProd(): | ||
| return list(Product.objects.all()) | ||
|
|
||
|
|
||
| @staticmethod | ||
| def getProdById(prod_id): | ||
| try: | ||
| obj_id = ObjectId(prod_id) | ||
| return Product.objects.get(id=obj_id) | ||
|
|
||
| except InvalidId: | ||
| raise ValueError("Invalid ID format. Must be a 12-byte input or 24-character hex string.") | ||
|
|
||
| except Product.DoesNotExist: | ||
| return None | ||
|
|
||
|
|
||
| @staticmethod | ||
| def updateProd(prod_id , prod_details): | ||
|
|
||
| prod = ProductRepository.getProdById(prod_id) | ||
|
|
||
| if prod: | ||
| for key,value in prod_details.items(): | ||
| setattr(prod,key,value) | ||
| prod.save() | ||
| return prod | ||
|
|
||
| return None | ||
|
|
||
|
|
||
| @staticmethod | ||
| def deleteProd(prod_id): | ||
| print(f"Looking for product with ID: {prod_id}") | ||
| prod = ProductRepository.getProdById(prod_id) | ||
| print(f"Product fetched: {prod}") | ||
|
|
||
| if prod: | ||
| prod.delete() | ||
| return True | ||
|
|
||
| return None | ||
|
|
||
|
|
||
|
|
||
| @staticmethod | ||
| def update_product_categories(product, updated_categories): | ||
| product.category = updated_categories | ||
| product.save() | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid '*'. This allows requests from any domain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed '*' from ALLOWED_HOSTS and restricted it to localhost and 127.0.0.1 for development. Also ensured CORS_ALLOWED_ORIGINS only allow localhost addresses. Please let me know if any further changes are needed!