[DEPR] FFI-3 (Course Enrollment by Tenant) feature

[MaferMazu]

Proposal Date

Oct 1, 2024

Target Ticket Acceptance Date

Oct 10, 2024

Earliest Open edX Named Release Without This Functionality

Sumac - 2024-10

Rationale

From the user story in the FFI-3 document, this feature shows in the learning dashboard te course enrollment by tenant, and also modifies the APIs api/enrollment/v1/enrollment and api/mobile/v1/users/{username}/course_enrollments/.

Como estudiante
Quiero ver los cursos del sitio al que estoy registrado
Para no confundirme con los cursos de otros sitios

Este feature se refiere a modificar las APIs que exponen los enrollments (a móviles o a desktop u otras apps) de forma que solo entreguen información sobre los enrollments en el site actual.

But since Redwood and MFEs, we have already seen different enrollments by tenant.

Removal

• (FFI-3/DS-300) filter user's course enrollment by microsite request #156
• feat: adds the CourseEnrollmentSiteFilterRequested filter openedx/openedx-filters#47

Replacement

In our Redwood instance, we can already see different course enrollments by tenant without this feature.

Deprecation

Leave in the Custom changes and in the FFI-3 doc, that this feature is deprecated

Additional Info

📌 If a not Nimbus instance needs to specifically have the APIs api/enrollment/v1/enrollment and api/mobile/v1/users/{username}/course_enrollments/ filtered, please raise the hand to decline this proposal.

I think it is a specific and unnecessary change, but if necessary, we can implement it as a filter.

Task List

• Revert (FFI-3/DS-300) filter user's course enrollment by microsite request #156
• Revert feat: adds the CourseEnrollmentSiteFilterRequested filter openedx/openedx-filters#47
• Leave in the Custom changes and in the FFI-3 doc, that this feature is deprecated.

[felipemontoya]

Reverting openedx/openedx-filters/pull/47 is not actually an option. This filter was accepted by the filters library more than one year ago. If we want to remove it we need to properly deprecate it.

@mariajgrimaldi what are your thoughts on this? do you know why the dashboard mfe is working out of the box given that the CourseEnrollmentQuerysetRequested filter is never in use at edx-platform?

[mariajgrimaldi]

@felipemontoya: Why can't we skip the deprecation process, as is, since the filter was never included in any service or library to be used (https://github.com/search?q=CourseEnrollmentQuerysetRequested&type=code), so it is not used by the community? IMHO, a good enough removal would be to open a PR, remove the filter, and release the library with a major bump. If you disagree, we can discuss the conditions where a proper deprecation process is needed here.

[mariajgrimaldi]

do you know why the dashboard mfe is working out of the box given that the CourseEnrollmentQuerysetRequested filter is never in use at edx-platform?

I'm not sure how this is working. The enrollments are filtered by site org: https://github.com/openedx/edx-platform/blob/open-release/redwood.master/lms/djangoapps/learner_home/views.py#L504-L506, but that's also how the legacy FE works. I'll be looking into it.

[DonatoBD]

Initially, I didn't understand why we were considering removing a filter that already exists until I realized that the edx-platform PR was closed. So, as far as I'm concerned, there's no point in keeping it when it's not used in edx-platform

[MaferMazu]

@DonatoBD, the question here is:
Does a not Nimbus instance need to specifically have the APIs api/enrollment/v1/enrollment and api/mobile/v1/users/{username}/course_enrollments/ filtered?

If not, we can deprecate this feature.

[DonatoBD]

Apart from NELC, I would say that no other Stratus has that restriction at the API level. @johanseto, do you know if NELC uses/needs that enrollment API filtered in any custom development?

[johanseto]

@DonatoBD Yes, recently, we needed to filter course enrollments by tenant for the mobile API. /api/mobile/v4/users/{username}/course_enrollments/
We merged it in our fork nelc/edx-platform#36,
but also we created a PR for upstream with that.
openedx/edx-platform#35500

[MaferMazu]

Based on the comments, I think the option that requires minor effort and more value is helping with this PR openedx/edx-platform#35500, and reverting the filter change in openedx-filters and eox-tenant; because with the maintaining the filter option, we should edit the current filter, edit the pipeline in eox-tenant and create a new PR in edx-platform, and I am not sure if the value justifies that effort.

I would go with the idea of reverting and helping the NELC effort.

But if you think it is better to have the filter and approach to the community with that, we can discuss it.

What do you think?

cc @DonatoBD @mariajgrimaldi @felipemontoya

Update
The only thing I see as a possible blocker is that openedx/edx-platform#35500 is adding more to the idea of site_configuration, and I don't know if the community still accepts that kind of PRs, on the other hand, the PR looks good, makes sense and is a small change.

[felipemontoya]

The problem I see with the NELC PR is that it is a bandaid trying to add tenant support in an API that has never had tenant support in the past. This in theory needs product approval and it won't get it as the the right way to do it is via a filter, not in the is_org method, but in the get_queryset method.

If reverting the filter as it is not being used (as Majo said) is an option, we can always create a general filter that can serve anywhere we are fetching a list of courses.

In the long run FFI-3 is not a high priority as we are not currently thinking of migrating the multitenant saas.