Merge pull request #502 from adityacp/fix_assignment_upload

Fix assignment upload

7 files changed, 72 insertions, 53 deletions

diff --git a/yaksh/models.py b/yaksh/models.py
index 49a2384..480a111 100644
--- a/yaksh/models.py
+++ b/yaksh/models.py
@@ -94,8 +94,10 @@ MOD_GROUP_NAME = 'moderator'
 
 
 def get_assignment_dir(instance, filename):
-    upload_dir = instance.question_paper.quiz.description.replace(" ", "_")
-    return os.sep.join((upload_dir, instance.user.username,
+    folder_name = instance.course.name.replace(" ", "_")
+    sub_folder_name = instance.question_paper.quiz.description.replace(
+        " ", "_")
+    return os.sep.join((folder_name, sub_folder_name, instance.user.username,
                         str(instance.assignmentQuestion.id),
                         filename
                         ))
@@ -2234,20 +2236,19 @@ class AnswerPaper(models.Model):
 ##############################################################################
 class AssignmentUploadManager(models.Manager):
 
-    def get_assignments(self, qp, que_id=None, user_id=None):
+    def get_assignments(self, qp, que_id=None, user_id=None, course_id=None):
         if que_id and user_id:
             assignment_files = AssignmentUpload.objects.filter(
                         assignmentQuestion_id=que_id, user_id=user_id,
-                        question_paper=qp
+                        question_paper=qp, course_id=course_id
                         )
             file_name = User.objects.get(id=user_id).get_full_name()
         else:
             assignment_files = AssignmentUpload.objects.filter(
-                        question_paper=qp
+                        question_paper=qp, course_id=course_id
                         )
-
             file_name = "{0}_Assignment_files".format(
-                            assignment_files[0].question_paper.quiz.description
+                            assignment_files[0].course.name
                             )
 
         return assignment_files, file_name
@@ -2259,6 +2260,7 @@ class AssignmentUpload(models.Model):
     assignmentQuestion = models.ForeignKey(Question)
     assignmentFile = models.FileField(upload_to=get_assignment_dir)
     question_paper = models.ForeignKey(QuestionPaper, blank=True, null=True)
+    course = models.ForeignKey(Course, null=True, blank=True)
     objects = AssignmentUploadManager()
 
 
diff --git a/yaksh/templates/yaksh/grade_user.html b/yaksh/templates/yaksh/grade_user.html
index 8430e91..2e5a403 100644
--- a/yaksh/templates/yaksh/grade_user.html
+++ b/yaksh/templates/yaksh/grade_user.html
@@ -62,7 +62,7 @@ $(document).ready(function()
 
 {% if has_quiz_assignments %}
 
-<a href="{{URL_ROOT}}/exam/manage/download/quiz_assignments/{{quiz_id}}/">
+<a href="{{URL_ROOT}}/exam/manage/download/quiz_assignments/{{quiz_id}}/{{course_id}}">
     Download All Assignments</a>
 {% endif %}
 
@@ -194,7 +194,7 @@ Status : <b style="color: red;"> Failed </b><br/>
     <h5>Student answer: </h5>
     {% if question.type == "upload" %}
     {% if has_user_assignments %}
-        <a href="{{URL_ROOT}}/exam/manage/download/user_assignment/{{question.id}}/{{data.user.id}}/{{paper.question_paper.quiz.id}}">
+        <a href="{{URL_ROOT}}/exam/manage/download/user_assignment/{{question.id}}/{{data.user.id}}/{{paper.question_paper.quiz.id}}/{{course_id}}">
         <div class="panel">
         Assignment File for {{ data.user.get_full_name.title }}
         </div>
diff --git a/yaksh/templates/yaksh/view_answerpaper.html b/yaksh/templates/yaksh/view_answerpaper.html
index 7cbec91..8f3fad7 100644
--- a/yaksh/templates/yaksh/view_answerpaper.html
+++ b/yaksh/templates/yaksh/view_answerpaper.html
@@ -124,7 +124,7 @@
         {% endfor %}
         </div>
         {% elif question.type == "upload" and has_user_assignment %}
-                <a href="{{URL_ROOT}}/exam/download/user_assignment/{{question.id}}/{{data.user.id}}/{{paper.question_paper.quiz.id}}">
+                <a href="{{URL_ROOT}}/exam/download/user_assignment/{{question.id}}/{{data.user.id}}/{{paper.question_paper.quiz.id}}/{{course_id}}">
                 <div class="well well-sm">
                 <div class="panel">
                 Assignment File for {{ data.user.get_full_name.title }}
diff --git a/yaksh/test_models.py b/yaksh/test_models.py
index e0235fe..c9cb953 100644
--- a/yaksh/test_models.py
+++ b/yaksh/test_models.py
@@ -4,7 +4,7 @@ from yaksh.models import User, Profile, Question, Quiz, QuestionPaper,\
     QuestionSet, AnswerPaper, Answer, Course, StandardTestCase,\
     StdIOBasedTestCase, FileUpload, McqTestCase, AssignmentUpload,\
     LearningModule, LearningUnit, Lesson, LessonFile, CourseStatus, \
-    TestCaseOrder, create_group
+    create_group
 from yaksh.code_server import (
     ServerPool, get_result as get_result_from_code_server
     )
@@ -25,7 +25,7 @@ from yaksh import settings
 
 
 def setUpModule():
-    mod_group = Group.objects.create(name='moderator')
+    Group.objects.create(name='moderator')
 
     # create user profile
     user = User.objects.create_user(username='creator',
@@ -125,6 +125,7 @@ class GlobalMethodsTestCases(unittest.TestCase):
             Group.objects.get(name='moderator')
         )
 
+
 ###############################################################################
 class LessonTestCases(unittest.TestCase):
     def setUp(self):
@@ -1903,7 +1904,7 @@ class AssignmentUploadTestCases(unittest.TestCase):
         self.user2.last_name = "user3"
         self.user2.save()
         self.quiz = Quiz.objects.get(description="demo quiz 1")
-
+        self.course = Course.objects.get(name="Python Course")
         self.questionpaper = QuestionPaper.objects.create(
             quiz=self.quiz, total_marks=0.0, shuffle_questions=True
         )
@@ -1919,17 +1920,19 @@ class AssignmentUploadTestCases(unittest.TestCase):
         file_path2 = os.path.join(tempfile.gettempdir(), "upload2.txt")
         self.assignment1 = AssignmentUpload.objects.create(
             user=self.user1, assignmentQuestion=self.question,
-            assignmentFile=file_path1, question_paper=self.questionpaper
+            assignmentFile=file_path1, question_paper=self.questionpaper,
+            course=self.course
             )
         self.assignment2 = AssignmentUpload.objects.create(
             user=self.user2, assignmentQuestion=self.question,
-            assignmentFile=file_path2, question_paper=self.questionpaper
+            assignmentFile=file_path2, question_paper=self.questionpaper,
+            course=self.course
             )
 
     def test_get_assignments_for_user_files(self):
         assignment_files, file_name = AssignmentUpload.objects.get_assignments(
                                     self.questionpaper, self.question.id,
-                                    self.user1.id
+                                    self.user1.id, self.course.id
                                     )
         self.assertIn("upload1.txt", assignment_files[0].assignmentFile.name)
         self.assertEqual(assignment_files[0].user, self.user1)
@@ -1939,15 +1942,15 @@ class AssignmentUploadTestCases(unittest.TestCase):
 
     def test_get_assignments_for_quiz_files(self):
         assignment_files, file_name = AssignmentUpload.objects.get_assignments(
-                                    self.questionpaper
-                                    )
+            self.questionpaper, course_id=self.course.id
+            )
         files = [os.path.basename(file.assignmentFile.name)
                  for file in assignment_files]
         question_papers = [file.question_paper for file in assignment_files]
         self.assertIn("upload1.txt", files)
         self.assertIn("upload2.txt", files)
         self.assertEqual(question_papers[0].quiz, self.questionpaper.quiz)
-        actual_file_name = self.quiz.description.replace(" ", "_")
+        actual_file_name = self.course.name.replace(" ", "_")
         file_name = file_name.replace(" ", "_")
         self.assertIn(actual_file_name, file_name)
 
diff --git a/yaksh/test_views.py b/yaksh/test_views.py
index 3520c61..dbd9547 100644
--- a/yaksh/test_views.py
+++ b/yaksh/test_views.py
@@ -948,13 +948,14 @@ class TestDownloadAssignment(TestCase):
         # create assignment file
         assignment_file1 = SimpleUploadedFile("file1.txt", b"Test")
         assignment_file2 = SimpleUploadedFile("file2.txt", b"Test")
-        SimpleUploadedFile("file3.txt", b"Test")
         self.assignment1 = AssignmentUpload.objects.create(
             user=self.student1, assignmentQuestion=self.question,
+            course=self.course,
             assignmentFile=assignment_file1, question_paper=self.question_paper
             )
         self.assignment2 = AssignmentUpload.objects.create(
             user=self.student2, assignmentQuestion=self.question,
+            course=self.course,
             assignmentFile=assignment_file2, question_paper=self.question_paper
             )
 
@@ -970,23 +971,25 @@ class TestDownloadAssignment(TestCase):
         self.learning_module.delete()
         self.learning_unit.delete()
         self.mod_group.delete()
-        dir_name = self.quiz.description.replace(" ", "_")
+        dir_name = self.course.name.replace(" ", "_")
         file_path = os.sep.join((settings.MEDIA_ROOT, dir_name))
         if os.path.exists(file_path):
             shutil.rmtree(file_path)
 
     def test_download_assignment_denies_student(self):
         """
-            Check download assignment denies student
+            Check download assignment denies student not enrolled in a course
         """
         self.client.login(
             username=self.student1.username,
             password=self.student1_plaintext_pass
         )
-        response = self.client.get(reverse('yaksh:download_quiz_assignment',
-                                           kwargs={'quiz_id': self.quiz.id}),
-                                   follow=True
-                                   )
+        response = self.client.get(
+            reverse('yaksh:download_quiz_assignment',
+                    kwargs={'quiz_id': self.quiz.id,
+                            "course_id": self.course.id}),
+            follow=True
+            )
         self.assertEqual(response.status_code, 404)
 
     def test_download_assignment_per_quiz(self):
@@ -997,11 +1000,13 @@ class TestDownloadAssignment(TestCase):
             username=self.user.username,
             password=self.user_plaintext_pass
         )
-        response = self.client.get(reverse('yaksh:download_quiz_assignment',
-                                           kwargs={'quiz_id': self.quiz.id}),
-                                   follow=True
-                                   )
-        file_name = "{0}_Assignment_files.zip".format(self.quiz.description)
+        response = self.client.get(
+            reverse('yaksh:download_quiz_assignment',
+                    kwargs={'quiz_id': self.quiz.id,
+                            'course_id': self.course.id}),
+            follow=True
+            )
+        file_name = "{0}_Assignment_files.zip".format(self.course.name)
         file_name = file_name.replace(" ", "_")
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.get('Content-Disposition'),
@@ -1026,7 +1031,8 @@ class TestDownloadAssignment(TestCase):
             reverse('yaksh:download_user_assignment',
                     kwargs={'quiz_id': self.quiz.id,
                             'question_id': self.question.id,
-                            'user_id': self.student2.id
+                            'user_id': self.student2.id,
+                            'course_id': self.course.id
                             }),
             follow=True
             )
diff --git a/yaksh/urls.py b/yaksh/urls.py
index 8f3401b..8397017 100644
--- a/yaksh/urls.py
+++ b/yaksh/urls.py
@@ -8,7 +8,8 @@ urlpatterns = [
     url(r'^update_email/$', views.update_email, name="update_email"),
     url(r'^activate/(?P<key>.+)$', views.activate_user, name="activate"),
     url(r'^new_activation/$', views.new_activation, name='new_activation'),
-    url(r'^toggle_moderator/$', views.toggle_moderator_role, name='toggle_moderator'),
+    url(r'^toggle_moderator/$', views.toggle_moderator_role,
+        name='toggle_moderator'),
     url(r'^quizzes/$', views.quizlist_user, name='quizlist_user'),
     url(r'^quizzes/(?P<enrolled>\w+)/$', views.quizlist_user,
         name='quizlist_user'),
@@ -41,6 +42,9 @@ urlpatterns = [
         name='self_enroll'),
     url(r'^view_answerpaper/(?P<questionpaper_id>\d+)/(?P<course_id>\d+)$',
         views.view_answerpaper, name='view_answerpaper'),
+    url(r'^download/user_assignment/(?P<question_id>\d+)/(?P<user_id>\d+)/'
+        '(?P<quiz_id>\d+)/(?P<course_id>\d+)$',
+        views.download_assignment_file, name="download_user_assignment"),
     url(r'^show_lesson/(?P<lesson_id>\d+)/(?P<module_id>\d+)/'
         '(?P<course_id>\d+)/$', views.show_lesson, name='show_lesson'),
     url(r'^quizzes/view_module/(?P<module_id>\d+)/(?P<course_id>\d+)/$',
@@ -160,10 +164,11 @@ urlpatterns = [
     url(r'^manage/courses/download_course_csv/(?P<course_id>\d+)/$',
         views.download_course_csv, name="download_course_csv"),
     url(r'^manage/download/user_assignment/(?P<question_id>\d+)/'
-        '(?P<user_id>\d+)/(?P<quiz_id>\d+)/$',
+        '(?P<user_id>\d+)/(?P<quiz_id>\d+)/(?P<course_id>\d+)$',
         views.download_assignment_file, name="download_user_assignment"),
-    url(r'^manage/download/quiz_assignments/(?P<quiz_id>\d+)/$',
-        views.download_assignment_file, name="download_quiz_assignment"),
+    url(r'^manage/download/quiz_assignments/(?P<quiz_id>\d+)/'
+        '(?P<course_id>\d+)$', views.download_assignment_file,
+        name="download_quiz_assignment"),
     url(r'^manage/courses/download_yaml_template/',
         views.download_yaml_template, name="download_yaml_template"),
     url(r'^manage/download_sample_csv/',
diff --git a/yaksh/views.py b/yaksh/views.py
index ecd7efd..8ccc12b 100644
--- a/yaksh/views.py
+++ b/yaksh/views.py
@@ -30,8 +30,8 @@ from yaksh.code_server import get_result as get_result_from_code_server
 from yaksh.models import (
     Answer, AnswerPaper, AssignmentUpload, Course, FileUpload, Profile,
     QuestionPaper, QuestionSet, Quiz, Question, TestCase, User,
-    FIXTURES_DIR_PATH, MOD_GROUP_NAME, Lesson, LessonFile, LearningUnit, LearningModule,
-    CourseStatus
+    FIXTURES_DIR_PATH, MOD_GROUP_NAME, Lesson, LessonFile, LearningUnit,
+    LearningModule, CourseStatus
 )
 from yaksh.forms import (
     UserRegisterForm, UserLoginForm, QuizForm, QuestionForm,
@@ -77,7 +77,7 @@ def is_moderator(user, group_name=MOD_GROUP_NAME):
 def add_as_moderator(users, group_name=MOD_GROUP_NAME):
     """ add users to moderator group """
     try:
-        group = Group.objects.get(name=group_name)
+        Group.objects.get(name=group_name)
     except Group.DoesNotExist:
         raise Http404('The Group {0} does not exist.'.format(group_name))
     for user in users:
@@ -729,19 +729,17 @@ def check(request, q_id, attempt_num=None, questionpaper_id=None,
             for fname in assignment_filename:
                 fname._name = fname._name.replace(" ", "_")
                 assignment_files = AssignmentUpload.objects.filter(
-                            assignmentQuestion=current_question,
-                            assignmentFile__icontains=fname, user=user,
-                            question_paper=questionpaper_id)
+                    assignmentQuestion=current_question, course_id=course_id,
+                    assignmentFile__icontains=fname, user=user,
+                    question_paper=questionpaper_id)
                 if assignment_files.exists():
-                    assign_file = assignment_files.get(
-                            assignmentQuestion=current_question,
-                            assignmentFile__icontains=fname, user=user,
-                            question_paper=questionpaper_id)
+                    assign_file = assignment_files.first()
                     if os.path.exists(assign_file.assignmentFile.path):
                         os.remove(assign_file.assignmentFile.path)
                     assign_file.delete()
                 AssignmentUpload.objects.create(
                     user=user, assignmentQuestion=current_question,
+                    course_id=course_id,
                     assignmentFile=fname, question_paper_id=questionpaper_id
                 )
             user_answer = 'ASSIGNMENT UPLOADED'
@@ -1890,10 +1888,10 @@ def view_answerpaper(request, questionpaper_id, course_id):
         data = AnswerPaper.objects.get_user_data(user, questionpaper_id,
                                                  course_id)
         has_user_assignment = AssignmentUpload.objects.filter(
-            user=user,
+            user=user, course_id=course.id,
             question_paper_id=questionpaper_id
         ).exists()
-        context = {'data': data, 'quiz': quiz,
+        context = {'data': data, 'quiz': quiz, 'course_id': course.id,
                    "has_user_assignment": has_user_assignment}
         return my_render_to_response(
             request, 'yaksh/view_answerpaper.html', context
@@ -2100,13 +2098,18 @@ def update_email(request):
 
 @login_required
 @email_verified
-def download_assignment_file(request, quiz_id, question_id=None, user_id=None):
+def download_assignment_file(request, quiz_id, course_id,
+                             question_id=None, user_id=None):
     user = request.user
-    if not is_moderator(user):
-        raise Http404("You are not allowed to view this page")
-    qp = QuestionPaper.objects.get(quiz_id=quiz_id)
+    course = get_object_or_404(Course, pk=course_id)
+    if (not course.is_creator(user) and not course.is_teacher(user) and
+            not course.is_student(user)):
+        raise Http404("You are not allowed to download files for {0}".format(
+            course.name)
+        )
+    qp = get_object_or_404(QuestionPaper, quiz_id=quiz_id)
     assignment_files, file_name = AssignmentUpload.objects.get_assignments(
-        qp, question_id, user_id
+        qp, question_id, user_id, course_id
     )
     zipfile_name = string_io()
     zip_file = zipfile.ZipFile(zipfile_name, "w")