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

feat: Publish mobile SKUs to LMS #4057

Merged
merged 1 commit into from
Nov 23, 2023
Merged

feat: Publish mobile SKUs to LMS #4057

merged 1 commit into from
Nov 23, 2023

Conversation

moeez96
Copy link
Contributor

@moeez96 moeez96 commented Nov 21, 2023

⛔️ DEPRECATION WARNING

This repository is deprecated and in maintainence-only operation while we work on a replacement, please see this announcement for more information.

Although we have stopped integrating new contributions, we always appreciate security disclosures and patches sent to [email protected]

Anyone internally merging to this repository is expected to release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.

Required Testing

  • Before deploying this change, complete a purchase in the stage environment.
    (^ We can remove that manual check once REV-2624 is done and the corresponding e2e test runs again)

Description

This PR adds to changes to the publishing of seats of a course to the LMS as Course modes:

  1. Do not include mobile seats while serializing course seats to be published as course modes in LMS.
  2. Add mobile SKUs value in serialized seat.

Supporting information

Jira ticket: https://2u-internal.atlassian.net/browse/LEARNER-9682
Supporting LMS PR: openedx/edx-platform#33754

Testing instructions

  1. Create verified mobile seats for a course and add SKU in the mobile seats prefixed with 'mobile.android.' and 'mobile.ios.'
  2. Enter ecommerce shell on local and publish the course you created the seat for.
  3. Verify in the debugger that SKUs are added to the serialized data sent to the LMS. SKU values will be added in the verified seat serialized JSON, while the audit seat will have None values for mobile SKUs.

Copy link
Contributor

@zubair-ce07 zubair-ce07 left a comment

Choose a reason for hiding this comment

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

LGTM. We will need to take care of price_excl_tax field as in latest django-oscar version it is renamed to price. But for now its 👍

@moeez96 moeez96 merged commit e53555c into master Nov 23, 2023
10 checks passed
@moeez96 moeez96 deleted the LEARNER-9682 branch November 23, 2023 05:35
christopappas pushed a commit that referenced this pull request Dec 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants