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

refactor: use CourseEnrollmentQuerysetRequested from openedx-filters #159

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

JuanDavidBuitrago
Copy link
Member

@JuanDavidBuitrago JuanDavidBuitrago commented Dec 20, 2022

Description

This PR make a refactor to use FFI-3 with openedx-filters
Refactor from #156

Testing instructions

  1. Create two Tenant sites and add at less one course on each one. Go on to create sites: http://{lms.base}:8000/admin/eox_tenant/tenantconfig/
  2. In a course, in Studio, set Mobile Course Available true. Go on to configure the key: http://{studio.base}:8001/settings/advanced/{your_course_id}
  3. Enroll a user (user don't have to be a staff user) in a course on different sites.
  4. Add the following configurations to your configuration site.
"OPEN_EDX_FILTERS_CONFIG": {
    "org.openedx.learning.course_enrollment_queryset.requested.v1": {
        "fail_silently": false,
        "pipeline": [
            "eox_tenant.filters.pipeline.FilterUserCourseEnrollmentsByTenant"
        ]
    }
},
  1. Make a request in:
  • my_microsite.{lms.base}:8000/api/enrollment/v1/enrollment
  • my_microsite.{lms.base}:8000/api/mobile/v1/users/{username}/course_enrollments/
  1. You should see the courses where you are enrolled. Check when you don't use OPEN_EDX_FILTERS_CONFIG in your site, the request show all courses.

More information

You can try the steps in #156 with edx-platform branch

@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/course_enrollment_filter branch 2 times, most recently from ad4579b to 084f19c Compare March 2, 2023 11:06
@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as ready for review March 2, 2023 11:13
@JuanDavidBuitrago JuanDavidBuitrago requested a review from a team March 2, 2023 11:14
@JuanDavidBuitrago JuanDavidBuitrago self-assigned this Mar 2, 2023
@Alec4r
Copy link
Member

Alec4r commented May 10, 2023

Hi @JuanDavidBuitrago , I wanna know if this PR is still important, or maybe we could close it.

@santiagosuarezedunext
Copy link

I undertand that this PR need review so I am going to create a card for it to check it in the next sprint

@JuanDavidBuitrago
Copy link
Member Author

Hi @Alec4r Its important, but first we need upstream PR is merge: openedx/edx-platform#31331

@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as ready for review July 18, 2023 17:24
@Asespinel
Copy link
Contributor

@JuanDavidBuitrago I checked the PR openedx/edx-platform#31331 and it's not yet closed. Can we review this PR? Or do we need to wait for the openedx-filter to pass all the tests first?

@JuanDavidBuitrago
Copy link
Member Author

I'm going to continue working on it, it has to be merged in this sprint

@JuanDavidBuitrago JuanDavidBuitrago merged commit 9ffa5a7 into master Jul 30, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants