refactor: core/views/index.py

core/templates/core/search.jinja

@@ -9,19 +9,17 @@
 {% block content %}
   <h4>{% trans %}Users{% endtrans %}</h4>
   <ul>
-    {% for i in result.users %}
-      {% if user.can_view(i) %}
+    {% for user in users %}
       <li>
-          {{ user_link_with_pict(i) }}
+        {{ user_link_with_pict(user) }}
       </li>
-      {% endif %}
     {% endfor %}
   </ul>
   <h4>{% trans %}Clubs{% endtrans %}</h4>
   <ul>
-    {% for i in result.clubs %}
+    {% for club in clubs %}
       <li>
-        <a href="{{ url("club:club_view", club_id=i.id) }}">{{ i }}</a>
+        <a href="{{ url("club:club_view", club_id=club.id) }}">{{ club }}</a>
       </li>
     {% endfor %}
   </ul>

core/tests/test_core.py

@@ -35,6 +35,7 @@ from pytest_django.asserts import assertInHTML, assertRedirects
 
 from antispam.models import ToxicDomain
 from club.models import Club, Membership
+from core.baker_recipes import subscriber_user
 from core.markdown import markdown
 from core.models import AnonymousUser, Group, Page, User, validate_promo
 from core.utils import get_last_promo, get_semester_code, get_start_of_semester
@@ -551,3 +552,10 @@ def test_allow_fragment_mixin():
     assert not TestAllowFragmentView.as_view()(request)
     request.headers = {"HX-Request": True, **base_headers}
     assert TestAllowFragmentView.as_view()(request)
+
+
+@pytest.mark.django_db
+def test_search_view(client: Client):
+    client.force_login(subscriber_user.make())
+    response = client.get(reverse("core:search", query={"query": "foo"}))
+    assert response.status_code == 200

core/tests/test_notification.py

@@ -0,0 +1,64 @@
+from datetime import timedelta
+from operator import attrgetter
+
+import pytest
+from bs4 import BeautifulSoup
+from django.test import Client, TestCase
+from django.urls import reverse
+from django.utils.timezone import now
+from model_bakery import baker, seq
+from pytest_django.asserts import assertRedirects
+
+from core.baker_recipes import subscriber_user
+from core.models import Notification
+
+
+@pytest.mark.django_db
+class TestNotificationList(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = subscriber_user.make()
+        url = reverse("core:user_profile", kwargs={"user_id": cls.user.id})
+        cls.notifs = baker.make(
+            Notification,
+            user=cls.user,
+            url=url,
+            viewed=False,
+            date=seq(now() - timedelta(days=1), timedelta(hours=1)),
+            _quantity=10,
+            _bulk_create=True,
+        )
+
+    def test_list(self):
+        self.client.force_login(self.user)
+        response = self.client.get(reverse("core:notification_list"))
+        assert response.status_code == 200
+        soup = BeautifulSoup(response.text, "lxml")
+        ul = soup.find("ul", id="notifications")
+        elements = list(ul.find_all("li"))
+        assert len(elements) == len(self.notifs)
+        notifs = sorted(self.notifs, key=attrgetter("date"), reverse=True)
+        for element, notif in zip(elements, notifs, strict=True):
+            assert element.find("a")["href"] == reverse(
+                "core:notification", kwargs={"notif_id": notif.id}
+            )
+
+    def test_read_all(self):
+        self.client.force_login(self.user)
+        response = self.client.get(
+            reverse("core:notification_list", query={"read_all": None})
+        )
+        assert response.status_code == 200
+        assert not self.user.notifications.filter(viewed=True).exists()
+
+
+@pytest.mark.django_db
+def test_notification_redirect(client: Client):
+    user = subscriber_user.make()
+    url = reverse("core:user_profile", kwargs={"user_id": user.id})
+    notif = baker.make(Notification, user=user, url=url, viewed=False)
+    client.force_login(user)
+    response = client.get(reverse("core:notification", kwargs={"notif_id": notif.id}))
+    assertRedirects(response, url)
+    notif.refresh_from_db()
+    assert notif.viewed is True

core/urls.py

@@ -24,6 +24,7 @@
 from django.urls import path, re_path, register_converter
 from django.views.generic import RedirectView
 
+from com.views import NewsListView
 from core.converters import (
     BooleanStringConverter,
     FourDigitYearConverter,
@@ -53,6 +54,7 @@ from core.views import (
     PagePropView,
     PageRevView,
     PageView,
+    SearchView,
     SithLoginView,
     SithPasswordChangeDoneView,
     SithPasswordChangeView,
@@ -76,13 +78,9 @@ from core.views import (
     UserUpdateProfileView,
     UserView,
     delete_user_godfather,
-    index,
     logout,
     notification,
     password_root_change,
-    search_json,
-    search_user_json,
-    search_view,
     send_file,
 )
 
@@ -91,13 +89,11 @@ register_converter(TwoDigitMonthConverter, "mm")
 register_converter(BooleanStringConverter, "bool")
 
 urlpatterns = [
-    path("", index, name="index"),
+    path("", NewsListView.as_view(), name="index"),
     path("notifications/", NotificationList.as_view(), name="notification_list"),
     path("notification/<int:notif_id>/", notification, name="notification"),
     # Search
-    path("search/", search_view, name="search"),
-    path("search_json/", search_json, name="search_json"),
-    path("search_user/", search_user_json, name="search_user"),
+    path("search/", SearchView.as_view(), name="search"),
     # Login and co
     path("login/", SithLoginView.as_view(), name="login"),
     path("logout/", logout, name="logout"),

core/views/index.py

@@ -22,106 +22,49 @@
 #
 #
 
-import json
-
 from django.conf import settings
-from django.contrib.auth.decorators import login_required
-from django.core import serializers
+from django.contrib.auth.mixins import LoginRequiredMixin
+from django.db.models import F
 from django.db.models.query import QuerySet
-from django.http import JsonResponse
-from django.shortcuts import redirect, render
-from django.utils import html
-from django.utils.text import slugify
-from django.views.generic import ListView
-from haystack.query import SearchQuerySet
+from django.http import HttpRequest
+from django.shortcuts import get_object_or_404, redirect
+from django.views.generic import ListView, TemplateView
 
 from club.models import Club
 from core.models import Notification, User
+from core.schemas import UserFilterSchema
 
 
-def index(request, context=None):
-    from com.views import NewsListView
-
-    return NewsListView.as_view()(request)
-
-
-class NotificationList(ListView):
+class NotificationList(LoginRequiredMixin, ListView):
     model = Notification
     template_name = "core/notification_list.jinja"
 
     def get_queryset(self) -> QuerySet[Notification]:
-        if self.request.user.is_anonymous:
-            return Notification.objects.none()
-        # TODO: Bulk update in django 2.2
         if "see_all" in self.request.GET:
             self.request.user.notifications.filter(viewed=False).update(viewed=True)
         return self.request.user.notifications.order_by("-date")[:20]
 
 
-def notification(request, notif_id):
-    notif = Notification.objects.filter(id=notif_id).first()
-    if notif:
+def notification(request: HttpRequest, notif_id: int):
+    notif = get_object_or_404(Notification, id=notif_id)
     if notif.type not in settings.SITH_PERMANENT_NOTIFICATIONS:
         notif.viewed = True
     else:
         notif.callback()
     notif.save()
     return redirect(notif.url)
-    return redirect("/")
 
 
-def search_user(query):
-    try:
-        # slugify turns everything into ascii and every whitespace into -
-        # it ends by removing duplicate - (so ' - ' will turn into '-')
-        # replace('-', ' ') because search is whitespace based
-        query = slugify(query).replace("-", " ")
-        # TODO: is this necessary?
-        query = html.escape(query)
-        res = (
-            SearchQuerySet()
-            .models(User)
-            .autocomplete(auto=query)
-            .order_by("-last_login")
-            .load_all()[:20]
+class SearchView(LoginRequiredMixin, TemplateView):
+    template_name = "core/search.jinja"
+
+    def get_context_data(self, **kwargs):
+        users, clubs = [], []
+        if query := self.request.GET.get("query"):
+            users = list(
+                UserFilterSchema(search=query)
+                .filter(User.objects.viewable_by(self.request.user))
+                .order_by(F("last_login").desc(nulls_last=True))
             )
-        return [r.object for r in res]
-    except TypeError:
-        return []
-
-
-def search_club(query, *, as_json=False):
-    clubs = []
-    if query:
-        clubs = Club.objects.filter(name__icontains=query).all()
-        clubs = clubs[:5]
-        if as_json:
-            # Re-loads json to avoid double encoding by JsonResponse, but still benefit from serializers
-            clubs = json.loads(serializers.serialize("json", clubs, fields=("name")))
-        else:
-            clubs = list(clubs)
-    return clubs
-
-
-@login_required
-def search_view(request):
-    result = {
-        "users": search_user(request.GET.get("query", "")),
-        "clubs": search_club(request.GET.get("query", "")),
-    }
-    return render(request, "core/search.jinja", context={"result": result})
-
-
-@login_required
-def search_user_json(request):
-    result = {"users": search_user(request.GET.get("query", ""))}
-    return JsonResponse(result)
-
-
-@login_required
-def search_json(request):
-    result = {
-        "users": search_user(request.GET.get("query", "")),
-        "clubs": search_club(request.GET.get("query", ""), as_json=True),
-    }
-    return JsonResponse(result)
+            clubs = list(Club.objects.filter(name__icontains=query)[:5])
+        return super().get_context_data(**kwargs) | {"users": users, "clubs": clubs}

matmat/views.py

@@ -24,6 +24,7 @@ from ast import literal_eval
 from enum import Enum
 
 from django import forms
+from django.db.models import F
 from django.http.response import HttpResponseRedirect
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -34,7 +35,7 @@ from phonenumber_field.widgets import RegionalPhoneNumberWidget
 
 from core.auth.mixins import FormerSubscriberMixin
 from core.models import User
-from core.views import search_user
+from core.schemas import UserFilterSchema
 from core.views.forms import SelectDate
 
 # Enum to select search type
@@ -126,11 +127,13 @@ class SearchFormListView(FormerSubscriberMixin, SingleObjectMixin, ListView):
                 q = q.filter(phone=self.valid_form["phone"]).all()
             elif self.search_type == SearchType.QUICK:
                 if self.valid_form["quick"].strip():
-                    q = search_user(self.valid_form["quick"])
+                    q = list(
+                        UserFilterSchema(search=self.valid_form["quick"])
+                        .filter(User.objects.viewable_by(self.request.user))
+                        .order_by(F("last_login").desc(nulls_last=True))
+                    )
                 else:
                     q = []
-                if not self.can_see_hidden and len(q) > 0:
-                    q = [user for user in q if user.is_viewable]
             else:
                 search_dict = {}
                 for key, value in self.valid_form.items():