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: improve payment mock in test #324

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ def get_edx_successful_payment_info(order: CTOrder):
return None, None


# TODO update function and its return value name
# the return value could be either stripe payment intent id or PayPal order ID
def get_edx_payment_intent_id(order: CTOrder) -> Union[str, None]:
def get_edx_psp_payment_id(order: CTOrder) -> Union[str, None]:
"""
Retrieve the payment service provider (PSP) payment ID from a given
order which could be either a Stripe payment intent ID or a PayPal order ID.
"""
pmt, _ = get_edx_successful_payment_info(order)
if pmt:
return pmt.interface_id
Expand Down
15 changes: 8 additions & 7 deletions commerce_coordinator/apps/commercetools/sub_messages/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
get_edx_lms_user_id,
get_edx_lms_user_name,
get_edx_order_workflow_state_key,
get_edx_payment_intent_id,
get_edx_product_course_run_key,
get_edx_psp_payment_id,
is_edx_lms_order
)
from commerce_coordinator.apps.commercetools.catalog_info.utils import (
Expand Down Expand Up @@ -269,24 +269,25 @@ def _prepare_segment_event_properties(in_order, return_line_item_return_id):
logger.info(f'[CT-{tag}] order {order_id} is not an edX order, message id: {message_id}')
return True

payment_intent_id = get_edx_payment_intent_id(order)
# psp_payment_id will be payment intent id for stripe and paypal order id for paypal
psp_payment_id = get_edx_psp_payment_id(order)
lms_user_name = get_edx_lms_user_name(customer)
lms_user_id = get_edx_lms_user_id(customer)

logger.info(f'[CT-{tag}] calling PSP to refund payment "{payment_intent_id}", message id: {message_id}')
logger.info(f'[CT-{tag}] calling PSP to refund payment "{psp_payment_id}", message id: {message_id}')

# Return payment if payment id is set
if payment_intent_id is not None:
if psp_payment_id is not None:
result = OrderRefundRequested.run_filter(
order_id=order_id, return_line_item_return_id=return_line_item_return_id, message_id=message_id
)

if 'refund_response' in result and result['refund_response']:
if result['refund_response'] == 'charge_already_refunded':
logger.info(f'[CT-{tag}] payment intent {payment_intent_id} already has refunded transaction, '
logger.info(f'[CT-{tag}] payment {psp_payment_id} already has refunded transaction, '
f'sending Slack notification, message id: {message_id}')
else:
logger.info(f'[CT-{tag}] payment intent {payment_intent_id} refunded for message id: {message_id}')
logger.info(f'[CT-{tag}] payment {psp_payment_id} refunded for message id: {message_id}')
segment_event_properties = _prepare_segment_event_properties(order, return_line_item_return_id)

for line_item in get_edx_items(order):
Expand Down Expand Up @@ -321,7 +322,7 @@ def _prepare_segment_event_properties(in_order, return_line_item_return_id):
properties=segment_event_properties
)
else: # pragma no cover
logger.info(f'[CT-{tag}] payment intent {payment_intent_id} not refunded, '
logger.info(f'[CT-{tag}] payment {psp_payment_id} not refunded, '
f'sending Slack notification, message id: {message_id}')

logger.info(f'[CT-{tag}] Finished return for order: {order_id}, line item: {return_line_item_return_id}, '
Expand Down
9 changes: 8 additions & 1 deletion commerce_coordinator/apps/commercetools/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from commercetools.platform.models import CustomFields as CTCustomFields
from commercetools.platform.models import FieldContainer as CTFieldContainer
from commercetools.platform.models import LineItemReturnItem as CTLineItemReturnItem
from commercetools.platform.models import MoneyType as CTMoneyType
from commercetools.platform.models import Order as CTOrder
from commercetools.platform.models import Payment as CTPayment
from commercetools.platform.models import PaymentState
Expand All @@ -22,6 +23,7 @@
from commercetools.platform.models import ReturnPaymentState, ReturnShipmentState
from commercetools.platform.models import Transaction as CTTransaction
from commercetools.platform.models import TransactionState, TransactionType
from commercetools.platform.models import TypedMoney as CTTypedMoney
from commercetools.platform.models import TypeReference as CTTypeReference
from commercetools.platform.models.state import State as CTLineItemState
from commercetools.platform.models.state import StateTypeEnum as CTStateType
Expand Down Expand Up @@ -170,7 +172,12 @@ def gen_payment():
amount_planned=4900,
payment_method_info={},
payment_status=PaymentState.PAID,
transactions=[gen_transaction(TransactionType.REFUND, 4900)],
transactions=[gen_transaction(TransactionType.REFUND, CTTypedMoney(
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember we updated this mock function with the CT Money type but mocking started creating issues in other cases where this mock result was used. How have we resolved that?

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 didn't get that issue. I think that there must have been something wrong locally (maybe some unintentional code change) at that time.

currency_code='USD',
cent_amount=1000,
type=CTMoneyType.CENT_PRECISION,
fraction_digits=2,
))],
interface_interactions=[]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
from unittest import TestCase
from unittest.mock import MagicMock, call, patch

from commercetools.platform.models import MoneyType as CTMoneyType
from commercetools.platform.models import Order as CTOrder
from commercetools.platform.models import ReturnInfo as CTReturnInfo
from commercetools.platform.models import ReturnPaymentState as CTReturnPaymentState
from commercetools.platform.models import TypedMoney as CTTypedMoney
from edx_django_utils.cache import TieredCache

from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient
Expand Down Expand Up @@ -255,18 +253,6 @@ def setUp(self):
self.mock.update_return_payment_state_after_successful_refund
}
)
# TODO: Properly mock the Payment object.
payment = self.mock.get_payment_by_key.return_value
amount = CTTypedMoney(
currency_code='USD',
cent_amount=1000,
type=CTMoneyType.CENT_PRECISION,
fraction_digits=2,
)
for transaction in payment.transactions:
transaction.amount = amount

self.payment_mock = payment

def tearDown(self):
MonkeyPatch.unmonkey(CommercetoolsAPIClient)
Expand Down Expand Up @@ -330,12 +316,12 @@ def test_correct_arguments_passed_valid_stripe_refund(
mock_values.customer_mock.assert_called_once_with(mock_values.customer_id)
_stripe_api_mock.return_value.refund_payment_intent.assert_called_once()

@patch('commerce_coordinator.apps.commercetools.sub_messages.tasks.get_edx_payment_intent_id')
@patch('commerce_coordinator.apps.commercetools.sub_messages.tasks.get_edx_psp_payment_id')
@patch('commerce_coordinator.apps.commercetools.sub_messages.tasks.OrderRefundRequested.run_filter')
def test_refund_already_charged(
self,
_return_filter_mock: MagicMock,
_mock_payment_intent: MagicMock,
_mock_psp_payment_id: MagicMock,
):
"""
Check calling uut with mock_parameters yields call to client with
Expand All @@ -344,6 +330,6 @@ def test_refund_already_charged(
mock_values = self.mock
mock_values.order_mock.return_value.return_info = []
_return_filter_mock.return_value = {'refund_response': 'charge_already_refunded'}
_mock_payment_intent.return_value = 'mock_payment_intent_id'
_mock_psp_payment_id.return_value = 'mock_payment_intent_id'

self.get_uut()(*self.unpack_for_uut(self.mock.example_payload))
Loading