edx-platform icon indicating copy to clipboard operation
edx-platform copied to clipboard

FC-0001: ecommerce Basket Page -> micro-frontend

Open UvgenGen opened this issue 2 years ago • 5 comments

Description

Replace the Basket page with a new micro-frontend-based implementation: edx/frontend-app-payment

Supporting information

https://github.com/openedx/public-engineering/issues/68 Original Jira Issue: https://openedx.atlassian.net/browse/DEPR-42 PR to ecommerce: https://github.com/openedx/ecommerce/pull/3829 https://github.com/openedx/ecommerce/pull/3718

UvgenGen avatar May 12 '22 10:05 UvgenGen

Thanks for the pull request, @UvgenGen! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

openedx-webhooks avatar May 12 '22 10:05 openedx-webhooks

@dianakhuang Can you review and check with Purchase team if they are ok with you reviewing/merging this ?

natabene avatar May 17 '22 17:05 natabene

@UvgenGen I'm looking into why so many tests are getting cancelled, but the failing test looks legitimate. It wants makemigrations run for the change to the default text for CommerceConfiguration.basket_checkout_page.

E assert 'No changes detected' in "Migrations for 'commerce':\n lms/djangoapps/commerce/migrations/0009_alter_commerceconfiguration_basket_checkout_pag..., help_text='Path to course(s) checkout page hosted by the E-Commerce service.', max_length=255),\n ),\n ]\n"

bmtcril avatar Sep 26 '22 12:09 bmtcril

@bmtcril Should I add a migration in this PR or add skip for this test and add a migration separately?

UvgenGen avatar Sep 26 '22 17:09 UvgenGen

@UvgenGen it will need to be part of this PR.

bmtcril avatar Sep 26 '22 20:09 bmtcril

@bmtcril - would you mind reviewing the latest commit?

mphilbrick211 avatar Dec 12 '22 21:12 mphilbrick211

Hi @bmtcril - just following up on this to see if this can be reviewed/merged?

mphilbrick211 avatar Jan 23 '23 21:01 mphilbrick211