Skip to content

Commit

Permalink
feat: Stop syncing mobile skus on discovery (#4097)
Browse files Browse the repository at this point in the history
Since mobile skus are not required any place other than ecommerce there is no need to sync those skus on discovery.
Mobile skus will stay in ecommerce and any service who needs this data can fetch them from eommerce.
  • Loading branch information
jawad-khan authored Oct 2, 2023
1 parent 203f3c9 commit 5ffbddc
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 1 deletion.
5 changes: 5 additions & 0 deletions course_discovery/apps/course_metadata/data_loaders/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,11 @@ def update_seat(self, course_run, product_body):
price = Decimal(stock_record['price_excl_tax'])
sku = stock_record['partner_sku']

# For more context see ADR docs/decisions/0025-dont-sync-mobile-skus-on-discovery.rst
if "mobile" in sku:
logger.warning("Skipping mobile seat with sku [%s]", sku)
return

try:
currency = Currency.objects.get(code=currency_code)
except Currency.DoesNotExist:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,24 @@
}
]
},
# Mobile seat which we will discarded since we arn't syncing mobile skus see decicion 0025
{
"structure": "child",
"expires": "2017-01-01T12:00:00Z",
"attribute_values": [
{
"name": "certificate_type",
"value": "verified"
}
],
"stockrecords": [
{
"price_currency": "EUR",
"price_excl_tax": "250.00",
"partner_sku": "mobile.android.sku003",
}
]
},
{
"structure": "standalone",
"expires": "2017-01-01T12:00:00Z",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,9 @@ def get_product_bulk_sku(self, seat_type, course_run, products):
def assert_seats_loaded(self, body, mock_products):
""" Assert a Seat corresponding to the specified data body was properly loaded into the database. """
course_run = CourseRun.objects.get(key=body['id'])
products = [p for p in body['products'] if p['structure'] == 'child']
products = [p for p in body['products'] if p['structure'] == 'child' and
'mobile' not in p['stockrecords'][0]['partner_sku']]

# Verify that the old seat is removed
assert course_run.seats.count() == len(products)
assert course_run.draft_version.seats.count() == len(products)
Expand Down
34 changes: 34 additions & 0 deletions docs/decisions/0025-dont-sync-mobile-skus-on-discovery.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
25. Don't sync mobile skus in discovery
------------------------------------------------------------------

Status
------

Accepted (September 2023).

Context
-------

While implementing in-app payments, the mobile team decided to use the current e-commerce implementation as its backend. This way, all the analytics, sales, and refunds records will stay in a single place.
Mobile platforms (e.g., Play Store and App Store) apply some restrictions on product prices and SKU names. Therefore, a separate verified seat was needed for mobile.
The seat was added on the e-commerce side, and it worked perfectly with Django Oscars on e-commerce. However, when Discovery consumed this data, course publishers started having problems publishing a course run.
During a spike, it turned out that Discovery was expecting only a single verified seat for a course run, and the code was written according to this logic. Not just Discovery, but all other services behave in the same way. Since all services fetch data from Discovery, adding another verified seat directly into Discovery is not a good idea. Any service can start having problems, and we might not even know if there is any problem with that service or not.

Decision
--------

Since mobile SKUs aren't required anywhere other than e-commerce, there is no need to sync those SKUs in Discovery.
Mobile SKUs will stay in e-commerce, and any service that needs this data can fetch them from e-commerce.
Any mobile SKU that is synced in Discovery will be deleted, and all syncing jobs will discard them while fetching data from e-commerce.
Mobile SKUs, as `documented`_, will be created in a certain format, i.e., ``mobile.{platform name}.{web SKU}``. For example, a web SKU with value 123 will have an iOS SKU of ``mobile.ios.123``. In this way, it is easy to identify mobile SKUs from web SKUs, and we can filter any SKU in Discovery that has "mobile" in its name.

.. _documented: https://2u-internal.atlassian.net/wiki/spaces/MOBL/pages/283508791/Enable+IAP+for+a+course+in+mobile#iOS

Consequences
------------

Discovery will not sync mobile SKUs from e-commerce. Mobile SKUs will reside only in the e-commerce service and can be fetched from there.

Any service can differentiate between mobile and web SKUs of the verified seat category by the "mobile" keyword in the SKU value.
When there is a price change on the publisher, the course run will be updated, and Discovery will push updated data to e-commerce.
E-commerce will detect this change and update SKUs' prices on relevant mobile platforms (Android, iOS) through APIs.

0 comments on commit 5ffbddc

Please sign in to comment.