From c8dfc32f81a8268b2d10592ae1d941f4344e5355 Mon Sep 17 00:00:00 2001 From: Voxin Muyli Date: Fri, 21 Jul 2023 10:20:15 +0200 Subject: [PATCH 1/8] =?UTF-8?q?Oscarapi=20always=20accessed=20and=20prcess?= =?UTF-8?q?ed=20the=20basket=20even=20if=20it=20wasn't=20ne=E2=80=A6=20(#3?= =?UTF-8?q?16)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Oscarapi always accessed and prcessed the basket even if it wasn't needed. This solves that and also solves a problem with logout being broken because cookies failed to be deleted. Fixes #311 * Removed print statement --- oscarapi/middleware.py | 52 +++++++++++++++++++---------- oscarapi/tests/unit/testbasket.py | 2 +- oscarapi/tests/unit/testcheckout.py | 2 +- oscarapi/tests/unit/testproduct.py | 2 +- oscarapi/tests/unit/testuser.py | 2 +- 5 files changed, 39 insertions(+), 21 deletions(-) diff --git a/oscarapi/middleware.py b/oscarapi/middleware.py index 4b727b7c..0c3c5bb8 100644 --- a/oscarapi/middleware.py +++ b/oscarapi/middleware.py @@ -19,6 +19,7 @@ get_basket, ) +from django.utils.functional import SimpleLazyObject, empty from oscarapi.utils.loading import get_api_class from oscarapi.utils.request import get_domain from oscarapi.utils.session import session_id_from_parsed_session_uri, get_session @@ -156,29 +157,46 @@ class ApiBasketMiddleWare(BasketMiddleware, IsApiRequest): def __call__(self, request): if self.is_api_request(request): - request.cookies_to_delete = [] - # we should make sure that any cookie baskets are turned into - # session baskets, since oscarapi uses only baskets from the - # session. - cookie_key = self.get_cookie_key(request) - basket = self.get_cookie_basket( - cookie_key, - request, - Exception("get_cookie_basket doesn't use the manager argument"), - ) + def load_basket_into_session(): + request.cookies_to_delete = [] + # we should make sure that any cookie baskets are turned into + # session baskets, since oscarapi uses only baskets from the + # session. + cookie_key = self.get_cookie_key(request) + + basket = self.get_cookie_basket( + cookie_key, + request, + Exception("get_cookie_basket doesn't use the manager argument"), + ) + + if basket is not None: + # when a basket exists and we are already allowed to access + # this basket + if request_allows_access_to_basket(request, basket): + pass + else: + store_basket_in_session(basket, request.session) + + request.basket = SimpleLazyObject(load_basket_into_session) - if basket is not None: - # when a basket exists and we are already allowed to access - # this basket - if request_allows_access_to_basket(request, basket): - pass - else: - store_basket_in_session(basket, request.session) + response = self.get_response(request) + return self.process_response(request, response) return super(ApiBasketMiddleWare, self).__call__(request) def process_response(self, request, response): + if not hasattr(request, "basket"): + return response + + # If the basket was never initialized we can safely return + if ( + isinstance(request.basket, SimpleLazyObject) + and request.basket._wrapped is empty # pylint: disable=protected-access + ): + return response + if ( self.is_api_request(request) and hasattr(request, "user") diff --git a/oscarapi/tests/unit/testbasket.py b/oscarapi/tests/unit/testbasket.py index 536a8c5e..648abb22 100644 --- a/oscarapi/tests/unit/testbasket.py +++ b/oscarapi/tests/unit/testbasket.py @@ -1,6 +1,6 @@ import json -from mock import patch +from unittest.mock import patch from django.contrib.auth import get_user_model from django.urls import reverse diff --git a/oscarapi/tests/unit/testcheckout.py b/oscarapi/tests/unit/testcheckout.py index bdebc8c9..4d71a3e9 100644 --- a/oscarapi/tests/unit/testcheckout.py +++ b/oscarapi/tests/unit/testcheckout.py @@ -1,5 +1,5 @@ from decimal import Decimal -from mock import patch +from unittest.mock import patch from django.contrib.auth import get_user_model from django.urls import reverse diff --git a/oscarapi/tests/unit/testproduct.py b/oscarapi/tests/unit/testproduct.py index 19b6a700..8abedbf6 100644 --- a/oscarapi/tests/unit/testproduct.py +++ b/oscarapi/tests/unit/testproduct.py @@ -1,4 +1,4 @@ -import mock +from unittest import mock import decimal import datetime import json diff --git a/oscarapi/tests/unit/testuser.py b/oscarapi/tests/unit/testuser.py index 8443a88a..4fcb7036 100644 --- a/oscarapi/tests/unit/testuser.py +++ b/oscarapi/tests/unit/testuser.py @@ -1,4 +1,4 @@ -from mock import patch +from unittest.mock import patch from django.contrib.auth import get_user_model from django.urls import reverse From 79cecbc136cdf75c5f47beaf68fa7002e3c431ce Mon Sep 17 00:00:00 2001 From: Snake-Soft <40820005+snake-soft@users.noreply.github.com> Date: Fri, 21 Jul 2023 10:36:47 +0200 Subject: [PATCH 2/8] Fix Django 4 errors (#317) * Ugettext is Removed in Django 4.0 * Signal kwarg providing_args is Disallowed in Django 4.0 * Test against Django 4.2 and remove python 3.7 --------- Co-authored-by: Frank Hennige Co-authored-by: Joey Jurjens --- .github/workflows/ci.yml | 4 ++-- oscarapi/middleware.py | 2 +- oscarapi/serializers/basket.py | 2 +- oscarapi/serializers/fields.py | 2 +- oscarapi/serializers/product.py | 2 +- oscarapi/signals.py | 9 ++++++++- oscarapi/utils/categories.py | 2 +- oscarapi/views/basket.py | 2 +- 8 files changed, 16 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c06479fb..62adcda0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,8 +33,8 @@ jobs: strategy: fail-fast: true matrix: - python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"] - django-version: ["3.2"] + python-version: ["3.8", "3.9", "3.10", "3.11"] + django-version: ["3.2", "4.2"] oscar-version: ["3.2"] steps: - name: Download src dir diff --git a/oscarapi/middleware.py b/oscarapi/middleware.py index 0c3c5bb8..418c617b 100644 --- a/oscarapi/middleware.py +++ b/oscarapi/middleware.py @@ -5,7 +5,7 @@ from django.contrib.sessions.middleware import SessionMiddleware from django.core.exceptions import PermissionDenied from django.http.response import HttpResponse -from django.utils.translation import ugettext as _ +from django.utils.translation import gettext as _ from oscar.core.loading import get_class diff --git a/oscarapi/serializers/basket.py b/oscarapi/serializers/basket.py index 5b9fa4e9..1f9ee3ff 100644 --- a/oscarapi/serializers/basket.py +++ b/oscarapi/serializers/basket.py @@ -2,7 +2,7 @@ import logging from decimal import Decimal -from django.utils.translation import ugettext as _ +from django.utils.translation import gettext as _ from django.contrib.auth import get_user_model from rest_framework import serializers diff --git a/oscarapi/serializers/fields.py b/oscarapi/serializers/fields.py index 3e56feb3..1e0599f8 100644 --- a/oscarapi/serializers/fields.py +++ b/oscarapi/serializers/fields.py @@ -7,7 +7,7 @@ from urllib.error import HTTPError from django.conf import settings as django_settings from django.db import IntegrityError -from django.utils.translation import ugettext as _ +from django.utils.translation import gettext as _ from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.files import File from django.utils.functional import cached_property diff --git a/oscarapi/serializers/product.py b/oscarapi/serializers/product.py index 6112f5aa..b480d6f3 100644 --- a/oscarapi/serializers/product.py +++ b/oscarapi/serializers/product.py @@ -2,7 +2,7 @@ import logging from copy import deepcopy -from django.utils.translation import ugettext as _ +from django.utils.translation import gettext as _ from rest_framework import serializers from rest_framework.fields import empty diff --git a/oscarapi/signals.py b/oscarapi/signals.py index e3476577..17d20090 100644 --- a/oscarapi/signals.py +++ b/oscarapi/signals.py @@ -1,3 +1,10 @@ +import django from django.dispatch import Signal -oscarapi_post_checkout = Signal(providing_args=["order", "user", "request", "response"]) + +oscarapi_post_checkout_args = ["order", "user", "request", "response"] + +if django.VERSION >= (3, 0): + oscarapi_post_checkout = Signal() +else: + oscarapi_post_checkout = Signal(providing_args=oscarapi_post_checkout_args) diff --git a/oscarapi/utils/categories.py b/oscarapi/utils/categories.py index ac9d35bb..539b7d6c 100644 --- a/oscarapi/utils/categories.py +++ b/oscarapi/utils/categories.py @@ -1,4 +1,4 @@ -from django.utils.translation import ugettext as _ +from django.utils.translation import gettext as _ from rest_framework.exceptions import NotFound diff --git a/oscarapi/views/basket.py b/oscarapi/views/basket.py index 905dfedf..07732688 100644 --- a/oscarapi/views/basket.py +++ b/oscarapi/views/basket.py @@ -1,5 +1,5 @@ # pylint: disable=W0632 -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import gettext_lazy as _ from oscar.apps.basket import signals from oscar.core.loading import get_model, get_class From a60a14f53a98c458d7b8e7333108880e53ad9ff1 Mon Sep 17 00:00:00 2001 From: Voxin Muyli Date: Thu, 31 Aug 2023 08:22:18 +0200 Subject: [PATCH 3/8] Fix broken basket middleware (#319) * new release * Undo broken lazy basket. Added prepare=False to get_basket because we don't really need to apply any discounts if we only need to find the basket id. * Added ApiBasketMiddleWare test and filter in get_anonymous_basket * removed useless code --- oscarapi/basket/operations.py | 3 +- oscarapi/middleware.py | 56 ++++++---------- oscarapi/tests/unit/testbasket.py | 106 +++++++++++++++++++++++++++++- oscarapi/tests/utils.py | 6 ++ setup.py | 2 +- 5 files changed, 132 insertions(+), 41 deletions(-) diff --git a/oscarapi/basket/operations.py b/oscarapi/basket/operations.py index d0e7821d..34e9d766 100644 --- a/oscarapi/basket/operations.py +++ b/oscarapi/basket/operations.py @@ -74,7 +74,7 @@ def get_anonymous_basket(request): basket_id = get_basket_id_from_session(request) try: - basket = Basket.open.get(pk=basket_id) + basket = Basket.open.get(pk=basket_id, owner=None) except Basket.DoesNotExist: basket = None @@ -83,7 +83,6 @@ def get_anonymous_basket(request): def get_user_basket(user): "get basket for a user." - try: basket, __ = Basket.open.get_or_create(owner=user) except Basket.MultipleObjectsReturned: diff --git a/oscarapi/middleware.py b/oscarapi/middleware.py index 418c617b..2f27439d 100644 --- a/oscarapi/middleware.py +++ b/oscarapi/middleware.py @@ -5,7 +5,7 @@ from django.contrib.sessions.middleware import SessionMiddleware from django.core.exceptions import PermissionDenied from django.http.response import HttpResponse -from django.utils.translation import gettext as _ +from django.utils.translation import ugettext as _ from oscar.core.loading import get_class @@ -19,7 +19,6 @@ get_basket, ) -from django.utils.functional import SimpleLazyObject, empty from oscarapi.utils.loading import get_api_class from oscarapi.utils.request import get_domain from oscarapi.utils.session import session_id_from_parsed_session_uri, get_session @@ -157,46 +156,29 @@ class ApiBasketMiddleWare(BasketMiddleware, IsApiRequest): def __call__(self, request): if self.is_api_request(request): + request.cookies_to_delete = [] + # we should make sure that any cookie baskets are turned into + # session baskets, since oscarapi uses only baskets from the + # session. + cookie_key = self.get_cookie_key(request) - def load_basket_into_session(): - request.cookies_to_delete = [] - # we should make sure that any cookie baskets are turned into - # session baskets, since oscarapi uses only baskets from the - # session. - cookie_key = self.get_cookie_key(request) - - basket = self.get_cookie_basket( - cookie_key, - request, - Exception("get_cookie_basket doesn't use the manager argument"), - ) - - if basket is not None: - # when a basket exists and we are already allowed to access - # this basket - if request_allows_access_to_basket(request, basket): - pass - else: - store_basket_in_session(basket, request.session) - - request.basket = SimpleLazyObject(load_basket_into_session) + basket = self.get_cookie_basket( + cookie_key, + request, + Exception("get_cookie_basket doesn't use the manager argument"), + ) - response = self.get_response(request) - return self.process_response(request, response) + if basket is not None: + # when a basket exists and we are already allowed to access + # this basket + if request_allows_access_to_basket(request, basket): + pass + else: + store_basket_in_session(basket, request.session) return super(ApiBasketMiddleWare, self).__call__(request) def process_response(self, request, response): - if not hasattr(request, "basket"): - return response - - # If the basket was never initialized we can safely return - if ( - isinstance(request.basket, SimpleLazyObject) - and request.basket._wrapped is empty # pylint: disable=protected-access - ): - return response - if ( self.is_api_request(request) and hasattr(request, "user") @@ -208,7 +190,7 @@ def process_response(self, request, response): # We just have to make sure it is stored as a cookie, because it # could have been created by oscarapi. cookie_key = self.get_cookie_key(request) - basket = get_basket(request) + basket = get_basket(request, prepare=False) cookie = self.get_basket_hash(basket.id) # Delete any surplus cookies diff --git a/oscarapi/tests/unit/testbasket.py b/oscarapi/tests/unit/testbasket.py index 648abb22..cbbf62df 100644 --- a/oscarapi/tests/unit/testbasket.py +++ b/oscarapi/tests/unit/testbasket.py @@ -1,7 +1,7 @@ import json from unittest.mock import patch - +from django.test import override_settings from django.contrib.auth import get_user_model from django.urls import reverse @@ -1147,3 +1147,107 @@ def test_get_user_basket_with_multiple_baskets(self): user_basket = get_user_basket(user) self.assertEqual(Basket.open.count(), 1) self.assertEqual(user_basket, Basket.open.first()) + + +@override_settings( + MIDDLEWARE=( + "django.middleware.common.CommonMiddleware", + "django.middleware.csrf.CsrfViewMiddleware", + "django.contrib.sessions.middleware.SessionMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + "django.contrib.messages.middleware.MessageMiddleware", + "django.middleware.clickjacking.XFrameOptionsMiddleware", + "oscarapi.middleware.ApiBasketMiddleWare", + "django.contrib.flatpages.middleware.FlatpageFallbackMiddleware", + ) +) +class ApiBasketMiddleWareTest(APITest): + fixtures = [ + "product", + "productcategory", + "productattribute", + "productclass", + "productattributevalue", + "category", + "attributeoptiongroup", + "attributeoption", + "stockrecord", + "partner", + "option", + ] + + def test_basket_login_logout(self): + self.assertEqual( + Basket.objects.count(), 0, "Initially there should be no baskets" + ) + + # add something to the anonymous basket, so we get a cookie basket + url = reverse("basket:add", kwargs={"pk": 1}) + post_params = {"child_id": 2, "action": "add", "quantity": 5} + response = self.client.post(url, post_params, follow=True) + + self.assertEqual( + Basket.objects.count(), + 1, + "After posting to the basket, 1 basket should be created.", + ) + self.assertIn( + "oscar_open_basket", + self.client.cookies, + "An basket cookie should have been created", + ) + self.assertStartsWith(self.client.cookies["oscar_open_basket"].value, "1") + + # retrieve the basket with oscarapi. + self.response = self.get("api-basket") + self.response.assertValueEqual( + "owner", None, "The basket should not have an owner" + ) + self.response.assertValueEqual("id", 1) + self.assertStartsWith(self.client.cookies["oscar_open_basket"].value, "1") + + # now lets log in with oscarapi + response = self.post("api-login", username="nobody", password="nobody") + # and lets retrieve the basket + self.response = self.get("api-basket") + self.response.assertValueEqual( + "owner", + "http://testserver/api/users/2/", + "the basket after login should have an owner", + ) + self.assertEqual( + self.client.cookies["oscar_open_basket"].value, + "1:Rdm76bzEHM-N1G6WSTj0Zu9ByZ80a8ggxSkqqvGbC6s", + "After logging out the cookie unfortunately does not go away", + ) + + response = self.client.post(url, post_params, follow=True) + self.assertEqual(response.status_code, 200) + self.assertEqual(Basket.objects.count(), 2) + self.assertEqual( + self.client.cookies["oscar_open_basket"].value, + "", + "The basket cookie should be removed", + ) + + self.response = self.delete("api-login") + self.assertStartsWith( + self.client.cookies["oscar_open_basket"].value, + "", + "After loging out, nothing happened to the basket cookie", + ) + + self.response = self.get("api-basket") + self.response.assertValueEqual( + "owner", None, "The logged out user's basket should not have an owner" + ) + self.assertEqual( + Basket.objects.count(), + 3, + "A new basket should be created for the anonymous (logged out) user", + ) + self.assertStartsWith( + self.client.cookies["oscar_open_basket"].value, + "3", + "The basket cookie is re-established after accessing the basket when logged out", + ) diff --git a/oscarapi/tests/utils.py b/oscarapi/tests/utils.py index 363ecb43..a275f290 100644 --- a/oscarapi/tests/utils.py +++ b/oscarapi/tests/utils.py @@ -121,6 +121,12 @@ def reload_modules(modules=()): for module in modules: reload_module(module) + def assertStartsWith(self, value, startvalue, message=""): + if not value.startswith(startvalue): + self.fail( + "'%s' does not start with '%s'. %s" % (value, startvalue, message) + ) + class ParsedResponse(object): def __init__(self, response, unittestcase): diff --git a/setup.py b/setup.py index 6e16e290..b4b56d13 100755 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -__version__ = "3.2.2" +__version__ = "3.2.3" setup( # package name in pypi From 50ece310ff39fdbb8bfa408f8770e41b34d00760 Mon Sep 17 00:00:00 2001 From: Lars van de Kerkhof Date: Thu, 31 Aug 2023 16:23:31 +0200 Subject: [PATCH 4/8] New release --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index b4b56d13..4f5e62c0 100755 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -__version__ = "3.2.3" +__version__ = "3.2.4" setup( # package name in pypi From 296466caaa1b34a1e6f8b8fd7efceb42afdb73cb Mon Sep 17 00:00:00 2001 From: Viggodevries Date: Fri, 8 Sep 2023 09:55:45 +0200 Subject: [PATCH 5/8] add os build and python 3.11 --- .readthedocs.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.readthedocs.yml b/.readthedocs.yml index f968835c..8948d3d3 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -1,10 +1,15 @@ version: 2 +build: + os: "ubuntu-22.04" + tools: + python: "3.11" + sphinx: configuration: docs/source/conf.py python: - version: 3.7 + version: 3.11 install: - method: pip path: . From 5b372fb36369090d5c9f787a290630976e85d228 Mon Sep 17 00:00:00 2001 From: Viggodevries Date: Fri, 8 Sep 2023 10:01:28 +0200 Subject: [PATCH 6/8] Add 3.2.4 release note --- docs/source/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 24461cf2..69564e34 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -2,6 +2,11 @@ Changelog ========= +3.2.4 (2023-09-08) +------------------ +this release is compatible with Oscar 3.2.1. This adds support for Django versions up to Django 4.2 + + 3.2.0 (2023-03-03) ------------------ This release is compatible with Oscar 3.2 . Supported Django versions are 3.2, and supported Python versions are 3.7, 3.8, 3.9, 3.10 and 3.11. This is my (maerteijn) last release as maintainer of OscarAPI. From ea0ca38c47c548e82262efd0df4ffd678e02b63e Mon Sep 17 00:00:00 2001 From: Viggodevries Date: Fri, 8 Sep 2023 10:11:02 +0200 Subject: [PATCH 7/8] fix .yml file --- .readthedocs.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.readthedocs.yml b/.readthedocs.yml index 8948d3d3..5be54601 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -9,10 +9,9 @@ sphinx: configuration: docs/source/conf.py python: - version: 3.11 install: + - requirements: docs/requirements.txt - method: pip path: . extra_requirements: - dev - - requirements: docs/requirements.txt \ No newline at end of file From c76343a1447cf82b6670d045df66670aebc331de Mon Sep 17 00:00:00 2001 From: Viggo de Vries Date: Thu, 28 Dec 2023 15:31:10 +0100 Subject: [PATCH 8/8] Update settings.py --- oscarapi/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oscarapi/settings.py b/oscarapi/settings.py index c1abc4c3..542e11ba 100644 --- a/oscarapi/settings.py +++ b/oscarapi/settings.py @@ -40,7 +40,7 @@ "OSCARAPI_VOUCHER_FIELDS", default=("name", "code", "start_datetime", "end_datetime"), ) -"ik ben harrie" + BASKET_FIELDS = overridable( "OSCARAPI_BASKET_FIELDS", default=(