Skip to content

[BB-6438] Check if feature is enabled before calling enterprise api - #30693

Merged
macdiesel merged 1 commit into
openedx:masterfrom
open-craft:pooja/fix-page-banner-message-not-seen
Jul 26, 2022
Merged

[BB-6438] Check if feature is enabled before calling enterprise api#30693
macdiesel merged 1 commit into
openedx:masterfrom
open-craft:pooja/fix-page-banner-message-not-seen

Conversation

@pkulkark

@pkulkark pkulkark commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Description

This PR fixes the issue of django messages being read before redirecting to dashboard page, due to enterprise api being
called even if the feature is disabled. Before maple release, there used to be a waffle switch named ENABLE_MULTIPLE_USER_ENTERPRISES_FEATURE that was checked before calling the api. This was deprecated in edx-platform#28057 but was not replaced by any other check for enterprise integration. And so after each login, the enterprise api would be called, which would return 404 but was fetching the django messages (which clears it) and so page banner messages wouldn't show up when the user was redirected to the dashboard page.

Supporting information

JIRA: BB-6438

Testing instructions

  1. Deploy this branch
  2. Create a new user account and activate it
  3. In lms.yml and studio.yml files, under AUTH_PASSWORD_VALIDATORS update the minimum password length requirement to longer than the one you set in the previous step.
  4. Ensure you enable the complaince check by the setting the following config and restart the services:
PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG:
    ENFORCE_COMPLIANCE_ON_LOGIN: true
    GENERAL_USER_COMPLIANCE_DEADLINE: '2022-08-01 00:00:00+00:00'
  1. Now login with the same user credentials and verify that the warning message is seen on the dashboard.

Reviewers
TBD

This fixes the issue of django messages being read before
redirecting to dashboard page, due to enterprise api being
called even if the feature is disabled.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 5, 2022
@openedx-webhooks

Copy link
Copy Markdown

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

@navinkarkera navinkarkera 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.

👍

  • I tested this on sandbox
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@pkulkark

pkulkark commented Jul 7, 2022

Copy link
Copy Markdown
Contributor Author

@natabene This is good for edX's review.

@natabene

Copy link
Copy Markdown
Contributor

@pkulkark Thank you for your contribution!

@natabene

Copy link
Copy Markdown
Contributor

@pkulkark I have lined this up for our review.

@macdiesel
macdiesel merged commit 6d80bdd into openedx:master Jul 26, 2022
@openedx-webhooks

Copy link
Copy Markdown

@pkulkark 🎉 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

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

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.

6 participants