Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fixed issue where git logs aren't functioning

Browse Source 
Use the course key as the primary key for storing coure import logs in
mongo, instead of the deprecated course key (this only applies to
courses imported via git_import.py).

No effort is made to migrate existing logs, since they are ephemeral.
This commit is contained in:
Amir Qayyum Khan
2018-02-16 15:47:47 +05:00
parent fa50a39d55
commit 9124933925
4 changed files with 35 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
lms/djangoapps/dashboard/git_import.py
View File

@@ -16,7 +16,7 @@ from django.core.management.base import CommandError
from django.utils import timezone
from django.utils.translation import ugettext_lazy as _
from xmodule.util.sandboxing import DEFAULT_PYTHON_LIB_FILENAME
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import CourseLocator
from dashboard.models import CourseImportLog
@@ -298,8 +298,13 @@ def add_repo(repo, rdir_in, branch=None):
# this is needed in order for custom course scripts to work
match = re.search(r'(?ms)===> IMPORTING courselike (\S+)', ret_import)
if match:
course_id = match.group(1)
course_key = CourseKey.from_string(course_id)
course_id = match.group(1).split('/')
# we need to transform course key extracted from logs into CourseLocator instance, because
# we are using split module store and course keys store as instance of CourseLocator.
# please see common.lib.xmodule.xmodule.modulestore.split_mongo.split.SplitMongoModuleStore#make_course_key
# We want set course id in CourseImportLog as CourseLocator. So that in split module
# environment course id remain consistent as CourseLocator instance.
course_key = CourseLocator(*course_id)
cdir = '{0}/{1}'.format(git_repo_dir, course_key.course)
log.debug('Studio course dir = %s', cdir)

3
lms/djangoapps/dashboard/models.py
View File

@@ -13,6 +13,9 @@ class CourseImportLog(mongoengine.Document):
import_log = mongoengine.StringField(max_length=20 * 65535)
git_log = mongoengine.StringField(max_length=65535)
repo_dir = mongoengine.StringField(max_length=128)
commit = mongoengine.StringField(max_length=40, null=True)
author = mongoengine.StringField(max_length=500, null=True)
date = mongoengine.DateTimeField()
created = mongoengine.DateTimeField()
meta = {'indexes': ['course_id', 'created'],
'allow_inheritance': False}

10
lms/djangoapps/dashboard/sysadmin.py
View File

@@ -623,16 +623,10 @@ class GitLogs(TemplateView):
raise Http404
cilset = CourseImportLog.objects.order_by('-created')
else:
try:
course = get_course_by_id(course_id)
except Exception:
log.info('Cannot find course %s', course_id)
raise Http404
# Allow only course team, instructors, and staff
if not (request.user.is_staff or
CourseInstructorRole(course.id).has_user(request.user) or
CourseStaffRole(course.id).has_user(request.user)):
CourseInstructorRole(course_id).has_user(request.user) or
CourseStaffRole(course_id).has_user(request.user)):
raise Http404
log.debug('course_id=%s', course_id)
cilset = CourseImportLog.objects.filter(

38
lms/djangoapps/dashboard/tests/test_sysadmin.py
View File

@@ -15,7 +15,7 @@ from django.urls import reverse
from django.test.client import Client
from django.test.utils import override_settings
from pytz import UTC
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import CourseLocator
from six import text_type
from dashboard.git_import import GitImportErrorNoDir
@@ -24,7 +24,10 @@ from student.roles import CourseStaffRole, GlobalStaff
from student.tests.factories import UserFactory
from util.date_utils import DEFAULT_DATE_TIME_FORMAT, get_time_display
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.django_utils import (
TEST_DATA_SPLIT_MODULESTORE,
SharedModuleStoreTestCase
)
from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM
TEST_MONGODB_LOG = {
@@ -46,7 +49,8 @@ class SysadminBaseTestCase(SharedModuleStoreTestCase):
TEST_REPO = 'https://github.com/mitocw/edx4edx_lite.git'
TEST_BRANCH = 'testing_do_not_delete'
TEST_BRANCH_COURSE = CourseKey.from_string('MITx/edx4edx_branch/edx4edx')
TEST_BRANCH_COURSE = CourseLocator.from_string('course-v1:MITx+edx4edx_branch+edx4edx')
MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
def setUp(self):
"""Setup test case by adding primary user."""
@@ -78,7 +82,7 @@ class SysadminBaseTestCase(SharedModuleStoreTestCase):
course = def_ms.courses.get(course_path, None)
except AttributeError:
# Using mongo store
course = def_ms.get_course(CourseKey.from_string('MITx/edx4edx/edx4edx'))
course = def_ms.get_course(CourseLocator('MITx', 'edx4edx', 'edx4edx'))
# Delete git loaded course
response = self.client.post(
@@ -168,11 +172,11 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
self.assertNotEqual('xml', def_ms.get_modulestore_type(None))
self._add_edx4edx()
course = def_ms.get_course(CourseKey.from_string('MITx/edx4edx/edx4edx'))
course = def_ms.get_course(CourseLocator('MITx', 'edx4edx', 'edx4edx'))
self.assertIsNotNone(course)
self._rm_edx4edx()
course = def_ms.get_course(CourseKey.from_string('MITx/edx4edx/edx4edx'))
course = def_ms.get_course(CourseLocator('MITx', 'edx4edx', 'edx4edx'))
self.assertIsNone(course)
def test_course_info(self):
@@ -185,10 +189,9 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
table_re = re.compile(r"""
<tr>\s+
<td>edX\sAuthor\sCourse</td>\s+ # expected test git course name
<td>MITx/edx4edx/edx4edx</td>\s+ # expected test git course_id
<td>course-v1:MITx\+edx4edx\+edx4edx</td>\s+ # expected test git course_id
<td>[a-fA-F\d]{40}</td> # git sha1 hash
""", re.VERBOSE)
self._setstaff_login()
self._mkdir(settings.GIT_REPO_DIR)
@@ -212,11 +215,11 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
response = self.client.get(reverse('gitlogs'))
# Check that our earlier import has a log with a link to details
self.assertIn('/gitlogs/MITx/edx4edx/edx4edx', response.content)
self.assertIn('/gitlogs/course-v1:MITx+edx4edx+edx4edx', response.content)
response = self.client.get(
reverse('gitlogs_detail', kwargs={
'course_id': 'MITx/edx4edx/edx4edx'}))
'course_id': 'course-v1:MITx+edx4edx+edx4edx'}))
self.assertIn('======&gt; IMPORTING course',
response.content)
@@ -260,7 +263,10 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
response = self.client.get(
reverse('gitlogs_detail', kwargs={
'course_id': 'Not/Real/Testing'}))
self.assertEqual(404, response.status_code)
self.assertIn(
'No git import logs have been recorded for this course.',
response.content
)
def test_gitlog_no_logs(self):
"""
@@ -279,7 +285,7 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
response = self.client.get(
reverse('gitlogs_detail', kwargs={
'course_id': 'MITx/edx4edx/edx4edx'
'course_id': 'course-v1:MITx+edx4edx+edx4edx'
})
)
self.assertIn(
@@ -301,7 +307,7 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
for _ in xrange(15):
CourseImportLog(
course_id=CourseKey.from_string("test/test/test"),
course_id=CourseLocator.from_string("test/test/test"),
location="location",
import_log="import_log",
git_log="git_log",
@@ -341,13 +347,13 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
self.assertEqual(response.status_code, 404)
# Or specific logs
response = self.client.get(reverse('gitlogs_detail', kwargs={
'course_id': 'MITx/edx4edx/edx4edx'
'course_id': 'course-v1:MITx+edx4edx+edx4edx'
}))
self.assertEqual(response.status_code, 404)
# Add user as staff in course team
def_ms = modulestore()
course = def_ms.get_course(CourseKey.from_string('MITx/edx4edx/edx4edx'))
course = def_ms.get_course(CourseLocator('MITx', 'edx4edx', 'edx4edx'))
CourseStaffRole(course.id).add_users(self.user)
self.assertTrue(CourseStaffRole(course.id).has_user(self.user))
@@ -357,7 +363,7 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
response = self.client.get(
reverse('gitlogs_detail', kwargs={
'course_id': 'MITx/edx4edx/edx4edx'
'course_id': 'course-v1:MITx+edx4edx+edx4edx'
}))
self.assertIn('======&gt; IMPORTING course',
response.content)