Remove navbar cache (#1849)

* Remove navbar cache

Co-authored-by: Richard Ebeling <He3lixxx@users.noreply.github.com>

evap/evaluation/models.py

@@ -126,10 +126,6 @@ def archive_results(self):
         self.results_are_archived = True
         self.save()
 
-    @classmethod
-    def get_all_with_unarchived_results(cls):
-        return cls.objects.filter(results_are_archived=False).distinct()
-
     @classmethod
     def get_all_with_published_unarchived_results(cls):
         return cls.objects.filter(

evap/evaluation/templates/base.html

@@ -37,9 +37,7 @@
         {% endblock %}
 
         <div class="sticky-top d-print-none">
-            {% cache 3600 navbar request.user.email LANGUAGE_CODE %}
-                {% include_navbar user LANGUAGE_CODE %}
-            {% endcache %}
+            {% include_navbar user LANGUAGE_CODE %}
 
             {% block breadcrumb_bar %}
             {% endblock %}

evap/evaluation/templatetags/navbar_templatetags.py

@@ -1,3 +1,4 @@
+from django.db.models import Q
 from django.template import Library
 
 from evap.evaluation.models import Semester
@@ -8,12 +9,26 @@
 
 @register.inclusion_tag("navbar.html")
 def include_navbar(user, language):
+    semesters_with_unarchived_results_or_grade_documents = Semester.objects.filter(
+        Q(results_are_archived=False) | Q(grade_documents_are_deleted=False)
+    )
+
+    semesters_with_unarchived_results = {
+        semester
+        for semester in semesters_with_unarchived_results_or_grade_documents
+        if not semester.results_are_archived
+    }
+    semesters_with_grade_documents = {
+        semester
+        for semester in semesters_with_unarchived_results_or_grade_documents
+        if not semester.grade_documents_are_deleted
+    }
+
     return {
         "user": user,
         "current_language": language,
         "languages": LANGUAGES,
-        "published_result_semesters": Semester.get_all_with_published_unarchived_results(),
-        "result_semesters": Semester.get_all_with_unarchived_results(),
-        "grade_document_semesters": Semester.objects.filter(grade_documents_are_deleted=False),
+        "result_semesters": semesters_with_unarchived_results,
+        "grade_document_semesters": semesters_with_grade_documents,
         "debug": DEBUG,
     }

evap/evaluation/views.py

@@ -18,7 +18,6 @@
 from evap.evaluation.forms import DelegatesForm, LoginEmailForm, NewKeyForm
 from evap.evaluation.models import EmailTemplate, FaqSection, Semester
 from evap.middleware import no_login_required
-from evap.staff.tools import delete_navbar_cache_for_users
 
 logger = logging.getLogger(__name__)
 
@@ -96,9 +95,6 @@ def index(request):
         )
         return render(request, "index.html", template_data)
 
-    # the cached navbar might contain CSRF tokens that are invalid after a new login
-    delete_navbar_cache_for_users([request.user])
-
     # check for redirect variable
     redirect_to = request.GET.get("next", None)
     if redirect_to is not None and url_has_allowed_host_and_scheme(redirect_to, None):

evap/results/tests/test_views.py

@@ -1,4 +1,3 @@
-import random
 from io import StringIO
 from unittest.mock import patch
 
@@ -459,16 +458,24 @@ def test_heading_question_filtering(self):
         self.assertIn(likert_question.text, page)
         self.assertNotIn(heading_question_2.text, page)
 
+    @override_settings(VOTER_COUNT_NEEDED_FOR_PUBLISHING_RATING_RESULTS=0)
     def test_default_view_is_public(self):
         cache_results(self.evaluation)
-        random.seed(42)  # use explicit seed to always choose the same "random" slogan
+
+        # the view=public button should have class "active". The rest in-between is just noise.
+        expected_button = (
+            f'<a href="/results/semester/{self.evaluation.course.semester.pk}/evaluation/{self.evaluation.pk}'
+            + '?view=public" role="button" class="btn btn-sm btn-light active"'
+        )
+
         page_without_get_parameter = self.app.get(self.url, user=self.manager)
-        random.seed(42)
+        self.assertContains(page_without_get_parameter, expected_button)
+
         page_with_get_parameter = self.app.get(self.url + "?view=public", user=self.manager)
-        random.seed(42)
+        self.assertContains(page_with_get_parameter, expected_button)
+
         page_with_random_get_parameter = self.app.get(self.url + "?view=asdf", user=self.manager)
-        self.assertEqual(page_without_get_parameter.body, page_with_get_parameter.body)
-        self.assertEqual(page_without_get_parameter.body, page_with_random_get_parameter.body)
+        self.assertContains(page_with_random_get_parameter, expected_button)
 
     def test_wrong_state(self):
         helper_exit_staff_mode(self)

evap/staff/tests/test_tools.py

@@ -1,43 +1,10 @@
 from django.contrib.auth.models import Group
-from django.core.cache import cache
-from django.core.cache.utils import make_template_fragment_key
 from django.test import TestCase
 from model_bakery import baker
 
 from evap.evaluation.models import Contribution, Course, Evaluation, UserProfile
-from evap.evaluation.tests.tools import WebTest
 from evap.rewards.models import RewardPointGranting, RewardPointRedemption
-from evap.staff.tools import (
-    delete_navbar_cache_for_users,
-    merge_users,
-    remove_user_from_represented_and_ccing_users,
-    user_edit_link,
-)
-
-
-class NavbarCacheTest(WebTest):
-    def test_navbar_cache_deletion_for_users(self):
-        user1 = baker.make(UserProfile, email="user1@institution.example.com")
-        user2 = baker.make(UserProfile, email="user2@institution.example.com")
-
-        # create navbar caches for anonymous user, user1 and user2
-        self.app.get("/")
-        self.app.get("/results/", user="user1@institution.example.com")
-        self.app.get("/results/", user="user2@institution.example.com")
-
-        cache_key1 = make_template_fragment_key("navbar", [user1.email, "en"])
-        cache_key2 = make_template_fragment_key("navbar", [user2.email, "en"])
-        cache_key_anonymous = make_template_fragment_key("navbar", ["", "en"])
-
-        self.assertIsNotNone(cache.get(cache_key1))
-        self.assertIsNotNone(cache.get(cache_key2))
-        self.assertIsNotNone(cache.get(cache_key_anonymous))
-
-        delete_navbar_cache_for_users([user2])
-
-        self.assertIsNotNone(cache.get(cache_key1))
-        self.assertIsNone(cache.get(cache_key2))
-        self.assertIsNotNone(cache.get(cache_key_anonymous))
+from evap.staff.tools import merge_users, remove_user_from_represented_and_ccing_users, user_edit_link
 
 
 class MergeUsersTest(TestCase):

evap/staff/tools.py

@@ -5,8 +5,6 @@
 from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.models import Group
-from django.core.cache import cache
-from django.core.cache.utils import make_template_fragment_key
 from django.core.exceptions import SuspiciousOperation
 from django.db import transaction
 from django.db.models import Count
@@ -63,15 +61,6 @@ def get_import_file_content_or_raise(user_id, import_type):
         return file.read()
 
 
-def delete_navbar_cache_for_users(users):
-    # delete navbar cache from base.html
-    for user in users:
-        key = make_template_fragment_key("navbar", [user.email, "de"])
-        cache.delete(key)
-        key = make_template_fragment_key("navbar", [user.email, "en"])
-        cache.delete(key)
-
-
 def create_user_list_html_string_for_message(users):
     return format_html_join("", "<br />{} {} ({})", ((user.first_name, user.last_name, user.email) for user in users))
 

evap/staff/views.py

@@ -102,7 +102,6 @@
     ImportType,
     bulk_update_users,
     delete_import_file,
-    delete_navbar_cache_for_users,
     find_unreviewed_evaluations,
     get_import_file_content_or_raise,
     import_file_exists,
@@ -557,10 +556,6 @@ def semester_create(request):
 
     if form.is_valid():
         semester = form.save()
-        delete_navbar_cache_for_users(
-            [user for user in UserProfile.objects.all() if user.is_reviewer or user.is_grade_publisher]
-        )
-
         messages.success(request, _("Successfully created semester."))
         return redirect("staff:semester_view", semester.id)
 
@@ -606,9 +601,6 @@ def semester_delete(request):
     Evaluation.objects.filter(course__semester=semester).delete()
     Course.objects.filter(semester=semester).delete()
     semester.delete()
-    delete_navbar_cache_for_users(
-        [user for user in UserProfile.objects.all() if user.is_reviewer or user.is_grade_publisher]
-    )
     return redirect("staff:index")
 
 
@@ -652,7 +644,6 @@ def semester_import(request, semester_id):
                 )
                 importer_log.forward_messages_to_django(request)
                 delete_import_file(request.user.id, import_type)
-                delete_navbar_cache_for_users(UserProfile.objects.all())
                 return redirect("staff:semester_view", semester_id)
 
     test_passed = import_file_exists(request.user.id, import_type)
@@ -910,7 +901,6 @@ def semester_delete_grade_documents(request):
     if not semester.grade_documents_can_be_deleted:
         raise SuspiciousOperation("Deleting grade documents for this semester is not allowed")
     semester.delete_grade_documents()
-    delete_navbar_cache_for_users(UserProfile.objects.all())
     return HttpResponse()  # 200 OK
 
 
@@ -923,7 +913,6 @@ def semester_archive_results(request):
     if not semester.results_can_be_archived:
         raise SuspiciousOperation("Archiving results for this semester is not allowed")
     semester.archive_results()
-    delete_navbar_cache_for_users(UserProfile.objects.all())
     return HttpResponse()  # 200 OK
 
 
@@ -1212,9 +1201,6 @@ def notify_reward_points(grantings, **_kwargs):
         else:
             messages.success(request, _("Successfully updated evaluation."))
 
-        delete_navbar_cache_for_users(evaluation.participants.all())
-        delete_navbar_cache_for_users(UserProfile.objects.filter(contributions__evaluation=evaluation))
-
         return redirect("staff:semester_view", semester.id)
 
     assert set(Answer.__subclasses__()) == {TextAnswer, RatingAnswerCounter}
@@ -2093,7 +2079,6 @@ def notify_reward_points(grantings, **_kwargs):
 
     if form.is_valid():
         form.save()
-        delete_navbar_cache_for_users([user])
         messages.success(request, _("Successfully updated user."))
         for message in form.remove_messages:
             messages.warning(request, message)