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

Fixed course being accessed from a different org site #588

Open
wants to merge 1 commit into
base: develop-koa
Choose a base branch
from

Conversation

manan-memon
Copy link

@manan-memon manan-memon commented Oct 21, 2024

This PR fixes the issue of a course from different organization being accessed on LMS site of another one.

JIRA: https://edlyio.atlassian.net/browse/EDLY-7116

Copy link

@muhammadali286 muhammadali286 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to test it thoroughly for its ripple affects. Please coordinate with QA accordingly.

lms/djangoapps/courseware/courses.py Outdated Show resolved Hide resolved
Comment on lines 41 to 43
course_key = CourseKey.from_string(course_id)
course = get_course_by_id(course_key)
check_if_course_org_in_site_org_filters(course_key)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we course objects when check_if_course_org_in_site_org_filters is also doing this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the function to use is_course_org_same_as_site_org from edly app, now the course object is only fetched in the view.

Copy link

@Anas-hameed Anas-hameed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants