mitodl / mitxonline

BSD 3-Clause "New" or "Revised" License
4 stars 2 forks source link

Open edX: hide "Order History" from the MFE user menu when it isn't configured #172

Closed pdpinch closed 2 years ago

pdpinch commented 3 years ago

As a open edX operator, I'd like to be able to run the site without ecommerce (i.e. courses are free). If I don't configure a URL for "Order History" it shouldn't appear in the user menu.

Acceptance Criteria:

Related issues

We have a couple of other MFE issues currently open:

pdpinch commented 3 years ago

Wow, your PR was merged fast! If I had a chance to review, I would have asked for the commit message to follow conventional commmits. I think you just needed a "fix: " at the start of it.

umarmughal824 commented 3 years ago

@pdpinch As the upstream PR is merged so what's next should I close this ticket?

pdpinch commented 3 years ago

Can you coordinate with @mitodl/devops to:

  1. Configure the setting for ORDER_HISTORY_URL
  2. deploy the MFE to the various mitxonline instances
blarghmatey commented 3 years ago

What is the intended destination for that URL?

umarmughal824 commented 3 years ago

@blarghmatey I have merged the PR in upstream with working screenshots. There you could get the idea from these screenshots how to configure it https://github.com/edx/frontend-component-header-edx/pull/161

blarghmatey commented 3 years ago

Thanks, understood now. We'll update the build to set that to an empty value to hide the link.

blarghmatey commented 3 years ago

I set the value for the learner MFE build, but it seems that we'll need to wait for that project to update its dependencies to use the newer version of the header component.

umarmughal824 commented 3 years ago

@pdpinch @blarghmatey they have already created a new release after merging my PR, we could use it https://github.com/edx/frontend-component-header-edx/releases/tag/v5.0.12

pdpinch commented 3 years ago

What's the dependency chain from front-app-learning to frontend-component-header-edx? Do we need to open another PR (or PRs) to see Umar's change?

umarmughal824 commented 3 years ago

What's the dependency chain from front-app-learning to frontend-component-header-edx? Do we need to open another PR (or PRs) to see Umar's change?

@blarghmatey could you please confirm that?

umarmughal824 commented 3 years ago

further work will be done on this ticket https://github.com/mitodl/mitxonline/issues/252

pdpinch commented 2 years ago

I think we need to also make this change in frontend-component-header

See https://github.com/openedx/build-test-release-wg/issues/109#issuecomment-974658913

It sounds like someone from OpenCraft is planning to open a PR

asadiqbal08 commented 2 years ago

I believe, It should be fixed now as some of the Header MFE PRs are merged and we added the corresponding condition there to take care of it. We need to test it after deployment. Any instance where we have deployed the frontend-component-header-mitol

@pdpinch @blarghmatey @Wassaf-Shahzad

pdpinch commented 2 years ago

@briangrossman @asadiqbal08 can we close this?

briangrossman commented 2 years ago

@pdpinch Yes. Now we can.