Skip to content

feat: implements SHOW_REGISTRATION_LINKS feature toggle - #32783

Merged
navinkarkera merged 1 commit into
openedx:masterfrom
open-craft:yusuf-musleh/hide-registration-form-feature-toggle
Sep 1, 2023
Merged

feat: implements SHOW_REGISTRATION_LINKS feature toggle#32783
navinkarkera merged 1 commit into
openedx:masterfrom
open-craft:yusuf-musleh/hide-registration-form-feature-toggle

Conversation

@yusuf-musleh

Copy link
Copy Markdown
Member

Description

This PR implements a SHOW_REGISTRATION_LINKS configuration variable to allow hiding the links to the registration form, while keeping the functionality for public registration enabled, through the /register url directly or through the API.

(cherry picked from commit 3025ab5)

Supporting information

Private-ref: BB-7694

Testing instructions

  1. Set the SHOW_REGISTRATION_LINKS flag to False in both lms and cms
  2. Check LMS: http://localhost:18000/ has no register button at the top
  3. Check LMS: http://localhost:18000/register still works does work
  4. Check LMS: http://localhost:18000/login doesn't show links to the registration form
  5. Check CMS (studio): http://localhost:18010/ doesn't show links to register an account

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 19, 2023
@openedx-webhooks

openedx-webhooks commented Jul 19, 2023

Copy link
Copy Markdown

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

@yusuf-musleh
yusuf-musleh force-pushed the yusuf-musleh/hide-registration-form-feature-toggle branch 3 times, most recently from b3e65cf to 6683c28 Compare July 20, 2023 08:44
Comment thread lms/static/js/student_account/views/FormView.js Outdated
@yusuf-musleh
yusuf-musleh force-pushed the yusuf-musleh/hide-registration-form-feature-toggle branch from 6683c28 to 14f8da3 Compare August 1, 2023 11:20

@viadanna viadanna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved 👍

  • I tested this:
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@yusuf-musleh
yusuf-musleh force-pushed the yusuf-musleh/hide-registration-form-feature-toggle branch from 14f8da3 to 2b4c41a Compare August 2, 2023 11:47
@yusuf-musleh

Copy link
Copy Markdown
Member Author

Thanks @viadanna.

@mphilbrick211 This PR is ready.

@mphilbrick211

Copy link
Copy Markdown

Hi @openedx/vanguards! Is this something one of you could help review/merge? thanks!

@mphilbrick211 mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Aug 23, 2023
@zainab-amir

Copy link
Copy Markdown
Contributor

@mphilbrick211 yes, we can help review this. I will create a ticket in the backlog. Please note we are in the middle of a sprint and will be able to pick it next week.

@mphilbrick211

Copy link
Copy Markdown

@mphilbrick211 yes, we can help review this. I will create a ticket in the backlog. Please note we are in the middle of a sprint and will be able to pick it next week.

@zainab-amir - perfect, thanks so much!

@mubbsharanwar mubbsharanwar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this flag and how is this different from ALLOW_PUBLIC_ACCOUNT_CREATION ?

@yusuf-musleh

Copy link
Copy Markdown
Member Author

What is the purpose of this flag and how is this different from ALLOW_PUBLIC_ACCOUNT_CREATION ?

@mubbsharanwar ALLOW_PUBLIC_ACCOUNT_CREATION completely disables new account creation, however this flag SHOW_REGISTRATION_LINKS just hides/shows the UI needed to reach the signup page/form while the account creation functionality is still enabled, learners would be able to sign up by going to the register page URL directly and create an account.

@zainab-amir

Copy link
Copy Markdown
Contributor

@yusuf-musleh can you comment on the product vision for this? In what cases would we want the link to not be visible but registration page to be accessible via url?

@yusuf-musleh

Copy link
Copy Markdown
Member Author

@zainab-amir The main use case for this is when you want to limit/enforce your users (learners) to sign up through your own custom portal that makes API calls to LMS in order to create new users. This could be to enforce a certain standard or requirement that you have for your organization.

Setting the ALLOW_PUBLIC_ACCOUNT_CREATION flag to False breaks that, as it completely disables the account creation. We've implemented this for a client that had this use case setup and thought it might be useful to upstream it as it is generic.

Accessing the /register page through the url directly is sort of a nice to have/fallback in case you want to manually send the link to your learners, but isn't the main benefit gained from this change.

@mubbsharanwar

Copy link
Copy Markdown
Contributor

Hi @yusuf-musleh,
You added this feature for the legacy design/flow. Please replicate the same feature for the new micro frontend frontend-app-authn.

@yusuf-musleh
yusuf-musleh force-pushed the yusuf-musleh/hide-registration-form-feature-toggle branch from 2ee57b7 to ddeff2b Compare September 1, 2023 11:35
@navinkarkera
navinkarkera force-pushed the yusuf-musleh/hide-registration-form-feature-toggle branch from ddeff2b to 0631059 Compare September 1, 2023 12:44
@navinkarkera
navinkarkera merged commit 26d8c21 into openedx:master Sep 1, 2023
@navinkarkera
navinkarkera deleted the yusuf-musleh/hide-registration-form-feature-toggle branch September 1, 2023 13:06
@openedx-webhooks

Copy link
Copy Markdown

@yusuf-musleh 🎉 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
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@yusuf-musleh

Copy link
Copy Markdown
Member Author

Hi @yusuf-musleh, You added this feature for the legacy design/flow. Please replicate the same feature for the new micro frontend frontend-app-authn.

@mubbsharanwar Thanks for the review/approval, I've created an internal ticket to replicate the same functionality on the new micro frontend.

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 waiting for eng review PR is ready for review. Review and merge it, or suggest changes.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

8 participants