fix: fixed the lms link imports to avoid a runtime error #166

Asespinel · 2025-06-09T23:19:22Z

Description

This PR resolves a critical issue where the LMS crashes when the Feedback XBlock is loaded. The problem stems from the XBlock importing a utility function from the openedx.core.djangoapps.content.search.models module, which is only available in the CMS (Studio) context. More information here: #164

Because the content.search app is not installed or initialized in the LMS, attempting to import from it causes runtime errors, resulting in LMS failures when rendering or initializing the XBlock.

Solution

To fix this, the offending import has been fully removed, and the required functionality has been re-implemented locally within the utils.py file of the Feedback XBlock. This decouples the XBlock from platform-specific CMS-only dependencies, ensuring it works in both LMS and Studio environments.

This solution avoids runtime exceptions and ensures the XBlock operates independently of internal platform modules not shared across services.

🔍 Why this matters

Prevents LMS from crashing due to unavailable CMS-only Django apps.

Makes the Feedback XBlock more robust, self-contained, and platform-agnostic.

Enables safe use of the XBlock across different Open edX deployments and service boundaries.

How to test

Install the xblock using this branch
Install the component in studio adding "feedback" to the advanced settings in studio
Go to your course as an admin and go to the Instructor tab
You should see no errors and the xblock rendering correctly

openedx-webhooks · 2025-06-09T23:19:26Z

Thanks for the pull request, @Asespinel!

This repository is currently maintained by @openedx/axim-engineering.

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.

codecov · 2025-06-09T23:58:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.75%. Comparing base (a7db6e1) to head (794d752).
⚠️ Report is 47 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   92.76%   93.75%   +0.98%     
==========================================
  Files           5        5              
  Lines         221      240      +19     
==========================================
+ Hits          205      225      +20     
+ Misses         16       15       -1

Flag	Coverage Δ
unittests	`93.75% <100.00%> (+0.98%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Asespinel · 2025-06-10T00:10:51Z

@kdmccormick @openedx/axim-engineering this PR solves the issue discussed in openedx/openedx-platform#36802 please let me know if you need something else

mphilbrick211 · 2025-08-14T19:14:05Z

@kdmccormick are you still able to review this?

kdmccormick · 2025-08-14T19:47:37Z

Yep, but I will not be able to get to it until next week. If there are any edx-platform CCs that are available to take a look in the meantime they're welcome to review instead.

mphilbrick211 · 2025-10-08T20:12:10Z

Yep, but I will not be able to get to it until next week. If there are any edx-platform CCs that are available to take a look in the meantime they're welcome to review instead.

Bumping this @kdmccormick :)

kdmccormick

Looks good, thanks for the fix! Can you bump the patch version so that this can be released? Then I'll be happy to approve and merge.

kdmccormick · 2025-10-16T20:42:44Z

feedback/utils.py

@@ -1,6 +1,44 @@
 """Utilities for feedback app"""

+# feedbackblock/feedback/utils.py


Suggested change

# feedbackblock/feedback/utils.py

comment is not necessary

kdmccormick · 2025-10-16T20:43:38Z

feedback/utils.py

+from urllib.parse import urlencode, urlparse, urlunparse
+from opaque_keys.edx.keys import UsageKey
+from django.conf import settings


please follow the style guide for imports

Suggested change

from urllib.parse import urlencode, urlparse, urlunparse

from opaque_keys.edx.keys import UsageKey

from django.conf import settings

from urllib.parse import urlencode, urlparse, urlunparse

from django.conf import settings

from opaque_keys.edx.keys import UsageKey

kdmccormick · 2025-10-16T20:44:21Z

feedbacktests/test_utils.py

+from feedback.utils import get_lms_link_for_item
+from opaque_keys.edx.keys import UsageKey
+from unittest.mock import Mock
+import sys
+import types


import style

Suggested change

from feedback.utils import get_lms_link_for_item

from opaque_keys.edx.keys import UsageKey

from unittest.mock import Mock

import sys

import types

import sys

import types

from unittest.mock import Mock

from opaque_keys.edx.keys import UsageKey

from feedback.utils import get_lms_link_for_item

kdmccormick · 2025-10-16T20:46:29Z

feedback/utils.py

+
+    try:
+        # pylint: disable=import-outside-toplevel
+        from openedx.core.djangoapps.site_configuration.models import SiteConfiguration


generally speaking, it is a bad pattern for plugins to import code from edx-platform, as it creates coupling between something that is supposed to be stable (the plugin) and something that is inherently unstable (edx-platform modules). However, I don't know of any better way to let this block access SiteConfiguration other than adding an new XBlock runtime service, and I don't think a new SiteConfig runtime service is a good idea either given that we want to move away from SiteConfig. So, long story short, this is a bad pattern, but I'm OK with it for this PR given that there's no better way and we need a quick fix for this bug.

mphilbrick211 · 2025-12-19T18:36:45Z

@kdmccormick can this be merged?

feanil · 2025-12-22T19:27:04Z

Looks like we're waiting for a version bump on this PR so that we can release this after it's merged.

kdmccormick · 2026-01-07T16:46:55Z

We're also waiting for the review feedback to be addressed. @Asespinel , anything we can do to help unblock this?

mphilbrick211 · 2026-01-23T13:31:14Z

We're also waiting for the review feedback to be addressed. @Asespinel , anything we can do to help unblock this?

Friendly ping on this, @Asespinel

fix: fixed the lms link imports to avoid a runtime error

6cf2cd1

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

openedx-webhooks added this to Contributions Jun 9, 2025

github-project-automation bot moved this to Needs Triage in Contributions Jun 9, 2025

Asespinel added 4 commits June 9, 2025 18:35

fix: fixed import error to get lms link function

e4d7859

chore: fixed unit test import

bdc932d

chore: disable pylint warning

46e7e3d

chore: fixed quality test

08c58d5

Asespinel mentioned this pull request Jun 10, 2025

fix: added content search to lms installed apps openedx/openedx-platform#36802

Closed

Asespinel added 4 commits June 9, 2025 19:54

chore: added unit test for new function

a03dac8

chore: added more tests for the new function

2913fb0

chore: added none import test case

85a68b4

fix: fixed the lms link imports to avoid a runtime error

794d752

kdmccormick self-requested a review June 10, 2025 15:38

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Jun 12, 2025

kdmccormick reviewed Oct 16, 2025

View reviewed changes

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

kdmccormick moved this from In Eng Review to Waiting on Author in Contributions Nov 10, 2025

mphilbrick211 moved this from Waiting on Author to Ready to Merge in Contributions Dec 19, 2025

		@@ -1,6 +1,44 @@
		"""Utilities for feedback app"""

		# feedbackblock/feedback/utils.py

fix: fixed the lms link imports to avoid a runtime error #166

Are you sure you want to change the base?

fix: fixed the lms link imports to avoid a runtime error #166

Uh oh!

Conversation

Asespinel commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

How to test

Uh oh!

openedx-webhooks commented Jun 9, 2025

Uh oh!

codecov bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Asespinel commented Jun 10, 2025

Uh oh!

mphilbrick211 commented Aug 14, 2025

Uh oh!

kdmccormick commented Aug 14, 2025

Uh oh!

mphilbrick211 commented Oct 8, 2025

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

kdmccormick Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

mphilbrick211 commented Dec 19, 2025

Uh oh!

feanil commented Dec 22, 2025

Uh oh!

kdmccormick commented Jan 7, 2026

Uh oh!

mphilbrick211 commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Asespinel commented Jun 9, 2025 •

edited

Loading

codecov bot commented Jun 9, 2025 •

edited

Loading