search

land-of-apps / django-oscar

Domain-driven e-commerce for Django
http://oscarcommerce.com
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Exclusive voucher offers are not exclusive #14

Open kgilpin opened 1 month ago

kgilpin commented 1 month ago

Exclusive voucher offers are not exclusive

Issue Summary

When you create two new offers with the voucher offer type and make it exclusive, it's still possible to add both to your basket. Though reproducing was a bit strange, as it depends on the order you're adding the voucher.

I had to add my latest created voucher first and then the first one I created. The other way around resulted in an message that it wasn't possible to add that voucher to my basket.

Steps to Reproduce

github-actions[bot] commented 1 month ago

Title: Fix the Handling of Exclusive Voucher Offers

Problem:

Exclusive voucher offers are not being enforced correctly. Users can add two exclusive vouchers to their basket, which violates the exclusivity constraint. Depending on the order in which the vouchers are added, they may or may not be added successfully.

Analysis:

The root cause of this problem is likely the logic that handles the validation and application of vouchers to the basket. The logic for adding vouchers should check if any exclusive vouchers are already applied to the basket and prevent adding another if so. The issue could be in both the validation logic and the application process of vouchers in the basket handling code.

  1. Form Validation: Ensure that when a voucher is being added, the form validation checks if any exclusive voucher is already present in the basket.
  2. Voucher Application Logic: When applying a voucher, ensure the code correctly checks for existing exclusive vouchers in the basket and prevents further additions.
  3. Communication: Properly inform the user via error messages when an exclusive voucher cannot be added due to the presence of another exclusive voucher.

Proposed Changes:

  1. File: src/oscar/apps/basket/views.py

    • Modify the form_valid method in VoucherAddView to include the logic to check for existing exclusive vouchers before adding a new voucher.

  2. File: src/oscar/apps/offer/applicator.py

    • Enhance the applicator logic to ensure existing exclusive vouchers block the addition of new exclusive vouchers.

  3. File: tests/integration/basket/test_views.py

    • Add and update integration tests in the TestVoucherAddView and TestVoucherRemoveView classes to ensure exclusive vouchers cannot be applied if another exclusive voucher is already present in the basket.
    • Cover the scenario for both orders of adding vouchers (new first, old first).

  4. File: tests/integration/basket/test_utils.py

    • Ensure utility methods and fixtures used in tests correctly handle exclusive vouchers, ensuring they reflect realistic scenarios.

Steps:

  1. src/oscar/apps/basket/views.py:

    • Before adding a new voucher in the form_valid method, check if an exclusive voucher already exists in the basket. If so, display an error message and prevent adding the new voucher.
    def form_valid(self, form):
    code = form.cleaned_data["code"]
    if not self.request.basket.id:
        return redirect_to_referrer(self.request, "basket:summary")
    if self.request.basket.contains_voucher(code):
        messages.error(
            self.request,
            _("You have already added the '%(code)s' voucher to your basket")
            % {"code": code},
        )
    else:
        try:
            voucher = self.voucher_model._default_manager.get(code=code)
        except self.voucher_model.DoesNotExist:
            messages.error(
                self.request,
                _("No voucher found with code '%(code)s'") % {"code": code},
            )
        else:
            # Check for existing exclusive vouchers
            if voucher.exclusive and any(v.exclusive for v in self.request.basket.vouchers.all()):
                messages.error(
                    self.request,
                    _("Cannot add an exclusive voucher when another exclusive voucher is already in the basket.")
                )
            else:
                self.apply_voucher_to_basket(voucher)
    return redirect_to_referrer(self.request, "basket:summary")

  2. src/oscar/apps/offer/applicator.py:

    • Ensure that exclusive vouchers prevent further addition of vouchers if they are active and available to the user.
    def get_offers(self, basket, user=None):
    offers = super(Applicator, self).get_offers(basket, user)
    exclusive_voucher_applied = any(v.exclusive for v in basket.vouchers.all())
    for voucher in basket.vouchers.all():
        available_to_user, __ = voucher.is_available_to_user(user=user)
        if voucher.is_active() and available_to_user:
            basket_offers = voucher.offers.all()
            for offer in basket_offers:
                if exclusive_voucher_applied and offer.exclusive:
                    continue
                offer.set_voucher(voucher)
            offers = list(chain(offers, basket_offers))
    return offers

  3. tests/integration/basket/test_views.py:

    • Add tests to validate that exclusive vouchers cannot be added if another exclusive voucher is already present.
    class TestVoucherAddView(TestCase):
    def test_exclusive_voucher_addition(self):
        voucher1 = factories.VoucherFactory(exclusive=True)
        voucher2 = factories.VoucherFactory(exclusive=True)
        self.assertTrue(voucher1.is_active())
        self.assertTrue(voucher2.is_active())

        data1 = {"code": voucher1.code}
        data2 = {"code": voucher2.code}
        request = RequestFactory().post("/", data=data1)
        request.basket.save()
        view = views.VoucherAddView.as_view()
        response = view(request)  # Add the first exclusive voucher
        self.assertEqual(response.status_code, 302)
        self.assertEqual(request.basket.vouchers.count(), 1)

        request = RequestFactory().post("/", data=data2)
        response = view(request)  # Attempt to add a second exclusive voucher
        self.assertEqual(response.status_code, 302)
        self.assertEqual(request.basket.vouchers.count(), 1)  # Should not be added

    # Add more tests for different scenarios of voucher addition order

By implementing these changes, the exclusivity constraint will be correctly enforced for vouchers in the basket, and the user will be appropriately notified if an exclusive voucher cannot be added.

kgilpin commented 1 month ago