Fixed bug of checking out the same branch multiple times and code review fixes

Translator comments and language fixes

lms/djangoapps/dashboard/git_import.py

@@ -39,7 +39,13 @@ class GitImportError(Exception):
     CANNOT_PULL = _('git clone or pull failed!')
     XML_IMPORT_FAILED = _('Unable to run import command.')
     UNSUPPORTED_STORE = _('The underlying module store does not support import.')
+    # Translators: This is an error message when they ask for a
+    # particular version of a git repository and that version isn't
+    # available from the remote source they specified
     REMOTE_BRANCH_MISSING = _('The specified remote branch is not available.')
+    # Translators: Error message shown when they have asked for a git
+    # repository branch, a specific version within a repository, that
+    # doesn't exist, or there is a problem changing to it.
     CANNOT_BRANCH = _('Unable to switch to specified branch. Please check '
                       'your branch name.')
 
@@ -89,7 +95,7 @@ def switch_branch(branch, rdir):
         raise GitImportError(GitImportError.CANNOT_BRANCH)
     branches = []
     for line in output.split('\n'):
-        branches.append(line.strip())
+        branches.append(line.replace('*', '').strip())
 
     if branch not in branches:
         # Checkout with -b since it is remote only
@@ -109,8 +115,12 @@ def switch_branch(branch, rdir):
         raise GitImportError(GitImportError.CANNOT_BRANCH)
 
 
-def add_repo(repo, rdir_in, branch):
-    """This will add a git repo into the mongo modulestore"""
+def add_repo(repo, rdir_in, branch=None):
+    """
+    This will add a git repo into the mongo modulestore.
+    If branch is left as None, it will fetch the most recent
+    version of the current branch.
+    """
     # pylint: disable=R0915
 
     # Set defaults even if it isn't defined in settings

lms/djangoapps/dashboard/management/commands/git_add_course.py

@@ -25,6 +25,10 @@ class Command(BaseCommand):
     Pull a git repo and import into the mongo based content database.
     """
 
+    # Translators: A git repository is a place to store a grouping of
+    # versioned files. A branch is a sub grouping of a repository that
+    # has a specific version of the repository. A modulestore is the database used
+    # to store the courses for use on the Web site.
     help = ('Usage: '
             'git_add_course repository_url [directory to check out into] [repository_branch] '
             '\n{0}'.format(_('Import the specified git repository and optional branch into the '
@@ -41,8 +45,8 @@ class Command(BaseCommand):
                                'the git URL')
 
         if len(args) > 3:
-            raise CommandError('This script requires no more than three '
-                               'arguments')
+            raise CommandError('Expected no more than three '
+                               'arguments; recieved {0}'.format(len(args)))
 
         rdir_arg = None
         branch = None

lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py

@@ -46,6 +46,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
     TEST_COURSE = 'MITx/edx4edx/edx4edx'
     TEST_BRANCH = 'testing_do_not_delete'
     TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx'
+    GIT_REPO_DIR = getattr(settings, 'GIT_REPO_DIR')
 
     def assertCommandFailureRegexp(self, regex, *args):
         """
@@ -63,27 +64,27 @@ class TestGitAddCourse(ModuleStoreTestCase):
         self.assertCommandFailureRegexp(
             'This script requires at least one argument, the git URL')
         self.assertCommandFailureRegexp(
-            'This script requires no more than three arguments',
+            'Expected no more than three arguments; recieved 4',
             'blah', 'blah', 'blah', 'blah')
         self.assertCommandFailureRegexp(
             'Repo was not added, check log output for details',
             'blah')
         # Test successful import from command
-        if not os.path.isdir(getattr(settings, 'GIT_REPO_DIR')):
-            os.mkdir(getattr(settings, 'GIT_REPO_DIR'))
-        self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR'))
+        if not os.path.isdir(self.GIT_REPO_DIR):
+            os.mkdir(self.GIT_REPO_DIR)
+        self.addCleanup(shutil.rmtree, self.GIT_REPO_DIR)
 
         # Make a course dir that will be replaced with a symlink
         # while we are at it.
-        if not os.path.isdir(getattr(settings, 'GIT_REPO_DIR') / 'edx4edx'):
-            os.mkdir(getattr(settings, 'GIT_REPO_DIR') / 'edx4edx')
+        if not os.path.isdir(self.GIT_REPO_DIR / 'edx4edx'):
+            os.mkdir(self.GIT_REPO_DIR / 'edx4edx')
 
         call_command('git_add_course', self.TEST_REPO,
-                     getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite')
+                     self.GIT_REPO_DIR / 'edx4edx_lite')
 
         # Test with all three args (branch)
         call_command('git_add_course', self.TEST_REPO,
-                     getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite',
+                     self.GIT_REPO_DIR / 'edx4edx_lite',
                      self.TEST_BRANCH)
 
 
@@ -94,8 +95,8 @@ class TestGitAddCourse(ModuleStoreTestCase):
         with self.assertRaisesRegexp(GitImportError, GitImportError.NO_DIR):
             git_import.add_repo(self.TEST_REPO, None, None)
 
-        os.mkdir(getattr(settings, 'GIT_REPO_DIR'))
-        self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR'))
+        os.mkdir(self.GIT_REPO_DIR)
+        self.addCleanup(shutil.rmtree, self.GIT_REPO_DIR)
 
         with self.assertRaisesRegexp(GitImportError, GitImportError.URL_BAD):
             git_import.add_repo('foo', None, None)
@@ -117,7 +118,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
         """
         Test repo that is in detached head state.
         """
-        repo_dir = getattr(settings, 'GIT_REPO_DIR')
+        repo_dir = self.GIT_REPO_DIR
         # Test successful import from command
         try:
             os.mkdir(repo_dir)
@@ -135,7 +136,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
         """
         Exercise branching code of import
         """
-        repo_dir = getattr(settings, 'GIT_REPO_DIR')
+        repo_dir = self.GIT_REPO_DIR
         # Test successful import from command
         if not os.path.isdir(repo_dir):
             os.mkdir(repo_dir)
@@ -153,6 +154,12 @@ class TestGitAddCourse(ModuleStoreTestCase):
         # Validate that it is different than master
         self.assertIsNotNone(def_ms.get_course(self.TEST_BRANCH_COURSE))
 
+        # Attempt to check out the same branch again to validate branch choosing
+        # works
+        git_import.add_repo(self.TEST_REPO,
+                            repo_dir / 'edx4edx_lite',
+                            self.TEST_BRANCH)
+
         # Delete to test branching back to master
         delete_course(def_ms, contentstore(),
                       def_ms.get_course(self.TEST_BRANCH_COURSE).location,
@@ -176,7 +183,7 @@ class TestGitAddCourse(ModuleStoreTestCase):
                                 cwd=bare_repo)
 
         # Build repo dir
-        repo_dir = getattr(settings, 'GIT_REPO_DIR')
+        repo_dir = self.GIT_REPO_DIR
         if not os.path.isdir(repo_dir):
             os.mkdir(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
@@ -206,7 +213,6 @@ class TestGitAddCourse(ModuleStoreTestCase):
             ['git', 'remote', 'rename', 'origin', 'blah', ],
             stderr=subprocess.STDOUT, cwd=rdir
         )
-        try:
+        with self.assertRaises(GitImportError):
             git_import.switch_branch('master', rdir)
-        except GitImportError:
         self.assertIn('Getting a list of remote branches failed', output.getvalue())

lms/djangoapps/dashboard/sysadmin.py

@@ -438,6 +438,10 @@ class Courses(SysadminDashboardView):
             )
         except subprocess.CalledProcessError as ex:
             log.exception('Git pull or clone output was: %r', ex.output)
+            # Translators: unable to download the course content from
+            # the source git repository. Clone occurs if this is brand
+            # new, and pull is when it is being updated from the
+            # source.
             return _('Unable to clone or pull repository. Please check '
                      'your url. Output was: {0!r}'.format(ex.output))
 
@@ -451,7 +455,11 @@ class Courses(SysadminDashboardView):
                 git_import.switch_branch(branch, gdir)
             except GitImportError as ex:
                 return str(ex)
-            msg += u'<p>{0}: {1}</p>'.format(_('Successfully switched to branch'), branch)
+            # Translators: This is a git repository branch, which is a
+            # specific version of a courses content
+            msg += u'<p>{0}</p>'.format(
+                _('Successfully switched to branch: '
+                  '{branch_name}'.format(branch_name=branch)))
 
         self.def_ms.try_load_course(os.path.abspath(gdir))
         errlog = self.def_ms.errored_courses.get(cdir, '')

lms/templates/sysadmin_dashboard.html

@@ -126,12 +126,16 @@ textarea {
   <ul class="list-input">
 	<li class="field text">
 	  <label for="repo_location">
+		## Translators: Repo is short for git repository or source of
+		## courseware
 		${_('Repo Location')}:
 	  </label>
 	  <input type="text" name="repo_location" style="width:60%" />
 	</li>
 	<li class="field text">
 	  <label for="repo_location">
+		## Translators: Repo is short for git repository or source of
+		## courseware and branch is a specific version within that repository
 		${_('Repo Branch (optional)')}:
 	  </label>
 	  <input type="text" name="repo_branch" style="width:60%" />
@@ -207,6 +211,7 @@ textarea {
 
   </section>
   <div style="text-align:right; float: right"><span id="djangopid">${_('Django PID')}: ${djangopid}</span>
+  ## Translators: A version number appears after this string
   | <span id="edxver">${_('Platform Version')}: ${edx_platform_version}</span></div>
 </div>
 </section>