From ec989e499962b32d6c46ae56384e777d3171d892 Mon Sep 17 00:00:00 2001
From: Jason Bau <jbau@stanford.edu>
Date: Wed, 4 Sep 2013 11:29:22 -0700
Subject: [PATCH] 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               | 13 +++++++------
 lms/djangoapps/courseware/tests/test_courses.py    | 10 +++++++---
 lms/static/sass/course/instructor/_instructor.scss |  6 ++++++
 lms/templates/courseware/instructor_dashboard.html | 10 +++++-----
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index b998d7eb11..b39e2d6315 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/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
diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py
index 6bba730100..20cb83a411 100644
--- a/lms/djangoapps/courseware/tests/test_courses.py
+++ b/lms/djangoapps/courseware/tests/test_courses.py
@@ -1,8 +1,11 @@
 # -*- 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'))
\ No newline at end of file
+        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'))
diff --git a/lms/static/sass/course/instructor/_instructor.scss b/lms/static/sass/course/instructor/_instructor.scss
index eee4afffc6..6ec7f617ab 100644
--- a/lms/static/sass/course/instructor/_instructor.scss
+++ b/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;
diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html
index 35b37dc291..effa5852d8 100644
--- a/lms/templates/courseware/instructor_dashboard.html
+++ b/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>