Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

feat: add additional fields to EnterpriseLearnerOfferApiSerializer #3963

Merged
merged 5 commits into from
May 10, 2023
Merged

feat: add additional fields to EnterpriseLearnerOfferApiSerializer #3963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
69830e5
feat: add additional fields to EnterpriseLearnerOfferApiSerializer
adamstankiewicz May 8, 2023
68fc826
chore: tests
adamstankiewicz May 9, 2023
80063b7
chore: quality
adamstankiewicz May 9, 2023
990090d
chore: updates
adamstankiewicz May 9, 2023
5acf7cf
chore: quality
adamstankiewicz May 9, 2023
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
56 changes: 41 additions & 15 deletions ecommerce/extensions/api/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1137,46 +1137,71 @@ class Meta:

def _serialize_remaining_balance_value(conditional_offer):
"""
Change value into string and return it unless it is None.
Calculate and return remaining balance on the offer.
"""
remaining_balance = calculate_remaining_offer_balance(conditional_offer)
if remaining_balance is not None:
remaining_balance = str(remaining_balance)
return remaining_balance


class EnterpriseLearnerOfferApiSerializer(serializers.BaseSerializer): # pylint: disable=abstract-method
def _serialize_remaining_balance_for_user(conditional_offer, request):
"""
Serializer for EnterpriseOffer learner endpoint.
Determines the remaining balance for the user.
"""
if request and conditional_offer.max_user_discount is not None:
return str(conditional_offer.max_user_discount - sum_user_discounts_for_offer(request.user, conditional_offer))
Copy link
Contributor

Choose a reason for hiding this comment

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

why is balance str()-ed but applications are (presumably) ints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iloveagent57 I think the current state of the enterprise-learner-offers endpoint returns the balance as a str() because course-discovery also returns price as a string, presumably so that clients don't have to format the returned value to have the 2 decimals. If returning a float with the UI wanted to display the price as $199.00, would need to transform 199 or 199.0 to 199.00 for display purposes, where if its given by the API as a string, the client can just render it as is.

For this purpose of this PR, I'm just following convention with what already exists. That said, I'm not sure what the "standard"/"correct" way of returning price should be. I could see some tradeoffs with both approaches.

I kept applications as ints because there's no chance for them to be floats; the decimal formatting is a non-issue for applications.

return None

Uses serializers.BaseSerializer to keep this lightweight.

def _serialize_remaining_applications_value(conditional_offer):
"""
Calculate and return remaining number of applications on the offer.
"""
if conditional_offer.max_global_applications is not None:
return conditional_offer.max_global_applications - conditional_offer.num_applications
return None

def _serialize_remaining_balance_for_user(self, instance):
request = self.context.get('request')

if request and instance.max_user_discount is not None:
return str(instance.max_user_discount - sum_user_discounts_for_offer(request.user, instance))
def _serialize_remaining_applications_for_user(conditional_offer, request):
"""
Determines the remaining number of applications (enrollments) for the user.
"""
if request and conditional_offer.max_user_applications is not None:
return conditional_offer.max_user_applications - conditional_offer.get_num_user_applications(request.user)
return None

return None

class EnterpriseLearnerOfferApiSerializer(serializers.BaseSerializer): # pylint: disable=abstract-method
"""
Serializer for EnterpriseOffer learner endpoint.

Uses serializers.BaseSerializer to keep this lightweight.
"""

def to_representation(self, instance):
representation = OrderedDict()

representation['id'] = instance.id
representation['max_discount'] = instance.max_discount
representation['enterprise_customer_uuid'] = instance.condition.enterprise_customer_uuid
representation['enterprise_catalog_uuid'] = instance.condition.enterprise_customer_catalog_uuid
representation['is_current'] = instance.is_current
representation['status'] = instance.status
representation['start_datetime'] = instance.start_datetime
representation['end_datetime'] = instance.end_datetime
representation['enterprise_catalog_uuid'] = instance.condition.enterprise_customer_catalog_uuid
representation['usage_type'] = get_benefit_type(instance.benefit)
representation['discount_value'] = instance.benefit.value
representation['status'] = instance.status
representation['remaining_balance'] = _serialize_remaining_balance_value(instance)
representation['is_current'] = instance.is_current
representation['max_discount'] = instance.max_discount
representation['max_global_applications'] = instance.max_global_applications
representation['max_user_applications'] = instance.max_user_applications
representation['max_user_discount'] = instance.max_user_discount
representation['num_applications'] = instance.num_applications
representation['remaining_balance_for_user'] = self._serialize_remaining_balance_for_user(instance)
representation['remaining_balance'] = _serialize_remaining_balance_value(instance)
representation['remaining_applications'] = _serialize_remaining_applications_value(instance)
representation['remaining_balance_for_user'] = \
_serialize_remaining_balance_for_user(instance, request=self.context.get('request'))
representation['remaining_applications_for_user'] = \
_serialize_remaining_applications_for_user(instance, request=self.context.get('request'))

return representation

Expand All @@ -1199,6 +1224,7 @@ def to_representation(self, instance):
representation['enterprise_catalog_uuid'] = instance.condition.enterprise_customer_catalog_uuid
representation['display_name'] = generate_offer_display_name(instance)
representation['remaining_balance'] = _serialize_remaining_balance_value(instance)
representation['remaining_applications'] = _serialize_remaining_applications_value(instance)
representation['is_current'] = instance.is_current

return representation
Expand Down
97 changes: 92 additions & 5 deletions ecommerce/extensions/api/v2/tests/test_serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,19 @@
class EnterpriseLearnerOfferApiSerializerTests(TestCase):

@ddt.data(
(100, '74.5'),
(None, None)
(100, 25.5, '74.5'),
(None, None, None)
)
@ddt.unpack
@mock.patch('ecommerce.extensions.api.serializers.sum_user_discounts_for_offer')
def test_serialize_remaining_balance_for_user(
self,
max_user_discount,
expected_remaining_balance,
existing_user_spend,
expected_remaining_balance_for_user,
mock_sum_user_discounts_for_offer
):
mock_sum_user_discounts_for_offer.return_value = 25.5
mock_sum_user_discounts_for_offer.return_value = existing_user_spend
enterprise_customer_uuid = str(uuid4())
condition = extended_factories.EnterpriseCustomerConditionFactory(
enterprise_customer_uuid=enterprise_customer_uuid
Expand All @@ -45,4 +46,90 @@ def test_serialize_remaining_balance_for_user(
}
)
data = serializer.to_representation(enterprise_offer)
assert data['remaining_balance_for_user'] == expected_remaining_balance
assert data['remaining_balance_for_user'] == expected_remaining_balance_for_user

@ddt.data(
(5250, '5250'),
Copy link
Contributor

Choose a reason for hiding this comment

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

question unrelated to your change: why are we returning a string-ified number in this serializer?

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 think the answer to this might be covered in the response to your other comment?

(None, None)
)
@ddt.unpack
@mock.patch('ecommerce.extensions.api.serializers.calculate_remaining_offer_balance')
def test_serialize_remaining_balance(
self,
max_discount,
expected_remaining_balance,
mock_calculate_remaining_offer_balance
):
mock_calculate_remaining_offer_balance.return_value = max_discount
enterprise_customer_uuid = str(uuid4())
condition = extended_factories.EnterpriseCustomerConditionFactory(
enterprise_customer_uuid=enterprise_customer_uuid
)
enterprise_offer = extended_factories.EnterpriseOfferFactory.create(
partner=self.partner,
condition=condition,
)
serializer = EnterpriseLearnerOfferApiSerializer(
data=enterprise_offer,
context={
'request': mock.MagicMock(user=UserFactory())
}
)
data = serializer.to_representation(enterprise_offer)
assert data['remaining_balance'] == expected_remaining_balance

@ddt.data(
(1000, 1000),
(None, None)
)
@ddt.unpack
def test_serialize_remaining_applications_for_user(
self,
max_user_applications,
expected_remaining_applications_for_user,
):
enterprise_customer_uuid = str(uuid4())
condition = extended_factories.EnterpriseCustomerConditionFactory(
enterprise_customer_uuid=enterprise_customer_uuid
)
enterprise_offer = extended_factories.EnterpriseOfferFactory.create(
partner=self.partner,
condition=condition,
max_user_applications=max_user_applications,
)
serializer = EnterpriseLearnerOfferApiSerializer(
data=enterprise_offer,
context={
'request': mock.MagicMock(user=UserFactory())
}
)
data = serializer.to_representation(enterprise_offer)
assert data['remaining_applications_for_user'] == expected_remaining_applications_for_user

@ddt.data(
(2, 2),
(None, None)
)
@ddt.unpack
def test_serialize_remaining_applications(
self,
max_global_applications,
expected_remaining_applications,
):
enterprise_customer_uuid = str(uuid4())
condition = extended_factories.EnterpriseCustomerConditionFactory(
enterprise_customer_uuid=enterprise_customer_uuid
)
enterprise_offer = extended_factories.EnterpriseOfferFactory.create(
partner=self.partner,
condition=condition,
max_global_applications=max_global_applications,
)
serializer = EnterpriseLearnerOfferApiSerializer(
data=enterprise_offer,
context={
'request': mock.MagicMock(user=UserFactory())
}
)
data = serializer.to_representation(enterprise_offer)
assert data['remaining_applications'] == expected_remaining_applications