fix: make discovery service dependency optional for discussion notifications by asajjad2 · Pull Request #37039 · openedx/openedx-platform

asajjad2 · 2025-07-17T13:06:40Z

What are the relevant tickets?

Description (What does it do?)

When the Discussions MFE is enabled via the discussions.enable_discussions_mfe Waffle flag, users encounter noisy error logs like:

Unable to retrieve details about Data for course_run course-v1:MITxT+21A.819.2x+3T2022 because Catalog Integration is not enabled

This error occurs in deployments where the Discovery service (Catalog Integration) is not configured, but the system still attempts to fetch course duration data for notification filtering.

The discussion notification system uses filter_audit_expired_users_with_no_role to exclude audit learners whose access has expired. This filtering logic calls:

get_expected_duration() to determine course duration
get_course_run_details() to fetch weeks_to_complete from Discovery
check_catalog_integration_and_get_user() which logs an error when Catalog Integration is disabled

This PR modifies get_expected_duration() in course_date_signals/utils.py to:

Check if Catalog Integration is enabled before attempting to call Discovery
Use the default minimum duration (4 weeks) when Discovery is unavailable
Maintain existing behavior when Discovery is enabled

Additional Changes

This PR also adds configurable Django settings COURSE_DURATION_MIN_WEEKS (default: 4) and COURSE_DURATION_MAX_WEEKS (default: 18) to allow deployments to customize course duration bounds without code changes.

Current State - The course duration bounds are currently hard-coded as constants:

MIN_DURATION = timedelta(weeks=4)(4 weeks minimum)
MAX_DURATION = timedelta(weeks=18)(18 weeks maximum)

This PR makes these durations configurable through Django settings to allow deployments to customize course duration bounds without code changes:

COURSE_DURATION_MIN_WEEKS (default: 4) - replaces the hard-coded MIN_DURATION
COURSE_DURATION_MAX_WEEKS (default: 18) - replaces the hard-coded MAX_DURATION

This change enables deployments to adjust the audit access duration policy to better suit their needs without requiring code modifications.

How can this be tested?

Verify the fix eliminates error logs when CatalogIntegration.is_enabled() returns False
Confirm existing functionality works when Discovery is enabled

openedx-webhooks · 2025-07-17T13:06:51Z

Thanks for the pull request, @asajjad2!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

pdpinch · 2025-07-17T13:13:03Z

openedx/core/djangoapps/course_date_signals/utils.py

+    """
+    Check if catalog integration is enabled
+    """
+    catalog_integration = CatalogIntegration.current()


I don't have an issue with this per se, but wouldn't it be clearer to test the setting that enables catalog integration?

I only found this setting related to catalog existence on which we could place a check COURSE_CATALOG_API_URL. The setting has a default value that's always present even when Discovery isn't running, so checking it alone might incorrectly return True in broken configurations.

The CatalogIntegration.is_enabled() method, on the other hand is consistent with how other catalog functions determine enablement throughout the codebase and also seems to provides proper administrative control through the database.

pdpinch · 2025-08-25T21:26:04Z

openedx/core/djangoapps/course_date_signals/utils.py

+)
+MAX_DURATION = timedelta(
+    weeks=getattr(settings, 'COURSE_DURATION_MAX_WEEKS', 18)
+)


Can you clarify in the PR description that MIN_DURATION and MAX_DURATION are currently hard-coded to 4 weeks and 18 weeks respectively, and that this PR makes them configurable?

I feel like this is going to require some input from the product working group, but having this context will help.

@pdpinch Updated in the additional changes section

mphilbrick211 · 2025-10-06T21:37:20Z

@pdpinch @asajjad2 hi there! Is this still in progress?

pdpinch · 2025-10-17T17:29:03Z

@asajjad2 can you resolve the conflicts and rebase?

I would still like to get a little feedback from the product WG, since this includes adding a setting -- well, actually exposing an existing setting.

mphilbrick211 · 2025-11-13T22:43:41Z

@asajjad2 can you resolve the conflicts and rebase?

I would still like to get a little feedback from the product WG, since this includes adding a setting -- well, actually exposing an existing setting.

@asajjad2 friendly ping on this!

asajjad2 · 2025-11-28T11:02:06Z

@mphilbrick211 updated.

mphilbrick211 · 2025-12-09T21:57:01Z

@pdpinch hi there! Are you able to review this for us?

pdpinch · 2026-01-02T20:03:37Z

lms/envs/common.py

@asajjad2 I don't think these changes should be here. Maybe they were removed while your PR has been open?

@pdpinch updated.

Anas12091101

Tested locally. LGTM 👍

pdpinch · 2026-01-19T19:34:58Z

@asajjad2 can you squash your commits?

…cations test: patch CatalogIntegration.is_enabled in expiration tests to restore mock behavior test: patch CatalogIntegration.is_enabled in setUp/tearDown feat: make course duration limits configurable refactor: clears ambigous name fix: remove unsued env vars

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 17, 2025

openedx-webhooks added this to Contributions Jul 17, 2025

github-project-automation bot moved this to Needs Triage in Contributions Jul 17, 2025

pdpinch reviewed Jul 17, 2025

View reviewed changes

asajjad2 changed the title ~~fix: make Discovery service dependency optional for discussion notifications~~ fix: make discovery service dependency optional for discussion notifications Jul 18, 2025

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Jul 28, 2025

mphilbrick211 requested a review from pdpinch July 28, 2025 16:56

mphilbrick211 moved this from Waiting on Author to Ready for Review in Contributions Jul 28, 2025

pdpinch reviewed Aug 25, 2025

View reviewed changes

asajjad2 force-pushed the areeb/optional-discovery-dependency branch from bdb9af1 to d41236d Compare September 3, 2025 06:05

mphilbrick211 added the needs reviewer assigned PR needs to be (re-)assigned a new reviewer label Sep 17, 2025

mphilbrick211 removed the needs reviewer assigned PR needs to be (re-)assigned a new reviewer label Nov 13, 2025

mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Nov 13, 2025

mphilbrick211 added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Nov 13, 2025

asajjad2 force-pushed the areeb/optional-discovery-dependency branch from d41236d to cf92d5e Compare November 28, 2025 10:35

mphilbrick211 removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Dec 9, 2025

pdpinch reviewed Jan 2, 2026

View reviewed changes

asajjad2 force-pushed the areeb/optional-discovery-dependency branch from 69efda7 to c8e190c Compare January 6, 2026 07:54

Anas12091101 assigned Anas12091101 and unassigned Anas12091101 Jan 8, 2026

Anas12091101 self-requested a review January 8, 2026 12:48

asajjad2 force-pushed the areeb/optional-discovery-dependency branch 2 times, most recently from 242bdd2 to a2b731c Compare January 9, 2026 12:57

Anas12091101 approved these changes Jan 9, 2026

View reviewed changes

asajjad2 force-pushed the areeb/optional-discovery-dependency branch from a6317cd to c1b33d2 Compare January 14, 2026 13:01

asajjad2 force-pushed the areeb/optional-discovery-dependency branch 2 times, most recently from d629985 to bbcaf2f Compare January 20, 2026 12:18

asajjad2 force-pushed the areeb/optional-discovery-dependency branch from bbcaf2f to 6b43768 Compare February 17, 2026 14:15

Conversation

asajjad2 commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are the relevant tickets?

Description (What does it do?)

Additional Changes

How can this be tested?

Uh oh!

openedx-webhooks commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdpinch Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

asajjad2 Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

pdpinch Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

asajjad2 Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mphilbrick211 commented Oct 6, 2025

Uh oh!

pdpinch commented Oct 17, 2025

Uh oh!

mphilbrick211 commented Nov 13, 2025

Uh oh!

asajjad2 commented Nov 28, 2025

Uh oh!

mphilbrick211 commented Dec 9, 2025

Uh oh!

pdpinch Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

asajjad2 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Anas12091101 left a comment

Choose a reason for hiding this comment

Uh oh!

pdpinch commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

asajjad2 commented Jul 17, 2025 •

edited

Loading

openedx-webhooks commented Jul 17, 2025 •

edited

Loading

asajjad2 Aug 27, 2025 •

edited

Loading