Core Changes - OmnipreneurshipAcademy/edx-platform GitHub Wiki

Core Changes

This page maintains the log of all the core changes that we made in the core code of edx-platform.

Documenting a Core Change

Change Title (PR) (LP-XXXX)

  1. Describe Core Changes. (Add core file paths)
  2. Why this core change is inevitable?
  3. Why can't we move this change to openedx/adg?

1. Registration form Customisation (PR) (LP-2439)

  1. Updated registration form as per the requirements by the ADG team. Here is the link to the design document.

lms/static/js/student_account/views/RegisterView.js removed line which was adding 'hidden' class using $('.optional-fields').addClass('hidden');

openedx/core/djangoapps/user_authn/views/login_form.py Updated import to use overridden RegistrationFormFactory

openedx/core/djangoapps/user_authn/views/register.py Conditionally overridden RegistrationFormFactory import

  1. Overridden the form factory to avoid core changes, used conditional imports to use the overridden form factory and updated RegisterView.js to remove a class on optional fields.

    • Needed to use the same views for registration and design didn't required any customisation so used that by just importing new form factory.
    • Removed redundant class which was hiding elements in RegisterView.js

2. ADG Testing pieline (PR) (LP-2431)

  1. Added new method pytest_collection_modifyitems in edx-platform/conftest.py from line # 38 to 58

Reason: This method is necessary to skip edx tests that are failing. This method cannot be created in any other file.

  1. Updated edx-platform/codecov.yml

Reason: codecov.yml contains configuration for coverage reporting. Its configurations are updated as per ADG needs. This file cannot be moved to any other location.

3. Send all OTB emails using mandrill templates (PR) (LP-2418)

In this PR we send all edx out of the emails using our Mandrill templates as edx is already sending those emails using edx-ace so we aren't removing this implementation as we don't want to break edx unit tests for that we added an implementation that check(is_testing_environment) if server is running in testing environment or local or production environment.

List of core changes

  1. Change AUTOMATIC_AUTH_FOR_TESTING setting in lms/envs/devstack.py at line#323 to send account activation email on local server.
  2. Added account activation email in openedx/core/djangoapps/user_authn/views/register.py > def create_account_with_params() method from line#222-225
  3. Added reset/change password email in openedx/core/djangoapps/user_authn/views/password_reset.py > def save() method from line#186-189
  4. Added enrollment invitation email lms/djangoapps/instructor/enrollment.py > def enroll_email() from line#163-166 and line#175-178
  5. Added following changes in common/djangoapps/student/views/management.py
    1. Added required imports from line#45-50
    2. Added enrollment email in def change_enrollment() from line#369-370
    3. Added email change verification email in def do_email_change_request() from line#681-684
    4. Added email change confirmation emails in def confirm_email_change() from line#792-795 and line#808-811

4. Pytest fitxture added in root conftest.py file

A pytest fitxture mute_signals is added in root conftest.py file so that it can be used explicitly in ADG tests where required.

  • /conftest.py (61 - 79)
Why this core change is inevitable? Why you can't move this change to philu_overrides?

The best place for fixtures which you want to use in any ADG app is conftest.py file at root.

Link of the ticket?

LP-2447

Link of the PR?

PR

5. Sign-in and Forgot password (PR) (LP-2348) LP-2349 (LP-2562 - PR)

  1. Updated form placeholders, instructions and labels as required by the designs here

lms/static/js/student_account/views/LoginView.js updated success message for forgot password

openedx/core/djangoapps/user_authn/views/login_form.py Overridden the get_login_session_form method.

openedx/core/djangoapps/user_authn/views/password_reset.py Overridden the get_password_reset_form method.

webpack.common.config.js Updated path to PasswordResetConfirmation react component as we have overridden and moved it to adg directory

  1. Form needed updates and did those updates in overridden form and used that in place of core forms, Success message was loaded from a backbone view as it was hard coded so had to do core change, Path must be changed for the component as we have overridden it.

    • Overridden methods in adg directory to use those I had to do a few core changes
    • Overridden react component and also had to override the path to that.
    • Success message was hard coded so had to change that in core file. (Success message was changed again under the scope of LP-2562)

6. Theme Integration

  1. Integrated ADG theme

.gitignore added theme folder to ignore from git.

.tx/config ADG transifex configurations added.

conf/locale/config.yaml Updated locales used only ar, en

lms/envs/common.py Updated languages, kept only arabic and english

lms/envs/devstack.py Updated ADG theme DIRS

cms/static/js/i18n/ar/djangojs.js UPDATED TRANSLATIONS

  1. Added and updated required settings for localization. Also updated the translations.

7. Password eye icon integration (PR) (LP-2477)

  1. Integrated ADG theme

cms/envs/common.py Added context processor for cdn link ADG_CONTEXT_PROCESSORS.

cms/envs/common.py ADG context processor for CDN-link added.

  1. Needed to pass cdn link to templates as it was used in many templates.

8. Mailchimp integration (PR) (LP-2447)

  1. Moved installed apps to a separate file and used that to integrate it in installed apps for lms and cms.

cms/envs/common.py Added adg apps named ADG_COMMON_INSTALLED_APPS and SUSPEND_RECEIVERS is added to suspend receivers for testing.

cms/envs/production.py Added mailchimp LIST_ID and API_KEY

cms/envs/test.py Added test settings for mailchimp

conftest.py Added fixture to mute signals while testing.

lms/envs/common.py added setting SUSPEND_RECEIVERS and added ADG_COMMON_INSTALLED_APPS.

lms/envs/production.py Added mailchimp list id and API key settings.

lms/envs/test.py Added test settings for mailchimp list id, API key and suspend receivers.

  1. Added settings required for mailchimp syncing.

9. Header and Footer designs (PR) (LP-2469)

  1. Updated header and footer.

lms/envs/common.py Updated MKTG_URL_LINK_MAP.

  1. Updated marketing link map for header and footer.

10. Setup Mandrill Client for ADG (PR) (LP-2390)

  1. Setup for mandrill client.

.github/workflows/quality.yml Updated github action to install adg requirements.

lms/envs/common.py Added from email address NOTIFICATION_FROM_EMAIL.

lms/envs/production.py Added MANDRILL_API_KEY.

pavelib/prereqs.py Updated PRIVATE_REQS

  1. Added/Updated required settings for mandrill

11. ADG header integration (PR) (LP-2347)

  1. Integration ADG header and added CDN_LINK in settings.

lms/envs/common.py Added CDN_LINK for S3.

  1. Added CDN_LINK for S3

12. ADG setup (PR) (LP-2373)

  1. Added applications app in ADG and setup urls with lms

lms/urls.py Added adg_url_patterns.

  1. Added adg urls in lms urls.

13. Language selector JS (PR 1, post fix PR) (LP-2341)

common/static/js/src/lang_edx.js changed settings_language_selector.

These changes are to support readio button for language switching, out of the box edX just support dropdown to show all languages and to switch b/w the languages.

14. Fix tests (PR) (LP-2500)

  1. Added All languages list in lms/envs/test.py and cms/envs/test.py. (I have added a direct commit to koa-master branch and cms/envs/test.py was updated in that commit.)

lms/envs/test.py Added LANGUAGES = [list of all languages]

cms/envs/test.py Added LANGUAGES = [list of all languages]. Added under this commit.

  1. Tests were failing due to updated languages list. We changed languages to make changes for transifex supported langs.

  2. Can override using decorator in tests but this was used at multiple places so added it in test.py.

15. Fix tests under edX upgrade (commit) (LP-2503)

  1. Fixed test in commerce using back code as it was fixed in edX.

lms/djangoapps/commerce/api/v1/tests/test_views.py Fixed at line 179

  1. Tests were failing due to year change.

  2. Back coded to fix our jobs.

16. Integrate MSP assessment XBlock with ADG (PR) (LP-2574)

  1. Added the following line in edx-platform/setup.py (line #38)

"msp_dashboard = msp_assessment.msp_dashboard.tabs:MspDashboardTab",

  1. This core change is necessary to install the msp assessment xblock and make it functional.

  2. The change has to be made in the setup script for the Open edx package, which is a core file.

17. Filter Courses on Course Catalog (PR) (LP-2523)

  1. Changed imports in lms/djangoapps/courseware/views/views.py Removed import at line 67 and added import at 92.

  2. This is necessary to use our method of listing down the courses.

  3. Moved the method to adg directory but the import was to be done in the core file.

18. Integrate Course About page (PR) (LP-2501)

  1. The following changes were made in lms/djangoapps/courseware/views/views.py

Imported function get_extra_course_about_context at line #92.

Updated course_about context with a call to the get_extra_course_about_context function, at line #987-988.

  1. This is necessary as we need the extra context to cater adg course about page requirements.

  2. All of the relevant code is in an adg directory named courseware_override. However, it had to be imported and called in the core file in order to reflect the changes.

19. Redirection and Textual Issues on ADG Platform (PR) (LP-2588)

Upon hitting the root url, authenticated users were redirected to dashboard and unauthenticated users to home page. However, we want both users (authenticated and unauthenticated) to be redirected to home page except for when they login or signup, then they should be redirected to dashboard.

  1. For this purpose a new helper function has been added which checks if the user is being referred from login or signup and following changes have been made in lms/djangoapps/branding/views.py

add a new import

from openedx.adg.lms.branding_extension.helpers import is_referred_by_login_or_register

change the docstring of index view to

Redirects to main page -- info page if user authenticated and redirected from login or register, or marketing if not

change the first condition in index view to

if request.user.is_authenticated and is_referred_by_login_or_register(request)

20. Updating py2neo package in requrement files LP-2613

Changed py2neo==3.1.2 to -e git+https://github.com/technige/[email protected]#egg=py2neo==3.1.2 in all edx requrement files

Reason

This change cannot be averted. Using py2neo==3.1.2 breaks edx setup specially pipelines. See context of LP-2613 for detail.

21. Add upcomming webinars in learn page LP-2623

Add upcomming webinars in context of def courses of edx-platform/lms/djangoapps/courseware/views/views.py

Reason

We need to add upcomming webinars in learn page for that we need to get all upcomming webinars in asscending order and add in course's context which will than render in courseware/courses.html. See context of LP-2623 for detail.

22. Send account activation emails (PR) (LP-2661)

  1. ADG email templates were not used and default edX OOTB emails were sent in 2 cases, which were missed when we overridden all the emails. To send ADG emails we had to do some core changes. common/djangoapps/third_party_auth/views.py Line 21 & Line 56 openedx/core/djangoapps/user_authn/views/login.py Line 32, 33 & Line 178 openedx/core/djangoapps/user_authn/views/register.py Line 39 & Line 237
  2. Cannot move all the logic in adg so had to replace the method call which sends email.
  3. It was all core functionality and we just need to send different emails so had to do core changes.

23. Back port fix: Reduce safe-sessions false alarms (PR) (LP-2688)

  1. Back port fix: Reduce safe-sessions false alarms.
  2. Because edX has changed the core code due the same issue we are facing.
  3. Why can't we move this change to openedx/adg? As edX already has changed this core code, we are just back porting it.

24. Add omni academy support url (PR) (LP-2662)

  1. Updated a setting to show term of service and updated login support url. lms/envs/common.py Line 2734 lms/static/js/student_account/views/LoginView.js Line 229 and Line 236
  2. Updated a setting so settings file changed. Updated js file to use the dynamic link.

25. Back port fix: [SE-4101] fix: address VisibleBlocks caching race condition (PR) (LP-2805)

  1. Back port [SE-4101] fix: address VisibleBlocks caching race condition.(https://github.com/edx/edx-platform/pull/27359)
  2. Because edX has changed the core code due to the same issue we are facing.
  3. Why can't we move this change to openedx/adg? As edX already has changed this core code, we are just back porting it.

26. Segment Integration for ADG (PR) (LP-2831)

  1. We need to emit events for when a course is started and when one is completed (passed or failed). We also need an event that is emitted when a new user is registered. Therefore, we made the following core changes:

lms/djangoapps/grades/models.py - Line #31-33, #660-662

common/djangoapps/student/models.py - Line #58-59, #1360-1362

openedx/core/djangoapps/user_authn/views/register.py - Line #85, #246-249

  1. Because we need to emit this event when an enrolment or grade calculation happens. As we are using openedx's code for enrolment and for grading, we have to add the event emits there as well.
  2. All the logic is already written in openedx/adg. We just need to call our emit function in the core code as we are using that functionality and we want to emit events related to it as well.

27. Populate and link the Terms of Use and Privacy Policy pages for ADG (PR) (LP-2354)

  1. We need to update settings

lms/envs/common.py - Line #2743

  1. Because we need to add link for privacy page
  2. Because its a settings map, We need to update it here.

28. [BACKPORT] Transcripts for videos are not uploading (PR) (LP-2846)

  1. It is fix backported from open edX.

cms/djangoapps/contentstore/views/transcript_settings.py - Line #236 cms/djangoapps/contentstore/views/transcripts_ajax.py - Line #112, 225 common/lib/xmodule/xmodule/video_module/video_handlers.py - Line #494

  1. Because its is change in core code and backported from openedx.

29. Implement registration for invited admins (PR) (LP-2847)

  1. Add logic to set default value if user is registering himself through Invitation email

/openedx/core/djangoapps/user_authn/views/registration_form.py - Line 439

  1. Because we are using core functionality for registration and we just want prefilled email and company if user is registering himself through invitation email
  2. Because registration is core functionality we don't have overridden registration process. So have to add this check if user is_invited then set default values in registration form

30. Integrate Delete Account, Reset Password & SSO Functionality on Account Page (PR) (LP-2938)

  1. We need to remove the notifications flow for account deletion as it was failing.

/openedx/core/djangoapps/user_api/accounts/views.py - Line #447-467

  1. Openedx default notification flow breaks when user is deleted. We do not need any notification on deletion anyway so we have decided to remove that flow altogether.

  2. As it is a part of core functionality that is failing, we will have to remove it from the core file. We cannot do anything from openedx/adg in this case.

31. Confirm enrollment & Unenroll from other versions of a course (PR) (LP-2972)

  1. We need to unenroll user from all other enrolled versions when a user tries to enroll in more than 1 version of a course.

common/djangoapps/student/views/management.py - Line #50, Line #370-378

  1. Need to inject custom logic in enrollment.

  2. Enrollment is core functionality, we need to inject custom logic.

32. Fix failing unit test (PR) (LP-2987)

  1. A type error was being caused because, instead of string an object was being passed to mode as argument in create method of CourseEnrollmentFactory. Hence, in

lms/djangoapps/experiments/tests/test_views_custom.py - Line #200

mode=course_mode,

was changed to

mode=course_mode.slug,