Reorganized URL patterns in all apps

...to comply with the style guide.
The path segments (i.e. the text between the forward slashes) now have a more consistent structure,
including being prefixed by `admin/` and `api/` - as described in `CONTRIBUTING.md` - by placing the `path()` calls
inside "custom `urlpatterns` lists", like `adminpatterns` and `apipatterns` - also described in `CONTRIBUTING.md`.

Of the views whose paths have been changed, the ones that I deemed as relevant, have been added to the "Old URLs"
in `web/urls.py` to (permanently) redirect from the old to the new path.

Also added permission checks for all admin URLs (indlucing admin API URLs) that require the requesting user to have
the `internal.is_internal` permission.

Also:
* Reordered the tests in all apps' `test_urls.py` to match the order of the paths listed in each app's `urls.py`
* Improved some of the parts in `CONTRIBUTING.md` on things related to path naming and organization
* Added some missing `ContentBox` tests to `internal/tests/test_urls.py`,
  as well as all the member GET URLs, as it's useful to have all URLs listed in one place, even if it duplicates some testing
* Added some missing tests for the `event_ticket_cancel` URL in `news/tests/test_urls.py`

CHANGELOG.md

@@ -21,15 +21,20 @@ A summary of changes made to the codebase, grouped per deployment.
   - This is only visible to users who have permission to modify those content boxes through Django admin
 - Changed cropping of images in article and event lists so that they always have equal proportions
 - Added a title to the quota admin panel, and switched out the "Add new quota" button for a green plus button, like other admin panel pages
+- Added an additional permission check for all admin pages (incl. API endpoints), that requires the user to have the `internal.is_internal` permission
 
 ### Fixes
 
 - Fixed board members not being allowed to add, change and delete reservation rules
-  (e.g. [the reservation rules for 3D printers](https://makentnu.no/reservation/machinetypes/1/rules/))
+  (e.g. [the reservation rules for 3D printers](https://makentnu.no/reservation/machinetypes/1/reservationrules/))
 
 ### Other changes
 
 - Enabled automated code quality checks from [Code Climate](https://codeclimate.com/quality)
+- Restructured lots of URLs to comply with the style guide
+  - The path segments (i.e. `/the-text-between-the-forward-slashes/`) now all have a more consistent structure,
+    including prefixing all admin pages' paths with `admin/` and all API endpoints with `api/`
+  - Permanent redirects have been added for the URLs deemed most relevant, to redirect from the old to the new URL
 - 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),
@@ -78,8 +83,8 @@ A summary of changes made to the codebase, grouped per deployment.
 - Made the red "Canceled" ribbon on tickets transparent when clicking / hovering over it, to be able to read the text behind it
 - Added a "help text" yellow question mark icon next to the "Discord username" field in member info forms
   (displayed when clicked / hovered over)
-- Made the user dropdowns on both [the quota admin page](https://makentnu.no/reservation/quota/) and
-  [the quota form page](https://makentnu.no/reservation/quota/add/), function equally
+- Made the user dropdowns on both [the quota admin page](https://makentnu.no/admin/reservation/quotas/) and
+  [the quota form page](https://makentnu.no/admin/reservation/quotas/add/), function equally
 - When changing machine type in the reservation creation form, made the first machine of that type be automatically selected
 - Place files uploaded through CKEditor in a separate folder for each model
 - Made all pages have a consistent (browser tab) title format
@@ -154,12 +159,12 @@ A summary of changes made to the codebase, grouped per deployment.
 ### New features
 
 - Added a URL which always links to the current week for a machine reservation calendar
-  - This URL can be copied by right-clicking the "View in calendar" button for a machine on [the machine list page](https://makentnu.no/reservation/),
-    and selecting "Copy link address"
+  - This URL can be copied by right-clicking the "View in calendar" button for a machine on
+    [the machine list page](https://makentnu.no/reservation/machines/), and selecting "Copy link address"
 - Made machine streams work with the new Raspberry Pi setup
   - Also, only the visible streams are connected to;
     once the page is scrolled so that a stream image is no longer rendered, the stream for that machine is disconnected,
-    which makes [the machine list page](https://makentnu.no/reservation/) use considerably less data (depending on how the page is scrolled)
+    which makes [the machine list page](https://makentnu.no/reservation/machines/) use considerably less data (depending on how the page is scrolled)
     - This does not apply to the machine detail (calendar) page, as there is always only one stream on the page
 
 ### Improvements
@@ -168,7 +173,7 @@ A summary of changes made to the codebase, grouped per deployment.
 - Renamed a lot of templates (and CSS and JavaScript files) to comply with the style guide
 - Improved word breaking (splitting a word between two lines, often using a hyphen) multiple places, like in titles and descriptions
 - Added translations to the spreadsheet containing course registrations, that can be downloaded from
-  [the course registrations page](https://makentnu.no/reservation/course/)
+  [the course registrations page](https://makentnu.no/admin/reservation/courses/)
 - Improved the permission check for [the admin panel](https://makentnu.no/admin/)
 - Added edit and delete buttons on the machine detail (calendar) page
 - On the history page for a documentation page (on [docs.makentnu.no](https://docs.makentnu.no/)),
@@ -246,7 +251,7 @@ A summary of changes made to the codebase, grouped per deployment.
 ### Improvements
 
 - Significantly improved page performance when watching streams that fail to connect
-  (most notably on [the machine list](https://makentnu.no/reservation/))
+  (most notably on [the machine list](https://makentnu.no/reservation/machines/))
 - Added warning message when there are gaps between the reservation rules of a machine type
 - Reordered [admin panel](https://makentnu.no/admin/) buttons
 - The code that reduces the size of uploaded images, now does not use the "reduced" image if it's not actually smaller -
@@ -339,7 +344,7 @@ A summary of changes made to the codebase, grouped per deployment.
 
 ### New features
 
-- Added an [admin page for FAQ categories](https://makentnu.no/faq/admin/categories/)
+- Added an [admin page for FAQ categories](https://makentnu.no/admin/faq/categories/)
 - For machines with streams: Added a new "no stream" image, in addition to images that are shown when the stream is down *and* the machine has either
   status "Maintenance" or "Out of order"
 
@@ -399,13 +404,14 @@ A summary of changes made to the codebase, grouped per deployment.
 ### New features
 
 - Added a "Special 3D printers" machine type
-  - This is listed on [the machine list (reservations) page](https://makentnu.no/reservation/) when one or more machines of this type have been added
+  - This is listed on [the machine list (reservations) page](https://makentnu.no/reservation/machines/) when one or more machines of this type
+    have been added
 - Added an "Advanced course" checkbox to course registrations
   - Checking this checkbox will grant users permission to create reservations for the special 3D printers
 - The course registration form now checks whether the submitted card number is actually (by accident) the phone number of NTNU's Building security
 - Changed the URL for the email lists from [/email](https://makentnu.no/email/) to [/about/contact](https://makentnu.no/about/contact/)
 - Added a counter at the bottom of [the member list](https://i.makentnu.no/members/)
-  and [course registration list](https://makentnu.no/reservation/course/), that shows how many members and course registrations are displayed,
+  and [course registration list](https://makentnu.no/admin/reservation/courses/), that shows how many members and course registrations are displayed,
   respectively
 - Articles, events, profile pictures and other things you can upload images for, now support GIFs
 
@@ -451,7 +457,7 @@ A summary of changes made to the codebase, grouped per deployment.
 ### New features
 
 - Added an [internal home page](https://i.makentnu.no/) (currently blank)
-- Added an [FAQ page](https://makentnu.no/faq/), including the ability to [add questions through the admin page](https://makentnu.no/faq/admin/)
+- Added an [FAQ page](https://makentnu.no/faq/), including the ability to [add questions through the admin page](https://makentnu.no/admin/faq/)
 
 ### Fixes
 

CONTRIBUTING.md

@@ -356,14 +356,16 @@ Sort the contents of a class in the following order:
 In general, names of views related to model objects should comply with one of the following patterns:
 * `<Model name><Noun or verb>View` - in most cases;
 * `Admin<Model name><Noun or verb>View` - for views that only admins should have access to;
-* `API<Model name><Noun or verb>View` - for views responding with JSON;
-* `AdminAPI<Model name><Noun or verb>View` - for views responding with JSON that only admins should have access to;
+* `API<Model name><Noun or verb>View` - for views responding exclusively with JSON;
+* `AdminAPI<Model name><Noun or verb>View` - for views responding exclusively with JSON that only admins should have access to;
 * where:
   * `<Model name>` is the name of the model class that the view is related to.
   * `<Noun or verb>` is a word concisely outlining the contents of the view.
     * If the view inherits from one of Django's top-level generic views (`ListView`, `DetailView`, `CreateView`, `UpdateView` or `DeleteView`),
       the word should be the name of the generic view - without the `View` suffix.
 
+(Note that the `Admin`/`API`/`AdminAPI` prefixes correspond with the path prefixes mentioned in [Path prefixes](#path-prefixes).)
+
 #### View class order
 
 Sort views in the same order as they appear in the app's `urls.py` (see [Path order](#path-order)).
@@ -400,6 +402,17 @@ or `test_get_related_events_returns_expected_events()` (for a model method named
 
 In general, try to make paths as [RESTful](https://hackernoon.com/restful-api-designing-guidelines-the-best-practices-60e1d954e7c9) as possible.
 
+Let all paths end with a `/`
+(except if the first argument to `path()` would have been just `"/"`, in which case the argument should be an empty string).
+
+###### Path naming conventions:
+
+Use `kebab-case`, and concatenate compound nouns - as long as it's still fairly legible.
+Examples:
+* `machinetypes/` is preferable over `machine-types/`;
+* `softwareengineers/` is preferable over `software-engineers/` - even if there's a double E, which makes it slightly less legible;
+* `find-free-slots/` is preferable over `findfreeslots/`.
+
 For paths that refer to views inheriting from one of the following
 [generic views](https://docs.djangoproject.com/en/stable/ref/class-based-views/flattened-index/),
 the path specified is encouraged:
@@ -416,15 +429,26 @@ This makes the paths consistent with the ones used by the
 [Django admin site](https://docs.djangoproject.com/en/stable/ref/contrib/admin/#reversing-admin-urls),
 and the names of the corresponding [default model permissions](https://docs.djangoproject.com/en/stable/topics/auth/default/#default-permissions).
 
+###### Path prefixes:
+
+Prefix all endpoints' paths with the following patterns:
+* `admin/` - if the endpoint/webpage should only be accessed by admins;
+* `api/` - if the endpoint responds exclusively with JSON;
+* `api/admin/` - if the endpoint responds exclusively with JSON **and** should only be accessed by admins.
+
+See [`urlpatterns` for admin/API view paths](#urlpatterns-for-adminapi-view-paths) for how to manage this with `urlpatterns`.
+
+_The language path prefix is not affected by the above rules,
+so e.g. the English version of `/api/admin/some-path/` being `/en/api/admin/some-path/` is perfectly fine._
+
+###### Other path grouping:
+
 If a model is conceptually subordinated another model (e.g. an event occurrence model that is connected to an event model),
 the paths for the views related to that "sub-model" should be relative to the paths of the "super-model" -
 while still complying with the guidelines above.
 For example: `event/<int:pk>/occurrences/<int:occurrence_pk>/change/`.
 
-Lastly, let all paths end with a `/`
-(except if the first argument to `path()` would have been `"/"`, in which case it should be an empty string).
-
-#### Path name
+#### Path `name` (the argument of `path()`)
 
 Use `snake_case`.
 
@@ -471,13 +495,11 @@ event_occurrence_urlpatterns = [
     path("", ..., name='event_occurrence_list'),
     path("<int:pk>/", ..., name='event_occurrence_detail'),
 ]
-
 specific_event_urlpatterns = [
     path("", ..., name='event_detail'),
     path("change/", ..., name='event_update'),
     path("occurrences/", include(event_occurrence_urlpatterns)),
 ]
-
 event_urlpatterns = [
     path("", ..., name='event_list'),
     path("add/", ..., name='event_create'),
@@ -493,15 +515,17 @@ urlpatterns = [
 
 For each app's `urls.py` file, place paths inside lists with the following names:
 * `adminpatterns` - if they refer to a view that only admins should have access to;
-* `apipatterns` - if they refer to a view responding with JSON;
-* `adminapipatterns` - if they refer to a view responding with JSON that only admins should have access to.
+* `apipatterns` - if they refer to a view responding exclusively with JSON;
+* `adminapipatterns` - if they refer to a view responding exclusively with JSON that only admins should have access to.
 
-Each of these lists should now only contain paths referring to views with the corresponding prefixes listed in [View class name](#view-class-name).
+_Each of these lists should now only contain paths referring to views with the corresponding class name prefixes listed in
+[View class name](#view-class-name)._
 
 These lists should then be imported in [`web/urls.py`](src/web/urls.py), and `include()`d in
 `admin_urlpatterns`, `api_urlpatterns` and `admin_api_urlpatterns`, respectively -
 with the same path route argument as the app's other paths.
-(This ensures that all paths start with the relevant `admin/`, `api/` or `api/admin/` prefix.)
+(This ensures that all paths start with the relevant `admin/`, `api/` or `api/admin/` prefix;
+see [Path prefixes](#path-prefixes).)
 For example:
 ```python
 from django.urls import include, path

src/announcements/tests/test_urls.py

@@ -20,9 +20,12 @@ def setUp(self):
 
     def test_all_get_request_paths_succeed(self):
         path_predicates = [
+            # specific_announcement_adminpatterns
+            Get(reverse('announcement_update', args=[self.announcement1.pk]), public=False),
+
+            # adminpatterns
             Get(reverse('admin_announcement_list'), public=False),
             Get(reverse('announcement_create'), public=False),
-            Get(reverse('announcement_update', args=[self.announcement1.pk]), public=False),
         ]
         assert_requesting_paths_succeeds(self, path_predicates)
 

src/announcements/urls.py

@@ -1,11 +1,20 @@
-from django.urls import path
+from django.urls import include, path
 
 from . import views
 
 
 urlpatterns = [
-    path("admin/", views.AdminAnnouncementListView.as_view(), name='admin_announcement_list'),
+]
+
+# --- Admin URL patterns (imported in `web/urls.py`) ---
+
+specific_announcement_adminpatterns = [
+    path("change/", views.AnnouncementUpdateView.as_view(), name='announcement_update'),
+    path("delete/", views.AnnouncementDeleteView.as_view(), name='announcement_delete'),
+]
+
+adminpatterns = [
+    path("", views.AdminAnnouncementListView.as_view(), name='admin_announcement_list'),
     path("add/", views.AnnouncementCreateView.as_view(), name='announcement_create'),
-    path("<int:pk>/change/", views.AnnouncementUpdateView.as_view(), name='announcement_update'),
-    path("<int:pk>/delete/", views.AnnouncementDeleteView.as_view(), name='announcement_delete'),
+    path("<int:pk>/", include(specific_announcement_adminpatterns)),
 ]

src/checkin/local_scanner.py

@@ -1,4 +1,5 @@
 import requests
+from django.urls import reverse
 
 """
 Module for testing the RFID functionality on makentnu.no when running locally and/or without an RFID scanner.
@@ -15,8 +16,8 @@ def _card(path, card_id, secret):
 
 
 def register(card_id="0123456789", secret=""):
-    return _card("checkin/register/card/", card_id, secret)
+    return _card(reverse('admin_register_card'), card_id, secret)
 
 
 def check(card_id="0123456789", secret=""):
-    return _card("checkin/post/", card_id, secret)
+    return _card(reverse('admin_check_in'), card_id, secret)

src/checkin/tests/test_urls.py

@@ -31,8 +31,11 @@ def setUp(self):
 
     def test_all_get_request_paths_succeed(self):
         path_predicates = [
+            # urlpatterns
             Get(reverse('user_skill_list'), public=True),
             Get(reverse('profile_detail'), public=False),
+
+            # adminpatterns
             Get(reverse('admin_suggest_skill'), public=False),
         ]
         assert_requesting_paths_succeeds(self, path_predicates)

src/checkin/urls.py

@@ -1,17 +1,31 @@
 from django.contrib.auth.decorators import login_required
-from django.urls import path
+from django.urls import include, path
 
 from . import views
 
 
 urlpatterns = [
     path("", views.UserSkillListView.as_view(), name='user_skill_list'),
     path("profile/", login_required(views.ProfileDetailView.as_view()), name='profile_detail'),
-    path("profile/change/image/", login_required(views.AdminProfilePictureUpdateView.as_view()), name='admin_profile_picture_update'),
-    path("post/", views.AdminCheckInView.as_view()),
-    path("register/card/", views.AdminRegisterCardView.as_view()),
-    path("register/profile/", login_required(views.AdminAPIRegisterProfileView.as_view()), name='admin_api_register_profile'),
-    path("suggest/", login_required(views.AdminSuggestSkillView.as_view()), name='admin_suggest_skill'),
-    path("suggest/vote/", login_required(views.AdminAPISuggestSkillVoteView.as_view()), name='admin_api_suggest_skill_vote'),
-    path("suggest/<int:pk>/delete/", login_required(views.AdminAPISuggestSkillDeleteView.as_view()), name='admin_api_suggest_skill_delete'),
+]
+
+# --- Admin URL patterns (imported in `web/urls.py`) ---
+
+adminpatterns = [
+    path("profile/change/image/", views.AdminProfilePictureUpdateView.as_view(), name='admin_profile_picture_update'),
+    path("post/", views.AdminCheckInView.as_view(), name='admin_check_in'),
+    path("register/card/", views.AdminRegisterCardView.as_view(), name='admin_register_card'),
+    path("suggest/", views.AdminSuggestSkillView.as_view(), name='admin_suggest_skill'),
+]
+
+# --- Admin API URL patterns (imported in `web/urls.py`) ---
+
+suggest_skill_adminapipatterns = [
+    path("vote/", views.AdminAPISuggestSkillVoteView.as_view(), name='admin_api_suggest_skill_vote'),
+    path("<int:pk>/delete/", views.AdminAPISuggestSkillDeleteView.as_view(), name='admin_api_suggest_skill_delete'),
+]
+
+adminapipatterns = [
+    path("register/profile/", views.AdminAPIRegisterProfileView.as_view(), name='admin_api_register_profile'),
+    path("suggest/", include(suggest_skill_adminapipatterns)),
 ]

src/docs/tests/test_urls.py

@@ -37,18 +37,24 @@ def setUp(self):
 
     def test_all_get_request_paths_succeed(self):
         path_predicates = [
-            Get(self.reverse('home'), public=False),
+            # urlpatterns
+            Get('/robots.txt', public=True, translated=False),
+            Get('/.well-known/security.txt', public=True, translated=False),
+
+            # specific_documentation_page_urlpatterns
             Get(self.reverse('documentation_page_detail', self.page1.pk), public=False),
             Get(self.reverse('documentation_page_history_detail', self.page1.pk), public=False),
             Get(self.reverse('documentation_page_content_detail', self.page1.pk, self.content1.pk), public=False),
             Get(self.reverse('documentation_page_content_detail', self.page1.pk, self.content2.pk), public=False),
-            Get(self.reverse('documentation_page_create'), public=False),
             Get(self.reverse('documentation_page_update', self.page1.pk), public=False),
+            # documentation_page_urlpatterns
+            Get(self.reverse('documentation_page_create'), public=False),
+
+            # unsafe_urlpatterns
+            Get(self.reverse('home'), 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),
         ]
         assert_requesting_paths_succeeds(self, path_predicates, 'docs')