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

fix: Disable mobile payment refunds from ecommerce #8

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 21 additions & 1 deletion ecommerce/extensions/dashboard/orders/tests.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@


import json
import os
from unittest import SkipTest, skipIf

Expand All @@ -16,6 +16,8 @@
from ecommerce.extensions.dashboard.tests import DashboardViewTestMixin
from ecommerce.extensions.fulfillment.signals import SHIPPING_EVENT_NAME
from ecommerce.extensions.fulfillment.status import LINE, ORDER
from ecommerce.extensions.iap.constants import MOBILE_PAYMENT_PROCESSORS
from ecommerce.extensions.payment.models import PaymentProcessorResponse
from ecommerce.extensions.refund.tests.mixins import RefundTestMixin
from ecommerce.extensions.test.factories import create_order
from ecommerce.tests.factories import UserFactory
Expand Down Expand Up @@ -240,6 +242,24 @@ def test_create_refund_error(self):
'A refund cannot be created for these lines. They may have already been refunded.',
MSG.ERROR)

def test_mobile_refund_error(self):
"""Verify the view creates a Refund for the Order and selected Lines."""
# Create Order and Lines
order = self.create_order(user=self.user)
PaymentProcessorResponse.objects.create(basket=order.basket,
transaction_id="test-transaction",
processor_name=MOBILE_PAYMENT_PROCESSORS[0],
response=json.dumps({'state': 'approved'}))

self.assertFalse(order.refunds.exists())
# Validate the Refund
response = self._request_refund(order)
self.assertFalse(order.refunds.exists())

self.assert_message_equals(response,
"Mobile payment refunds aren't allowed in ecommerce.",
MSG.ERROR)


class OrderDetailViewBrowserTests(OrderViewTestsMixin, RefundTestMixin, OrderViewBrowserTestBase):

Expand Down
39 changes: 25 additions & 14 deletions ecommerce/extensions/dashboard/orders/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from oscar.core.loading import get_model

from ecommerce.extensions.dashboard.views import FilterFieldsMixin
from ecommerce.extensions.iap.constants import MOBILE_PAYMENT_PROCESSORS

Order = get_model('order', 'Order')
Partner = get_model('partner', 'Partner')
Expand Down Expand Up @@ -62,20 +63,30 @@ class OrderDetailView(CoreOrderDetailView):
line_actions = ('change_line_statuses', 'create_shipping_event', 'create_payment_event', 'create_refund')

def create_refund(self, request, order, lines, _quantities): # pylint: disable=unused-argument
refund = Refund.create_with_lines(order, lines)

if refund:
data = {
'link_start': '<a href="{}" target="_blank">'.format(
reverse('dashboard:refunds-detail', kwargs={'pk': refund.pk})),
'link_end': '</a>',
'refund_id': refund.pk
}
message = _('{link_start}Refund #{refund_id}{link_end} created! '
'Click {link_start}here{link_end} to view it.').format(**data)
messages.success(request, mark_safe(message))
else:
message = _('A refund cannot be created for these lines. They may have already been refunded.')
if self._is_order_from_mobile(order):
message = _("Mobile payment refunds aren't allowed in ecommerce.")
messages.error(request, message)

else:
refund = Refund.create_with_lines(order, lines)

if refund:
data = {
'link_start': '<a href="{}" target="_blank">'.format(
reverse('dashboard:refunds-detail', kwargs={'pk': refund.pk})),
'link_end': '</a>',
'refund_id': refund.pk
}
message = _('{link_start}Refund #{refund_id}{link_end} created! '
'Click {link_start}here{link_end} to view it.').format(**data)
messages.success(request, mark_safe(message))
else:
message = _('A refund cannot be created for these lines. They may have already been refunded.')
messages.error(request, message)

return self.reload_page()

def _is_order_from_mobile(self, order):
processor_responses = order.basket.paymentprocessorresponse_set.all()
return any(response.processor_name in MOBILE_PAYMENT_PROCESSORS

Choose a reason for hiding this comment

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

I suspect MOBILE_PAYMENT_PROCESSORS is fairly small, but I think iterating over the processor_responses variable will make 1 db call for each item (unless it had been prefetched ahead of time). You might consider doing something like order.basket.paymentprocessorresponse_set.filter(name__in= MOBILE_PAYMENT_PROCESSORS).exists() which I THINK might do it in 1 DB call (untested of course though)

Not a blocking comment by any means, but think it's worth thinking about potentially

for response in processor_responses)
1 change: 1 addition & 0 deletions ecommerce/extensions/iap/constants.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
MOBILE_PAYMENT_PROCESSORS = ['android-iap', 'ios-iap']
ANDROID_SKU_PREFIX = 'android'
IOS_SKU_PREFIX = 'ios'
MISSING_WEB_SEAT_ERROR = "Couldn't find existing web seat for course [%s]"
Loading