Skip to content

Comments

FC-0001: Account pages -> micro-frontend#30336

Merged
abdullahwaheed merged 7 commits intoopenedx:masterfrom
raccoongang:depr/account-pages
Mar 7, 2023
Merged

FC-0001: Account pages -> micro-frontend#30336
abdullahwaheed merged 7 commits intoopenedx:masterfrom
raccoongang:depr/account-pages

Conversation

@UvgenGen
Copy link
Contributor

@UvgenGen UvgenGen commented May 2, 2022

Merge after the Olive release.

Description

replaced edx-platform's older implementation of the Django-server-side rendering of the following pages with new micro-frontend-based implementations:

PR to frontend-platform: openedx/frontend-platform#326
PR to frontend-component-header: openedx/frontend-component-header#210

Supporting information

openedx/public-engineering#71
Original Jira Issue: https://openedx.atlassian.net/browse/DEPR-17

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels May 2, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented May 2, 2022

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.

@natabene
Copy link
Contributor

natabene commented May 3, 2022

@dianakhuang This seems to be with arch-bom, do you mind reviewing and merging?

@abdullahwaheed
Copy link
Contributor

@UvgenGen i have reviewed and tested your changes and resolved merge conflicts locally. I cannot push my changes in your forked repo. Please either give me write access to your repo or i'll provide the patch file for changes.

@UvgenGen
Copy link
Contributor Author

@abdullahwaheed Could you provide the patch file please?

@abdullahwaheed
Copy link
Contributor

@UvgenGen the patch couldn't created for all the commits as i have merged it with master and resolved conflicts only. Could you please rebase it with master and i'll help you resolve the conflicts

@UvgenGen
Copy link
Contributor Author

@abdullahwaheed yes, sure. I rebased this PR and resolved conflicts. Files affected: lms/djangoapps/discussion/views.py and common/djangoapps/Third_Party_auth/tests/specs/base.py.

@arbrandes arbrandes linked an issue Oct 18, 2022 that may be closed by this pull request
Copy link
Contributor

@abdullahwaheed abdullahwaheed left a comment

Choose a reason for hiding this comment

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

tested with respective changes. LGTM

@mphilbrick211 mphilbrick211 added awaiting prioritization FC Relates to an Axim Funded Contribution project labels Dec 16, 2022
@abdullahwaheed
Copy link
Contributor

@UvgenGen could you please rebase it again

@UvgenGen
Copy link
Contributor Author

@abdullahwaheed PR rebased)

@e0d
Copy link
Contributor

e0d commented Jan 20, 2023

@UvgenGen should the name of this PR be updated? We're after olive, can this now be merged?

@UvgenGen
Copy link
Contributor Author

@e0d I removed Note: don't merge in description, did I understand you correctly?. Yes, I think nothing prevents merge it now)

@e0d
Copy link
Contributor

e0d commented Jan 20, 2023

@dianakhuang is this good to merge now?
@arbrandes FYI as MFE guru and Olive release lead.

Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@dianakhuang
Copy link
Contributor

I will try to get it on FED-BOM's agenda to review and help get this merged. Very excited for this to land!

@abdullahwaheed
Copy link
Contributor

@UvgenGen need rebasing again due to frequent changes in platform :)
or you can give me write access to your forked repo so i can update changes.

@UvgenGen
Copy link
Contributor Author

@abdullahwaheed PR rebased)

@mphilbrick211
Copy link

mphilbrick211 commented Feb 24, 2023

Hi @UvgenGen @arbrandes @dianakhuang - is this good to merge once the branch conflicts are resolved?

@UvgenGen UvgenGen force-pushed the depr/account-pages branch from ed4e690 to 027e429 Compare March 3, 2023 12:21
@UvgenGen
Copy link
Contributor Author

UvgenGen commented Mar 3, 2023

@mphilbrick211 PR rebased, conflicts resolved)

@mphilbrick211
Copy link

@arbrandes @dianakhuang Would one of you be able to merge this?

@abdullahwaheed abdullahwaheed merged commit 0f02c7b into openedx:master Mar 7, 2023
@openedx-webhooks
Copy link

@UvgenGen 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

dianakhuang pushed a commit that referenced this pull request Mar 7, 2023
rgraber pushed a commit that referenced this pull request Mar 7, 2023
abdullahwaheed added a commit that referenced this pull request Oct 19, 2023
feat: Account and profile MFE legacy removal - redeployment

* Revert "Revert "FC-0001: Account pages -> micro-frontend (#30336)" (#31888)"

This reverts commit 90c4ca6.

* refactor: removed filters test from user_api accounts

---------

Co-authored-by: Bilal Qamar <59555732+BilalQamar95@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[DEPR]: Account pages -> micro-frontend