fix merge conflicts

Author: orronai

Diff for: lms/lmsdb/bootstrap.py

@@ -296,11 +296,8 @@ def _add_exercise_course_id_and_number_columns_constraint() -> bool:
             migrate(
                 migrator.add_index('exercise', ('course_id', 'number'), True),
             )
-        except OperationalError as e:
-            if 'already exists' in str(e):
-                log.info(f'index exercise already exists: {e}')
-            else:
-                raise
+        except (OperationalError, ProgrammingError) as e:
+            log.info(f'Index exercise course and number already exists: {e}')
         db_config.database.commit()
 
 
@@ -313,11 +310,8 @@ def _add_user_course_constaint() -> bool:
                     'usercourse', ('user_id', 'course_id'), True,
                 ),
             )
-        except OperationalError as e:
-            if 'already exists' in str(e):
-                log.info(f'index usercourse already exists: {e}')
-            else:
-                raise
+        except (OperationalError, ProgrammingError) as e:
+            log.info(f'Index usercourse user and course already exists: {e}')
         db_config.database.commit()
 
 

Diff for: lms/lmsdb/models.py

@@ -655,16 +655,11 @@ def view_solution(self) -> None:
     def start_checking(self) -> bool:
         return self.set_state(Solution.STATES.IN_CHECKING)
 
-    def set_state(
-        self, new_state: SolutionState,
-        assessment: Optional[SolutionAssessment] = None, **kwargs,
-    ) -> bool:
+    def set_state(self, new_state: SolutionState, **kwargs) -> bool:
         # Optional: filter the old state of the object
         # to make sure that no two processes set the state together
         requested_solution = (Solution.id == self.id)
         updates_dict = {Solution.state.name: new_state.name}
-        if assessment is not None:
-            updates_dict[Solution.assessment.name] = assessment
         changes = Solution.update(
             **updates_dict, **kwargs,
         ).where(requested_solution)
@@ -807,19 +802,20 @@ def _base_next_unchecked(cls):
             cls.submission_timestamp.asc(),
         )
 
+    def change_assessment(self, assessment_id: Optional[int] = None) -> bool:
+        assessment = SolutionAssessment.get_or_none(
+            SolutionAssessment.id == assessment_id,
+        )
+        requested_solution = (Solution.id == self.id)
+        updates_dict = {Solution.assessment.name: assessment}
+        changes = Solution.update(**updates_dict).where(requested_solution)
+        return changes.execute() == 1
+
     def mark_as_checked(
         self,
-        assessment_id: Optional[int] = None,
         by: Optional[Union[User, int]] = None,
     ) -> bool:
-        assessment = SolutionAssessment.get_or_none(
-            SolutionAssessment.id == assessment_id,
-        )
-        return self.set_state(
-            Solution.STATES.DONE,
-            assessment=assessment,
-            checker=by,
-        )
+        return self.set_state(Solution.STATES.DONE, checker=by)
 
     @classmethod
     def next_unchecked(cls) -> Optional['Solution']:

Diff for: lms/lmsweb/translations/he/LC_MESSAGES/messages.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version:  1.0\n"
 "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n"
-"POT-Creation-Date: 2021-10-11 10:38+0300\n"
+"POT-Creation-Date: 2021-10-15 14:37+0300\n"
 "PO-Revision-Date: 2021-09-29 11:30+0300\n"
 "Last-Translator: Or Ronai\n"
 "Language: he\n"
@@ -127,7 +127,7 @@ msgstr "%(solver)s הגיב לך על בדיקת תרגיל \"%(subject)s\"."
 msgid "%(checker)s replied for \"%(subject)s\"."
 msgstr "%(checker)s הגיב לך על תרגיל \"%(subject)s\"."
 
-#: models/solutions.py:75
+#: models/solutions.py:78
 #, python-format
 msgid "Your solution for the \"%(subject)s\" exercise has been checked."
 msgstr "הפתרון שלך לתרגיל \"%(subject)s\" נבדק."

Diff for: lms/lmsweb/views.py

@@ -692,17 +692,20 @@ def shared_solution(shared_url: str, file_id: Optional[int] = None):
     )
 
 
+@webapp.route('/assessment/<int:solution_id>', methods=['POST'])
+@login_required
+@managers_only
+def assessment(solution_id: int):
+    assessment_id = request.json.get('assessment')
+    updated = solutions.change_assessment(solution_id, assessment_id)
+    return jsonify({'success': updated})
+
+
 @webapp.route('/checked/<int:exercise_id>/<int:solution_id>', methods=['POST'])
 @login_required
 @managers_only
-def done_checking(exercise_id, solution_id):
-    if request.method == 'POST':
-        assessment_id = request.json.get('assessment')
-    else:  # it's a GET
-        assessment_id = request.args.get('assessment')
-    is_updated = solutions.mark_as_checked(
-        solution_id, current_user.id, assessment_id,
-    )
+def done_checking(exercise_id: int, solution_id: int):
+    is_updated = solutions.mark_as_checked(solution_id, current_user.id)
     next_solution = solutions.get_next_unchecked(exercise_id)
     next_solution_id = getattr(next_solution, 'id', None)
     return jsonify({'success': is_updated, 'next': next_solution_id})

Diff for: lms/models/solutions.py

@@ -65,13 +65,16 @@ def get_message_and_addressee(
     return msg, addressee
 
 
-def mark_as_checked(
-    solution_id: int, checker_id: int, assessment_id: Optional[int] = None,
+def change_assessment(
+    solution_id: int, assessment_id: Optional[int] = None,
 ) -> bool:
     checked_solution: Solution = Solution.get_by_id(solution_id)
-    is_updated = checked_solution.mark_as_checked(
-        assessment_id=assessment_id, by=checker_id,
-    )
+    return checked_solution.change_assessment(assessment_id=assessment_id)
+
+
+def mark_as_checked(solution_id: int, checker_id: int) -> bool:
+    checked_solution: Solution = Solution.get_by_id(solution_id)
+    is_updated = checked_solution.mark_as_checked(by=checker_id)
     msg = _(
         'Your solution for the "%(subject)s" exercise has been checked.',
         subject=checked_solution.exercise.subject,

Diff for: lms/static/checker.js

@@ -1,7 +1,5 @@
 function trackFinished(exerciseId, solutionId, element) {
   element.addEventListener('click', () => {
-    const assessment = document.querySelector('input[name="assessment"]:checked');
-    const assessmentValue = (assessment !== null) ? assessment.value : null;
     const xhr = new XMLHttpRequest();
     xhr.open('POST', `/checked/${exerciseId}/${solutionId}`, true);
     xhr.setRequestHeader('Content-Type', 'application/json');
@@ -20,20 +18,33 @@ function trackFinished(exerciseId, solutionId, element) {
       }
     };
 
-    xhr.send(JSON.stringify({
-      assessment: assessmentValue,
-    }));
+    xhr.send(JSON.stringify({}));
   });
 }
 
 function changeAssessmentsAttributes(assessmentGroup, item) {
   if (item.value == assessmentGroup.dataset.checkedid) {
     item.removeAttribute('checked');
     item.checked = false;
-    assessmentGroup.dataset.checkedid = 'None';
+    assessmentGroup.dataset.checkedid = null;
   } else {
     assessmentGroup.dataset.checkedid = item.value;
   }
+  document.activeElement.blur();
+
+  const xhr = new XMLHttpRequest();
+  xhr.open('POST', `/assessment/${solutionId}`, true);
+    xhr.setRequestHeader('Content-Type', 'application/json');
+    xhr.responseType = 'json';
+    xhr.onreadystatechange = () => {
+      if (xhr.readyState === 4) {
+        if (xhr.status !== 200) {
+          console.log(xhr.status);
+        }
+      }
+    };
+
+    xhr.send(JSON.stringify({assessment: assessmentGroup.dataset.checkedid}));
 }
 
 function trackAssessmentButtons() {

Diff for: lms/static/my.css

@@ -795,7 +795,7 @@ code .grader-add .fa {
 }
 
 #courses-list {
-  overflow-y: scroll;
+  overflow-y: auto;
   max-height: 10em;
 }
 

Diff for: lms/templates/view.html

@@ -3,7 +3,7 @@
 {% block page_content %}
 <div id="page-view">
   <div id="view-head">
-    <h1>{{ _('Exercise view') }} {{ solution['exercise']['id'] }}: {{ solution['exercise']['subject'] }}</h1>
+    <h1>{{ _('Exercise view') }} {{ solution['exercise']['number'] }}: {{ solution['exercise']['subject'] }}</h1>
     {% if not is_manager and not shared_url %}
       {% if solution['state'] == 'DONE' %}
         <p class="checked-msg {{ direction }}"><strong>{{ _('Your solution had checked!') }}</strong> {{ _('Click on the red lines in order to see the comments.') }}</p>

Diff for: tests/test_notifications.py

@@ -178,7 +178,7 @@ def test_user_commented_after_check(
         client = conftest.get_logged_user(student_user.username)
 
         # Marking the solution as checked
-        solutions.mark_as_checked(solution.id, staff_user.id, 1)
+        solutions.mark_as_checked(solution.id, staff_user.id)
         solution = Solution.get_by_id(solution.id)
 
         # Sending comments after solution checked

Diff for: tests/test_solutions.py

@@ -132,7 +132,7 @@ def test_mark_as_checked(
     ):
         # Basic functionality
         assert solution.state == Solution.STATES.CREATED.name
-        marked = solutions.mark_as_checked(solution.id, staff_user.id, 2)
+        marked = solutions.mark_as_checked(solution.id, staff_user.id)
         # HELL WITH PEEWEE!!!
         solution = Solution.get_by_id(solution.id)
         assert marked
@@ -142,7 +142,7 @@ def test_mark_as_checked(
         # Not duplicating things
         staff_user2 = conftest.create_staff_user(index=1)
         solution2 = conftest.create_solution(exercise, student_user)
-        marked = solutions.mark_as_checked(solution2.id, staff_user2.id, 3)
+        marked = solutions.mark_as_checked(solution2.id, staff_user2.id)
         solution2 = Solution.get_by_id(solution2.id)
         assert solution2.state == Solution.STATES.DONE.name
         assert solution2.checker == staff_user2
@@ -171,21 +171,21 @@ def test_get_next_unchecked(
         assert unchecked.exercise.id == solution1.exercise.id
         assert unchecked == solution1
 
-        solutions.mark_as_checked(solution1.id, staff_user, 4)
+        solutions.mark_as_checked(solution1.id, staff_user)
         unchecked = solutions.get_next_unchecked(exercise.id)
         assert unchecked is not None
         assert unchecked.exercise.id == solution3.exercise.id
         assert unchecked == solution3
 
-        solutions.mark_as_checked(solution3.id, staff_user, 1)
+        solutions.mark_as_checked(solution3.id, staff_user)
         unchecked = solutions.get_next_unchecked(exercise.id)
         assert unchecked is None
 
         unchecked = solutions.get_next_unchecked()
         assert unchecked is not None
         assert unchecked == solution2
 
-        solutions.mark_as_checked(solution2.id, staff_user, 2)
+        solutions.mark_as_checked(solution2.id, staff_user)
         unchecked = solutions.get_next_unchecked()
         assert unchecked is None
 
@@ -563,7 +563,7 @@ def test_last_view_status(
         solution = Solution.get_by_id(solution.id)
         assert solution.last_status_view == SolutionStatusView.NOT_CHECKED.name
 
-        solutions.mark_as_checked(solution.id, staff_user.id, 3)
+        solutions.mark_as_checked(solution.id, staff_user.id)
         solution = Solution.get_by_id(solution.id)
         assert solution.last_status_view == SolutionStatusView.NOT_CHECKED.name
         client.get(f'/view/{solution.id}')
@@ -590,8 +590,6 @@ def test_done_checking(
         client = conftest.get_logged_user(staff_user.username)
         response = client.post(
             f'/checked/{solution.exercise.id}/{solution.id}',
-            data=json.dumps({'assessment': 1}),
-            content_type='application/json',
         )
         assert response.status_code == 200
 
@@ -636,14 +634,30 @@ def test_solutions_of_user(
         solution: Solution, _assessments,
     ):
         conftest.create_usercourse(student_user, course)
-        solutions.mark_as_checked(solution.id, staff_user.id, 2)
+        client = conftest.get_logged_user(staff_user.username)
+        client.post(
+            f'/assessment/{solution.id}',
+            data=json.dumps({'assessment': 2}),
+            content_type='application/json',
+        )
         solution = Solution.get_by_id(solution.id)
+        assert solution.assessment.name == 'Nice'
+
+        client.post(
+            f'/assessment/{solution.id}',
+            data=json.dumps({'assessment': None}),
+            content_type='application/json',
+        )
 
         exercise2 = conftest.create_exercise(course, 2)
         solution2 = conftest.create_solution(exercise2, student_user)
-        solutions.mark_as_checked(solution2.id, staff_user.id)
+        client.post(
+            f'/assessment/{solution2.id}',
+            data=json.dumps({'assessment': 3}),
+            content_type='application/json',
+        )
         solution2 = Solution.get_by_id(solution2.id)
 
         exercises = solution.of_user(student_user.id, from_all_courses=True)
-        assert exercises[0].get('assessment') == 'Nice'
-        assert exercises[1].get('assessment') is None
+        assert exercises[0].get('assessment') is None
+        assert exercises[1].get('assessment') == 'Try again'