Skip to content

Commit

Permalink
feat!: remove effort field from CourseCatalogData (#131)
Browse files Browse the repository at this point in the history
  • Loading branch information
Rebecca Graber authored Oct 20, 2022
1 parent 84ce4d0 commit d2f4261
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 6 deletions.
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.

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)

0 comments on commit d2f4261

Please sign in to comment.