Merge pull request #3640 from edx/zub/bugfix/std1595-importcoursepermission

remove redundent code on course import

cms/djangoapps/contentstore/tests/utils.py

@@ -4,16 +4,16 @@ Utilities for contentstore tests
 
 import json
 
-from student.models import Registration
 from django.contrib.auth.models import User
 from django.test.client import Client
 from django.test.utils import override_settings
 
+from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from contentstore.tests.modulestore_config import TEST_MODULESTORE
 from contentstore.utils import get_modulestore
-from xmodule.modulestore.django import loc_mapper
+from student.models import Registration
 
 
 def parse_json(response):
@@ -98,9 +98,9 @@ class CourseTestCase(ModuleStoreTestCase):
         )
         self.store = get_modulestore(self.course.location)
 
-    def create_non_staff_authed_user_client(self):
+    def create_non_staff_authed_user_client(self, authenticate=True):
         """
-        Create a non-staff user, log them in, and return the client, user to use for testing.
+        Create a non-staff user, log them in (if authenticate=True), and return the client, user to use for testing.
         """
         uname = 'teststudent'
         password = 'foo'
@@ -113,7 +113,8 @@ class CourseTestCase(ModuleStoreTestCase):
         nonstaff.save()
 
         client = Client()
-        client.login(username=uname, password=password)
+        if authenticate:
+            client.login(username=uname, password=password)
         return client, nonstaff
 
     def populate_course(self):

cms/djangoapps/contentstore/views/import_export.py

@@ -4,38 +4,35 @@ courses
 """
 import logging
 import os
-import tarfile
-import shutil
 import re
-from tempfile import mkdtemp
+import shutil
+import tarfile
 from path import path
+from tempfile import mkdtemp
 
 from django.conf import settings
-from django.http import HttpResponse
 from django.contrib.auth.decorators import login_required
-from django_future.csrf import ensure_csrf_cookie
-from django.core.servers.basehttp import FileWrapper
-from django.core.files.temp import NamedTemporaryFile
 from django.core.exceptions import SuspiciousOperation, PermissionDenied
-from django.http import HttpResponseNotFound
-from django.views.decorators.http import require_http_methods, require_GET
+from django.core.files.temp import NamedTemporaryFile
+from django.core.servers.basehttp import FileWrapper
+from django.http import HttpResponse, HttpResponseNotFound
 from django.utils.translation import ugettext as _
+from django.views.decorators.http import require_http_methods, require_GET
 
+from django_future.csrf import ensure_csrf_cookie
 from edxmako.shortcuts import render_to_response
-
-from xmodule.modulestore.xml_importer import import_from_xml
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore.xml_exporter import export_to_xml
-from xmodule.modulestore.django import modulestore, loc_mapper
 from xmodule.exceptions import SerializationError
-
+from xmodule.modulestore.django import modulestore, loc_mapper
 from xmodule.modulestore.locator import BlockUsageLocator
-from .access import has_course_access
+from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore.xml_exporter import export_to_xml
 
-from util.json_request import JsonResponse
+from .access import has_course_access
 from extract_tar import safetar_extractall
-from student.roles import CourseInstructorRole, CourseStaffRole
 from student import auth
+from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
+from util.json_request import JsonResponse
 
 
 __all__ = ['import_handler', 'import_status_handler', 'export_handler']
@@ -232,10 +229,6 @@ def import_handler(request, tag=None, package_id=None, branch=None, version_guid
                     session_status[key] = 3
                     request.session.modified = True
 
-                    auth.add_users(request.user, CourseInstructorRole(new_location), request.user)
-                    auth.add_users(request.user, CourseStaffRole(new_location), request.user)
-                    logging.debug('created all course groups at {0}'.format(new_location))
-
                 # Send errors to client with stage at which error occurred.
                 except Exception as exception:   # pylint: disable=W0703
                     log.exception(

cms/djangoapps/contentstore/views/tests/test_import_export.py

@@ -1,25 +1,28 @@
 """
 Unit tests for course import and export
 """
+import copy
+import json
+import logging
 import os
 import shutil
 import tarfile
 import tempfile
-import copy
 from path import path
-import json
-import logging
-from uuid import uuid4
 from pymongo import MongoClient
+from uuid import uuid4
 
-from contentstore.tests.utils import CourseTestCase
 from django.test.utils import override_settings
 from django.conf import settings
-from xmodule.modulestore.django import loc_mapper
 
 from xmodule.contentstore.django import _CONTENTSTORE
+from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.tests.factories import ItemFactory
 
+from contentstore.tests.utils import CourseTestCase
+from student import auth
+from student.roles import CourseInstructorRole, CourseStaffRole
+
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
 TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
 
@@ -107,6 +110,46 @@ class ImportTestCase(CourseTestCase):
 
         self.assertEquals(resp.status_code, 200)
 
+    def test_import_in_existing_course(self):
+        """
+        Check that course is imported successfully in existing course and users have their access roles
+        """
+        # Create a non_staff user and add it to course staff only
+        __, nonstaff_user = self.create_non_staff_authed_user_client(authenticate=False)
+        auth.add_users(self.user, CourseStaffRole(self.course.location), nonstaff_user)
+
+        course = self.store.get_item(self.course_location)
+        self.assertIsNotNone(course)
+        display_name_before_import = course.display_name
+
+        # Check that global staff user can import course
+        with open(self.good_tar) as gtar:
+            args = {"name": self.good_tar, "course-data": [gtar]}
+            resp = self.client.post(self.url, args)
+        self.assertEquals(resp.status_code, 200)
+
+        course = self.store.get_item(self.course_location)
+        self.assertIsNotNone(course)
+        display_name_after_import = course.display_name
+
+        # Check that course display name have changed after import
+        self.assertNotEqual(display_name_before_import, display_name_after_import)
+
+        # Now check that non_staff user has his same role
+        self.assertFalse(CourseInstructorRole(self.course_location).has_user(nonstaff_user))
+        self.assertTrue(CourseStaffRole(self.course_location).has_user(nonstaff_user))
+
+        # Now course staff user can also successfully import course
+        self.client.login(username=nonstaff_user.username, password='foo')
+        with open(self.good_tar) as gtar:
+            args = {"name": self.good_tar, "course-data": [gtar]}
+            resp = self.client.post(self.url, args)
+        self.assertEquals(resp.status_code, 200)
+
+        # Now check that non_staff user has his same role
+        self.assertFalse(CourseInstructorRole(self.course_location).has_user(nonstaff_user))
+        self.assertTrue(CourseStaffRole(self.course_location).has_user(nonstaff_user))
+
     ## Unsafe tar methods #####################################################
     # Each of these methods creates a tarfile with a single type of unsafe
     # content.