fix: assessments buttons (#339)

* fix: assessments buttons

- Unfocus after clicking an assessment
- Assessment is now changed after select and not after fully checked solution

Author: orronai

lms/lmsdb/models.py

@@ -619,16 +619,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)
@@ -771,19 +766,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']:

lms/lmsweb/views.py

@@ -673,17 +673,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})

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,

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() {

tests/test_notifications.py

@@ -175,7 +175,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

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'