Skip to content

C
course-discovery
Commit 907c3b4e by Clinton Blackburn
Options

Merge pull request #70 from edx/clintonb/seat-data-loader-fix 

Updated E-Commerce API data loader
parents e1dc2fca 52370d8f
Showing with 84 additions and 37 deletions
course_discovery/apps/course_metadata/data_loaders.py
...@@ -343,19 +343,21 @@ class EcommerceApiDataLoader(AbstractDataLoader): ...@@ -343,19 +343,21 @@ class EcommerceApiDataLoader(AbstractDataLoader):
logger.warning('Could not find course run [%s]', course_run_key) logger.warning('Could not find course run [%s]', course_run_key)
return None return None
for product in body['products']: for product_body in body['products']:
if product['structure'] != 'child': if product_body['structure'] != 'child':
continue continue
product = self.clean_strings(product) product_body = self.clean_strings(product_body)
self.update_seat(course_run, product) self.update_seat(course_run, product_body)
# Remove seats which no longer exist for that course run # Remove seats which no longer exist for that course run
certificate_types = [self.get_certificate_type(product) for product in body['products'] certificate_types = [self.get_certificate_type(product) for product in body['products']
if product['structure'] == 'child'] if product['structure'] == 'child']
course_run.seats.exclude(type__in=certificate_types).delete() course_run.seats.exclude(type__in=certificate_types).delete()
def update_seat(self, course_run, product): def update_seat(self, course_run, product_body):
currency_code = product['stockrecords'][0]['price_currency'] stock_record = product_body['stockrecords'][0]
currency_code = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
try: try:
currency = Currency.objects.get(code=currency_code) currency = Currency.objects.get(code=currency_code)
...@@ -363,22 +365,23 @@ class EcommerceApiDataLoader(AbstractDataLoader): ...@@ -363,22 +365,23 @@ class EcommerceApiDataLoader(AbstractDataLoader):
logger.warning("Could not find currency [%s]", currency_code) logger.warning("Could not find currency [%s]", currency_code)
return None return None
product_values = { attributes = {attribute['name']: attribute['value'] for attribute in product_body['attribute_values']}
'type': Seat.AUDIT,
'currency': currency, seat_type = attributes.get('certificate_type', Seat.AUDIT)
'upgrade_deadline': product.get('expires'), credit_provider = attributes.get('credit_provider')
'price': Decimal(product.get('price', 0.0)),
} credit_hours = attributes.get('credit_hours')
if credit_hours:
credit_hours = int(credit_hours)
for att in product['attribute_values']: defaults = {
if att['name'] == 'certificate_type': 'price': price,
product_values['type'] = att['value'] 'upgrade_deadline': self.parse_date(product_body.get('expires')),
elif att['name'] == 'credit_provider': 'credit_hours': credit_hours,
product_values['credit_provider'] = att['value'] }
elif att['name'] == 'credit_hours':
product_values['credit_hours'] = att['value']
course_run.seats.update_or_create(type=product.get('type'), defaults=product_values) course_run.seats.update_or_create(type=seat_type, credit_provider=credit_provider, currency=currency,
defaults=defaults)
def get_certificate_type(self, product): def get_certificate_type(self, product):
return next( return next(
......
course_discovery/apps/course_metadata/tests/test_data_loaders.py
""" Tests for data loaders. """ """ Tests for data loaders. """
import datetime import datetime
import json import json
from decimal import Decimal
from urllib.parse import parse_qs, urlparse, urljoin from urllib.parse import parse_qs, urlparse, urljoin
import ddt import ddt
...@@ -10,7 +11,7 @@ from django.test import TestCase, override_settings ...@@ -10,7 +11,7 @@ from django.test import TestCase, override_settings
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from pytz import UTC from pytz import UTC
from course_discovery.apps.course_metadata.data_loaders import( from course_discovery.apps.course_metadata.data_loaders import (
OrganizationsApiDataLoader, CoursesApiDataLoader, DrupalApiDataLoader, EcommerceApiDataLoader, AbstractDataLoader OrganizationsApiDataLoader, CoursesApiDataLoader, DrupalApiDataLoader, EcommerceApiDataLoader, AbstractDataLoader
) )
from course_discovery.apps.course_metadata.models import ( from course_discovery.apps.course_metadata.models import (
...@@ -155,6 +156,9 @@ class OrganizationsApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -155,6 +156,9 @@ class OrganizationsApiDataLoaderTests(DataLoaderTestMixin, TestCase):
for datum in data: for datum in data:
self.assert_organization_loaded(datum) self.assert_organization_loaded(datum)
# Verify multiple calls to ingest data do NOT result in data integrity errors.
self.loader.ingest()
@ddt.ddt @ddt.ddt
@override_settings(COURSES_API_URL=COURSES_API_URL) @override_settings(COURSES_API_URL=COURSES_API_URL)
...@@ -302,6 +306,9 @@ class CoursesApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -302,6 +306,9 @@ class CoursesApiDataLoaderTests(DataLoaderTestMixin, TestCase):
for datum in data: for datum in data:
self.assert_course_run_loaded(datum) self.assert_course_run_loaded(datum)
# Verify multiple calls to ingest data do NOT result in data integrity errors.
self.loader.ingest()
def test_get_pacing_type_field_missing(self): def test_get_pacing_type_field_missing(self):
""" Verify the method returns None if the API response does not include a pacing field. """ """ Verify the method returns None if the API response does not include a pacing field. """
self.assertIsNone(self.loader.get_pacing_type({})) self.assertIsNone(self.loader.get_pacing_type({}))
...@@ -523,6 +530,9 @@ class DrupalApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -523,6 +530,9 @@ class DrupalApiDataLoaderTests(DataLoaderTestMixin, TestCase):
Course.objects.get(key=self.EXISTING_COURSE['course_key'], title=self.EXISTING_COURSE['title']) Course.objects.get(key=self.EXISTING_COURSE['course_key'], title=self.EXISTING_COURSE['title'])
# Verify multiple calls to ingest data do NOT result in data integrity errors.
self.loader.ingest()
@ddt.data( @ddt.data(
('', ''), ('', ''),
('<h1>foo</h1>', '# foo'), ('<h1>foo</h1>', '# foo'),
...@@ -555,10 +565,10 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -555,10 +565,10 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
loader_class = EcommerceApiDataLoader loader_class = EcommerceApiDataLoader
def mock_api(self): def mock_api(self):
course_run_audit = CourseRunFactory(title='audit') course_run_audit = CourseRunFactory(title_override='audit')
course_run_verified = CourseRunFactory(title='verified') course_run_verified = CourseRunFactory(title_override='verified')
course_run_credit = CourseRunFactory(title='credit') course_run_credit = CourseRunFactory(title_override='credit')
course_run_no_currency = CourseRunFactory(title='no currency') course_run_no_currency = CourseRunFactory(title_override='no currency')
# create existing seats to be removed by ingest # create existing seats to be removed by ingest
SeatFactory(course_run=course_run_audit, type=Seat.PROFESSIONAL) SeatFactory(course_run=course_run_audit, type=Seat.PROFESSIONAL)
...@@ -580,12 +590,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -580,12 +590,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
}, },
{ {
"structure": "child", "structure": "child",
"price": "0.00",
"expires": None, "expires": None,
"attribute_values": [], "attribute_values": [],
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "USD", "price_currency": "USD",
"price_excl_tax": "0.00",
} }
] ]
} }
...@@ -604,7 +614,6 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -604,7 +614,6 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
}, },
{ {
"structure": "child", "structure": "child",
"price": "0.00",
"expires": None, "expires": None,
"attribute_values": [ "attribute_values": [
{ {
...@@ -615,12 +624,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -615,12 +624,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "EUR", "price_currency": "EUR",
"price_excl_tax": "0.00",
} }
] ]
}, },
{ {
"structure": "child", "structure": "child",
"price": "25.00",
"expires": "2017-01-01T12:00:00Z", "expires": "2017-01-01T12:00:00Z",
"attribute_values": [ "attribute_values": [
{ {
...@@ -631,12 +640,15 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -631,12 +640,15 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "EUR", "price_currency": "EUR",
"price_excl_tax": "25.00",
} }
] ]
} }
] ]
}, },
{ {
# This credit course has two credit seats to verify we are correctly finding/updating using the credit
# provider field.
"id": course_run_credit.key, "id": course_run_credit.key,
"products": [ "products": [
{ {
...@@ -649,18 +661,17 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -649,18 +661,17 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
}, },
{ {
"structure": "child", "structure": "child",
"price": "0.00",
"expires": None, "expires": None,
"attribute_values": [], "attribute_values": [],
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "USD", "price_currency": "USD",
"price_excl_tax": "0.00",
} }
] ]
}, },
{ {
"structure": "child", "structure": "child",
"price": "25.00",
"expires": "2017-01-01T12:00:00Z", "expires": "2017-01-01T12:00:00Z",
"attribute_values": [ "attribute_values": [
{ {
...@@ -671,12 +682,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -671,12 +682,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "USD", "price_currency": "USD",
"price_excl_tax": "25.00",
} }
] ]
}, },
{ {
"structure": "child", "structure": "child",
"price": "250.00",
"expires": "2017-06-01T12:00:00Z", "expires": "2017-06-01T12:00:00Z",
"attribute_values": [ "attribute_values": [
{ {
...@@ -699,6 +710,35 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -699,6 +710,35 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "USD", "price_currency": "USD",
"price_excl_tax": "250.00",
}
]
},
{
"structure": "child",
"expires": "2017-06-01T12:00:00Z",
"attribute_values": [
{
"name": "certificate_type",
"value": "credit"
},
{
"name": "credit_hours",
"value": 2
},
{
"name": "credit_provider",
"value": "acme"
},
{
"name": "verification_required",
"value": False
},
],
"stockrecords": [
{
"price_currency": "USD",
"price_excl_tax": "250.00",
} }
] ]
} }
...@@ -717,12 +757,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -717,12 +757,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
}, },
{ {
"structure": "child", "structure": "child",
"price": "0.00",
"expires": None, "expires": None,
"attribute_values": [], "attribute_values": [],
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "123", "price_currency": "123",
"price_excl_tax": "0.00",
} }
] ]
} }
...@@ -741,12 +781,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -741,12 +781,12 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
}, },
{ {
"structure": "child", "structure": "child",
"price": "0.00",
"expires": None, "expires": None,
"attribute_values": [], "attribute_values": [],
"stockrecords": [ "stockrecords": [
{ {
"price_currency": "USD", "price_currency": "USD",
"price_excl_tax": "0.00",
} }
] ]
} }
...@@ -794,8 +834,9 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -794,8 +834,9 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
# Validate each seat # Validate each seat
for product in products: for product in products:
price_currency = product['stockrecords'][0]['price_currency'] stock_record = product['stockrecords'][0]
price = float(product.get('price', 0.0)) price_currency = stock_record['price_currency']
price = Decimal(stock_record['price_excl_tax'])
certificate_type = Seat.AUDIT certificate_type = Seat.AUDIT
credit_provider = None credit_provider = None
credit_hours = None credit_hours = None
...@@ -814,7 +855,7 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -814,7 +855,7 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
elif att['name'] == 'credit_hours': elif att['name'] == 'credit_hours':
credit_hours = att['value'] credit_hours = att['value']
seat = course_run.seats.get(type=certificate_type) seat = course_run.seats.get(type=certificate_type, credit_provider=credit_provider, currency=price_currency)
self.assertEqual(seat.course_run, course_run) self.assertEqual(seat.course_run, course_run)
self.assertEqual(seat.type, certificate_type) self.assertEqual(seat.type, certificate_type)
...@@ -826,7 +867,7 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -826,7 +867,7 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
@responses.activate @responses.activate
def test_ingest(self): def test_ingest(self):
""" Verify the method ingests data from the Courses API. """ """ Verify the method ingests data from the E-Commerce API. """
data = self.mock_api() data = self.mock_api()
loaded_course_run_data = data[:-1] loaded_course_run_data = data[:-1]
loaded_seat_data = data[:-2] loaded_seat_data = data[:-2]
...@@ -846,6 +887,9 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase): ...@@ -846,6 +887,9 @@ class EcommerceApiDataLoaderTests(DataLoaderTestMixin, TestCase):
for datum in loaded_seat_data: for datum in loaded_seat_data:
self.assert_seats_loaded(datum) self.assert_seats_loaded(datum)
# Verify multiple calls to ingest data do NOT result in data integrity errors.
self.loader.ingest()
@ddt.unpack @ddt.unpack
@ddt.data( @ddt.data(
({"attribute_values": []}, Seat.AUDIT), ({"attribute_values": []}, Seat.AUDIT),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment