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 studio of another site #595

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

Conversation

manan-memon
Copy link

This PR fixes the issue of course endpoints of a different organization being accessed from another site for admin.

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

@@ -118,6 +131,10 @@ def has_studio_write_access(user, course_key):
:param user:
:param course_key: a CourseKey
"""
# verify if the course is from requesting site

Choose a reason for hiding this comment

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

nit: we can remove the inline comment.

Copy link
Author

Choose a reason for hiding this comment

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

Removed

Comment on lines 108 to 120
def is_course_org_in_site_filter_orgs(course_key):
"""
Check if the course organization is in the current site's organization filters.

:param course_key: a CourseKey object representing the course
:return: True if the course organization is in the site organization filters, False otherwise
"""
site_orgs = configuration_helpers.get_current_site_orgs()
if course_key.org not in site_orgs:
return False
return True


Choose a reason for hiding this comment

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

Isn't this function duplicate? Can you define a single function in EDLY app and import from there?

Copy link
Author

Choose a reason for hiding this comment

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

As we discussed, I have changed it to use the is_course_org_same_as_site_org from edly app

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.

4 participants