From 2103a95a279e7624159b62450e18345b82717385 Mon Sep 17 00:00:00 2001 From: Anders <6058745+ddabble@users.noreply.github.com> Date: Mon, 24 Apr 2023 20:02:08 +0200 Subject: [PATCH] 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 | 30 ++-- CONTRIBUTING.md | 48 +++++-- src/announcements/tests/test_urls.py | 5 +- src/announcements/urls.py | 17 ++- src/checkin/local_scanner.py | 5 +- src/checkin/tests/test_urls.py | 3 + src/checkin/urls.py | 30 ++-- src/docs/tests/test_urls.py | 14 +- src/docs/urls.py | 34 ++--- src/faq/tests/test_urls.py | 19 ++- src/faq/urls.py | 40 ++++-- src/groups/tests/test_urls.py | 5 +- src/groups/urls.py | 7 +- src/internal/tests/test_urls.py | 70 ++++++++-- src/internal/urls.py | 72 +++++----- src/internal/views.py | 3 +- .../static/make_queue/js/admin_quota_panel.js | 2 +- .../static/make_queue/js/calendar.js | 5 +- .../make_queue/js/printer_3d_course_form.js | 2 +- .../static/make_queue/js/reservation_form.js | 10 +- .../templates/make_queue/machine_detail.html | 1 + .../make_queue/reservation_form.html | 2 +- src/make_queue/tests/test_urls.py | 92 +++++++------ .../tests/views/test_machine_views.py | 2 +- .../tests/views/test_quota_views.py | 2 +- .../views/test_reservation_reservation.py | 30 ++-- src/make_queue/urls.py | 130 ++++++++++++------ src/make_queue/views/api/reservation.py | 3 +- .../views/reservation/reservation.py | 12 +- src/makerspace/tests/test_urls.py | 14 +- src/makerspace/urls.py | 31 +++-- src/news/tests/test_urls.py | 77 +++++++---- src/news/tests/views/test_article_views.py | 8 +- .../views/test_event_time_place_views.py | 10 +- src/news/urls.py | 38 +++-- src/news/views.py | 6 +- src/users/tests/test_urls.py | 1 + src/users/urls.py | 4 +- src/web/admin_urls.py | 5 +- src/web/tests/test_urls.py | 17 ++- src/web/tests/test_views.py | 3 + src/web/urls.py | 52 +++++-- 42 files changed, 639 insertions(+), 322 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d5489058..234a09e2c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53bb83ad6..90b5b9a20 100644 --- a/CONTRIBUTING.md +++ b/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: * `View` - in most cases; * `AdminView` - for views that only admins should have access to; -* `APIView` - for views responding with JSON; -* `AdminAPIView` - for views responding with JSON that only admins should have access to; +* `APIView` - for views responding exclusively with JSON; +* `AdminAPIView` - for views responding exclusively with JSON that only admins should have access to; * where: * `` is the name of the model class that the view is related to. * `` 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//occurrences//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("/", ..., 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 diff --git a/src/announcements/tests/test_urls.py b/src/announcements/tests/test_urls.py index 43bcdf59d..b220f33c1 100644 --- a/src/announcements/tests/test_urls.py +++ b/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) diff --git a/src/announcements/urls.py b/src/announcements/urls.py index f30f4d95c..e5cc3e1ac 100644 --- a/src/announcements/urls.py +++ b/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("/change/", views.AnnouncementUpdateView.as_view(), name='announcement_update'), - path("/delete/", views.AnnouncementDeleteView.as_view(), name='announcement_delete'), + path("/", include(specific_announcement_adminpatterns)), ] diff --git a/src/checkin/local_scanner.py b/src/checkin/local_scanner.py index 059a9ae66..c86969dcb 100644 --- a/src/checkin/local_scanner.py +++ b/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) diff --git a/src/checkin/tests/test_urls.py b/src/checkin/tests/test_urls.py index 7306b20e2..185a34a7e 100644 --- a/src/checkin/tests/test_urls.py +++ b/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) diff --git a/src/checkin/urls.py b/src/checkin/urls.py index 0369919c2..828a68b19 100644 --- a/src/checkin/urls.py +++ b/src/checkin/urls.py @@ -1,5 +1,5 @@ from django.contrib.auth.decorators import login_required -from django.urls import path +from django.urls import include, path from . import views @@ -7,11 +7,25 @@ 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//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("/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)), ] diff --git a/src/docs/tests/test_urls.py b/src/docs/tests/test_urls.py index f520d1e58..89c25dfe7 100644 --- a/src/docs/tests/test_urls.py +++ b/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') diff --git a/src/docs/urls.py b/src/docs/urls.py index 5ef65dd1f..5282878f9 100644 --- a/src/docs/urls.py +++ b/src/docs/urls.py @@ -2,7 +2,7 @@ from django.conf import settings from django.conf.urls.i18n import i18n_patterns from django.conf.urls.static import static -from django.urls import path, register_converter +from django.urls import include, path, register_converter from django.views.generic import TemplateView from util.url_utils import ckeditor_uploader_urls, debug_toolbar_urls, logout_urls, permission_required_else_denied @@ -16,34 +16,34 @@ path(".well-known/security.txt", TemplateView.as_view(template_name='web/security.txt', content_type='text/plain')), *debug_toolbar_urls(), - path("i18n/", decorator_include( - permission_required_else_denied('docs.view_page'), - 'django.conf.urls.i18n' - )), + path("i18n/", decorator_include(permission_required_else_denied('docs.view_page'), 'django.conf.urls.i18n')), *static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT), # For development only; Nginx is used in production *ckeditor_uploader_urls(), ] urlpatterns += logout_urls() +specific_documentation_page_urlpatterns = [ + path("", views.DocumentationPageDetailView.as_view(), name='documentation_page_detail'), + path("history/", views.DocumentationPageHistoryDetailView.as_view(), name='documentation_page_history_detail'), + path("history/change/", views.DocumentationPageVersionUpdateView.as_view(), name='documentation_page_version_update'), + path("history//", views.DocumentationPageContentDetailView.as_view(), name='documentation_page_content_detail'), + path("change/", views.DocumentationPageUpdateView.as_view(), name='documentation_page_update'), + path("delete/", views.DocumentationPageDeleteView.as_view(), name='documentation_page_delete'), +] +documentation_page_urlpatterns = [ + path("add/", views.DocumentationPageCreateView.as_view(), name='documentation_page_create'), + path("/", include(specific_documentation_page_urlpatterns)), +] + unsafe_urlpatterns = [ path("", views.DocumentationPageDetailView.as_view(is_main_page=True), name='home'), - path("page/add/", views.DocumentationPageCreateView.as_view(), name='documentation_page_create'), - path("page//", views.DocumentationPageDetailView.as_view(), name='documentation_page_detail'), - path("page//history/", views.DocumentationPageHistoryDetailView.as_view(), name='documentation_page_history_detail'), - path("page//history/change/", views.DocumentationPageVersionUpdateView.as_view(), name='documentation_page_version_update'), - path("page//history//", views.DocumentationPageContentDetailView.as_view(), - name='documentation_page_content_detail'), - path("page//change/", views.DocumentationPageUpdateView.as_view(), name='documentation_page_update'), - path("page//delete/", views.DocumentationPageDeleteView.as_view(), name='documentation_page_delete'), + path("page/", include(documentation_page_urlpatterns)), path("search/", views.DocumentationPageSearchView.as_view(), name='documentation_page_search'), ] urlpatterns += i18n_patterns( - path("", decorator_include( - permission_required_else_denied('docs.view_page'), - unsafe_urlpatterns - )), + path("", decorator_include(permission_required_else_denied('docs.view_page'), unsafe_urlpatterns)), prefix_default_language=False, ) diff --git a/src/faq/tests/test_urls.py b/src/faq/tests/test_urls.py index 23d938ab7..9ea1d4b99 100644 --- a/src/faq/tests/test_urls.py +++ b/src/faq/tests/test_urls.py @@ -21,20 +21,29 @@ def setUp(self): def test_all_get_request_paths_succeed(self): path_predicates = [ + # urlpatterns Get(reverse('faq_list'), public=True), - Get(reverse('admin_faq_panel'), public=False), - Get(reverse('admin_question_list'), public=False), - Get(reverse('question_create'), public=False), + + # specific_question_adminpatterns *[ Get(reverse('question_update', args=[question.pk]), public=False) for question in self.questions ], - Get(reverse('admin_category_list'), public=False), - Get(reverse('category_create'), public=False), + # question_adminpatterns + Get(reverse('admin_question_list'), public=False), + Get(reverse('question_create'), public=False), + + # specific_category_adminpatterns *[ Get(reverse('category_update', args=[category.pk]), public=False) for category in self.categories ], + # category_adminpatterns + Get(reverse('admin_category_list'), public=False), + Get(reverse('category_create'), public=False), + + # adminpatterns + Get(reverse('admin_faq_panel'), public=False), ] assert_requesting_paths_succeeds(self, path_predicates) diff --git a/src/faq/urls.py b/src/faq/urls.py index 8c3c1d90f..54e7f0b35 100644 --- a/src/faq/urls.py +++ b/src/faq/urls.py @@ -1,18 +1,36 @@ -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.FAQListView.as_view(), name='faq_list'), - path("admin/", login_required(views.AdminFAQPanelView.as_view()), name='admin_faq_panel'), - path("admin/questions/", login_required(views.AdminQuestionListView.as_view()), name='admin_question_list'), - path("admin/questions/add/", login_required(views.QuestionCreateView.as_view()), name='question_create'), - path("admin/questions//change/", login_required(views.QuestionUpdateView.as_view()), name='question_update'), - path("admin/questions//delete/", login_required(views.QuestionDeleteView.as_view()), name='question_delete'), - path("admin/categories/", login_required(views.AdminCategoryListView.as_view()), name='admin_category_list'), - path("admin/categories/add/", login_required(views.CategoryCreateView.as_view()), name='category_create'), - path("admin/categories//change/", login_required(views.CategoryUpdateView.as_view()), name='category_update'), - path("admin/categories//delete/", login_required(views.CategoryDeleteView.as_view()), name='category_delete'), +] + +# --- Admin URL patterns (imported in `web/urls.py`) --- + +specific_question_adminpatterns = [ + path("change/", views.QuestionUpdateView.as_view(), name='question_update'), + path("delete/", views.QuestionDeleteView.as_view(), name='question_delete'), +] +question_adminpatterns = [ + path("", views.AdminQuestionListView.as_view(), name='admin_question_list'), + path("add/", views.QuestionCreateView.as_view(), name='question_create'), + path("/", include(specific_question_adminpatterns)), +] + +specific_category_adminpatterns = [ + path("change/", views.CategoryUpdateView.as_view(), name='category_update'), + path("delete/", views.CategoryDeleteView.as_view(), name='category_delete'), +] +category_adminpatterns = [ + path("", views.AdminCategoryListView.as_view(), name='admin_category_list'), + path("add/", views.CategoryCreateView.as_view(), name='category_create'), + path("/", include(specific_category_adminpatterns)), +] + +adminpatterns = [ + path("", views.AdminFAQPanelView.as_view(), name='admin_faq_panel'), + path("questions/", include(question_adminpatterns)), + path("categories/", include(category_adminpatterns)), ] diff --git a/src/groups/tests/test_urls.py b/src/groups/tests/test_urls.py index 1d6019f2a..b5978cdef 100644 --- a/src/groups/tests/test_urls.py +++ b/src/groups/tests/test_urls.py @@ -22,10 +22,13 @@ def setUp(self): def test_all_get_request_paths_succeed(self): path_predicates = [ + # urlpatterns Get(reverse('committee_list'), public=True), Get(reverse('committee_detail', args=[self.committee1.pk]), public=True), - Get(reverse('committee_update', args=[self.committee1.pk]), public=False), + + # adminpatterns Get(reverse('admin_committee_list'), public=False), + Get(reverse('committee_update', args=[self.committee1.pk]), public=False), ] assert_requesting_paths_succeeds(self, path_predicates) diff --git a/src/groups/urls.py b/src/groups/urls.py index e58442ffa..eca0de3c0 100644 --- a/src/groups/urls.py +++ b/src/groups/urls.py @@ -6,6 +6,11 @@ urlpatterns = [ path("", views.CommitteeListView.as_view(), name='committee_list'), path("/", views.CommitteeDetailView.as_view(), name='committee_detail'), +] + +# --- Admin URL patterns (imported in `web/urls.py`) --- + +adminpatterns = [ + path("", views.AdminCommitteeListView.as_view(), name='admin_committee_list'), path("/change/", views.CommitteeUpdateView.as_view(), name='committee_update'), - path("admin/", views.AdminCommitteeListView.as_view(), name='admin_committee_list'), ] diff --git a/src/internal/tests/test_urls.py b/src/internal/tests/test_urls.py index 1521f6a65..69860f0d2 100644 --- a/src/internal/tests/test_urls.py +++ b/src/internal/tests/test_urls.py @@ -43,6 +43,17 @@ def setUp(self): self.member_editor_client.login(username=member_editor_user, password=password) self.home_content_box = ContentBox.objects.create(url_name='home') + self.dev_board_content_box = ContentBox.objects.create(url_name='dev-board') + self.event_board_content_box = ContentBox.objects.create(url_name='event-board') + self.mentor_board_content_box = ContentBox.objects.create(url_name='mentor-board') + self.pr_board_content_box = ContentBox.objects.create(url_name='pr-board') + self.MAKE_history_content_box = ContentBox.objects.create(url_name='make-history') + self.content_boxes = ( + self.home_content_box, + self.dev_board_content_box, self.event_board_content_box, self.mentor_board_content_box, self.pr_board_content_box, + self.MAKE_history_content_box, + ) + self.secret1 = Secret.objects.create(title="Key storage box", content="Code: 1234") self.secret2 = Secret.objects.create(title="YouTube account", content="

Email: make@gmail.com

Password: password

") self.secrets = (self.secret1, self.secret2) @@ -134,23 +145,64 @@ def test_permissions(self): self._test_internal_url('POST', reverse_internal('set_language'), {'language': 'en'}, expected_redirect_path="/en/") self._test_internal_url('POST', reverse_internal('set_language'), {'language': 'nb'}, expected_redirect_path="/") - def test_all_non_member_get_request_paths_succeed(self): + def test_all_get_request_paths_succeed(self): path_predicates = [ - Get(reverse_internal(self.home_content_box.url_name), public=False), - Get(reverse_internal('content_box_update', self.home_content_box.pk), public=False), + # urlpatterns + Get('/robots.txt', public=True, translated=False), + Get('/.well-known/security.txt', public=True, translated=False), - Get(reverse_internal('secret_list'), public=False), - Get(reverse_internal('secret_create'), public=False), + # committee_bulletin_urlpatterns + Get(reverse_internal(self.dev_board_content_box.url_name), public=False), + Get(reverse_internal(self.event_board_content_box.url_name), public=False), + Get(reverse_internal(self.mentor_board_content_box.url_name), public=False), + Get(reverse_internal(self.pr_board_content_box.url_name), public=False), + + # internal_content_box_urlpatterns + *[ + Get(reverse_internal('content_box_update', content_box.pk), public=False) + for content_box in self.content_boxes + ], + + # change_status_of_specific_member_urlpatterns + *[ + Get(reverse_internal('member_quit', member.pk), public=False) + for member in self.members + ], + *[ + Get(reverse_internal('member_retire', member.pk), public=False) + for member in self.members + ], + # change_specific_member_urlpatterns + *[ + Get(reverse_internal('member_update', member.pk), public=False) + for member in self.members + ], + # specific_member_urlpatterns + *[ + Get(reverse_internal('member_detail', member.pk), public=False) + for member in self.members + ], + # member_urlpatterns + Get(reverse_internal('member_list'), public=False), + Get(reverse_internal('member_create'), public=False), + + # specific_secret_urlpatterns Get(reverse_internal('secret_update', self.secret1.pk), public=False), Get(reverse_internal('secret_update', self.secret2.pk), public=False), + # secret_urlpatterns + Get(reverse_internal('secret_list'), public=False), + Get(reverse_internal('secret_create'), public=False), - Get(reverse_internal('quote_list'), public=False), - Get(reverse_internal('quote_create'), public=False), + # specific_quote_urlpatterns Get(reverse_internal('quote_update', self.quote1.pk), public=False), Get(reverse_internal('quote_update', self.quote2.pk), public=False), + # quote_urlpatterns + Get(reverse_internal('quote_list'), public=False), + Get(reverse_internal('quote_create'), public=False), - Get('/robots.txt', public=True, translated=False), - Get('/.well-known/security.txt', public=True, translated=False), + # internal_urlpatterns + Get(reverse_internal(self.home_content_box.url_name), public=False), + Get(reverse_internal(self.MAKE_history_content_box.url_name), public=False), ] assert_requesting_paths_succeeds(self, path_predicates, 'internal') diff --git a/src/internal/urls.py b/src/internal/urls.py index 8222d7667..33fc36665 100644 --- a/src/internal/urls.py +++ b/src/internal/urls.py @@ -14,10 +14,7 @@ path(".well-known/security.txt", TemplateView.as_view(template_name='web/security.txt', content_type='text/plain')), *debug_toolbar_urls(), - path("i18n/", decorator_include( - permission_required_else_denied('internal.is_internal'), - 'django.conf.urls.i18n' - )), + path("i18n/", decorator_include(permission_required_else_denied('internal.is_internal'), 'django.conf.urls.i18n')), *static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT), # For development only; Nginx is used in production *ckeditor_uploader_urls(), @@ -35,29 +32,44 @@ path("/change/", views.InternalContentBoxUpdateView.as_view(), name='content_box_update'), ] +change_status_of_specific_member_urlpatterns = [ + path("", views.MemberStatusUpdateView.as_view(), name='member_status_update'), + path("quit/", views.MemberQuitView.as_view(), name='member_quit'), + path("retire/", views.MemberRetireView.as_view(), name='member_retire'), +] +change_specific_member_urlpatterns = [ + path("", views.MemberUpdateView.as_view(), name='member_update'), + path("status/", include(change_status_of_specific_member_urlpatterns)), +] +specific_member_urlpatterns = [ + path("", views.MemberListView.as_view(), name='member_detail'), + path("change/", include(change_specific_member_urlpatterns)), + path("access//change/", views.SystemAccessUpdateView.as_view(), name='system_access_update'), +] member_urlpatterns = [ - path("members/", views.MemberListView.as_view(), name='member_list'), - path("members//", views.MemberListView.as_view(), name='member_detail'), - path("members/add/", views.MemberCreateView.as_view(), name='member_create'), - path("members//change/", views.MemberUpdateView.as_view(), name='member_update'), - path("members//change/status/", views.MemberStatusUpdateView.as_view(), name='member_status_update'), - path("members//change/status/quit/", views.MemberQuitView.as_view(), name='member_quit'), - path("members//change/status/retire/", views.MemberRetireView.as_view(), name='member_retire'), - path("members//access//change/", views.SystemAccessUpdateView.as_view(), name='system_access_update'), + path("", views.MemberListView.as_view(), name='member_list'), + path("add/", views.MemberCreateView.as_view(), name='member_create'), + path("/", include(specific_member_urlpatterns)), ] +specific_secret_urlpatterns = [ + path("change/", views.SecretUpdateView.as_view(), name='secret_update'), + path("delete/", views.SecretDeleteView.as_view(), name='secret_delete'), +] secret_urlpatterns = [ - path("secrets/", views.SecretListView.as_view(), name='secret_list'), - path("secrets/add/", views.SecretCreateView.as_view(), name='secret_create'), - path("secrets//change/", views.SecretUpdateView.as_view(), name='secret_update'), - path("secrets//delete/", views.SecretDeleteView.as_view(), name='secret_delete'), + path("", views.SecretListView.as_view(), name='secret_list'), + path("add/", views.SecretCreateView.as_view(), name='secret_create'), + path("/", include(specific_secret_urlpatterns)), ] +specific_quote_urlpatterns = [ + path("change/", views.QuoteUpdateView.as_view(), name='quote_update'), + path("delete/", views.QuoteDeleteView.as_view(), name='quote_delete'), +] quote_urlpatterns = [ - path("quotes/", views.QuoteListView.as_view(), name='quote_list'), - path("quotes/add/", views.QuoteCreateView.as_view(), name='quote_create'), - path("quotes//change/", views.QuoteUpdateView.as_view(), name='quote_update'), - path("quotes//delete/", views.QuoteDeleteView.as_view(), name='quote_delete'), + path("", views.QuoteListView.as_view(), name='quote_list'), + path("add/", views.QuoteCreateView.as_view(), name='quote_create'), + path("/", include(specific_quote_urlpatterns)), ] internal_urlpatterns = [ @@ -67,25 +79,13 @@ views.InternalContentBoxDetailView.get_path('make-history'), path("contentbox/", include(internal_content_box_urlpatterns)), - path("", decorator_include( - permission_required_else_denied('internal.view_member'), - member_urlpatterns - )), - path("", decorator_include( - permission_required_else_denied('internal.view_secret'), - secret_urlpatterns - )), - path("", decorator_include( - permission_required_else_denied('internal.view_quote'), - quote_urlpatterns - )), + path("members/", decorator_include(permission_required_else_denied('internal.view_member'), member_urlpatterns)), + path("secrets/", decorator_include(permission_required_else_denied('internal.view_secret'), secret_urlpatterns)), + path("quotes/", decorator_include(permission_required_else_denied('internal.view_quote'), quote_urlpatterns)), ] urlpatterns += i18n_patterns( - path("", decorator_include( - permission_required_else_denied('internal.is_internal'), - internal_urlpatterns - )), + path("", decorator_include(permission_required_else_denied('internal.is_internal'), internal_urlpatterns)), prefix_default_language=False, ) diff --git a/src/internal/views.py b/src/internal/views.py index 8a1f3f790..d846bb07e 100644 --- a/src/internal/views.py +++ b/src/internal/views.py @@ -215,10 +215,11 @@ def get_success_url(self): class SystemAccessUpdateView(PermissionRequiredMixin, PreventGetRequestsMixin, UpdateView): model = SystemAccess + pk_url_kwarg = 'system_access_pk' form_class = SystemAccessValueForm def get_queryset(self): - return get_object_or_404(Member, pk=self.kwargs['member_pk']).system_accesses + return get_object_or_404(Member, pk=self.kwargs['pk']).system_accesses def has_permission(self): system_access: SystemAccess = self.get_object() diff --git a/src/make_queue/static/make_queue/js/admin_quota_panel.js b/src/make_queue/static/make_queue/js/admin_quota_panel.js index f00bc98d6..781012d9b 100644 --- a/src/make_queue/static/make_queue/js/admin_quota_panel.js +++ b/src/make_queue/static/make_queue/js/admin_quota_panel.js @@ -9,7 +9,7 @@ $userDropdown.dropdown({ fullTextSearch: true, forceSelection: true, onChange: function (userPK, text, $choice) { - $.ajax(`${LANG_PREFIX}/reservation/quota/user/${userPK}/`, { + $.ajax(`${LANG_PREFIX}/admin/reservation/quotas/users/${userPK}/`, { success: function (data, textStatus) { $("#user-quotas").html(data); setUpDeleteModal(); diff --git a/src/make_queue/static/make_queue/js/calendar.js b/src/make_queue/static/make_queue/js/calendar.js index ce4bb8ca7..1bc8e2bb8 100644 --- a/src/make_queue/static/make_queue/js/calendar.js +++ b/src/make_queue/static/make_queue/js/calendar.js @@ -20,6 +20,7 @@ function ReservationCalendar($element, properties) { this.informationHeaders = $element.find("thead th").toArray(); this.days = $element.find("tbody .day .reservations").toArray(); this.$element = $element; + this.machineType = properties.machineType; this.machine = properties.machine; this.machineReservationURL = properties.machineReservationURL; this.selection = properties.selection; @@ -384,12 +385,12 @@ ReservationCalendar.prototype.update = function () { this.updateInformationHeaders(); const calendar = this; - $.get(`${window.location.origin}/reservation/calendar/${this.machine}/reservations/`, { + $.get(`${window.location.origin}/api/reservation/machines/${this.machine}/reservations/`, { start_date: this.date.djangoFormat(), end_date: this.date.nextWeek().djangoFormat(), }, (data) => calendar.updateReservations.apply(calendar, [data]), "json"); - $.get(`${window.location.origin}/reservation/calendar/${this.machine}/rules/`, {}, (data) => { + $.get(`${window.location.origin}/api/reservation/machinetypes/${this.machineType}/reservationrules/`, {}, (data) => { calendar.reservationRules = data.rules; }); }; diff --git a/src/make_queue/static/make_queue/js/printer_3d_course_form.js b/src/make_queue/static/make_queue/js/printer_3d_course_form.js index a3e580c85..37880d75a 100644 --- a/src/make_queue/static/make_queue/js/printer_3d_course_form.js +++ b/src/make_queue/static/make_queue/js/printer_3d_course_form.js @@ -5,7 +5,7 @@ $(".clear-dropdown").click(function () { }); $("input[name='username']").focusout((event) => { - $.ajax(`${LANG_PREFIX}/users/username/${$(event.target).val()}/`, { + $.ajax(`${LANG_PREFIX}/api/admin/users/username/${$(event.target).val()}/`, { success: function (data) { const fullName = data["full_name"]; if (fullName) diff --git a/src/make_queue/static/make_queue/js/reservation_form.js b/src/make_queue/static/make_queue/js/reservation_form.js index 8485495ac..32dc5c4b3 100644 --- a/src/make_queue/static/make_queue/js/reservation_form.js +++ b/src/make_queue/static/make_queue/js/reservation_form.js @@ -11,6 +11,7 @@ const reservations = []; const reservationRules = []; let canIgnoreRules = false; +const $machineTypeDropdown = $("#machine-type-dropdown"); const $machineNameDropdown = $("#machine-name-dropdown"); const $startTimeField = $("#start-time"); const $startTimeFieldInput = $startTimeField.find("input"); @@ -22,7 +23,7 @@ function getFutureReservations(machineID, forceNewTime) { /** * Retrieves future reservations and all reservation rules from the server. */ - let jsonUrl = `${LANG_PREFIX}/reservation/json/${machineID}/`; + let jsonUrl = `${LANG_PREFIX}/api/reservation/machines/${machineID}/data/`; const reservationPK = $("#reservation-form").data("reservation"); if (reservationPK) { jsonUrl += `?exclude_reservation=${reservationPK}`; @@ -226,7 +227,7 @@ $("#special-checkbox").checkbox({ }, }); -$("#machine-type-dropdown").dropdown({ +$machineTypeDropdown.dropdown({ onChange: function (selectedMachineType, text, $choice) { if (!$("#machine-type-dropdown").is(".disabled")) { $machineNameDropdown.toggleClass("disabled", false); @@ -302,12 +303,17 @@ function timeSelectionPopupHTML() { return $button; } +function getMachineType() { + return $machineTypeDropdown.dropdown("get value"); +} + function getMachine() { return $machineNameDropdown.dropdown("get value"); } const calendar = new ReservationCalendar($(".reservation-calendar"), { date: new Date(), + machineType: getMachineType(), machine: getMachine(), selection: true, canIgnoreRules: false, diff --git a/src/make_queue/templates/make_queue/machine_detail.html b/src/make_queue/templates/make_queue/machine_detail.html index e0e67aed7..fb274cd80 100644 --- a/src/make_queue/templates/make_queue/machine_detail.html +++ b/src/make_queue/templates/make_queue/machine_detail.html @@ -21,6 +21,7 @@