Skip to content
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

Product API refactor #90

Merged
merged 18 commits into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dshop/Dshop/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@
},
"TEST_REQUEST_DEFAULT_FORMAT": "json",
"DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.AutoSchema",
"DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.LimitOffsetPagination",
"DEFAULT_PAGINATION_CLASS": 'rest_framework.pagination.PageNumberPagination',
"PAGE_SIZE": 25,
"DEFAULT_FILTER_BACKENDS": ["django_filters.rest_framework.DjangoFilterBackend"]

Expand Down
2 changes: 2 additions & 0 deletions Dshop/Dshop/settings_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from Dshop.settings import * # noqa: W0401
REST_FRAMEWORK['PAGE_SIZE'] = 5 # noqa: W0401
4 changes: 2 additions & 2 deletions Dshop/apps/products_catalogue/api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
from .filters import ProductFilter


class ProductViewSet(viewsets.ModelViewSet):
class ProductViewSet(viewsets.ReadOnlyModelViewSet):
queryset = Product.objects.filter(is_active=True, parent_product=None)
serializer_class = ProductSerializer
filter_backends = [DjangoFilterBackend]
filterset_class = ProductFilter

permission_classes = (AllowAny, )

class CartAPIView(APIView):
permission_classes = (AllowAny ,)
Expand Down
145 changes: 97 additions & 48 deletions Dshop/apps/products_catalogue/tests/test_api_products.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
from rest_framework.test import APIClient
import pytest
from django.urls import reverse
from django.conf import settings
from apps.products_catalogue.models import Category, Product, ProductImage
# pylama:ignore=W0404, W0611
from apps.users.conftest import api_client, login_url, login_data, user_instance, user_instance_token
from apps.users.conftest import login_url, login_data, user_instance, user_instance_token
from PIL import Image
from django.core.files.uploadedfile import SimpleUploadedFile
import tempfile
import os

@pytest.fixture
def create_category():
return Category.objects.create(name='Test Category', is_active=True)


@pytest.fixture
Expand Down Expand Up @@ -45,6 +44,7 @@ def authenticated_api_client(api_client, user_instance_token):
return api_client



@pytest.fixture
def create_product_with_images(create_category):
product = Product.objects.create(
Expand All @@ -70,15 +70,13 @@ def create_product_with_images(create_category):


def assert_active_object(data):
assert data['category'] is not None

fields_values = {
"id": 1,
"category": 1,
"name": "main product",
"price": "11.00",
"short_description": "short desc",
"full_description": "full_description",
"name": "TV AMOLED",
"price": "3999.00",
"short_description": 'Test short description',
"full_description": 'Test full description',
"parent_product": None,
"images": []
}
Expand All @@ -100,71 +98,122 @@ def assert_product_with_images(data):


@pytest.mark.django_db
def test_product_detail_with_images(authenticated_api_client, create_product_with_images):
def test_product_detail_with_images(api_client_authed, create_product_with_images):
url = reverse('products-api-detail', kwargs={'pk': create_product_with_images.id})
response = authenticated_api_client.get(url)
response = api_client_authed.get(url)
yanazPL marked this conversation as resolved.
Show resolved Hide resolved

assert response.status_code == 200
assert_product_with_images(response.data)


@pytest.mark.django_db
def test_access_protected_resource(authenticated_api_client, create_active_product, create_inactive_product):
def test_get_list_one(set_test_pagination_size, tv_product, inactive_product):
url = reverse('products-api-list')
response = authenticated_api_client.get(url)
response = APIClient().get(url)
yanazPL marked this conversation as resolved.
Show resolved Hide resolved
assert response.status_code == 200

results = response.data.get('results', [])
assert len(results) == 1

product_data = results[0]

assert response.data['count'] == 1
assert response.data['previous'] is None
assert response.data['next'] is None
assert_active_object(product_data)


def test_access_protected_resource_without_authentication(api_client):
url = reverse('products-api-list')
response = api_client.get(url)
@pytest.mark.django_db
def test_product_detail_404():
url = reverse('products-api-detail', kwargs={'pk': 6669})
response = APIClient().get(url)
assert response.status_code == 404

assert response.status_code == 401

@pytest.fixture
yanazPL marked this conversation as resolved.
Show resolved Hide resolved
def set_test_pagination_size(settings, autouse=True):
settings.REST_FRAMEWORK['PAGE_SIZE'] = 5


def test_pagination_size_in_tests():
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove fixture since we moved to settings_tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to know if settings_tests are changed. So we know the cause of test's failures

assert settings.REST_FRAMEWORK['PAGE_SIZE'] == 5
# pagination size changed here: Dshop/Dshop/settings_tests.py

@pytest.mark.django_db
def test_product_detail(authenticated_api_client, create_active_product):
url = reverse('products-api-detail', kwargs={'pk': create_active_product.id})
response = authenticated_api_client.get(url)
def test_product_list_empty(set_test_pagination_size):
response = APIClient().get(reverse("products-api-list"))
assert response.status_code == 200
assert response.data['results'] == []
assert response.data['count'] == 0
assert response.data['previous'] is None
assert response.data['next'] is None



@pytest.mark.django_db
def test_product_detail(tv_product):
url = reverse('products-api-detail', kwargs={'pk': tv_product.id})
response = APIClient().get(url)
Copy link
Contributor

Choose a reason for hiding this comment

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

After this change, this test is only checking response code, not details. Please bring back ID and other details tests - the one removed. Or use a function to test product details.
Testing just response code is not enough.
Actually, this test_product_detail_with_images test, after changing to not authenticated user, would make the current one test not needed no? They both check the same thing.

Copy link
Contributor Author

@yanazPL yanazPL Mar 10, 2024

Choose a reason for hiding this comment

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

Assert added. Also test_product_detail_with_images doesn't need authentication any more.

I still think it may be beneficial to have separate tests for image related behavior and general object retrieve.
Especially since separate tests are already there. They don't overlap.

assert response.status_code == 200
assert response.data['id'] == create_active_product.id
assert response.data['name'] == "main product"
assert response.data['price'] == "11.00"
assert response.data['short_description'] == "short desc"
assert response.data['full_description'] == "full_description"



@pytest.mark.django_db
def test_create_product(authenticated_api_client, create_category):
url = reverse('products-api-list')
data = {
'category': create_category.id,
'name': 'Test Product',
'price': '19.99',
'short_description': 'Test short description',
'full_description': 'Test full description',
}
response = authenticated_api_client.post(url, data, format='json')
def test_product_list_pagination_ten_products_page_too_far(set_test_pagination_size, tv_product):
response = APIClient().get(f"{reverse('products-api-list')}?page=100")
assert response.status_code == 404


assert response.status_code == 201
assert response.data['name'] == "Test Product"
assert response.data['price'] == "19.99"
@pytest.mark.parametrize("page_suffix", ["", "?page=1"])
yanazPL marked this conversation as resolved.
Show resolved Hide resolved
@pytest.mark.django_db
def test_product_list_pagination_ten_products_page_1(set_test_pagination_size, page_suffix, ten_tv_products):#, ten_tv_products):
yanazPL marked this conversation as resolved.
Show resolved Hide resolved
response = APIClient().get(reverse("products-api-list") + page_suffix)
results = response.data['results']
assert len(results) == 5
assert results[0]['id'] == ten_tv_products[0].id
assert results[4]['id'] == ten_tv_products[4].id
assert response.data['count'] == 10
assert response.data['next'] == "http://testserver/api/products/?page=2"
assert response.data['previous'] is None


@pytest.mark.django_db
def test_product_list_pagination_ten_products_page_2(set_test_pagination_size, ten_tv_products):
response = APIClient().get(f"{reverse('products-api-list')}?page=2")
results = response.data['results']
assert response.status_code == 200
assert len(results) == 5
assert results[0]['id'] == ten_tv_products[5].id
assert results[4]['id'] == ten_tv_products[9].id
assert response.data['count'] == 10
assert response.data['next'] is None
assert response.data['previous'] == "http://testserver/api/products/"


@pytest.mark.django_db
def test_update_product(authenticated_api_client, create_active_product):
url = reverse('products-api-detail', kwargs={'pk': create_active_product.id})
data = {'name': 'Updated product name'}
response = authenticated_api_client.patch(url, data, format='json')
def test_product_list_pagination_forty_three_products_page_4(set_test_pagination_size, forty_three_tv_products):
response = APIClient().get(f"{reverse('products-api-list')}?page=4")
results = response.data['results']
assert response.status_code == 200
assert len(results) == 5
assert results[0]['id'] == forty_three_tv_products[15].id
assert results[4]['id'] == forty_three_tv_products[19].id
assert response.data['count'] == 43
assert response.data['next'] == "http://testserver/api/products/?page=5"
assert response.data['previous'] == "http://testserver/api/products/?page=3"


@pytest.mark.django_db
def test_product_list_pagination_forty_three_products_page_9(set_test_pagination_size, forty_three_tv_products):
response = APIClient().get(f"{reverse('products-api-list')}?page=9")
results = response.data['results']
assert response.status_code == 200
assert response.data['name'] == "Updated product name"
assert response.data['short_description'] == "short desc"
assert len(results) == 3
assert results[0]['id'] == forty_three_tv_products[40].id
assert results[2]['id'] == forty_three_tv_products[42].id
assert response.data['count'] == 43
assert response.data['next'] is None
assert response.data['previous'] == "http://testserver/api/products/?page=8"


@pytest.mark.django_db
def test_delete_unallowed_method(tv_product):
response = APIClient().delete(reverse('products-api-detail', kwargs={'pk': tv_product.pk}))
assert response.status_code == 405
7 changes: 0 additions & 7 deletions Dshop/apps/users/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,9 @@
from django.contrib.auth import get_user_model
from django.urls import reverse
from rest_framework.authtoken.models import Token
from rest_framework.test import APIClient

User = get_user_model()


@pytest.fixture
def api_client():
return APIClient()


@pytest.fixture
def login_url():
return reverse('api-login')
Expand Down
Loading
Loading