Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create a new setting USE_DISCUSSIONS_MFE #809

Merged
merged 9 commits into from
Mar 7, 2024

Conversation

luisfelipec95
Copy link

@luisfelipec95 luisfelipec95 commented Feb 27, 2024

Description

This PR adds a new setting USE_DISCUSSIONS_MFE that will allow Discussions MFE to be enabled or disabled in the tenant

Supporting information

JIRA Issue DS-806

Testing instructions

  1. Run a Palm environment using TVM or the Tutor docs in its Palm version
  2. Use this branch as your edx-platform version adding in the config.yml the following
EDX_PLATFORM_REPOSITORY: https://github.com/eduNEXT/edunext-platform.git
EDX_PLATFORM_VERSION: lfc/enable-discussions-mfe
  1. Install tutor-mfe directly in the Tutor environment tutor plugins install mfe, and enable it tutor plugins enable mfe
  2. tutor plugins install forum
  3. Update the environment settings tutor config save
  4. tutor dev launch
  5. Go to both container tutor dev exec lms/cms bash, install eox-tenant pip install eox-tenant, and run the migrations ./manage.py lms makemigrations and ./manage.py lms migrate
  6. Restart the containers tutor dev restart
  7. Go to the admin panel and create 2 new routes:
  • Domain name: site1.local.overhang.io
    {
        "DISCUSSIONS_MICROFRONTEND_URL": "http://site1.local.overhang.io:2002/discussions",
        "EDNX_USE_SIGNAL": true,
        "LEARNING_MICROFRONTEND_URL": "http://site1.local.overhang.io:2000/learning",
        "LMS_BASE": "site1.local.overhang.io:8000",
        "LMS_ROOT_URL": "http://site1.local.overhang.io:8000",
        "MFE_CONFIG": {
            "EXAMS_BASE_URL": "",
            "LMS_BASE_URL": "http://site1.local.overhang.io",
            "LOGIN_URL": "http://site1.local.overhang.io/login",
            "LOGOUT_URL": "http://site1.local.overhang.io/logout"
        },
        "PLATFORM_NAME": "site1",
        "SITE_NAME": "site1.local.overhang.io",
        "USE_DISCUSSIONS_MFE": false,
        "course_org_filter": [
            "site1"
        ]
    }
  • Domain name: site2.local.overhang.io
    {
        "DISCUSSIONS_MICROFRONTEND_URL": "http://site2.local.overhang.io:2002/discussions",
        "EDNX_USE_SIGNAL": true,
        "LEARNING_MICROFRONTEND_URL": "http://site2.local.overhang.io:2000/learning",
        "LMS_BASE": "site2.local.overhang.io:8000",
        "LMS_ROOT_URL": "http://site2.local.overhang.io:8000",
        "MFE_CONFIG": {
            "EXAMS_BASE_URL": "",
            "LMS_BASE_URL": "http://site2.local.overhang.io",
            "LOGIN_URL": "http://site2.local.overhang.io/login",
            "LOGOUT_URL": "http://site2.local.overhang.io/logout"
        },
        "PLATFORM_NAME": "site2",
        "SITE_NAME": "site2.local.overhang.io",
        "USE_DISCUSSIONS_MFE": true,
        "course_org_filter": [
            "site2"
        ]
    }
  1. Go to Studio and create two new courses one of them with the organization site1 and the other one with the organization site2
  2. Go to http://site1.local.overhang.io:8000/dashboard and clic on "Resume Course"
  3. Install CORS Everywhere Extension to prevent CORS error.
  4. Clic on "Discussions" Tab
  5. If USE_DISCUSSIONS_MFE: true:
    image
  6. If USE_DISCUSSIONS_MFE: false:
    image

@luisfelipec95 luisfelipec95 requested a review from a team February 29, 2024 14:49
Copy link

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

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

Additionally, before approving the PR, I would like to see the changes in amberes first because, according to my experience, there may be different settings that can change the way the code is executed, and it would be great to ensure the proper functioning

openedx/core/djangoapps/discussions/utils.py Outdated Show resolved Hide resolved
lms/djangoapps/discussion/tests/test_views.py Outdated Show resolved Hide resolved
luisfelipec95 and others added 2 commits March 5, 2024 17:14
Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>
@luisfelipec95 luisfelipec95 force-pushed the lfc/enable-discussions-mfe branch from 3b2f939 to 877ba07 Compare March 5, 2024 22:14
@bra-i-am bra-i-am self-requested a review March 6, 2024 13:58
Copy link

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

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

I've been testing the MFE in the amberes environment and I don't know if it is even a problem, but when the flag is turned from true to false, the MFE lasts some minutes being disabled; despite the time it takes, everything works as expected.

@luisfelipec95 luisfelipec95 requested a review from a team March 7, 2024 13:04
Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

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

This looks good to me. I only have one suggestion: change the variable name USE_DISCUSSIONS_MFE_FRONTEND in the PR name or when you squash the commit because it is not consistent with the code.
Remember to add the documentation about how to enable or disable this here: https://docs.google.com/document/d/1j_kyDTdDaS1jnXgK0zbJOdz1XJja5lXvBzye8dmUhYA/edit?usp=sharing

@luisfelipec95 luisfelipec95 changed the title feat: create a new setting USE_DISCUSSIONS_MFE_FRONTEND feat: create a new setting USE_DISCUSSIONS_MFE Mar 7, 2024
@luisfelipec95 luisfelipec95 merged commit fe07703 into ednx-release/palma.master Mar 7, 2024
35 of 36 checks passed
@luisfelipec95
Copy link
Author

This looks good to me. I only have one suggestion: change the variable name USE_DISCUSSIONS_MFE_FRONTEND in the PR name or when you squash the commit because it is not consistent with the code. Remember to add the documentation about how to enable or disable this here: https://docs.google.com/document/d/1j_kyDTdDaS1jnXgK0zbJOdz1XJja5lXvBzye8dmUhYA/edit?usp=sharing

Thanks!!

bra-i-am added a commit that referenced this pull request Apr 16, 2024
* feat: create a new setting USE_DISCUSSIONS_MFE

* feat: add new setting USE_DISCUSSIONS_MFE

* fix: quality checks

* fix: add setting override

* fix: add setting override

* fix: add setting override

* fix: pep 8 violations

* fix: update setting name

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>

* fix: change variable name

---------

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>
bra-i-am added a commit that referenced this pull request Apr 16, 2024
* feat: create a new setting USE_DISCUSSIONS_MFE

* feat: add new setting USE_DISCUSSIONS_MFE

* fix: quality checks

* fix: add setting override

* fix: add setting override

* fix: add setting override

* fix: pep 8 violations

* fix: update setting name

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>

* fix: change variable name

---------

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>
dcoa pushed a commit that referenced this pull request Aug 7, 2024
* feat: create a new setting USE_DISCUSSIONS_MFE

* feat: add new setting USE_DISCUSSIONS_MFE

* fix: quality checks

* fix: add setting override

* fix: add setting override

* fix: add setting override

* fix: pep 8 violations

* fix: update setting name

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>

* fix: change variable name

---------

Co-authored-by: Brayan Cerón <86393372+bra-i-am@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants