Sorting for participants in Issue #1975 (#2061)

* added sorting for participants (part of issue #1975)

* Co-authored-by: @till2

* Make datagrid self-repair on broken localstorage sorting entry

* Aggregated sorting for courses.

---------

Co-authored-by: Constantin Kühne <[email protected]>
Co-authored-by: Richard Ebeling <[email protected]>

Author: 3 people

evap/evaluation/models.py

@@ -813,6 +813,12 @@ def num_voters(self):
             return self._voter_count
         return self.voters.count()
 
+    @property
+    def voter_ratio(self):
+        if self.is_single_result or self.num_participants == 0:
+            return 0
+        return self.num_voters / self.num_participants
+
     @property
     def due_participants(self):
         return self.participants.exclude(pk__in=self.voters.all())

evap/results/templates/results_evaluation_detail.html

@@ -127,19 +127,17 @@ <h3>{{ evaluation.full_name }} ({{ evaluation.course.semester.name }})</h3>
                         <div data-col="name">{% trans 'Evaluation' %}</div>
                         <div data-col="semester">{% trans 'Semester' %}</div>
                         <div data-col="responsible">{% trans 'Responsible' %}</div>
-                        <div data-col="participants">{% trans 'Voters' %}</div>
+                        <div data-col="voters">{% trans 'Voters' %}</div>
                         <div data-col="result">{% trans 'Distribution and average grade' %}</div>
                 </div>
                 {% if course_evaluations %}
-                    {% include 'results_index_course.html' %}
+                    {% include 'results_index_course.html' with evaluations=course_evaluations %}
                     {% for course_evaluation in course_evaluations|dictsort:"name" %}
                         {% if course_evaluation.pk == evaluation.pk %}
                             {# we cannot add a class to the actual row because it is cached globally. so we use a + selector in css to style the following row. #}
                             <div class="current-row-follows"></div>
                         {% endif %}
-                        {% with evaluation=course_evaluation %}
-                            {% include 'results_index_evaluation.html' with links_to_results_page=evaluation|can_results_page_be_seen_by:request.user is_subentry=True %}
-                        {% endwith %}
+                        {% include 'results_index_evaluation.html' with evaluation=course_evaluation links_to_results_page=course_evaluation|can_results_page_be_seen_by:request.user is_subentry=True %}
                     {% endfor %}
                 {% else %}
                     {% include 'results_index_evaluation.html' with links_to_results_page=False is_subentry=False %}

evap/results/templates/results_index.html

@@ -77,7 +77,7 @@ <h5 class="card-title mb-lg-0">
                 <div data-col="name" class="col-order">{% trans 'Evaluation' %}</div>
                 <div data-col="semester" class="col-order">{% trans 'Semester' %}</div>
                 <div data-col="responsible" class="col-order">{% trans 'Responsible' %}</div>
-                <div data-col="participants">{% trans 'Voters' %}</div>
+                <div data-col="voters" class="col-order">{% trans 'Voters' %}</div>
                 <div data-col="result" class="col-order">{% trans 'Distribution and average grade' %}</div>
             </div>
             <div class="d-flex d-lg-none">
@@ -96,6 +96,7 @@ <h5 class="card-title mb-lg-0">
                     <option value="name">{% trans 'Evaluation' %}</option>
                     <option value="semester">{% trans 'Semester' %}</option>
                     <option value="responsible">{% trans 'Responsible' %}</option>
+                    <option value="voters">{% trans 'Voters' %}</option>
                     <option value="result">{% trans 'Distribution and average grade' %}</option>
                 </select>
             </div>

evap/results/templates/results_index_course_impl.html

@@ -17,6 +17,7 @@
     <div data-col="responsible" data-order="{{ course.responsibles.first.last_name|lower }}">
         {{ course.responsibles_names }}
     </div>
+    <div data-col="voters" data-order="{{ evaluations|aggregated_voters_order }}"></div>
     <div data-col="result" data-order="{{ course.avg_grade|default:7 }}">
         {% if course.not_all_evaluations_are_published %}
             <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Course result is not yet available.' %}">

evap/results/templates/results_index_evaluation_impl.html

@@ -1,5 +1,6 @@
 {% load evaluation_filters %}
 {% load results_templatetags %}
+{% load l10n %}
 
 <{% if links_to_results_page %}a href="{% url 'results:evaluation_detail' evaluation.course.semester.id evaluation.id %}"{% else %}div{% endif %}
         class="results-grid-row {% if not is_subentry %}heading-row{% else %}evaluation-row{% endif %}
@@ -49,14 +50,13 @@
             {{ evaluation.course.responsibles_names }}
         </div>
     {% endif %}
-    {% if evaluation.is_single_result %}
-        <div data-col="participants" class="text-center"><span class="fas fa-user"></span>&nbsp;{{ evaluation.single_result_rating_result.count_sum }}</div>
-    {% else %}
-        {% with num_participants=evaluation.num_participants num_voters=evaluation.num_voters %}
-            <div data-col="participants">{% include 'progress_bar.html' with done=num_voters total=num_participants %}</div>
-        {% endwith %}
-    {% endif %}
-
+    <div data-col="voters" class="text-center" data-order="{{ evaluation|voters_order }}">
+        {% if evaluation.is_single_result %}
+            <span class="fas fa-user"></span>&nbsp;{{ evaluation.single_result_rating_result.count_sum }}
+        {% else %}
+            {% include 'progress_bar.html' with done=evaluation.num_voters total=evaluation.num_participants %}
+        {% endif %}
+    </div>
     <div data-col="result"{% if not is_subentry %} data-order="{% if evaluation.is_single_result %}{{ evaluation.single_result_rating_result.average }}{% else %}{{ evaluation.avg_grade|default:7 }}{% endif %}"{% endif %}>
         {% if not evaluation.can_staff_see_average_grade %}
             <div class="d-flex" data-bs-toggle="tooltip" data-bs-placement="left" title="{% trans 'Evaluation result is not yet available.' %}">

evap/results/templatetags/results_templatetags.py

@@ -1,3 +1,5 @@
+from typing import Any, Iterable
+
 from django.template import Library
 
 from evap.results.tools import (
@@ -40,3 +42,14 @@ def has_answers(rating_result: RatingResult):
 @register.filter
 def is_published(rating_result: RatingResult):
     return RatingResult.is_published(rating_result)
+
+
+@register.filter
+def voters_order(evaluation) -> str:
+    """float to string conversion done in python to circumvent localization breaking number parsing"""
+    return str(evaluation.voter_ratio)
+
+
+@register.filter
+def aggregated_voters_order(evaluations: Iterable[Any]) -> str:
+    return str(max(evaluation.voter_ratio for evaluation in evaluations))

evap/results/views.py

@@ -61,7 +61,8 @@ def _delete_course_template_cache_impl(course):
 def update_template_cache(evaluations):
     assert all(evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING for evaluation in evaluations)
     evaluations = get_evaluations_with_course_result_attributes(get_evaluations_with_prefetched_data(evaluations))
-    courses_to_render = {evaluation.course for evaluation in evaluations if evaluation.course.evaluation_count > 1}
+
+    courses_and_evaluations = unordered_groupby((evaluation.course, evaluation) for evaluation in evaluations)
 
     current_language = translation.get_language()
 
@@ -72,25 +73,25 @@ def update_template_cache(evaluations):
         for lang in ["en", "de"]:
             translation.activate(lang)
 
-            for course in courses_to_render:
-                caches["results"].set(
-                    get_course_result_template_fragment_cache_key(course.id, lang),
-                    results_index_course_template.render({"course": course}),
-                )
-
-            for evaluation in evaluations:
-                assert evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING
-                is_subentry = evaluation.course.evaluation_count > 1
-                base_args = {"evaluation": evaluation, "is_subentry": is_subentry}
-
-                caches["results"].set(
-                    get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, True),
-                    results_index_evaluation_template.render({**base_args, "links_to_results_page": True}),
-                )
-                caches["results"].set(
-                    get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, False),
-                    results_index_evaluation_template.render({**base_args, "links_to_results_page": False}),
-                )
+            for course, course_evaluations in courses_and_evaluations.items():
+                if len(course_evaluations) > 1:
+                    caches["results"].set(
+                        get_course_result_template_fragment_cache_key(course.id, lang),
+                        results_index_course_template.render({"course": course, "evaluations": course_evaluations}),
+                    )
+
+                for evaluation in course_evaluations:
+                    assert evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING
+                    base_args = {"evaluation": evaluation, "is_subentry": len(course_evaluations) > 1}
+
+                    caches["results"].set(
+                        get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, True),
+                        results_index_evaluation_template.render({**base_args, "links_to_results_page": True}),
+                    )
+                    caches["results"].set(
+                        get_evaluation_result_template_fragment_cache_key(evaluation.id, lang, False),
+                        results_index_evaluation_template.render({**base_args, "links_to_results_page": False}),
+                    )
 
     finally:
         translation.activate(current_language)  # reset to previously set language to prevent unwanted side effects

evap/static/scss/components/_grid.scss

@@ -90,29 +90,29 @@
     @extend %table-grid;
 
     grid:
-        "name semester responsible participants result"
-        / auto    6rem       12rem         7rem  11rem;
+        "name semester responsible voters result"
+        / auto    6rem       12rem   7rem  11rem;
 
     @include media-breakpoint-down(lg) {
         grid:
             "name semester  responsible responsible"
-            "name semester participants      result"
+            "name semester       voters      result"
             / auto    3rem         7rem       10rem;
     }
 
     @include media-breakpoint-down(md) {
         grid:
             "        name        name    semester"
             " responsible responsible responsible"
-            "participants      result      result"
+            "      voters      result      result"
             /         1fr         1fr        6rem;
     }
 
     &:not(.grid-header) [data-col=responsible] {
         font-style: italic;
     }
 
-    [data-col=participants] {
+    [data-col=voters] {
         max-width: 8rem;
     }
 
@@ -164,7 +164,7 @@
     }
 }
 
-@each $col in name, semester, responsible, participants, result, answer, edit, review, flag {
+@each $col in name, semester, responsible, voters, result, answer, edit, review, flag {
     [data-col=#{$col}] {
         grid-area: $col;
     }

evap/static/ts/src/datagrid.ts

@@ -158,7 +158,13 @@ abstract class DataGrid {
             header.classList.remove("col-order-asc", "col-order-desc");
         }
         for (const [column, ordering] of this.state.order) {
-            this.sortableHeaders.get(column)!.classList.add(`col-order-${ordering}`);
+            const header = this.sortableHeaders.get(column);
+            if (header === undefined) {
+                // Silently ignore non-existing columns: They were probably renamed.
+                // A correct state will be built the next time the user sorts the datagrid.
+                continue;
+            }
+            header.classList.add(`col-order-${ordering}`);
         }
 
         this.rows.sort((a, b) => {