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

feat!: remove effort field from CourseCatalogData #131

Merged
merged 1 commit into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ Change Log
Unreleased
----------

[3.0.0] - 2022-10-19
--------------------
* **Breaking change**: Removed (optional) field ``effort`` from ``CourseCatalogData.`` Nothing should be relying on this field as it is not used by Course Discovery in Publisher-enabled setups.

[2.0.0] - 2022-10-18
--------------------
* **Breaking change**: Removed signal ``SUBSCRIPTION_LICENSE_MODIFIED`` and corresponding data class ``SubscriptionLicenseData``. This should only affect experimental event-bus code (which should also have been deleted by now).
Expand Down
20 changes: 17 additions & 3 deletions docs/decisions/0009-course-catalog-info-changed-design.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,23 @@ Context

The COURSE_CATALOG_INFO_CHANGED signal is used by Studio to convey that information relevant to the course catalog has changed. It was created with an eye towards eventually replacing the refresh-course-metadata batch job, which syncs all data between edx-platform and course-discovery.

Refresh-course-metadata functions differently depending on whether or not the system has Publisher enabled. Specifically, after requesting information from the ``/courses`` endpoint in Studio, Discovery will ignore certain fields if Publisher is enabled. These fields are mostly part of the ``media`` attribute of the API response. They are sent in a variety of ways (absolute urls, paths, sometimes divided by size, etc.)
Refresh-course-metadata functions differently depending on whether or not the system has Publisher enabled. Specifically, after requesting information from the ``/courses`` endpoint in Studio, Discovery will ignore certain fields if Publisher is enabled. Most, though not all, of these fields are sent as part of the ``media`` attribute of the API response. These media fields are sent in a variety of ways (absolute urls, paths, sometimes divided by size, etc.)

Decision
--------

The COURSE_CATALOG_INFO_CHANGED will only contain the information necessary to work in a Publisher-enabled environment. In particular, this means it will not contain the ``media`` field usually present in the Studio ``/courses`` API endpoint.
The COURSE_CATALOG_INFO_CHANGED will only contain the information necessary to work in a Publisher-enabled environment. In particular, this means it will not include some fields usually present in the Studio ``/courses`` API endpoint, for example:
- ``media``
- ``short_description``
- ``mobile_available``
- ``effort``

In Discovery, if Publisher is not enabled, the consumer will log a warning and not try to update anything.

Rationale
---------

The way we update media information in refresh-course-metadata is quite haphazard, with no real standard way of sending over the information or of storing it on the Discovery end. Replicating these structures in the COURSE_CATALOG_INFO_CHANGED signal would make the data definition confusing.
The way we update media information in refresh-course-metadata is quite haphazard, with no real standard way of sending over the information or of storing it on the Discovery end. Replicating these structures in the COURSE_CATALOG_INFO_CHANGED signal would make the data definition confusing. In addition, it is very difficult to test these other fields without a real non-Publisher environment that runs refresh-course-metadata.

Until these fields are needed by a consumer in a non-Publisher environment (if ever), it makes sense to defer trying to find a good solution to this problem.
rgraber marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -36,3 +40,13 @@ Github discussion
For discussion on the initial event design, see https://github.com/openedx/openedx-events/issues/72 .
For discussion on the removal of the media fields, see the comments on this PR: https://github.com/openedx/openedx-events/pull/81 .

Change history
--------------

2022-10-18
~~~~~~~~~~
- Updated `Decision` section to include more excluded fields

2022-09-14
~~~~~~~~~~
Initial commit
2 changes: 1 addition & 1 deletion openedx_events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
more information about the project.
"""

__version__ = "2.0.0"
__version__ = "3.0.0"
2 changes: 0 additions & 2 deletions openedx_events/content_authoring/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class CourseCatalogData:
course_key (CourseKey): identifier of the Course object.
name (str): course name
schedule_data (CourseScheduleData): scheduling information for the course
effort (str): estimated level of effort in hours per week (optional). Kept as a str to align with the lms model.
hidden (bool): whether the course is hidden from search (optional)
invitation_only (bool): whether the course requires an invitation to enroll
"""
Expand All @@ -53,6 +52,5 @@ class CourseCatalogData:

# additional marketing information
schedule_data = attr.ib(type=CourseScheduleData)
effort = attr.ib(type=str, default=None)
hidden = attr.ib(type=bool, default=False)
invitation_only = attr.ib(type=bool, default=False)