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

[WIP] feat: call python methods from forum v2 #35490

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Faraz32123
Copy link
Contributor

@Faraz32123 Faraz32123 commented Sep 16, 2024

This PR is still a work in progress and not yet ready for final review.

  • directly call python native APIs from forum v2
  • add forum to the edx-platform requirements

@Faraz32123 Faraz32123 requested a review from a team as a code owner September 16, 2024 11:31
- directly call python native APIs from forum v2 for pin, unpin thread,
commentables count_stats and get user's data by user_id
- add forum to the edx-platform requirements
- directly call python native APIs from forum v2 for get parent comment,
create parent comment and create child comment.
- rename retrieve_commentables_stats method to get_commentables_stats and
retrieve_user to get_user.
@Faraz32123 Faraz32123 marked this pull request as draft September 18, 2024 08:53
@Faraz32123 Faraz32123 changed the title feat: call python methods from forum v2 [WIP] feat: call python methods from forum v2 Sep 18, 2024
- refactored code and now pass proper parameters to python native APIs
instead of a single dict
@ormsbee
Copy link
Contributor

ormsbee commented Sep 20, 2024

As you folks are building this out, please use a CourseWaffleFlag to switch between running the old code that hits the service from edx-platform and the new code that uses the forum app. When a migration this big rolls out on a site that runs off of master (like 2U or one of MIT's sites), it will usually go:

  1. Turn it on for a few select courses to try it out, see if there are any major regressions.
  2. Turn it on by default for everyone.
  3. Turn it back off for a few select courses where weird edge case bugs arise.

We can turn this flag on by default for Sumac.

Ali-Salman29 and others added 4 commits September 24, 2024 10:58
- get_user API tests are now passing in test_views.py and test_serializers.py
- add get_user api patch in all tests
- fix httppretty request count in some tests
- fix test_patch_read_non_owner_user test
- add `ENABLE_FORUM_V2` course waffle flag to switch between old code i.e.
cs_comment_service and new code i.e. forum v2.
- mock course waffle flag is_enabled method i.e. ENABLE_FORUM_V2.is_enabled(),
so that old unit tests can be run and passed.
- refactor code(that parts of code whose native APIs are implemented till now)
where we call the native APIs
@Faraz32123 Faraz32123 force-pushed the feat/migrate_APIs_from_http_to_python_call branch from eb04713 to 60703b4 Compare September 27, 2024 09:56
- run `make compile-requirements forum`. Error: It appears that the
Python dependencies in this PR are inconsistent: A re-run of
`make compile-requirements` produced changes.
- fix quality checks
@Faraz32123 Faraz32123 force-pushed the feat/migrate_APIs_from_http_to_python_call branch from 60703b4 to 740b127 Compare September 27, 2024 10:39
- fix new tests related to discussion that failed after fixing previous tests
these are failing due to no.of argument difference
https://github.com/openedx/edx-platform/actions/runs/11069160532/job/30756121710?pr=35490
@Faraz32123 Faraz32123 force-pushed the feat/migrate_APIs_from_http_to_python_call branch from 3dd43f1 to 8cdc9fc Compare September 27, 2024 11:58
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