Make some views use URL query parameters instead of distinct paths

CHANGELOG.md

@@ -32,6 +32,11 @@ A summary of changes made to the codebase, grouped per deployment.
 - Enabled automated code quality checks from [Code Climate](https://codeclimate.com/quality)
 - Renamed lots of views, forms and templates to comply with
   [the style guides](https://github.com/MAKENTNU/web/blob/2826b57a6c6fe27446c88edb19ca167a728b5eb4/CONTRIBUTING.md#code-style-guides)
+- Changed multiple pages' URLs to use [query parameters](https://en.wikipedia.org/wiki/Query_string),
+  instead of having multiple distinct paths for practically the same page
+  (see [#650](https://github.com/MAKENTNU/web/pull/650) for a list of all the URLs affected)
+  - Permanent redirects have been added for the URLs deemed most relevant, to redirect from the old to the new URL; see [a list of the added URLs in
+    the code](https://github.com/MAKENTNU/web/pull/650/files#diff-37d0e0a00e828360d35d68a7ada510a98e03252045f3d51d36a81fedfaea7907R111-R115)
 - Changed some URLs so that they all consistently use `add` (instead of `create`) and `change` (instead of `edit` or `update`),
   to comply with [the style guide](https://github.com/MAKENTNU/web/blob/2826b57a6c6fe27446c88edb19ca167a728b5eb4/CONTRIBUTING.md#endpoint-pathroute)
 - Made the use of add/change verbs consistent among page titles and buttons, and some other parts of the website's UI

src/checkin/views.py

@@ -4,7 +4,7 @@
 
 from django.contrib import messages
 from django.contrib.auth.mixins import PermissionRequiredMixin
-from django.http import HttpResponse, HttpResponseRedirect, JsonResponse
+from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
 from django.utils import timezone
@@ -15,7 +15,7 @@
 
 from card import utils as card_utils
 from card.views import RFIDView
-from util.view_utils import PreventGetRequestsMixin
+from util.view_utils import PreventGetRequestsMixin, UTF8JsonResponse
 from .models import Profile, RegisterProfile, Skill, SuggestSkill, UserSkill
 
 
@@ -221,15 +221,15 @@ def post(self, request):
         if forced:
             Skill.objects.create(title=suggestion.title, title_en=suggestion.title_en, image=suggestion.image)
             suggestion.delete()
-            return JsonResponse(response_dict)
+            return UTF8JsonResponse(response_dict)
 
         suggestion.voters.add(request.user.profile)
         response_dict['skill_passed'] = suggestion.voters.count() >= 5
         if response_dict['skill_passed']:
             Skill.objects.create(title=suggestion.title, title_en=suggestion.title_en, image=suggestion.image)
             suggestion.delete()
 
-        return JsonResponse(response_dict)
+        return UTF8JsonResponse(response_dict)
 
 
 class AdminAPISuggestSkillDeleteView(PermissionRequiredMixin, PreventGetRequestsMixin, DeleteView):
@@ -238,7 +238,7 @@ class AdminAPISuggestSkillDeleteView(PermissionRequiredMixin, PreventGetRequests
 
     def delete(self, request, *args, **kwargs):
         self.get_object().delete()
-        return JsonResponse({'suggestion_deleted': True})
+        return UTF8JsonResponse({'suggestion_deleted': True})
 
 
 class AdminRegisterCardView(RFIDView):
@@ -271,7 +271,7 @@ def post(self, request):
                 request.user.card_number = card_number
                 request.user.save()
         RegisterProfile.objects.all().delete()
-        return JsonResponse(response_dict)
+        return UTF8JsonResponse(response_dict)
 
 
 class AdminProfilePictureUpdateView(PreventGetRequestsMixin, View):

src/docs/forms.py

@@ -77,3 +77,8 @@ def clean(self):
                     'current_content': _("The content does not belong to the given page"),
                 })
         return cleaned_data
+
+
+class DocumentationPageSearchQueryForm(forms.Form):
+    query = forms.CharField(required=False, max_length=1000, strip=False)
+    page = forms.IntegerField(required=False, min_value=1)

src/docs/templates/docs/documentation_page_search.html

@@ -2,7 +2,7 @@
 {% load i18n %}
 
 
-{% block title %}{% translate "Search results" %} | {% translate "Docs" %}{% endblock title %}
+{% block title %}{% translate "Search results" %}: "{{ query }}" | {% translate "Docs" %}{% endblock title %}
 
 {% block extra_head %}
     <script>

src/docs/tests/test_urls.py

@@ -45,6 +45,8 @@ def test_all_get_request_paths_succeed(self):
             Get(self.reverse('documentation_page_create'), public=False),
             Get(self.reverse('documentation_page_update', self.page1.pk), public=False),
             Get(self.reverse('documentation_page_search'), public=False),
+            Get(f"{self.reverse('documentation_page_search')}?query=lorem", public=False),
+            Get(f"{self.reverse('documentation_page_search')}?query=lorem&page=2", public=False),
             Get('/robots.txt', public=True, translated=False),
             Get('/.well-known/security.txt', public=True, translated=False),
         ]

src/docs/views.py

@@ -10,8 +10,8 @@
 from django.views.generic.edit import ModelFormMixin
 
 from util.templatetags.string_tags import title_en
-from util.view_utils import CustomFieldsetFormMixin, PreventGetRequestsMixin, insert_form_field_values
-from .forms import AddPageForm, ChangePageVersionForm, PageContentForm
+from util.view_utils import CustomFieldsetFormMixin, PreventGetRequestsMixin, QueryParameterFormMixin, insert_form_field_values
+from .forms import AddPageForm, ChangePageVersionForm, DocumentationPageSearchQueryForm, PageContentForm
 from .models import Content, MAIN_PAGE_TITLE, Page
 
 
@@ -166,8 +166,10 @@ class DocumentationPageDeleteView(PermissionRequiredMixin, PreventGetRequestsMix
     success_url = reverse_lazy('home')
 
 
-class DocumentationPageSearchView(TemplateView):
+class DocumentationPageSearchView(QueryParameterFormMixin, TemplateView):
+    form_class = DocumentationPageSearchQueryForm
     template_name = 'docs/documentation_page_search.html'
+
     page_size = 10
 
     @staticmethod
@@ -180,9 +182,9 @@ def pages_to_show(current_page, n_pages):
 
     def get_context_data(self, **kwargs):
         context_data = super().get_context_data(**kwargs)
+        query = self.query_params['query']
+        page = self.query_params['page'] or 1
 
-        query = self.request.GET.get('query', "")
-        page = int(self.request.GET.get('page', 1))
         pages = Page.objects.filter(Q(title__icontains=query) | Q(current_content__content__icontains=query))
         n_pages = ceil(pages.count() / self.page_size)
         context_data.update({

src/groups/tests/test_admin.py

@@ -1,51 +1,67 @@
-from django.contrib.admin.sites import AdminSite
-from django.test import TestCase
+from http import HTTPStatus
 
+from django.contrib.auth.models import Permission
+from django.contrib.contenttypes.models import ContentType
+from django.test import Client, TestCase
+
+from users.models import User
+from util.test_utils import set_without_duplicates
+from web.tests.test_urls import ADMIN_CLIENT_DEFAULTS, reverse_admin
 from ..admin import InheritanceGroupAdmin
 from ..models import InheritanceGroup
 
 
-class MockRequest:
-    pass
+class InheritanceGroupAdminTestCase(TestCase):
 
+    def setUp(self):
+        self.admin = User.objects.create_user("admin", is_staff=True, is_superuser=True)
+        self.admin_client = Client(**ADMIN_CLIENT_DEFAULTS)
+        self.admin_client.force_login(self.admin)
 
-class MockSuperUser:
+        self.org = InheritanceGroup.objects.create(name='Org')
+        self.mentor = InheritanceGroup.objects.create(name='Mentor')
+        self.dev = InheritanceGroup.objects.create(name='Dev')
+        self.event = InheritanceGroup.objects.create(name='Event')
+        self.chairperson = InheritanceGroup.objects.create(name='Leder')
 
-    def has_perm(self, *args, **kwargs):
-        return True
+        self.mentor.parents.add(self.org)
+        self.dev.parents.add(self.org)
+        self.event.parents.add(self.org)
+        self.chairperson.parents.add(self.mentor, self.dev, self.event)
 
+        self.org_perms = {
+            Permission.objects.create(codename='perm1', name="Perm 1", content_type=ContentType.objects.get_for_model(InheritanceGroup))
+        }
+        self.org.own_permissions.add(*self.org_perms)
 
-class InheritanceGroupAdminTestCase(TestCase):
+        self.admin_add_url = reverse_admin('groups_inheritancegroup_add')
+        self.admin_change_dev_url = reverse_admin('groups_inheritancegroup_change', args=[self.dev.pk])
 
-    def setUp(self):
-        self.site = AdminSite()
-        self.request = MockRequest()
-        self.request.user = MockSuperUser()
-        org = InheritanceGroup.objects.create(name='Org')
-        mentor = InheritanceGroup.objects.create(name='Mentor')
-        mentor.parents.add(org)
-        dev = InheritanceGroup.objects.create(name='Dev')
-        dev.parents.add(org)
-        arr = InheritanceGroup.objects.create(name='Arrangement')
-        arr.parents.add(org)
-        InheritanceGroup.objects.create(name='Leder').parents.add(mentor, dev, arr)
-
-    def test_get_form(self):
-        admin = InheritanceGroupAdmin(InheritanceGroup, self.site)
+    def test_form_contains_expected_fields_and_related_field_querysets(self):
+        response = self.admin_client.get(self.admin_add_url)
+        self.assertEqual(response.status_code, HTTPStatus.OK)
+        form = response.context['adminform']
         expected_fields = ['name', 'parents', 'own_permissions']
-        form = admin.get_form(self.request)
-        self.assertListEqual(list(form.base_fields), expected_fields)
-
-        expected_parents = InheritanceGroup.objects.all()
-        self.assertSetEqual(set(form.base_fields['parents'].queryset), set(expected_parents))
-
-        form = admin.get_form(self.request, obj=InheritanceGroup.objects.get(name='Dev'))
-        expected_parents = InheritanceGroup.objects.get(name='Dev').get_available_parents()
-        self.assertSetEqual(set(form.base_fields['parents'].queryset), set(expected_parents))
-
-    def test_inherited_permissions(self):
-        admin = InheritanceGroupAdmin(InheritanceGroup, self.site)
-        dev = InheritanceGroup.objects.get(name='Dev')
-        permissions = set(admin.get_inherited_permissions(dev))
-        for perm in dev.inherited_permissions:
-            self.assertIn(str(perm), permissions)
+        self.assertListEqual(list(form.fields), expected_fields)
+        self.assertSetEqual(
+            set_without_duplicates(self, form.fields['parents'].queryset),
+            set(InheritanceGroup.objects.all()),
+        )
+
+        response = self.admin_client.get(self.admin_change_dev_url)
+        self.assertEqual(response.status_code, HTTPStatus.OK)
+        form = response.context['adminform']
+        self.assertListEqual(list(form.fields), expected_fields)
+        self.assertSetEqual(
+            set_without_duplicates(self, form.fields['parents'].queryset),
+            set(self.dev.get_available_parents()),
+        )
+
+    def test_get_inherited_permissions_contains_expected_permissions(self):
+        response = self.admin_client.get(self.admin_change_dev_url)
+        self.assertEqual(response.status_code, HTTPStatus.OK)
+        admin: InheritanceGroupAdmin = response.context['adminform'].model_admin
+
+        permissions_string = admin.get_inherited_permissions(self.dev)
+        permissions = set_without_duplicates(self, permissions_string.splitlines())
+        self.assertSetEqual(permissions, {str(perm) for perm in self.org_perms})

src/groups/tests/test_models.py

@@ -8,7 +8,6 @@
 from ..models import Committee, InheritanceGroup
 
 
-# See the `0008_add_default_inheritancegroups_and_committees.py` migration for which InheritanceGroups are created by default
 class PermGroupTestCase(TestCase):
 
     def setUp(self):
@@ -199,7 +198,6 @@ def test_inherited_permissions(self):
                 self.assertIn(perm, inherited_permissions)
 
 
-# See the `0008_add_default_inheritancegroups_and_committees.py` migration for which Committees are created by default
 class CommitteeTestCase(TestCase):
 
     def setUp(self):