Edly: Integrate Forum V2 Application into edx-platform
This PR introduces the integration of the new Forum V2 application into the edx-platform, allowing a course-level choice between the legacy forum (V1) and the new Forum V2.
Key Changes 🚀🚀:
-
Waffle Flag for Forum V2: A new course waffle flag,
forum_v2.enable_forum_v2, has been introduced. When enabled, it will activate the Forum V2 application for the selected course, allowing Forum V1 and V2 to coexist in the platform, used on a per-course basis. -
Data Storage Options: By default, Forum V2 stores data in MongoDB. However, if MySQL is preferred (as is the default in Tutor), an additional course waffle flag,
forum_v2.enable_mysql_backend, can be used to switch the storage backend to MySQL. -
Course-Level Data Migration: A new management command,
forum_migrate_course_from_mongodb_to_mysql, enables data migration on a per-course basis from MongoDB to MySQL. This provides a smooth transition for courses already using Forum V2 with MongoDB who wish to switch to MySQL.
🛠 Note that this PR does not include all unit tests for the forum v2 native API. This is because migrating unit tests is taking much longer than expected. We will take out this PR from draft as soon as it is considered production-ready, despite the fact that some code in edx-platform might not be 100% covered.
@Faraz32123 Are the unit tests complete now? Did you test the new forum v2 with the discussions MFE?
@Faraz32123 Are the unit tests complete now? Did you test the new forum v2 with the discussions MFE?
- As for unit tests, current edx-platform tests that uses v1 are modified accordingly. Tests that uses v2 are under work and will come in a separate PR that will cover v2 native API calls in edx-platform.
- Yup, we have tested the forum v2 with the discussion MFE.
can @regisb you or someone with access run the workflows again. Thanks. Dependency conflicts with forum should be resolved now after we run CI again.
@Faraz32123, @regisb: What do you need at this point in order to get this PR merged?
@ormsbee All that's needed is a :+1: review. It's a fairly big PR, so if you want we can walk you through the changes in a live call. Who do you think would be most suited to review this? We can find someone internally with expertise on cs_comments_service.
@hurtstotouchfire: Wanted to make sure you folks were aware of this PR. This introduces the switching code between old and new forums. It should not affect the default site behavior, in that it will route to the cs_comments_service by default. But it will allow you folks to start experimenting with the new backend.
I know you've said that 2U doesn't need to be in the loop for PRs as long as the default behaviors are unchanged for non-tutor users. But I wanted to make sure you and your team had a chance to review if you wanted, and that you knew this was coming down the pipe in case you notice any regressions.
I'm doing a review as well, and we'll target a merge for later this week.
Finally got through the test code. Thank you for your patience. The only thing there that I'd ask is that we use the
override_waffle_flaghelper where it would be appropriate, instead of the manual patching of'openedx.core.djangoapps.discussions.config.waffle.ENABLE_FORUM_V2.is_enabled'. If that would cause problems that I'm not seeing, please let me know.
@ormsbee thanks for the review.
Basically while using override_waffle_flag, we were facing error: AssertionError: expected a course key or other learning context key which occurs in edx-platform/openedx/core/djangoapps/waffle_utils/__init__.py at line 121. This override_waffle_flag wasn't mocking the is_enabled() method of the CourseWaffleFlag. So we had just manually added patches for it.
override_waffle_flag would be helpful in case if we were just checking, ENABLE_FORUM_V2 is enabled or not. But as now we are checking ENABLE_FORUM_V2 for each course while calling APIs from V1 or V2, We had to patch is_enabled method.
Ah, I see, thank you.
Apologies for the delay in merging this. 2U had requested time to get their stage environment ready for testing, and they're also rolling out a database config change to prod this coming Monday (this is the long Thanksgiving weekend in the U.S.). The plan will be to merge this on Monday (December 2nd).
2U is all clear now, so this can be merged as soon as conflicts are resolved. Thank you for your patience.
@Faraz32123, @regisb: Just want to flag that there's a test failure.
:partying_face:
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.
2U Release Notice: This PR has been deployed to the edX production environment.
2U Release Notice: This PR has been deployed to the edX production environment.
2U Release Notice: This PR has been rolled back from the edX production environment.
2U Release Notice: This PR has been rolled back from the edX production environment.
2U Release Notice: This PR has been deployed to the edX production environment.
2U Release Notice: This PR has been deployed to the edX production environment.