Address review comments for lms_link_to_cms

* Uses override_settings to provide test variable
* Move location of cms link to upper right
* PEP8 / Pylink

lms/djangoapps/courseware/courses.py

@@ -296,17 +296,18 @@ def sort_by_announcement(courses):
 
     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 = re.match(format_str, course_id)
-    if m:
+    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.group('org'),
-                                                            course=m.group('course'),
-                                                            name=m.group('name'))
+                                                           org=m_obj.group('org'),
+                                                           course=m_obj.group('course'),
+                                                           name=m_obj.group('name'))
     return host

lms/djangoapps/courseware/tests/test_courses.py

 # -*- coding: utf-8 -*-
 from django.test import TestCase
 from django.http import Http404
+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):
         """
@@ -15,10 +18,11 @@ class CoursesTest(TestCase):
             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("//localhost:8001/", get_cms_course_link_by_id("blah_bad_course_id"))
-        self.assertEqual("//localhost:8001/", get_cms_course_link_by_id("too/too/many/slashes"))
-        self.assertEqual("//localhost:8001/org/num/course/name", get_cms_course_link_by_id('org/num/name'))
+        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'))
\ No newline at end of file

lms/static/sass/course/instructor/_instructor.scss

@@ -8,6 +8,12 @@
     right: 2em;
   }
 
+  .studio-edit-link{
+    position: absolute;
+    top: 3.5em;
+    right: 2em;
+  }
+
 	section.instructor-dashboard-content {
 		@extend .content;
     padding: 40px;

lms/templates/courseware/instructor_dashboard.html

@@ -111,6 +111,11 @@ function goto( mode)
   %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>
   %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>
@@ -130,11 +135,6 @@ function goto( mode)
           %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_ANALYTICS'):
              | <a href="#" onclick="goto('Analytics');" class="${modeflag.get('Analytics')}">${_("Analytics")}</a>
           %endif
-          %if studio_url:
-          ## not checking access because if user can see this, they are at least course staff (with studio edit access)
-             | <a href="${studio_url}" target="_blank">${_('Edit Course')}</a>
-          %endif
-
 	  ]
     </h2>