From 4099fc9af0ef9f867497f8c0fc04d4eb1ef8e08c Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Mon, 24 Feb 2014 13:18:22 -0500 Subject: [PATCH 01/14] Correct misaligned django-cas integration --- cms/djangoapps/contentstore/views/public.py | 4 ++++ cms/envs/aws.py | 10 ++++++++++ cms/urls.py | 6 ++++++ common/djangoapps/student/views.py | 3 +++ lms/envs/aws.py | 11 +++++++++++ lms/envs/common.py | 11 ----------- 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 63f95084aa..da805a7bd7 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -44,6 +44,10 @@ def login_page(request): # to course now that the user is authenticated via # the decorator. return redirect('/course') + if settings.FEATURES.get('AUTH_USE_CAS'): + # If CAS is enabled, redirect auth handling to there + return redirect(reverse('cas-login')) + return render_to_response( 'login.html', { diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 3147ad3cd7..383d6925e0 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -181,6 +181,16 @@ PLATFORM_NAME = ENV_TOKENS.get('PLATFORM_NAME', 'edX') if "TRACKING_IGNORE_URL_PATTERNS" in ENV_TOKENS: TRACKING_IGNORE_URL_PATTERNS = ENV_TOKENS.get("TRACKING_IGNORE_URL_PATTERNS") +# Django CAS external authentication settings +CAS_EXTRA_LOGIN_PARAMS = ENV_TOKENS.get("CAS_EXTRA_LOGIN_PARAMS", None) +if FEATURES.get('AUTH_USE_CAS'): + CAS_SERVER_URL = ENV_TOKENS.get("CAS_SERVER_URL", None) + AUTHENTICATION_BACKENDS = ( + 'django.contrib.auth.backends.ModelBackend', + 'django_cas.backends.CASBackend', + ) + INSTALLED_APPS += ('django_cas',) + MIDDLEWARE_CLASSES += ('django_cas.middleware.CASMiddleware',) ################ SECURE AUTH ITEMS ############################### # Secret things: passwords, access keys, etc. diff --git a/cms/urls.py b/cms/urls.py index 65d19e83f7..063995117e 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -115,6 +115,12 @@ if settings.FEATURES.get('ENABLE_SERVICE_STATUS'): url(r'^status/', include('service_status.urls')), ) +if settings.FEATURES.get('AUTH_USE_CAS'): + urlpatterns += ( + url(r'^cas-auth/login/$', 'external_auth.views.cas_login', name="cas-login"), + url(r'^cas-auth/logout/$', 'django_cas.views.logout', {'next_page': '/'}, name="cas-logout"), + ) + urlpatterns += patterns('', url(r'^admin/', include(admin.site.urls)),) # enable automatic login diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 2c802d3f4f..eacad70210 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -344,6 +344,9 @@ def signin_user(request): # branding and allow that to process the login if it # is enabled and the header is in the request. return redirect(reverse('root')) + if settings.FEATURES.get('AUTH_USE_CAS'): + # If CAS is enabled, redirect auth handling to there + return redirect(reverse('cas-login')) if request.user.is_authenticated(): return redirect(reverse('dashboard')) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index ee424f84cc..373704b5e2 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -258,6 +258,17 @@ SSL_AUTH_EMAIL_DOMAIN = ENV_TOKENS.get("SSL_AUTH_EMAIL_DOMAIN", "MIT.EDU") SSL_AUTH_DN_FORMAT_STRING = ENV_TOKENS.get("SSL_AUTH_DN_FORMAT_STRING", "/C=US/ST=Massachusetts/O=Massachusetts Institute of Technology/OU=Client CA v1/CN={0}/emailAddress={1}") +# Django CAS external authentication settings +CAS_EXTRA_LOGIN_PARAMS = ENV_TOKENS.get("CAS_EXTRA_LOGIN_PARAMS", None) +if FEATURES.get('AUTH_USE_CAS'): + CAS_SERVER_URL = ENV_TOKENS.get("CAS_SERVER_URL", None) + AUTHENTICATION_BACKENDS = ( + 'django.contrib.auth.backends.ModelBackend', + 'django_cas.backends.CASBackend', + ) + INSTALLED_APPS += ('django_cas',) + MIDDLEWARE_CLASSES += ('django_cas.middleware.CASMiddleware',) + HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS = ENV_TOKENS.get('HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS',{}) ############################## SECURE AUTH ITEMS ############### diff --git a/lms/envs/common.py b/lms/envs/common.py index 623c03098b..10287904b8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1196,17 +1196,6 @@ FEATURES['CLASS_DASHBOARD'] = False if FEATURES.get('CLASS_DASHBOARD'): INSTALLED_APPS += ('class_dashboard',) -######################## CAS authentication ########################### - -if FEATURES.get('AUTH_USE_CAS'): - CAS_SERVER_URL = 'https://provide_your_cas_url_here' - AUTHENTICATION_BACKENDS = ( - 'django.contrib.auth.backends.ModelBackend', - 'django_cas.backends.CASBackend', - ) - INSTALLED_APPS += ('django_cas',) - MIDDLEWARE_CLASSES += ('django_cas.middleware.CASMiddleware',) - ###################### Registration ################################## # For each of the fields, give one of the following values: From bc12869a008b5fe4903ce2250ec7ca95ba3f7bb0 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Tue, 4 Mar 2014 17:32:03 -0500 Subject: [PATCH 02/14] Add more tests for the container page --- .../contentstore/views/component.py | 2 +- .../contentstore/views/tests/test_helpers.py | 11 ++- cms/static/sass/views/_unit.scss | 2 +- common/test/acceptance/pages/xblock/acid.py | 11 ++- common/test/acceptance/tests/test_lms.py | 27 ++++-- common/test/acceptance/tests/test_studio.py | 87 +++++++++++++++---- requirements/edx/github.txt | 2 +- 7 files changed, 113 insertions(+), 29 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 551f0f6d57..a7946c55c9 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -306,7 +306,7 @@ def container_handler(request, tag=None, package_id=None, branch=None, version_g if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block) try: - old_location, course, xblock, __ = _get_item_in_course(request, locator) + __, course, xblock, __ = _get_item_in_course(request, locator) except ItemNotFoundError: return HttpResponseBadRequest() diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py index b0ca2f5529..d0bf3ffc24 100644 --- a/cms/djangoapps/contentstore/views/tests/test_helpers.py +++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py @@ -12,33 +12,42 @@ class HelpersTestCase(CourseTestCase): Unit tests for helpers.py. """ def test_xblock_studio_url(self): + course = self.course + # Verify course URL - self.assertEqual(xblock_studio_url(self.course), + self.assertEqual(xblock_studio_url(course), u'/course/MITx.999.Robot_Super_Course/branch/published/block/Robot_Super_Course') # Verify chapter URL chapter = ItemFactory.create(parent_location=self.course.location, category='chapter', display_name="Week 1") self.assertIsNone(xblock_studio_url(chapter)) + self.assertIsNone(xblock_studio_url(chapter, course)) # Verify lesson URL sequential = ItemFactory.create(parent_location=chapter.location, category='sequential', display_name="Lesson 1") self.assertIsNone(xblock_studio_url(sequential)) + self.assertIsNone(xblock_studio_url(sequential, course)) # Verify vertical URL vertical = ItemFactory.create(parent_location=sequential.location, category='vertical', display_name='Unit') self.assertEqual(xblock_studio_url(vertical), u'/unit/MITx.999.Robot_Super_Course/branch/published/block/Unit') + self.assertEqual(xblock_studio_url(vertical, course), + u'/unit/MITx.999.Robot_Super_Course/branch/published/block/Unit') # Verify child vertical URL child_vertical = ItemFactory.create(parent_location=vertical.location, category='vertical', display_name='Child Vertical') self.assertEqual(xblock_studio_url(child_vertical), u'/container/MITx.999.Robot_Super_Course/branch/published/block/Child_Vertical') + self.assertEqual(xblock_studio_url(child_vertical, course), + u'/container/MITx.999.Robot_Super_Course/branch/published/block/Child_Vertical') # Verify video URL video = ItemFactory.create(parent_location=child_vertical.location, category="video", display_name="My Video") self.assertIsNone(xblock_studio_url(video)) + self.assertIsNone(xblock_studio_url(video, course)) diff --git a/cms/static/sass/views/_unit.scss b/cms/static/sass/views/_unit.scss index fcc36e5399..61b1428b33 100644 --- a/cms/static/sass/views/_unit.scss +++ b/cms/static/sass/views/_unit.scss @@ -1399,7 +1399,7 @@ body.unit .xblock-type-container { // UI: special case discussion, HTML xmodule styling body.unit .component { - .xmodule_DiscussionModule, .xmodule_HtmlModule { + .xmodule_DiscussionModule, .xmodule_HtmlModule, .xblock { margin-top: ($baseline*1.5); } } diff --git a/common/test/acceptance/pages/xblock/acid.py b/common/test/acceptance/pages/xblock/acid.py index a3ae6d4885..b3c692d57b 100644 --- a/common/test/acceptance/pages/xblock/acid.py +++ b/common/test/acceptance/pages/xblock/acid.py @@ -34,6 +34,13 @@ class AcidView(PageObject): selector = '{} .acid-block {} .pass'.format(self.context_selector, test_selector) return bool(self.q(css=selector).execute(try_interval=0.1, timeout=3)) + def child_test_passed(self, test_selector): + """ + Return whether a particular :class:`.AcidParentBlock` test passed. + """ + selector = '{} .acid-parent-block {} .pass'.format(self.context_selector, test_selector) + return bool(self.q(css=selector).execute(try_interval=0.1, timeout=3)) + @property def init_fn_passed(self): """ @@ -47,8 +54,8 @@ class AcidView(PageObject): Whether the tests of children passed """ return all([ - self.test_passed('.child-counts-match'), - self.test_passed('.child-values-match') + self.child_test_passed('.child-counts-match'), + self.child_test_passed('.child-values-match') ]) @property diff --git a/common/test/acceptance/tests/test_lms.py b/common/test/acceptance/tests/test_lms.py index 621d1ac988..dc60ec99e5 100644 --- a/common/test/acceptance/tests/test_lms.py +++ b/common/test/acceptance/tests/test_lms.py @@ -359,6 +359,19 @@ class XBlockAcidBase(UniqueCourseTest): self.course_info_page = CourseInfoPage(self.browser, self.course_id) self.tab_nav = TabNavPage(self.browser) + + def validate_acid_block_view(self, acid_block): + """ + Verify that the LMS view for the Acid Block is correct + """ + self.assertTrue(acid_block.init_fn_passed) + self.assertTrue(acid_block.resource_url_passed) + self.assertTrue(acid_block.scope_passed('user_state')) + self.assertTrue(acid_block.scope_passed('user_state_summary')) + self.assertTrue(acid_block.scope_passed('preferences')) + self.assertTrue(acid_block.scope_passed('user_info')) + + def test_acid_block(self): """ Verify that all expected acid block tests pass in the lms. @@ -368,13 +381,7 @@ class XBlockAcidBase(UniqueCourseTest): self.tab_nav.go_to_tab('Courseware') acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]') - self.assertTrue(acid_block.init_fn_passed) - self.assertTrue(acid_block.child_tests_passed) - self.assertTrue(acid_block.resource_url_passed) - self.assertTrue(acid_block.scope_passed('user_state')) - self.assertTrue(acid_block.scope_passed('user_state_summary')) - self.assertTrue(acid_block.scope_passed('preferences')) - self.assertTrue(acid_block.scope_passed('user_info')) + self.validate_acid_block_view(acid_block) class XBlockAcidNoChildTest(XBlockAcidBase): @@ -420,7 +427,7 @@ class XBlockAcidChildTest(XBlockAcidBase): XBlockFixtureDesc('chapter', 'Test Section').add_children( XBlockFixtureDesc('sequential', 'Test Subsection').add_children( XBlockFixtureDesc('vertical', 'Test Unit').add_children( - XBlockFixtureDesc('acid', 'Acid Block').add_children( + XBlockFixtureDesc('acid_parent', 'Acid Parent Block').add_children( XBlockFixtureDesc('acid', 'First Acid Child', metadata={'name': 'first'}), XBlockFixtureDesc('acid', 'Second Acid Child', metadata={'name': 'second'}), XBlockFixtureDesc('html', 'Html Child', data="Contents"), @@ -430,6 +437,10 @@ class XBlockAcidChildTest(XBlockAcidBase): ) ).install() + def validate_acid_block_view(self, acid_block): + super(XBlockAcidChildTest, self).validate_acid_block_view() + self.assertTrue(acid_block.child_tests_passed) + # This will fail until we fix support of children in pure XBlocks @expectedFailure def test_acid_block(self): diff --git a/common/test/acceptance/tests/test_studio.py b/common/test/acceptance/tests/test_studio.py index 4bcaaa84c9..0fc9da84fd 100644 --- a/common/test/acceptance/tests/test_studio.py +++ b/common/test/acceptance/tests/test_studio.py @@ -147,6 +147,17 @@ class XBlockAcidBase(WebAppTest): self.auth_page.visit() + def validate_acid_block_preview(self, acid_block): + """ + Validate the Acid Block's preview + """ + self.assertTrue(acid_block.init_fn_passed) + self.assertTrue(acid_block.resource_url_passed) + self.assertTrue(acid_block.scope_passed('user_state')) + self.assertTrue(acid_block.scope_passed('user_state_summary')) + self.assertTrue(acid_block.scope_passed('preferences')) + self.assertTrue(acid_block.scope_passed('user_info')) + def test_acid_block_preview(self): """ Verify that all expected acid block tests pass in studio preview @@ -155,22 +166,13 @@ class XBlockAcidBase(WebAppTest): self.outline.visit() subsection = self.outline.section('Test Section').subsection('Test Subsection') unit = subsection.toggle_expand().unit('Test Unit').go_to() - container = unit.components[0].go_to_container() - acid_block = AcidView(self.browser, container.xblocks[0].preview_selector) - self.assertTrue(acid_block.init_fn_passed) - self.assertTrue(acid_block.child_tests_passed) - self.assertTrue(acid_block.resource_url_passed) - self.assertTrue(acid_block.scope_passed('user_state')) - self.assertTrue(acid_block.scope_passed('user_state_summary')) - self.assertTrue(acid_block.scope_passed('preferences')) - self.assertTrue(acid_block.scope_passed('user_info')) + acid_block = AcidView(self.browser, unit.components[0].preview_selector) + self.validate_acid_block_preview(acid_block) - # This will fail until we support editing on the container page - @expectedFailure def test_acid_block_editor(self): """ - Verify that all expected acid block tests pass in studio preview + Verify that all expected acid block tests pass in studio editor """ self.outline.visit() @@ -181,7 +183,6 @@ class XBlockAcidBase(WebAppTest): acid_block = AcidView(self.browser, unit.components[0].edit().editor_selector) self.assertTrue(acid_block.init_fn_passed) - self.assertTrue(acid_block.child_tests_passed) self.assertTrue(acid_block.resource_url_passed) self.assertTrue(acid_block.scope_passed('content')) self.assertTrue(acid_block.scope_passed('settings')) @@ -213,7 +214,36 @@ class XBlockAcidNoChildTest(XBlockAcidBase): ).install() -class XBlockAcidChildTest(XBlockAcidBase): +class XBlockAcidParentBase(XBlockAcidBase): + """ + Base class for tests that verify that parent XBlock integration is working correctly + """ + __test__ = False + + def validate_acid_block_preview(self, acid_block): + super(XBlockAcidParentBase, self).validate_acid_block_preview(acid_block) + self.assertTrue(acid_block.child_tests_passed) + + def test_acid_block_preview(self): + """ + Verify that all expected acid block tests pass in studio preview + """ + + self.outline.visit() + subsection = self.outline.section('Test Section').subsection('Test Subsection') + unit = subsection.toggle_expand().unit('Test Unit').go_to() + container = unit.components[0].go_to_container() + + acid_block = AcidView(self.browser, container.xblocks[0].preview_selector) + self.validate_acid_block_preview(acid_block) + + # This will fail until the container page supports editing + @expectedFailure + def test_acid_block_editor(self): + super(XBlockAcidParentBase, self).test_acid_block_editor() + + +class XBlockAcidEmptyParentTest(XBlockAcidParentBase): """ Tests of an AcidBlock with children """ @@ -232,7 +262,34 @@ class XBlockAcidChildTest(XBlockAcidBase): XBlockFixtureDesc('chapter', 'Test Section').add_children( XBlockFixtureDesc('sequential', 'Test Subsection').add_children( XBlockFixtureDesc('vertical', 'Test Unit').add_children( - XBlockFixtureDesc('acid', 'Acid Block').add_children( + XBlockFixtureDesc('acid_parent', 'Acid Parent Block').add_children( + ) + ) + ) + ) + ).install() + + +class XBlockAcidChildTest(XBlockAcidParentBase): + """ + Tests of an AcidBlock with children + """ + __test__ = True + + def setup_fixtures(self): + + course_fix = CourseFixture( + self.course_info['org'], + self.course_info['number'], + self.course_info['run'], + self.course_info['display_name'] + ) + + course_fix.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('vertical', 'Test Unit').add_children( + XBlockFixtureDesc('acid_parent', 'Acid Parent Block').add_children( XBlockFixtureDesc('acid', 'First Acid Child', metadata={'name': 'first'}), XBlockFixtureDesc('acid', 'Second Acid Child', metadata={'name': 'second'}), XBlockFixtureDesc('html', 'Html Child', data="Contents"), diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index e95079b9db..6a351aaa9c 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -25,4 +25,4 @@ -e git+https://github.com/edx/event-tracking.git@f0211d702d#egg=event-tracking -e git+https://github.com/edx/bok-choy.git@62de7b576a08f36cde5b030c52bccb1a2f3f8df1#egg=bok_choy -e git+https://github.com/edx-solutions/django-splash.git@9965a53c269666a30bb4e2b3f6037c138aef2a55#egg=django-splash --e git+https://github.com/edx/acid-block.git@bf61f0fcd5916a9236bb5681c98374a48a13a74c#egg=acid-xblock +-e git+https://github.com/edx/acid-block.git@459aff7b63db8f2c5decd1755706c1a64fb4ebb1#egg=acid-xblock From 9d55c88c76846d0bbf6501bb4d67f262e1529c6b Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Thu, 23 Jan 2014 13:17:50 -0500 Subject: [PATCH 03/14] Add option for importing a course from a named branch of a git repo --- lms/djangoapps/dashboard/git_import.py | 62 ++++++++- .../management/commands/git_add_course.py | 15 ++- .../commands/tests/test_git_add_course.py | 118 +++++++++++++++--- lms/djangoapps/dashboard/sysadmin.py | 39 +++--- .../dashboard/tests/test_sysadmin.py | 28 ++++- lms/templates/sysadmin_dashboard.html | 10 +- 6 files changed, 230 insertions(+), 42 deletions(-) diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index 812dc34796..7db862bc2a 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -39,7 +39,9 @@ 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.') - + REMOTE_BRANCH_MISSING = _('The specified remote branch is not available.') + CANNOT_BRANCH = _('Unable to switch to specified branch. Please check ' + 'your branch name.') def cmd_log(cmd, cwd): """ @@ -54,7 +56,60 @@ def cmd_log(cmd, cwd): return output -def add_repo(repo, rdir_in): +def switch_branch(branch, rdir): + """ + This will determine how to change the branch of the repo, and then + use the appropriate git commands to do so. + + Raises an appropriate GitImportError exception if there is any issues with changing + branches. + """ + # Get the latest remote + try: + cmd_log(['git', 'fetch', ], rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to fetch remote: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + + # Check if the branch is available from the remote. + cmd = ['git', 'ls-remote', 'origin', '-h', 'refs/heads/{0}'.format(branch), ] + try: + output = cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Getting a list of remote branches failed: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + if not branch in output: + raise GitImportError(GitImportError.REMOTE_BRANCH_MISSING) + # Check it the remote branch has already been made locally + cmd = ['git', 'branch', '-a', ] + try: + output = cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Getting a list of local branches failed: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + branches = [] + for line in output.split('\n'): + branches.append(line.strip()) + + if branch not in branches: + # Checkout with -b since it is remote only + cmd = ['git', 'checkout', '--force', '--track', + '-b', branch, 'origin/{0}'.format(branch), ] + try: + cmd_log(cmd, rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to checkout remote branch: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + # Go ahead and reset hard to the newest version of the branch now that we know + # it is local. + try: + cmd_log(['git', 'reset', '--hard', 'origin/{0}'.format(branch), ], rdir) + except subprocess.CalledProcessError as ex: + log.exception('Unable to reset to branch: %r', ex.output) + raise GitImportError(GitImportError.CANNOT_BRANCH) + + +def add_repo(repo, rdir_in, branch): """This will add a git repo into the mongo modulestore""" # pylint: disable=R0915 @@ -102,6 +157,9 @@ def add_repo(repo, rdir_in): log.exception('Error running git pull: %r', ex.output) raise GitImportError(GitImportError.CANNOT_PULL) + if branch: + switch_branch(branch, rdirp) + # get commit id cmd = ['git', 'log', '-1', '--format=%H', ] try: diff --git a/lms/djangoapps/dashboard/management/commands/git_add_course.py b/lms/djangoapps/dashboard/management/commands/git_add_course.py index 4a184e4cd2..af5c05c176 100644 --- a/lms/djangoapps/dashboard/management/commands/git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/git_add_course.py @@ -25,8 +25,10 @@ class Command(BaseCommand): Pull a git repo and import into the mongo based content database. """ - help = _('Import the specified git repository into the ' - 'modulestore and directory') + 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 ' + 'modulestore and optionally specified directory.'))) def handle(self, *args, **options): """Check inputs and run the command""" @@ -38,16 +40,19 @@ class Command(BaseCommand): raise CommandError('This script requires at least one argument, ' 'the git URL') - if len(args) > 2: - raise CommandError('This script requires no more than two ' + if len(args) > 3: + raise CommandError('This script requires no more than three ' 'arguments') rdir_arg = None + branch = None if len(args) > 1: rdir_arg = args[1] + if len(args) > 2: + branch = args[2] try: - dashboard.git_import.add_repo(args[0], rdir_arg) + dashboard.git_import.add_repo(args[0], rdir_arg, branch) except GitImportError as ex: raise CommandError(str(ex)) diff --git a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py index fd7c81dd56..9d04f48b5d 100644 --- a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py +++ b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py @@ -2,11 +2,12 @@ Provide tests for git_add_course management command. """ -import unittest +import logging import os import shutil import StringIO import subprocess +import unittest from django.conf import settings from django.core.management import call_command @@ -14,6 +15,9 @@ from django.core.management.base import CommandError from django.test.utils import override_settings from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase import dashboard.git_import as git_import from dashboard.git_import import GitImportError @@ -39,6 +43,9 @@ class TestGitAddCourse(ModuleStoreTestCase): """ TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git' + TEST_COURSE = 'MITx/edx4edx/edx4edx' + TEST_BRANCH = 'testing_do_not_delete' + TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx' def assertCommandFailureRegexp(self, regex, *args): """ @@ -56,16 +63,15 @@ class TestGitAddCourse(ModuleStoreTestCase): self.assertCommandFailureRegexp( 'This script requires at least one argument, the git URL') self.assertCommandFailureRegexp( - 'This script requires no more than two arguments', - 'blah', 'blah', 'blah') + 'This script requires no more than three arguments', + 'blah', 'blah', 'blah', 'blah') self.assertCommandFailureRegexp( 'Repo was not added, check log output for details', 'blah') # Test successful import from command - try: + if not os.path.isdir(getattr(settings, 'GIT_REPO_DIR')): os.mkdir(getattr(settings, 'GIT_REPO_DIR')) - except OSError: - pass + self.addCleanup(shutil.rmtree, getattr(settings, 'GIT_REPO_DIR')) # Make a course dir that will be replaced with a symlink # while we are at it. @@ -74,24 +80,28 @@ class TestGitAddCourse(ModuleStoreTestCase): call_command('git_add_course', self.TEST_REPO, getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite') - if os.path.isdir(getattr(settings, 'GIT_REPO_DIR')): - shutil.rmtree(getattr(settings, 'GIT_REPO_DIR')) + + # Test with all three args (branch) + call_command('git_add_course', self.TEST_REPO, + getattr(settings, 'GIT_REPO_DIR') / 'edx4edx_lite', + self.TEST_BRANCH) + def test_add_repo(self): """ Various exit path tests for test_add_repo """ with self.assertRaisesRegexp(GitImportError, GitImportError.NO_DIR): - git_import.add_repo(self.TEST_REPO, None) + 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')) with self.assertRaisesRegexp(GitImportError, GitImportError.URL_BAD): - git_import.add_repo('foo', None) + git_import.add_repo('foo', None, None) with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL): - git_import.add_repo('file:///foobar.git', None) + git_import.add_repo('file:///foobar.git', None, None) # Test git repo that exists, but is "broken" bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git')) @@ -101,7 +111,7 @@ class TestGitAddCourse(ModuleStoreTestCase): cwd=bare_repo) with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO): - git_import.add_repo('file://{0}'.format(bare_repo), None) + git_import.add_repo('file://{0}'.format(bare_repo), None, None) def test_detached_repo(self): """ @@ -114,9 +124,89 @@ class TestGitAddCourse(ModuleStoreTestCase): except OSError: pass self.addCleanup(shutil.rmtree, repo_dir) - git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite') + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None) subprocess.check_output(['git', 'checkout', 'HEAD~2', ], stderr=subprocess.STDOUT, cwd=repo_dir / 'edx4edx_lite') with self.assertRaisesRegexp(GitImportError, GitImportError.CANNOT_PULL): - git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite') + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', None) + + def test_branching(self): + """ + Exercise branching code of import + """ + repo_dir = getattr(settings, 'GIT_REPO_DIR') + # Test successful import from command + if not os.path.isdir(repo_dir): + os.mkdir(repo_dir) + self.addCleanup(shutil.rmtree, repo_dir) + + # Checkout non existent branch + with self.assertRaisesRegexp(GitImportError, GitImportError.REMOTE_BRANCH_MISSING): + git_import.add_repo(self.TEST_REPO, repo_dir / 'edx4edx_lite', 'asdfasdfasdf') + + # Checkout new branch + git_import.add_repo(self.TEST_REPO, + repo_dir / 'edx4edx_lite', + self.TEST_BRANCH) + def_ms = modulestore() + # Validate that it is different than master + self.assertIsNotNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + + # Delete to test branching back to master + delete_course(def_ms, contentstore(), + def_ms.get_course(self.TEST_BRANCH_COURSE).location, + True) + self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + git_import.add_repo(self.TEST_REPO, + repo_dir / 'edx4edx_lite', + 'master') + self.assertIsNone(def_ms.get_course(self.TEST_BRANCH_COURSE)) + self.assertIsNotNone(def_ms.get_course(self.TEST_COURSE)) + + def test_branch_exceptions(self): + """ + This wil create conditions to exercise bad paths in the switch_branch function. + """ + # create bare repo that we can mess with and attempt an import + bare_repo = os.path.abspath('{0}/{1}'.format(settings.TEST_ROOT, 'bare.git')) + os.mkdir(bare_repo) + self.addCleanup(shutil.rmtree, bare_repo) + subprocess.check_output(['git', '--bare', 'init', ], stderr=subprocess.STDOUT, + cwd=bare_repo) + + # Build repo dir + repo_dir = getattr(settings, 'GIT_REPO_DIR') + if not os.path.isdir(repo_dir): + os.mkdir(repo_dir) + self.addCleanup(shutil.rmtree, repo_dir) + + rdir = '{0}/bare'.format(repo_dir) + with self.assertRaisesRegexp(GitImportError, GitImportError.BAD_REPO): + git_import.add_repo('file://{0}'.format(bare_repo), None, None) + + # Get logger for checking strings in logs + output = StringIO.StringIO() + test_log_handler = logging.StreamHandler(output) + test_log_handler.setLevel(logging.DEBUG) + glog = git_import.log + glog.addHandler(test_log_handler) + + # Move remote so fetch fails + shutil.move(bare_repo, '{0}/not_bare.git'.format(settings.TEST_ROOT)) + try: + git_import.switch_branch('master', rdir) + except GitImportError: + self.assertIn('Unable to fetch remote', output.getvalue()) + shutil.move('{0}/not_bare.git'.format(settings.TEST_ROOT), bare_repo) + output.truncate(0) + + # Replace origin with a different remote + subprocess.check_output( + ['git', 'remote', 'rename', 'origin', 'blah', ], + stderr=subprocess.STDOUT, cwd=rdir + ) + try: + git_import.switch_branch('master', rdir) + except GitImportError: + self.assertIn('Getting a list of remote branches failed', output.getvalue()) diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 81c0e2824e..67f81cfe6f 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -272,7 +272,7 @@ class Users(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'users': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -316,7 +316,7 @@ class Users(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'users': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -348,7 +348,7 @@ class Courses(SysadminDashboardView): return info - def get_course_from_git(self, gitloc, datatable): + def get_course_from_git(self, gitloc, branch, datatable): """This downloads and runs the checks for importing a course in git""" if not (gitloc.endswith('.git') or gitloc.startswith('http:') or @@ -357,11 +357,11 @@ class Courses(SysadminDashboardView): "and be a valid url") if self.is_using_mongo: - return self.import_mongo_course(gitloc) + return self.import_mongo_course(gitloc, branch) - return self.import_xml_course(gitloc, datatable) + return self.import_xml_course(gitloc, branch, datatable) - def import_mongo_course(self, gitloc): + def import_mongo_course(self, gitloc, branch): """ Imports course using management command and captures logging output at debug level for display in template @@ -390,7 +390,7 @@ class Courses(SysadminDashboardView): error_msg = '' try: - git_import.add_repo(gitloc, None) + git_import.add_repo(gitloc, None, branch) except GitImportError as ex: error_msg = str(ex) ret = output.getvalue() @@ -411,7 +411,7 @@ class Courses(SysadminDashboardView): msg += "
{0}
".format(escape(ret)) return msg - def import_xml_course(self, gitloc, datatable): + def import_xml_course(self, gitloc, branch, datatable): """Imports a git course into the XMLModuleStore""" msg = u'' @@ -436,13 +436,23 @@ class Courses(SysadminDashboardView): cmd_output = escape( subprocess.check_output(cmd, stderr=subprocess.STDOUT, cwd=cwd) ) - except subprocess.CalledProcessError: - return _('Unable to clone or pull repository. Please check your url.') + except subprocess.CalledProcessError as ex: + log.exception('Git pull or clone output was: %r', ex.output) + return _('Unable to clone or pull repository. Please check ' + 'your url. Output was: {0!r}'.format(ex.output)) msg += u'
{0}
'.format(cmd_output) if not os.path.exists(gdir): msg += _('Failed to clone repository to {0}').format(gdir) return msg + # Change branch if specified + if branch: + try: + git_import.switch_branch(branch, gdir) + except GitImportError as ex: + return str(ex) + msg += u'

{0}: {1}

'.format(_('Successfully switched to branch'), branch) + self.def_ms.try_load_course(os.path.abspath(gdir)) errlog = self.def_ms.errored_courses.get(cdir, '') if errlog: @@ -494,7 +504,7 @@ class Courses(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'courses': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -511,8 +521,9 @@ class Courses(SysadminDashboardView): courses = self.get_courses() if action == 'add_course': gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '') + branch = request.POST.get('repo_branch', '').strip().replace(' ', '').replace(';', '') datatable = self.make_datatable() - self.msg += self.get_course_from_git(gitloc, datatable) + self.msg += self.get_course_from_git(gitloc, branch, datatable) elif action == 'del_course': course_id = request.POST.get('course_id', '').strip() @@ -563,7 +574,7 @@ class Courses(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'courses': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) @@ -602,7 +613,7 @@ class Staffing(SysadminDashboardView): 'msg': self.msg, 'djangopid': os.getpid(), 'modeflag': {'staffing': 'active-section'}, - 'mitx_version': getattr(settings, 'VERSION_STRING', ''), + 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), } return render_to_response(self.template_name, context) diff --git a/lms/djangoapps/dashboard/tests/test_sysadmin.py b/lms/djangoapps/dashboard/tests/test_sysadmin.py index fb20c4f345..19f97ff5e0 100644 --- a/lms/djangoapps/dashboard/tests/test_sysadmin.py +++ b/lms/djangoapps/dashboard/tests/test_sysadmin.py @@ -45,6 +45,10 @@ class SysadminBaseTestCase(ModuleStoreTestCase): Base class with common methods used in XML and Mongo tests """ + TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git' + TEST_BRANCH = 'testing_do_not_delete' + TEST_BRANCH_COURSE = 'MITx/edx4edx_branch/edx4edx' + def setUp(self): """Setup test case by adding primary user.""" super(SysadminBaseTestCase, self).setUp() @@ -58,11 +62,12 @@ class SysadminBaseTestCase(ModuleStoreTestCase): GlobalStaff().add_users(self.user) self.client.login(username=self.user.username, password='foo') - def _add_edx4edx(self): + def _add_edx4edx(self, branch=None): """Adds the edx4edx sample course""" - return self.client.post(reverse('sysadmin_courses'), { - 'repo_location': 'https://github.com/mitocw/edx4edx_lite.git', - 'action': 'add_course', }) + post_dict = {'repo_location': self.TEST_REPO, 'action': 'add_course', } + if branch: + post_dict['repo_branch'] = branch + return self.client.post(reverse('sysadmin_courses'), post_dict) def _rm_edx4edx(self): """Deletes the sample course from the XML store""" @@ -301,11 +306,24 @@ class TestSysadmin(SysadminBaseTestCase): self.assertIsNotNone(course) # Delete a course - response = self._rm_edx4edx() + self._rm_edx4edx() course = def_ms.courses.get('{0}/edx4edx_lite'.format( os.path.abspath(settings.DATA_DIR)), None) self.assertIsNone(course) + # Load a bad git branch + response = self._add_edx4edx('asdfasdfasdf') + self.assertIn(GitImportError.REMOTE_BRANCH_MISSING, + response.content.decode('utf-8')) + + # Load a course from a git branch + self._add_edx4edx(self.TEST_BRANCH) + course = def_ms.courses.get('{0}/edx4edx_lite'.format( + os.path.abspath(settings.DATA_DIR)), None) + self.assertIsNotNone(course) + self.assertIn(self.TEST_BRANCH_COURSE, course.location.course_id) + self._rm_edx4edx() + # Try and delete a non-existent course response = self.client.post(reverse('sysadmin_courses'), {'course_id': 'foobar/foo/blah', diff --git a/lms/templates/sysadmin_dashboard.html b/lms/templates/sysadmin_dashboard.html index 21dcf4c869..dcca467e3a 100644 --- a/lms/templates/sysadmin_dashboard.html +++ b/lms/templates/sysadmin_dashboard.html @@ -126,10 +126,16 @@ textarea {
    • +
  • + + +
@@ -201,6 +207,6 @@ textarea {
${_('Django PID')}: ${djangopid} - | ${_('Platform Version')}: ${mitx_version}
+ | ${_('Platform Version')}: ${edx_platform_version}
From 62628f6bd2103257f749330405ffe24572eed1bb Mon Sep 17 00:00:00 2001 From: Xavier Antoviaque Date: Wed, 5 Mar 2014 15:21:11 -0400 Subject: [PATCH 04/14] Only add mentoring application to INSTALLED_APPS when in virtualenv --- cms/envs/common.py | 29 +++++++++++++++++++---------- lms/envs/common.py | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index b914239585..e90d2b2139 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -24,6 +24,7 @@ Longer TODO: # want to import all variables from base settings files # pylint: disable=W0401, W0611, W0614 +import imp import sys import lms.envs.common from lms.envs.common import ( @@ -467,9 +468,6 @@ INSTALLED_APPS = ( # for course creator table 'django.contrib.admin', - # XBlocks containing migrations - 'mentoring', - # for managing course modes 'course_modes', @@ -536,11 +534,22 @@ MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = 5 MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = 15 * 60 -### JSdraw (only installed in some instances) +### Apps only installed in some instances -try: - import edx_jsdraw -except ImportError: - pass -else: - INSTALLED_APPS += ('edx_jsdraw',) +OPTIONAL_APPS = ( + 'edx_jsdraw', + 'mentoring', +) + +for app_name in OPTIONAL_APPS: + # First attempt to only find the module rather than actually importing it, + # to avoid circular references - only try to import if it can't be found + # by find_module, which doesn't work with import hooks + try: + imp.find_module(app_name) + except ImportError: + try: + __import__(app_name) + except ImportError: + continue + INSTALLED_APPS += (app_name,) diff --git a/lms/envs/common.py b/lms/envs/common.py index d2653ad881..769035b321 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -26,6 +26,7 @@ Longer TODO: import sys import os +import imp import json from path import path @@ -1163,9 +1164,6 @@ INSTALLED_APPS = ( 'reverification', 'embargo', - - # XBlocks containing migrations - 'mentoring', ) ######################### MARKETING SITE ############################### @@ -1447,11 +1445,22 @@ ALL_LANGUAGES = ( ) -### JSdraw (only installed in some instances) +### Apps only installed in some instances -try: - import edx_jsdraw -except ImportError: - pass -else: - INSTALLED_APPS += ('edx_jsdraw',) +OPTIONAL_APPS = ( + 'edx_jsdraw', + 'mentoring', +) + +for app_name in OPTIONAL_APPS: + # First attempt to only find the module rather than actually importing it, + # to avoid circular references - only try to import if it can't be found + # by find_module, which doesn't work with import hooks + try: + imp.find_module(app_name) + except ImportError: + try: + __import__(app_name) + except ImportError: + continue + INSTALLED_APPS += (app_name,) From b67b89e5624abbe9a3b023ff1fcaad7ffdf6397b Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Thu, 20 Feb 2014 11:45:13 -0500 Subject: [PATCH 05/14] 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 | 16 ++++++-- .../management/commands/git_add_course.py | 8 +++- .../commands/tests/test_git_add_course.py | 38 +++++++++++-------- lms/djangoapps/dashboard/sysadmin.py | 10 ++++- lms/templates/sysadmin_dashboard.html | 5 +++ 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index 7db862bc2a..a3391665fd 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/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 diff --git a/lms/djangoapps/dashboard/management/commands/git_add_course.py b/lms/djangoapps/dashboard/management/commands/git_add_course.py index af5c05c176..58092fe5c6 100644 --- a/lms/djangoapps/dashboard/management/commands/git_add_course.py +++ b/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 diff --git a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py b/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py index 9d04f48b5d..f88b8dd431 100644 --- a/lms/djangoapps/dashboard/management/commands/tests/test_git_add_course.py +++ b/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()) + self.assertIn('Getting a list of remote branches failed', output.getvalue()) diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 67f81cfe6f..c036dc2c0e 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/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'

{0}: {1}

'.format(_('Successfully switched to branch'), branch) + # Translators: This is a git repository branch, which is a + # specific version of a courses content + msg += u'

{0}

'.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, '') diff --git a/lms/templates/sysadmin_dashboard.html b/lms/templates/sysadmin_dashboard.html index dcca467e3a..35eefea6f3 100644 --- a/lms/templates/sysadmin_dashboard.html +++ b/lms/templates/sysadmin_dashboard.html @@ -126,12 +126,16 @@ textarea {