Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion lms/djangoapps/courseware/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
from fs.errors import ResourceNotFoundError
import logging
import inspect
import re

from path import path
from django.http import Http404

from django.conf import settings
from .module_render import get_module
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore import Location, XML_MODULESTORE_TYPE
Expand Down Expand Up @@ -294,3 +295,19 @@ def sort_by_announcement(courses):
courses = sorted(courses, key=key)

return courses


def get_cms_course_link_by_id(course_id):
"""
Returns a proto-relative link to course_index for editing the course in cms, assuming that the course is actually
cms-backed. If course_id is improperly formatted, just return the root of the cms
"""
format_str = r'^(?P<org>[^/]+)/(?P<course>[^/]+)/(?P<name>[^/]+)$'
host = "//{}/".format(settings.CMS_BASE) # protocol-relative
m_obj = re.match(format_str, course_id)
if m_obj:
return "{host}{org}/{course}/course/{name}".format(host=host,
org=m_obj.group('org'),
course=m_obj.group('course'),
name=m_obj.group('name'))
return host

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need the host or can it be a relative url? Can you get the url via Location().url() v this construction?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It does need the host because it's used for an external link (for the LMS to link to CMS). It's similar in spirit, (but in opposite direction) as https://github.com/edx/edx-platform/blob/master/cms/djangoapps/contentstore/utils.py#L92

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

and sure, I can switch to a better utility function.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@dmitchell actually, having looked at Location().url(), I don't think that's actually the right thing, mainly because the addressing scheme of the URL it returns is internal rather than a standard external url. eg:

In [8]: Location(loc_or_tag="internal", org="org", course="course", category="cat", name="name").url()
Out[8]: 'internal://org/course/cat/name'

I'd have to parse out the "protocol" part of that URL anyway to make it into an actual working URL for 'course_index' in CMS (which doesn't take location as is), so I'm thinking that not much is gained by existing library functions here. But maybe I'm missing some extra support for "course" modules like they exist for "static" ones?

Basically, what I really wanted to do was a django.core.urlresolvers.reverse, but couldn't because it was across different applications and settings files

14 changes: 13 additions & 1 deletion lms/djangoapps/courseware/tests/test_courses.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# -*- coding: utf-8 -*-
from django.test import TestCase
from django.http import Http404
from courseware.courses import get_course_by_id
from django.test.utils import override_settings
from courseware.courses import get_course_by_id, get_cms_course_link_by_id

CMS_BASE_TEST = 'testcms'

class CoursesTest(TestCase):
def test_get_course_by_id_invalid_chars(self):
Expand All @@ -14,3 +17,12 @@ def test_get_course_by_id_invalid_chars(self):
get_course_by_id('MITx/foobar/statistics=introduction')
get_course_by_id('MITx/foobar/business and management')
get_course_by_id('MITx/foobar/NiñøJoséMaríáßç')

@override_settings(CMS_BASE=CMS_BASE_TEST)
def test_get_cms_course_link_by_id(self):
"""
Tests that get_cms_course_link_by_id returns the right thing
"""
self.assertEqual("//{}/".format(CMS_BASE_TEST), get_cms_course_link_by_id("blah_bad_course_id"))
self.assertEqual("//{}/".format(CMS_BASE_TEST), get_cms_course_link_by_id("too/too/many/slashes"))
self.assertEqual("//{}/org/num/course/name".format(CMS_BASE_TEST), get_cms_course_link_by_id('org/num/name'))
21 changes: 14 additions & 7 deletions lms/djangoapps/instructor/views/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from courseware import grades
from courseware.access import (has_access, get_access_group_name,
course_beta_test_group_name)
from courseware.courses import get_course_with_access
from courseware.courses import get_course_with_access, get_cms_course_link_by_id
from courseware.models import StudentModule
from django_comment_common.models import (Role,
FORUM_ROLE_ADMINISTRATOR,
Expand Down Expand Up @@ -792,23 +792,30 @@ def get_analytics_result(analytics_name):
else:
instructor_tasks = None

# determine if this is a studio-backed course so we can 1) provide a link to edit this course in studio
# 2) enable course email
is_studio_course = modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE

email_editor = None
# HTML editor for email
if idash_mode == 'Email':
if idash_mode == 'Email' and is_studio_course:
html_module = HtmlDescriptor(course.system, {'data': html_message})
email_editor = wrap_xmodule(html_module.get_html, html_module, 'xmodule_edit.html')()
else:
email_editor = None

studio_url = None
if is_studio_course:
studio_url = get_cms_course_link_by_id(course_id)

# Flag for whether or not we display the email tab (depending upon
# what backing store this course using (Mongo vs. XML))
if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and \
modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE:
if settings.MITX_FEATURES['ENABLE_INSTRUCTOR_EMAIL'] and is_studio_course:
show_email_tab = True

# display course stats only if there is no other table to display:
course_stats = None
if not datatable:
course_stats = get_course_stats_table()

#----------------------------------------
# context for rendering

Expand All @@ -821,6 +828,7 @@ def get_analytics_result(analytics_name):
'course_stats': course_stats,
'msg': msg,
'modeflag': {idash_mode: 'selectedmode'},
'studio_url': studio_url,

'to_option': email_to_option, # email
'subject': email_subject, # email
Expand All @@ -843,7 +851,6 @@ def get_analytics_result(analytics_name):

return render_to_response('courseware/instructor_dashboard.html', context)


def _do_remote_gradebook(user, course, action, args=None, files=None):
'''
Perform remote gradebook action. Returns msg, datatable.
Expand Down
2 changes: 2 additions & 0 deletions lms/envs/aws.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@
SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE)
SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN')

CMS_BASE = ENV_TOKENS.get('CMS_BASE', 'studio.edx.org')

# allow for environments to specify what cookie name our login subsystem should use
# this is to fix a bug regarding simultaneous logins between edx.org and edge.edx.org which can
# happen with some browsers (e.g. Firefox)
Expand Down
3 changes: 3 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@
TEMPLATE_DEBUG = False
USE_TZ = True

# CMS base
CMS_BASE = 'localhost:8001'

# Site info
SITE_ID = 1
SITE_NAME = "edx.org"
Expand Down
6 changes: 6 additions & 0 deletions lms/static/sass/course/instructor/_instructor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
right: 2em;
}

.studio-edit-link{
position: absolute;
top: 3.5em;
right: 2em;
}

section.instructor-dashboard-content {
@extend .content;
padding: 40px;
Expand Down
9 changes: 7 additions & 2 deletions lms/templates/courseware/instructor_dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,13 @@
<div class="instructor-dashboard-wrapper">

%if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BETA_DASHBOARD'):
<div class="beta-button-wrapper"><a href="${ beta_dashboard_url }"> Try New Beta Dashboard </a></div>
<div class="beta-button-wrapper"><a href="${ beta_dashboard_url }">${_("Try New Beta Dashboard")}</a></div>
%endif
%if studio_url:
## not checking access because if user can see this, they are at least course staff (with studio edit access)
<div class="studio-edit-link"><a href="${studio_url}" target="_blank">${_('Edit Course In Studio')}</a></div>
%endif


<section class="instructor-dashboard-content">
<h1>${_("Instructor Dashboard")}</h1>
Expand All @@ -125,7 +130,7 @@ <h2 class="navbar">[ <a href="#" onclick="goto('Grades');" class="${modeflag.get
<a href="#" onclick="goto('Data');" class="${modeflag.get('Data')}">${_("DataDump")}</a> |
<a href="#" onclick="goto('Manage Groups');" class="${modeflag.get('Manage Groups')}">${_("Manage Groups")}</a>
%if show_email_tab:
| <a href="#" onclick="goto('Email')" class="${modeflag.get('Email')}">Email</a>
| <a href="#" onclick="goto('Email')" class="${modeflag.get('Email')}">${_("Email")}</a>
%endif
%if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_ANALYTICS'):
| <a href="#" onclick="goto('Analytics');" class="${modeflag.get('Analytics')}">${_("Analytics")}</a>
Expand Down