Skip to content

feat: implement SHOW_REGISTRATION_LINKS setting - #1065

Closed
CefBoud wants to merge 1 commit into
openedx:masterfrom
CefBoud:add_show_registration_links_setting
Closed

feat: implement SHOW_REGISTRATION_LINKS setting#1065
CefBoud wants to merge 1 commit into
openedx:masterfrom
CefBoud:add_show_registration_links_setting

Conversation

@CefBoud

@CefBoud CefBoud commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

Description

This PR implements the newly added SHOW_REGISTRATION_LINKS setting whose purpose is to hide the public registration functionality without disabling the register API.

ALLOW_PUBLIC_ACCOUNT_CREATION is already taken into account in the frontend-app-authn and is expected to be set through the runtime configuration.

This PR introduces the possibility of adding the SHOW_REGISTRATION_LINKS setting to the MFE runtime config so it can be taken into account for hiding the register functionality without disabling the API, thus replicating the legacy LMS flow.

How Has This Been Tested?

The steps to test are as follows:

  1. LMS configuration:
    • Set ENABLE_MFE_CONFIG_API to True in the LMS settings
    • Set SHOW_REGISTRATION_LINKS to False in the MFE_CONFIG dict in the LMS settings
  2. frontend-app-authn configuration:
  3. Launch LMS + frontend-app-authn in Devstack
  4. Head to http://localhost:1999/login. There should be no register tab.

NB: mind the MFE_CONFIG_API_CACHE_TIMEOUT setting if you want to play around with MFE_CONFIG. The result is cached for 5min by default. The runtime config also relies on client-side caching, so better to rely on private browsing if there is a config change.

Screenshots/sandbox (optional):

Include a link to the sandbox for design changes or screenshot for before and after. Remove this section if its not applicable.

Before After
image image

Merge Checklist

  • If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Sandbox, if applicable.
  • Is there adequate test coverage for your changes?

Post-merge Checklist

  • Deploy the changes to prod after verifying on stage or ask @openedx/vanguards to do it.
  • 🎉 🙌 Celebrate! Thanks for your contribution.

@CefBoud
CefBoud requested a review from a team October 2, 2023 10:16
@openedx-webhooks

openedx-webhooks commented Oct 2, 2023

Copy link
Copy Markdown

Thanks for the pull request, @CefBoud! 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.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 2, 2023
@CefBoud
CefBoud marked this pull request as draft October 2, 2023 10:17
@CefBoud CefBoud closed this Oct 2, 2023
@openedx-webhooks

Copy link
Copy Markdown

@CefBoud Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

jignaciopm pushed a commit to eduNEXT/frontend-app-authn that referenced this pull request Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants