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

feat: Enable mobile payments for new upcoming courses #15

Open
wants to merge 2 commits into
base: 2u/main
Choose a base branch
from

Conversation

jawad-khan
Copy link

@jawad-khan jawad-khan commented Dec 16, 2024

⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH

  • I have checked the branch to which I would like to merge.

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 enables mobile payments for upcoming courses. Whenever a new course is published through publisher ecommerce creates a web seat for it, this PR will create two mobile seats(for ios and android) alongside that mobile seat. In addition ios product will also be created on appstore.

Useful information to include:

  • This change will effect all mobile users.

Supporting information

Jira ticket: https://2u-internal.atlassian.net/browse/LEARNER-9951

Testing instructions

  1. Create course on publsiher and publish it.
  2. Confirm it on django oscar dashboard or on django admin that 2 mobile seats are also created for that course.
  3. Change expiry or price on discovery and verify that changes are reflected on mobile seats.

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, OpenEdx vs. edx.org differences, development vs. production environment differences, security, or accessibility.

self._update_mobile_seats(mobile_seats, web_seat, course)
else:
logger.info("Creating mobile seats for course [%s]", course.id)
create_mobile_seat(ANDROID_SKU_PREFIX, web_seat)
Copy link
Member

Choose a reason for hiding this comment

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

Wrap create_mobile_seat calls in try-except to catch specific failures (e.g., Android or iOS creation).
something like this

Suggested change
create_mobile_seat(ANDROID_SKU_PREFIX, web_seat)
try:
create_mobile_seat(ANDROID_SKU_PREFIX, web_seat)
except Exception as e:
logger.error("Failed to create Android mobile seat for course [%s]: %s", course.id, str(e))
return

self.create_course_and_seats()
updated_data = self.generate_update_payload()

with mock.patch.object(LMSPublisher, 'publish') as mock_publish:
Copy link
Member

Choose a reason for hiding this comment

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

can you remove mock from here and add something like this

Suggested change
with mock.patch.object(LMSPublisher, 'publish') as mock_publish:
@mock.patch.object(LMSPublisher, 'publish', return_value=None)
def test_course_update_creates_ios_seat_when_feature_enabled(self, mock_publish, mock_create_ios_product):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants