From 8bda42fef9d5c5f3cbb6e2b351014d6bcf5aeeca Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 16:09:40 +0300 Subject: [PATCH 1/4] refactor: Simplify promo activation logic in PromoActivationService This commit refactors the `PromoActivationService` to simplify its logic and improve readability. Key changes include: - Removed redundant `user_age is None` checks in the age targeting validation, assuming `user_age` is always present. - Streamlined the promo code acquisition process by removing unnecessary conditional branches and `PromoUnavailableError` exceptions. - Eliminated an unreachable `PromoActivationError`, making the code path for activating a promotion more direct. --- promo_code/user/services.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/promo_code/user/services.py b/promo_code/user/services.py index 43e9fab..629dbe2 100644 --- a/promo_code/user/services.py +++ b/promo_code/user/services.py @@ -79,14 +79,10 @@ def _validate_targeting(self): if target.get('country') and user_country != target['country'].lower(): raise TargetingError('Country mismatch.') - if target.get('age_from') and ( - user_age is None or user_age < target['age_from'] - ): + if target.get('age_from') and user_age < target['age_from']: raise TargetingError('Age mismatch.') - if target.get('age_until') and ( - user_age is None or user_age > target['age_until'] - ): + if target.get('age_until') and user_age > target['age_until']: raise TargetingError('Age mismatch.') def _validate_is_active(self): @@ -127,10 +123,7 @@ def _issue_promo_code(self) -> str: ) promo_locked.save(update_fields=['used_count']) promo_code_value = promo_locked.promo_common - else: - raise PromoUnavailableError() - - elif promo_locked.mode == business.constants.PROMO_MODE_UNIQUE: + else: unique_code = promo_locked.unique_codes.filter( is_used=False, ).first() @@ -139,8 +132,6 @@ def _issue_promo_code(self) -> str: unique_code.used_at = django.utils.timezone.now() unique_code.save(update_fields=['is_used', 'used_at']) promo_code_value = unique_code.code - else: - raise PromoUnavailableError() if promo_code_value: user.models.PromoActivationHistory.objects.create( @@ -149,7 +140,5 @@ def _issue_promo_code(self) -> str: ) return promo_code_value - raise PromoActivationError('Invalid promotion type.') - except business.models.Promo.DoesNotExist: raise PromoActivationError('Promo not found.') From c2af9bdf442f69e44605c237456b262005505ba1 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 16:10:53 +0300 Subject: [PATCH 2/4] chore: Update ruff linting rules This commit updates the `ruff.toml` configuration to expand the set of linting rules. The new rules `FIX` and `INT` have been added to the `select` list to improve code quality and catch a broader range of potential issues. --- ruff.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruff.toml b/ruff.toml index fad008f..fe248b6 100644 --- a/ruff.toml +++ b/ruff.toml @@ -3,7 +3,7 @@ exclude = ["migrations", "venv"] [lint] -select = ["N", "F", "W", "E", "I", "Q", "TID", "COM", "C4", "ERA", "RET", "PTH", "ISC", "C90", "T20", "SIM"] +select = ["C4","N", "F", "W", "E", "I", "Q", "TID", "COM", "C4", "ERA", "RET", "PTH", "ISC", "C90", "T20", "SIM", "FIX", "INT"] [format] From 0df06495820eadf4cc3de51bc68eb6e6b79a8d7e Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 16:14:53 +0300 Subject: [PATCH 3/4] test: Enhance test coverage for the user application This commit introduces a comprehensive set of tests for the `user` application, significantly improving its test coverage and ensuring the reliability of its components. Key additions include: - **Authentication:** Added tests for `CustomJWTAuthentication` to validate token handling, user type verification, and error scenarios like mismatched token versions or missing user IDs. - **Models:** Created new tests for `UserManager` and `User` model behaviors, including `__str__` representations and superuser creation. Added tests for related models like `PromoLike`, `PromoComment`, and `PromoActivationHistory`. - **Services:** Implemented tests for `PromoActivationService` to cover edge cases in targeting logic and race conditions where a promo might be deleted during activation. - **Anti-Fraud:** Added a test to ensure `AntiFraudService` correctly handles missing cache timeout values. - **Profile Operations:** Expanded tests for the user profile endpoint to cover partial updates (`PATCH`) of email and the `other` data field, including validation against existing emails. --- .../user/tests/auth/test_authentication.py | 94 ++++++++++++++++++ .../tests/user/operations/test_profile.py | 56 +++++++++++ .../user/tests/user/test_antifraud_service.py | 8 ++ promo_code/user/tests/user/test_models.py | 91 +++++++++++++++++ promo_code/user/tests/user/test_services.py | 99 +++++++++++++++++++ .../validations/test_profile_validation.py | 28 ++++++ 6 files changed, 376 insertions(+) create mode 100644 promo_code/user/tests/user/test_models.py create mode 100644 promo_code/user/tests/user/test_services.py diff --git a/promo_code/user/tests/auth/test_authentication.py b/promo_code/user/tests/auth/test_authentication.py index 2cefd79..1d1200d 100644 --- a/promo_code/user/tests/auth/test_authentication.py +++ b/promo_code/user/tests/auth/test_authentication.py @@ -1,5 +1,12 @@ +import uuid + +import django.test import rest_framework.status +import rest_framework_simplejwt.exceptions +import rest_framework_simplejwt.tokens +import business.models +import user.authentication import user.models import user.tests.auth.base @@ -27,3 +34,90 @@ def test_signin_success(self): response.status_code, rest_framework.status.HTTP_200_OK, ) + + +class CustomJWTAuthenticationTest(django.test.TestCase): + def setUp(self): + self.factory = django.test.RequestFactory() + self.authenticator = user.authentication.CustomJWTAuthentication() + self.user = user.models.User.objects.create( + name='testuser_uuid', + token_version=1, + ) + self.company = business.models.Company.objects.create( + name='testcompany_uuid', + token_version=1, + ) + + def _get_token_with_payload(self, payload): + token = rest_framework_simplejwt.tokens.AccessToken() + token.payload.update(payload) + return str(token) + + def test_authenticate_invalid_user_type(self): + payload = { + 'user_type': 'admin', + 'user_id': str(self.user.id), + 'token_version': self.user.token_version, + } + token = self._get_token_with_payload(payload) + request = self.factory.get('/api/test/') + request.META['HTTP_AUTHORIZATION'] = f'Bearer {token}' + with self.assertRaisesMessage( + rest_framework_simplejwt.exceptions.AuthenticationFailed, + 'Invalid user type', + ): + self.authenticator.authenticate(request) + + def test_authenticate_missing_id_in_token(self): + payload = { + 'user_type': 'user', + 'token_version': self.user.token_version, + } + token = self._get_token_with_payload(payload) + request = self.factory.get('/api/test/') + request.META['HTTP_AUTHORIZATION'] = f'Bearer {token}' + with self.assertRaisesMessage( + rest_framework_simplejwt.exceptions.AuthenticationFailed, + 'Missing user_id in token', + ): + self.authenticator.authenticate(request) + + def test_authenticate_mismatched_token_version(self): + payload = { + 'user_type': 'user', + 'user_id': str(self.user.id), + 'token_version': 1, + } + token = self._get_token_with_payload(payload) + self.user.token_version = 2 + self.user.save() + request = self.factory.get('/api/test/') + request.META['HTTP_AUTHORIZATION'] = f'Bearer {token}' + with self.assertRaisesMessage( + rest_framework_simplejwt.exceptions.AuthenticationFailed, + 'Token invalid', + ): + self.authenticator.authenticate(request) + + def test_authenticate_user_or_company_not_found(self): + non_existent_uuid = str(uuid.uuid4()) + payload = { + 'user_type': 'user', + 'user_id': non_existent_uuid, + 'token_version': 1, + } + token = self._get_token_with_payload(payload) + request = self.factory.get('/api/test/') + request.META['HTTP_AUTHORIZATION'] = f'Bearer {token}' + with self.assertRaisesMessage( + rest_framework_simplejwt.exceptions.AuthenticationFailed, + 'User or Company not found', + ): + self.authenticator.authenticate(request) + + def test_authenticate_raw_token_none(self): + request = self.factory.get('/api/test/') + request.META['HTTP_AUTHORIZATION'] = 'Token abcdefg' + result = self.authenticator.authenticate(request) + self.assertIsNone(result) diff --git a/promo_code/user/tests/user/operations/test_profile.py b/promo_code/user/tests/user/operations/test_profile.py index a65512d..46e767a 100644 --- a/promo_code/user/tests/user/operations/test_profile.py +++ b/promo_code/user/tests/user/operations/test_profile.py @@ -145,3 +145,59 @@ def test_auth_sign_in_new_password_succeeds(self): response.status_code, rest_framework.status.HTTP_200_OK, ) + + def test_patch_profile_update_other(self): + new_other = {'age': 30, 'country': 'ca'} + response = self.client.patch( + self.user_profile_url, + {'other': new_other}, + format='json', + ) + + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_200_OK, + ) + + self.assertEqual( + response.data.get('other'), + new_other, + ) + + get_resp = self.client.get(self.user_profile_url, format='json') + self.assertEqual( + get_resp.status_code, + rest_framework.status.HTTP_200_OK, + ) + self.assertEqual( + get_resp.data.get('other'), + new_other, + ) + + def test_patch_profile_update_mail(self): + new_email = 'new_creator@apple.com' + response = self.client.patch( + self.user_profile_url, + {'email': new_email}, + format='json', + ) + + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_200_OK, + ) + + self.assertEqual( + response.data.get('email'), + new_email, + ) + + get_resp = self.client.get(self.user_profile_url, format='json') + self.assertEqual( + get_resp.status_code, + rest_framework.status.HTTP_200_OK, + ) + self.assertEqual( + get_resp.data.get('email'), + new_email, + ) diff --git a/promo_code/user/tests/user/test_antifraud_service.py b/promo_code/user/tests/user/test_antifraud_service.py index 57c04e2..7dd8824 100644 --- a/promo_code/user/tests/user/test_antifraud_service.py +++ b/promo_code/user/tests/user/test_antifraud_service.py @@ -129,3 +129,11 @@ def test_handles_api_http_error(self, mock_cache, mock_post): result, {'ok': False, 'error': 'Anti-fraud service unavailable'}, ) + + def test_calculate_cache_timeout_none_when_missing(self): + self.assertIsNone( + self.service._calculate_cache_timeout(None), + ) + self.assertIsNone( + self.service._calculate_cache_timeout(''), + ) diff --git a/promo_code/user/tests/user/test_models.py b/promo_code/user/tests/user/test_models.py new file mode 100644 index 0000000..9d24b9d --- /dev/null +++ b/promo_code/user/tests/user/test_models.py @@ -0,0 +1,91 @@ +import django.test +import django.utils.timezone + +import business.models +import user.models + + +class UserManagerTests(django.test.TestCase): + def test_create_user_without_email_raises_error(self): + with self.assertRaises(ValueError): + user.models.User.objects.create_user( + email=None, + name='Test', + surname='User', + ) + + def test_create_superuser(self): + user_ = user.models.User.objects.create_superuser( + email='super@test.com', + name='Super', + surname='User', + password='password123', + ) + self.assertTrue(user_.is_staff) + self.assertTrue(user_.is_superuser) + self.assertEqual(user_.email, 'super@test.com') + + +class UserModelTests(django.test.TestCase): + def setUp(self): + self.user_ = user.models.User.objects.create_user( + email='test@example.com', + name='Test', + surname='User', + password='password123', + ) + + def test_user_str_representation(self): + self.assertEqual(str(self.user_), 'test@example.com') + + +class RelatedModelsStrTests(django.test.TestCase): + def setUp(self): + self.user_ = user.models.User.objects.create( + email='user@test.com', + name='Test', + surname='User', + ) + self.company = business.models.Company.objects.create( + email='company@test.com', + name='TestCorp', + ) + self.promo = business.models.Promo.objects.create( + company=self.company, + description='Test Promo', + max_count=100, + mode='COMMON', + ) + + def test_promo_like_str(self): + like = user.models.PromoLike.objects.create( + user=self.user_, + promo=self.promo, + ) + expected_str = f'{self.user_} likes {self.promo}' + self.assertEqual(str(like), expected_str) + + def test_promo_comment_str(self): + comment = user.models.PromoComment.objects.create( + author=self.user_, + promo=self.promo, + text='A test comment.', + ) + expected_str = ( + f'Comment by {self.user_.email} on promo {self.promo.id}' + ) + self.assertEqual(str(comment), expected_str) + + def test_promo_activation_history_str(self): + activation = user.models.PromoActivationHistory.objects.create( + user=self.user_, + promo=self.promo, + ) + activation.activated_at = django.utils.timezone.now() + activation.save() + + expected_str = ( + f'{self.user_} activated {self.promo.id} at ' + f'{activation.activated_at}' + ) + self.assertEqual(str(activation), expected_str) diff --git a/promo_code/user/tests/user/test_services.py b/promo_code/user/tests/user/test_services.py new file mode 100644 index 0000000..fc844fd --- /dev/null +++ b/promo_code/user/tests/user/test_services.py @@ -0,0 +1,99 @@ +import unittest.mock + +import django.test + +import business.constants +import business.models +import user.models +import user.services + + +@unittest.mock.patch( + 'user.antifraud_service.antifraud_service.get_verdict', + return_value={'ok': True}, +) +class PromoActivationServiceTestCase(django.test.TestCase): + def setUp(self): + self.user_ = user.models.User.objects.create_user( + name='John', + surname='Jones', + email='test@example.com', + password='password123', + other={'country': 'lu', 'age': 30}, + ) + self.promo = business.models.Promo.objects.create( + description='Test Promo', + active=True, + target={'country': 'lu', 'age_from': 20, 'age_until': 40}, + mode=business.constants.PROMO_MODE_COMMON, + max_count=100, + used_count=0, + promo_common='COMMON_CODE', + ) + + def test_targeting_fails_if_age_from_and_user_age_is_none( + self, + mock_antifraud, + ): + self.user_.other = {'country': 'lu', 'age': 30} + self.user_.save() + self.promo.target = {'age_until': 25} + self.promo.save() + + service = user.services.PromoActivationService( + user=self.user_, + promo=self.promo, + ) + + with self.assertRaisesRegex( + user.services.TargetingError, + 'Age mismatch.', + ): + service.activate() + + def test_targeting_fails_if_age_until_mismatch(self, mock_antifraud): + self.promo.target = {'age_until': 25} + self.promo.save() + + service = user.services.PromoActivationService( + user=self.user_, + promo=self.promo, + ) + + with self.assertRaisesRegex( + user.services.TargetingError, + 'Age mismatch.', + ): + service.activate() + + def test_targeting_fails_if_age_from_mismatch(self, mock_antifraud): + self.promo.target = {'age_from': 31} + self.promo.save() + + service = user.services.PromoActivationService( + user=self.user_, + promo=self.promo, + ) + + with self.assertRaisesRegex( + user.services.TargetingError, + 'Age mismatch.', + ): + service.activate() + + def test_activation_fails_if_promo_deleted_mid_process( + self, + mock_antifraud, + ): + service = user.services.PromoActivationService( + user=self.user_, + promo=self.promo, + ) + + self.promo.delete() + + with self.assertRaisesRegex( + user.services.PromoActivationError, + 'Promo not found.', + ): + service.activate() diff --git a/promo_code/user/tests/user/validations/test_profile_validation.py b/promo_code/user/tests/user/validations/test_profile_validation.py index 592b9a5..8b1b503 100644 --- a/promo_code/user/tests/user/validations/test_profile_validation.py +++ b/promo_code/user/tests/user/validations/test_profile_validation.py @@ -20,6 +20,21 @@ def setUp(self): format='json', ) token = response.data.get('access') + + self.second_user_email = 'another_user@example.com' + second_signup_data = { + 'name': 'Bill', + 'surname': 'Gates', + 'email': self.second_user_email, + 'password': 'MicrosoftRules2020!', + 'other': {'age': 65, 'country': 'us'}, + } + + self.client.post( + self.user_signup_url, + second_signup_data, + format='json', + ) self.client.credentials(HTTP_AUTHORIZATION='Bearer ' + token) def test_update_profile_empty_name_and_surname(self): @@ -91,3 +106,16 @@ def test_get_profile(self): 'other': {'age': 48, 'country': 'gb'}, } self.assertEqual(response.json(), expected) + + def test_patch_profile_update_email_to_existing_fails(self): + payload = {'email': self.second_user_email} + response = self.client.patch( + self.user_profile_url, + payload, + format='json', + ) + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_400_BAD_REQUEST, + ) + self.assertIn('email', response.data) From 6456c1aeb61ffcbd7312349a61b359506436fc64 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 16:16:50 +0300 Subject: [PATCH 4/4] test: Enhance test coverage for the business application This commit expands the test suite for the `business` application, improving validation and model testing. Key additions include: - **Authentication:** Added tests to `InvalidCompanyRegistrationTestCase` and `InvalidCompanyAuthenticationTestCase` to handle cases like missing email on creation and sign-in with an invalid email. - **Models:** Created `promo_code/business/tests/promocodes/test_models.py` to verify the `__str__` representations for the `Company`, `Promo`, and `PromoCode` models. - **Promo Creation:** Added validation tests in `TestPromoCreate` to ensure that `promo_common` is provided for 'COMMON' promos and `promo_unique` is provided for 'UNIQUE' promos. --- .../business/tests/auth/test_validation.py | 32 +++++++++++ .../business/tests/promocodes/test_models.py | 53 +++++++++++++++++++ .../validations/test_create_validation.py | 36 +++++++++++++ 3 files changed, 121 insertions(+) create mode 100644 promo_code/business/tests/promocodes/test_models.py diff --git a/promo_code/business/tests/auth/test_validation.py b/promo_code/business/tests/auth/test_validation.py index b96be7e..d51a522 100644 --- a/promo_code/business/tests/auth/test_validation.py +++ b/promo_code/business/tests/auth/test_validation.py @@ -90,6 +90,17 @@ def test_short_company_name(self): rest_framework.status.HTTP_400_BAD_REQUEST, ) + def test_create_company_missing_email_fiels(self): + with self.assertRaisesMessage( + ValueError, + 'The Email must be set', + ): + business.models.Company.objects.create_company( + name=self.valid_data['name'], + password=self.valid_data['password'], + email=None, + ) + class InvalidCompanyAuthenticationTestCase( business.tests.auth.base.BaseBusinessAuthTestCase, @@ -132,3 +143,24 @@ def test_signin_invalid_password(self): response.status_code, rest_framework.status.HTTP_401_UNAUTHORIZED, ) + + def test_signin_invalid_email(self): + business.models.Company.objects.create_company( + email=self.valid_data['email'], + name=self.valid_data['name'], + password=self.valid_data['password'], + ) + + data = { + 'email': 'example11@example.com', + 'password': self.valid_data['password'], + } + response = self.client.post( + self.company_signin_url, + data, + format='json', + ) + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_400_BAD_REQUEST, + ) diff --git a/promo_code/business/tests/promocodes/test_models.py b/promo_code/business/tests/promocodes/test_models.py new file mode 100644 index 0000000..91a68a4 --- /dev/null +++ b/promo_code/business/tests/promocodes/test_models.py @@ -0,0 +1,53 @@ +import django.test + +import business.constants +import business.models + + +class CompanyModelTests(django.test.TestCase): + def test_company_str_representation(self): + company = business.models.Company.objects.create( + email='contact@company.com', + name='My Awesome Company', + ) + self.assertEqual(str(company), 'My Awesome Company') + + +class PromoModelTests(django.test.TestCase): + def setUp(self): + self.company = business.models.Company.objects.create( + email='company@test.com', + name='TestCorp', + ) + self.common_promo = business.models.Promo.objects.create( + company=self.company, + description='A common promo', + max_count=100, + used_count=50, + mode=business.constants.PROMO_MODE_COMMON, + ) + + def test_promo_str_representation(self): + expected_str = ( + f'Promo {self.common_promo.id} ({self.common_promo.mode})' + ) + self.assertEqual(str(self.common_promo), expected_str) + + +class PromoCodeModelTests(django.test.TestCase): + def test_promo_code_str_representation(self): + company = business.models.Company.objects.create( + email='company@test.com', + name='TestCorp', + ) + promo = business.models.Promo.objects.create( + company=company, + description='Unique codes promo', + max_count=10, + mode=business.constants.PROMO_MODE_UNIQUE, + ) + promo_code = business.models.PromoCode.objects.create( + promo=promo, + code='UNIQUE123', + ) + self.assertEqual(str(promo_code), 'UNIQUE123') diff --git a/promo_code/business/tests/promocodes/validations/test_create_validation.py b/promo_code/business/tests/promocodes/validations/test_create_validation.py index f837a09..ef57159 100644 --- a/promo_code/business/tests/promocodes/validations/test_create_validation.py +++ b/promo_code/business/tests/promocodes/validations/test_create_validation.py @@ -306,6 +306,42 @@ def test_too_short_promo_common(self): rest_framework.status.HTTP_400_BAD_REQUEST, ) + def test_missing_promo_common_field_for_common_promo(self): + payload = { + 'description': 'Increased cashback 40% for new bank clients!', + 'max_count': 100, + 'target': {}, + 'active_from': '2028-12-20', + 'mode': 'COMMON', + } + response = self.client.post( + self.promo_list_create_url, + payload, + format='json', + ) + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_400_BAD_REQUEST, + ) + + def test_missing_promo_unique_field_for_unique_promo(self): + payload = { + 'description': 'Increased cashback 40% for new bank clients!', + 'max_count': 100, + 'target': {}, + 'active_from': '2028-12-20', + 'mode': 'UNIQUE', + } + response = self.client.post( + self.promo_list_create_url, + payload, + format='json', + ) + self.assertEqual( + response.status_code, + rest_framework.status.HTTP_400_BAD_REQUEST, + ) + @parameterized.parameterized.expand( [ (