From 7c1357b1689bcf150d35353f1c582dce9d0076f6 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Fri, 27 Sep 2024 12:02:19 +0100 Subject: [PATCH 01/13] :sparkles: Add initial edit view. --- importing/urls.py | 3 +- importing/views.py | 9 +++-- management/views.py | 72 ++++++++++++++++++++++++++++++++---- templates/object_detail.html | 8 +--- templates/object_edit.html | 17 +++++++++ templates/table.html | 7 +--- 6 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 templates/object_edit.html diff --git a/importing/urls.py b/importing/urls.py index b80fba78..27d24458 100644 --- a/importing/urls.py +++ b/importing/urls.py @@ -1,9 +1,10 @@ from django.urls import path -from .views import DataImportDetailView, DataImportListView +from .views import DataImportDetailView, DataImportEditView, DataImportListView app_name = "importing" urlpatterns = [ path("", DataImportListView.as_view(), name="dataimport_list"), path("/", DataImportDetailView.as_view(), name="dataimport_detail"), + path("edit/", DataImportEditView.as_view(), name="dataimport_edit"), ] diff --git a/importing/views.py b/importing/views.py index 2726ffc1..0835b876 100644 --- a/importing/views.py +++ b/importing/views.py @@ -1,4 +1,4 @@ -from management.views import CustomDetailView, CustomTableView +from management.views import CustomDetailView, CustomEditView, CustomTableView from .filters import DataImportFilter from .models import DataImport @@ -9,11 +9,14 @@ class DataImportDetailView(CustomDetailView): """View to view a data import.""" model = DataImport - show_list_btn = True class DataImportListView(CustomTableView): model = DataImport table_class = DataImportTable filterset_class = DataImportFilter - show_refresh_btn = True + + +class DataImportEditView(CustomEditView): + model = DataImport + fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] diff --git a/management/views.py b/management/views.py index eeeb1191..43f5f35b 100755 --- a/management/views.py +++ b/management/views.py @@ -7,6 +7,7 @@ from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.views.generic import CreateView, DetailView +from django.views.generic.edit import UpdateView from django_filters.views import FilterView from django_tables2 import SingleTableMixin from guardian.shortcuts import get_objects_for_user @@ -68,9 +69,7 @@ class CustomDetailView(LoginRequiredMixin, DetailView): """ template_name: str = "object_detail.html" - show_list_btn: bool = False show_delete_btn: bool = False - show_edit_btn: bool = False def get_object(self) -> Model: obj = get_object_or_404(self.model, pk=self.kwargs["pk"]) @@ -91,8 +90,9 @@ def get_context_data(self, **kwargs): context["form"] = self.get_form() context["title"] = self.model_description context["delete_url"] = self.delete_url if self.show_delete_btn else None - context["edit_url"] = self.edit_url if self.show_edit_btn else None - context["list_url"] = self.list_url if self.show_list_btn else None + context["edit_url"] = self.edit_url + context["list_url"] = self.list_url + context["pk"] = self.object.pk return context @property @@ -154,8 +154,6 @@ class CustomTableView(LoginRequiredMixin, SingleTableMixin, FilterView): template_name = "table.html" paginate_by = 10 - show_refresh_btn: bool = False - show_new_btn: bool = False def get_queryset(self): return get_objects_for_user(self.request.user, self.permission_required) @@ -163,8 +161,8 @@ def get_queryset(self): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["title"] = self.title - context["refresh"] = self.list_url if self.show_refresh_btn else None - context["new_url"] = self.create_url if self.show_new_btn else None + context["list_url"] = self.list_url + context["new_url"] = None return context @property @@ -190,3 +188,61 @@ def list_url(self) -> str: @property def create_url(self) -> str: return f"{self.app_label}:{self.model_name}_create" + + +class CustomEditView(LoginRequiredMixin, UpdateView): + """Generic edit view. + + This view is used to edit a model object. The user must have the permission to edit + the object, otherwise a 403 error is returned. + + The view includes a form with the object data, and the context includes the title of + the view and the URL to the list view. They follow the pattern + `app_label:model_name_action`. For example, the list URL for the `DataImport` model + would be `importing:dataimport_list`. + + The permissions required to edit the object are `app_label.change_model_name`. For + example, the permission required to edit a `DataImport` object would be + `importing.change_dataimport`. + + Users need to be logged in to access this view. + + Attributes: + template_name (str): Template to be used. + """ + + template_name = "object_edit.html" + + def get_object(self) -> Model: + obj = super().get_object() + if not self.request.user.has_perm( + f"{self.app_label}.change_{self.model_name}", obj + ): + raise PermissionDenied + return obj + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = self.model_description + context["list_url"] = self.list_url + return context + + @property + def app_label(self) -> str: + return self.model._meta.app_label + + @property + def model_name(self) -> str: + return self.model._meta.model_name + + @property + def list_url(self) -> str: + return f"{self.app_label}:{self.model_name}_list" + + @property + def model_description(self) -> str: + return self.model._meta.verbose_name.title() + + @property + def success_url(self) -> str: + return reverse_lazy(self.list_url) diff --git a/templates/object_detail.html b/templates/object_detail.html index 90dabcb0..6f870fae 100644 --- a/templates/object_detail.html +++ b/templates/object_detail.html @@ -10,12 +10,8 @@

- {% if list_url %} - Return to list - {% endif %} - {% if edit_url %} - Edit - {% endif %} + Return to list + Edit {% if delete_url %} Delete {% endif %} diff --git a/templates/object_edit.html b/templates/object_edit.html new file mode 100644 index 00000000..6c12f45f --- /dev/null +++ b/templates/object_edit.html @@ -0,0 +1,17 @@ +{% extends "base.html" %} +{% load crispy_forms_tags %} +{% block content %} +

+ {% block title %} + {{ title }} + {% endblock title %} + object edit +

+
+ {% csrf_token %} + + + Cancel + {{ form | crispy }} +
+{% endblock content %} diff --git a/templates/table.html b/templates/table.html index a2c74c5d..0210d18e 100644 --- a/templates/table.html +++ b/templates/table.html @@ -1,7 +1,6 @@ {% extends "base.html" %} {% load django_bootstrap5 %} -{% load crispy_forms_tags -%} +{% load crispy_forms_tags %} {% load render_table from django_tables2 %} {% block content %}

@@ -13,9 +12,7 @@

{% if new_url %} New {% endif %} - {% if refresh %} - Refresh - {% endif %} + Refresh {% if filter %}
From 9c3064036c6778fb3f55765972aab81844fc0774 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Fri, 27 Sep 2024 13:45:18 +0100 Subject: [PATCH 02/13] :sparkles: Add edit view. --- importing/views.py | 1 + management/admin.py | 22 ++-------------------- management/permissions.py | 27 +++++++++++++++++++++++++++ management/views.py | 21 +++++++++++++++------ measurement/admin.py | 4 ++-- templates/object_detail.html | 4 ++-- templates/object_edit.html | 7 ++++--- 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/importing/views.py b/importing/views.py index 0835b876..f3f6ff8f 100644 --- a/importing/views.py +++ b/importing/views.py @@ -20,3 +20,4 @@ class DataImportListView(CustomTableView): class DataImportEditView(CustomEditView): model = DataImport fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] + foreign_key_fields = ["station", "format"] diff --git a/management/admin.py b/management/admin.py index 8f347e44..c9294c90 100755 --- a/management/admin.py +++ b/management/admin.py @@ -7,6 +7,7 @@ from guardian.shortcuts import get_objects_for_user from .models import User +from .permissions import get_queryset # Set global preferences for the Django admin site admin.site.site_header = "Paricia Administration" @@ -65,7 +66,7 @@ def get_queryset(self, request): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Limit the queryset for foreign key fields.""" if db_field.name in self.foreign_key_fields: - kwargs["queryset"] = _get_queryset(db_field, request.user) + kwargs["queryset"] = get_queryset(db_field, request.user) if db_field.name == "owner" and not request.user.is_superuser: kwargs["initial"] = request.user.id kwargs["disabled"] = True @@ -80,25 +81,6 @@ def formfield_for_choice_field(self, db_field, request, **kwargs): return super().formfield_for_choice_field(db_field, request, **kwargs) -def _get_queryset(db_field, user): - """Return a queryset based on the permissions of the user. - - Returns queryset of public objects and objects that the user has change permisions - for. For the case of `Station` objects, having the `change` permission is - necessary to include the object in the queryset - being `Public` is not enough. - - """ - app_name = db_field.related_model._meta.app_label - model_name = db_field.related_model._meta.model_name - user_objects = get_objects_for_user(user, f"{app_name}.change_{model_name}") - public_objects = ( - db_field.related_model.objects.none() - if model_name == "station" - else db_field.related_model.objects.filter(visibility="public") - ) - return user_objects | public_objects - - class CustomUserAdmin(UserAdmin): """A slightly more restrictive user admin page.""" diff --git a/management/permissions.py b/management/permissions.py index 51a24a72..21d5313f 100755 --- a/management/permissions.py +++ b/management/permissions.py @@ -1,5 +1,7 @@ """Customised permissions.""" +from django.db import models as model +from guardian.shortcuts import get_objects_for_user from rest_framework.permissions import DjangoModelPermissions @@ -17,3 +19,28 @@ class CustomDjangoModelPermissions(DjangoModelPermissions): "PATCH": ["%(app_label)s.change_%(model_name)s"], "DELETE": ["%(app_label)s.delete_%(model_name)s"], } + + +def get_queryset(db_field: model.Field, user: model.Model) -> model.QuerySet: + """Return a queryset based on the permissions of the user. + + Returns queryset of public objects and objects that the user has change permisions + for. For the case of `Station` objects, having the `change` permission is + necessary to include the object in the queryset - being `Public` is not enough. + + Args: + db_field (model.Field): Field to filter. + user (model.Model): User to check permissions for. + + Returns: + model.QuerySet: Queryset of objects that the user has permissions for. + """ + app_name = db_field.related_model._meta.app_label + model_name = db_field.related_model._meta.model_name + user_objects = get_objects_for_user(user, f"{app_name}.change_{model_name}") + public_objects = ( + db_field.related_model.objects.none() + if model_name == "station" + else db_field.related_model.objects.filter(visibility="public") + ) + return user_objects | public_objects diff --git a/management/views.py b/management/views.py index 43f5f35b..93220949 100755 --- a/management/views.py +++ b/management/views.py @@ -13,6 +13,7 @@ from guardian.shortcuts import get_objects_for_user from .forms import CustomUserCreationForm +from .permissions import get_queryset def model_to_dict(instance: Model) -> dict: @@ -212,6 +213,7 @@ class CustomEditView(LoginRequiredMixin, UpdateView): """ template_name = "object_edit.html" + foreign_key_fields: list[str] = [] def get_object(self) -> Model: obj = super().get_object() @@ -224,7 +226,8 @@ def get_object(self) -> Model: def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["title"] = self.model_description - context["list_url"] = self.list_url + context["detail_url"] = self.detail_url + context["pk"] = self.object.pk return context @property @@ -235,14 +238,20 @@ def app_label(self) -> str: def model_name(self) -> str: return self.model._meta.model_name - @property - def list_url(self) -> str: - return f"{self.app_label}:{self.model_name}_list" - @property def model_description(self) -> str: return self.model._meta.verbose_name.title() @property def success_url(self) -> str: - return reverse_lazy(self.list_url) + return reverse_lazy(self.detail_url, kwargs={"pk": self.object.pk}) + + @property + def detail_url(self) -> str: + return f"{self.app_label}:{self.model_name}_detail" + + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Limit the queryset for foreign key fields.""" + if db_field.name in self.foreign_key_fields: + kwargs["queryset"] = get_queryset(db_field, request.user) + return super().formfield_for_foreignkey(db_field, request, **kwargs) diff --git a/measurement/admin.py b/measurement/admin.py index 22d2972a..f2795b94 100644 --- a/measurement/admin.py +++ b/measurement/admin.py @@ -2,7 +2,7 @@ from guardian.admin import GuardedModelAdmin from guardian.shortcuts import get_objects_for_user -from management.admin import _get_queryset +from management.admin import get_queryset from measurement.models import Measurement, Report @@ -47,7 +47,7 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): request.user, "station.change_station" ) elif db_field.name == "variable": - kwargs["queryset"] = _get_queryset(db_field, request.user) + kwargs["queryset"] = get_queryset(db_field, request.user) return super().formfield_for_foreignkey(db_field, request, **kwargs) diff --git a/templates/object_detail.html b/templates/object_detail.html index 6f870fae..034b269b 100644 --- a/templates/object_detail.html +++ b/templates/object_detail.html @@ -10,8 +10,8 @@

- Return to list - Edit + Return to list + Edit {% if delete_url %} Delete {% endif %} diff --git a/templates/object_edit.html b/templates/object_edit.html index 6c12f45f..ccf201c4 100644 --- a/templates/object_edit.html +++ b/templates/object_edit.html @@ -1,5 +1,5 @@ {% extends "base.html" %} -{% load crispy_forms_tags %} +{% load django_bootstrap5 %} {% block content %}

{% block title %} @@ -11,7 +11,8 @@

{% csrf_token %} - Cancel - {{ form | crispy }} + Cancel +

 

+ {% bootstrap_form form layout='horizontal' %} {% endblock content %} From 51d6ef0ec79d99b498c631ea39ac4625b6c1f313 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Fri, 27 Sep 2024 14:47:37 +0100 Subject: [PATCH 03/13] :sparkles: Add create view. --- importing/urls.py | 8 ++- importing/views.py | 13 +++- management/views.py | 111 +++++++++++++++++++++++++++++++++-- templates/object_create.html | 17 ++++++ templates/table.html | 5 +- 5 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 templates/object_create.html diff --git a/importing/urls.py b/importing/urls.py index 27d24458..746fdcdd 100644 --- a/importing/urls.py +++ b/importing/urls.py @@ -1,10 +1,16 @@ from django.urls import path -from .views import DataImportDetailView, DataImportEditView, DataImportListView +from .views import ( + DataImportCreateView, + DataImportDetailView, + DataImportEditView, + DataImportListView, +) app_name = "importing" urlpatterns = [ path("", DataImportListView.as_view(), name="dataimport_list"), path("/", DataImportDetailView.as_view(), name="dataimport_detail"), path("edit/", DataImportEditView.as_view(), name="dataimport_edit"), + path("create/", DataImportCreateView.as_view(), name="dataimport_create"), ] diff --git a/importing/views.py b/importing/views.py index f3f6ff8f..7ff34a4c 100644 --- a/importing/views.py +++ b/importing/views.py @@ -1,4 +1,9 @@ -from management.views import CustomDetailView, CustomEditView, CustomTableView +from management.views import ( + CustomCreateView, + CustomDetailView, + CustomEditView, + CustomTableView, +) from .filters import DataImportFilter from .models import DataImport @@ -21,3 +26,9 @@ class DataImportEditView(CustomEditView): model = DataImport fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] foreign_key_fields = ["station", "format"] + + +class DataImportCreateView(CustomCreateView): + model = DataImport + fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] + foreign_key_fields = ["station", "format"] diff --git a/management/views.py b/management/views.py index 93220949..c0ffaa6b 100755 --- a/management/views.py +++ b/management/views.py @@ -163,7 +163,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["title"] = self.title context["list_url"] = self.list_url - context["new_url"] = None + context["create_url"] = self.create_url return context @property @@ -206,6 +206,9 @@ class CustomEditView(LoginRequiredMixin, UpdateView): example, the permission required to edit a `DataImport` object would be `importing.change_dataimport`. + If successful or cancelled, the view redirects to the detail view of the created + object. + Users need to be logged in to access this view. Attributes: @@ -215,6 +218,23 @@ class CustomEditView(LoginRequiredMixin, UpdateView): template_name = "object_edit.html" foreign_key_fields: list[str] = [] + def get_form_class(self) -> forms.BaseModelForm: + class CustomCreateForm(forms.ModelForm): + foreign_key_fields = self.foreign_key_fields + + class Meta: + model = self.model + fields = self.fields + + def __init__(self, *args, **kwargs): + user = kwargs.pop("user") + super().__init__(*args, **kwargs) + for field in self._meta.model._meta.fields: + if field.name in self.foreign_key_fields: + self.fields[field.name].queryset = get_queryset(field, user) + + return CustomCreateForm + def get_object(self) -> Model: obj = super().get_object() if not self.request.user.has_perm( @@ -250,8 +270,87 @@ def success_url(self) -> str: def detail_url(self) -> str: return f"{self.app_label}:{self.model_name}_detail" - def formfield_for_foreignkey(self, db_field, request, **kwargs): - """Limit the queryset for foreign key fields.""" - if db_field.name in self.foreign_key_fields: - kwargs["queryset"] = get_queryset(db_field, request.user) - return super().formfield_for_foreignkey(db_field, request, **kwargs) + def get_form_kwargs(self): + """Add the user to the form kwargs, so we can filter the options.""" + kwargs = super().get_form_kwargs() + kwargs["user"] = self.request.user + return kwargs + + +class CustomCreateView(LoginRequiredMixin, CreateView): + """Generic create view. + + This view is used to create a new model object. The user must have the permission to + create the object, otherwise a 403 error is returned. + + The view includes a form with the object data, and the context includes the title of + the view and the URL to the list view. They follow the pattern + `app_label:model_name_action`. For example, the list URL for the `DataImport` model + would be `importing:dataimport_list`. + + If provided, the `foreign_key_fields` attribute is used to limit the queryset for + foreign key fields. + + If successful, the view redirects to the detail view of the created object. + + Users need to be logged in to access this view. + + Attributes: + template_name (str): Template to be used. + """ + + template_name = "object_create.html" + foreign_key_fields: list[str] = [] + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = self.model_description + context["list_url"] = self.list_url + return context + + def get_form_class(self) -> forms.BaseModelForm: + class CustomCreateForm(forms.ModelForm): + foreign_key_fields = self.foreign_key_fields + + class Meta: + model = self.model + fields = self.fields + + def __init__(self, *args, **kwargs): + user = kwargs.pop("user") + super().__init__(*args, **kwargs) + for field in self._meta.model._meta.fields: + if field.name in self.foreign_key_fields: + self.fields[field.name].queryset = get_queryset(field, user) + + return CustomCreateForm + + @property + def app_label(self) -> str: + return self.model._meta.app_label + + @property + def model_name(self) -> str: + return self.model._meta.model_name + + @property + def model_description(self) -> str: + return self.model._meta.verbose_name.title() + + @property + def success_url(self) -> str: + return reverse_lazy(self.detail_url, kwargs={"pk": self.object.pk}) + + @property + def detail_url(self) -> str: + return f"{self.app_label}:{self.model_name}_detail" + + @property + def list_url(self) -> str: + return f"{self.app_label}:{self.model_name}_list" + + def get_form_kwargs(self): + """Add the user to the form kwargs, so we can filter the options.""" + kwargs = super().get_form_kwargs() + kwargs["user"] = self.request.user + return kwargs diff --git a/templates/object_create.html b/templates/object_create.html new file mode 100644 index 00000000..fff898af --- /dev/null +++ b/templates/object_create.html @@ -0,0 +1,17 @@ +{% extends "base.html" %} +{% load django_bootstrap5 %} +{% block content %} +

+ {% block title %} + {{ title }} + {% endblock title %} + object creation +

+
+ {% csrf_token %} + + Cancel +

 

+ {% bootstrap_form form layout='horizontal' %} +
+{% endblock content %} diff --git a/templates/table.html b/templates/table.html index 0210d18e..77d373aa 100644 --- a/templates/table.html +++ b/templates/table.html @@ -9,10 +9,9 @@

{% endblock title %}

- {% if new_url %} - New - {% endif %} Refresh + New +

 

{% if filter %}
From b748cf05769c0f1180052d76b45b4dd3a9177804 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 06:21:13 +0100 Subject: [PATCH 04/13] :bug: Correctly handle uploaded files --- management/views.py | 9 +++++---- templates/object_create.html | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/management/views.py b/management/views.py index c0ffaa6b..46988d70 100755 --- a/management/views.py +++ b/management/views.py @@ -317,11 +317,12 @@ class Meta: fields = self.fields def __init__(self, *args, **kwargs): - user = kwargs.pop("user") + user = kwargs.pop("user") if "user" in kwargs else None super().__init__(*args, **kwargs) - for field in self._meta.model._meta.fields: - if field.name in self.foreign_key_fields: - self.fields[field.name].queryset = get_queryset(field, user) + if user: + for field in self._meta.model._meta.fields: + if field.name in self.foreign_key_fields: + self.fields[field.name].queryset = get_queryset(field, user) return CustomCreateForm diff --git a/templates/object_create.html b/templates/object_create.html index fff898af..fa595bc0 100644 --- a/templates/object_create.html +++ b/templates/object_create.html @@ -7,7 +7,7 @@

{% endblock title %} object creation

- + {% csrf_token %} Cancel From d8d93bbb8080741c54ffb297bdd0160d5abdd9b1 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 06:27:15 +0100 Subject: [PATCH 05/13] :bug: Conditionally filter based on user. --- management/views.py | 9 +++++---- templates/object_edit.html | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/management/views.py b/management/views.py index 46988d70..901f94c7 100755 --- a/management/views.py +++ b/management/views.py @@ -227,11 +227,12 @@ class Meta: fields = self.fields def __init__(self, *args, **kwargs): - user = kwargs.pop("user") + user = kwargs.pop("user") if "user" in kwargs else None super().__init__(*args, **kwargs) - for field in self._meta.model._meta.fields: - if field.name in self.foreign_key_fields: - self.fields[field.name].queryset = get_queryset(field, user) + if user: + for field in self._meta.model._meta.fields: + if field.name in self.foreign_key_fields: + self.fields[field.name].queryset = get_queryset(field, user) return CustomCreateForm diff --git a/templates/object_edit.html b/templates/object_edit.html index ccf201c4..6bbce447 100644 --- a/templates/object_edit.html +++ b/templates/object_edit.html @@ -7,7 +7,7 @@

{% endblock title %} object edit

- + {% csrf_token %} From 6fe58e858e24e753d67833ae1693193c29fd7e2c Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 06:32:34 +0100 Subject: [PATCH 06/13] :bug: Reprocess data if file changes. --- importing/models.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/importing/models.py b/importing/models.py index e22239b0..aeb18cf4 100755 --- a/importing/models.py +++ b/importing/models.py @@ -107,6 +107,10 @@ def clean(self) -> None: if not tz: raise ValidationError("Station must have a timezone set.") + # If the file has changed, we reprocess the data + if self.pk and self.rawfile != self.__class__.objects.get(pk=self.pk).rawfile: + self.reprocess = True + if self.reprocess: self.status = "N" self.reprocess = False From 9001bde92dac06f658aaa60eca280f3ac84f373c Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 10:53:12 +0100 Subject: [PATCH 07/13] :bug: Use a normal form with disabled entries in the detail view. --- management/views.py | 38 ++++++++++-------------------------- templates/object_detail.html | 20 +++---------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/management/views.py b/management/views.py index 901f94c7..cd8ac2dc 100755 --- a/management/views.py +++ b/management/views.py @@ -1,9 +1,7 @@ -from contextlib import suppress - from django import forms from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import PermissionDenied -from django.db.models import ForeignKey, Model +from django.db.models import Model from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.views.generic import CreateView, DetailView @@ -16,29 +14,6 @@ from .permissions import get_queryset -def model_to_dict(instance: Model) -> dict: - """Convert a model instance to a dictionary. - - For ForeignKey fields, the related model instance is converted to a string, so that - a meaningful representation is shown in the dictionary instead of the primary key. - - Args: - instance (Model): Model instance to convert. - - Returns: - dict: Dictionary with the model instance data. - """ - data = {} - for field in instance._meta.fields: - data[field.name] = field.value_from_object(instance) - if isinstance(field, ForeignKey): - with suppress(field.related_model.DoesNotExist): - data[field.name] = str( - field.related_model.objects.get(pk=data[field.name]) - ) - return data - - class SignUpView(CreateView): form_class = CustomUserCreationForm success_url = reverse_lazy("login") @@ -71,6 +46,7 @@ class CustomDetailView(LoginRequiredMixin, DetailView): template_name: str = "object_detail.html" show_delete_btn: bool = False + fields = "__all__" def get_object(self) -> Model: obj = get_object_or_404(self.model, pk=self.kwargs["pk"]) @@ -82,9 +58,15 @@ def get_form(self): class DetailForm(forms.ModelForm): class Meta: model = self.model - fields = "__all__" + fields = self.fields + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + for field in self.fields.keys(): + self.fields[field].widget.attrs["disabled"] = True + self.fields[field].widget.attrs["readonly"] = True - return DetailForm(data=model_to_dict(self.object)) + return DetailForm(instance=self.object) def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) diff --git a/templates/object_detail.html b/templates/object_detail.html index 034b269b..8d4306e3 100644 --- a/templates/object_detail.html +++ b/templates/object_detail.html @@ -1,5 +1,5 @@ {% extends "base.html" %} -{% load render_table from django_tables2 %} +{% load django_bootstrap5 %} {% block content %}

{% block title %} @@ -20,22 +20,8 @@

- - - - - - - - - {% for field in form %} - - - - - {% endfor %} - -
FieldValue
{{ field.label }}{{ field.value }}
+

 

+ {% bootstrap_form form layout='horizontal' %}
{% endblock content %} From 3140456f5135e0377df1a33484d2ceb8fc7754d2 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 10:54:07 +0100 Subject: [PATCH 08/13] :memo: Add docstring for DataImport views --- importing/views.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/importing/views.py b/importing/views.py index 7ff34a4c..e5c20672 100644 --- a/importing/views.py +++ b/importing/views.py @@ -17,18 +17,24 @@ class DataImportDetailView(CustomDetailView): class DataImportListView(CustomTableView): + """View to list all data imports.""" + model = DataImport table_class = DataImportTable filterset_class = DataImportFilter class DataImportEditView(CustomEditView): + """View to edit a data import.""" + model = DataImport fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] foreign_key_fields = ["station", "format"] class DataImportCreateView(CustomCreateView): + """View to create a data import.""" + model = DataImport fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] foreign_key_fields = ["station", "format"] From 7d95c092fd8c2f4648aa41d7f44b3614fa1073a0 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 13:14:36 +0100 Subject: [PATCH 09/13] :sparkles: Add delete view generic and for importing. --- importing/urls.py | 2 ++ importing/views.py | 7 ++++ management/tools.py | 25 ++++++++++++++ management/views.py | 67 ++++++++++++++++++++++++++++++++++-- templates/object_delete.html | 46 +++++++++++++++++++++++++ templates/object_detail.html | 4 +-- 6 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 management/tools.py create mode 100644 templates/object_delete.html diff --git a/importing/urls.py b/importing/urls.py index 746fdcdd..b8a2fc59 100644 --- a/importing/urls.py +++ b/importing/urls.py @@ -2,6 +2,7 @@ from .views import ( DataImportCreateView, + DataImportDeleteView, DataImportDetailView, DataImportEditView, DataImportListView, @@ -13,4 +14,5 @@ path("/", DataImportDetailView.as_view(), name="dataimport_detail"), path("edit/", DataImportEditView.as_view(), name="dataimport_edit"), path("create/", DataImportCreateView.as_view(), name="dataimport_create"), + path("delete/", DataImportDeleteView.as_view(), name="dataimport_delete"), ] diff --git a/importing/views.py b/importing/views.py index e5c20672..0c733a10 100644 --- a/importing/views.py +++ b/importing/views.py @@ -1,5 +1,6 @@ from management.views import ( CustomCreateView, + CustomDeleteView, CustomDetailView, CustomEditView, CustomTableView, @@ -38,3 +39,9 @@ class DataImportCreateView(CustomCreateView): model = DataImport fields = ["visibility", "station", "format", "rawfile", "reprocess", "observations"] foreign_key_fields = ["station", "format"] + + +class DataImportDeleteView(CustomDeleteView): + """View to delete a data import.""" + + model = DataImport diff --git a/management/tools.py b/management/tools.py new file mode 100644 index 00000000..966a00db --- /dev/null +++ b/management/tools.py @@ -0,0 +1,25 @@ +from django.contrib.admin.utils import NestedObjects +from django.db import models +from django.utils.encoding import force_str +from django.utils.text import capfirst + + +def get_deleted_objects( + objs: list[models.Model], +) -> tuple[list[str], dict[str, int], list[str]]: + """Return information about related objects to be deleted.""" + collector = NestedObjects(using="default") + collector.collect(objs) + + def format_callback(obj): + opts = obj._meta + no_edit_link = f"{capfirst(opts.verbose_name)}: {force_str(obj)}" + return no_edit_link + + to_delete = collector.nested(format_callback) + protected = [format_callback(obj) for obj in collector.protected] + model_count = { + model._meta.verbose_name_plural: len(objs) + for model, objs in collector.model_objs.items() + } + return to_delete, model_count, protected diff --git a/management/views.py b/management/views.py index cd8ac2dc..2ca66b61 100755 --- a/management/views.py +++ b/management/views.py @@ -4,7 +4,7 @@ from django.db.models import Model from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy -from django.views.generic import CreateView, DetailView +from django.views.generic import CreateView, DeleteView, DetailView from django.views.generic.edit import UpdateView from django_filters.views import FilterView from django_tables2 import SingleTableMixin @@ -12,6 +12,7 @@ from .forms import CustomUserCreationForm from .permissions import get_queryset +from .tools import get_deleted_objects class SignUpView(CreateView): @@ -45,7 +46,6 @@ class CustomDetailView(LoginRequiredMixin, DetailView): """ template_name: str = "object_detail.html" - show_delete_btn: bool = False fields = "__all__" def get_object(self) -> Model: @@ -72,7 +72,7 @@ def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["form"] = self.get_form() context["title"] = self.model_description - context["delete_url"] = self.delete_url if self.show_delete_btn else None + context["delete_url"] = self.delete_url context["edit_url"] = self.edit_url context["list_url"] = self.list_url context["pk"] = self.object.pk @@ -338,3 +338,64 @@ def get_form_kwargs(self): kwargs = super().get_form_kwargs() kwargs["user"] = self.request.user return kwargs + + +class CustomDeleteView(LoginRequiredMixin, DeleteView): + """Generic delete view. + + This view is used to delete a model object. The user must have the permission to + delete the object, otherwise a 403 error is returned. A confirmation page is shown + with the related objects that will be deleted. + + The permissions required to delete the object are `app_label.delete_model_name`. For + example, the permission required to delete a `DataImport` object would be + `importing.delete_dataimport`. + + If successful, the view redirects to the list view. + + Users need to be logged in to access this view. + + Attributes: + template_name (str): Template to be used. + """ + + template_name = "object_delete.html" + + def get_object(self) -> Model: + obj = super().get_object() + if not self.request.user.has_perm( + f"{self.app_label}.delete_{self.model_name}", obj + ): + raise PermissionDenied + return obj + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + deletable_objects, model_count, protected = get_deleted_objects([self.object]) + + context["deletable_objects"] = deletable_objects + context["model_count"] = dict(model_count).items() + context["protected"] = protected + context["title"] = self.model_description + context["list_url"] = self.list_url + return context + + @property + def app_label(self) -> str: + return self.model._meta.app_label + + @property + def model_name(self) -> str: + return self.model._meta.model_name + + @property + def model_description(self) -> str: + return self.model._meta.verbose_name.title() + + @property + def list_url(self) -> str: + return f"{self.app_label}:{self.model_name}_list" + + @property + def success_url(self) -> str: + return reverse_lazy(self.list_url) diff --git a/templates/object_delete.html b/templates/object_delete.html new file mode 100644 index 00000000..0f5d09d5 --- /dev/null +++ b/templates/object_delete.html @@ -0,0 +1,46 @@ +{% extends "base.html" %} +{% load django_bootstrap5 %} +{% block content %} +

+ {% block title %} + {{ title }} + {% endblock title %} + object delete +

+ + {% csrf_token %} +

Are you sure you want to delete "{{ object }}"?

+ {% bootstrap_form form layout='horizontal' %} + Return to list + + +

 

+ + Summary of objects to be deleted: +
+
+ + + + + + + + + {% for model_name, object_count in model_count %} + + + + + {% endfor %} + +
NameAmount
{{ model_name|capfirst }}{{ object_count }}
+
+
+ Individual objects to be deleted: +

+

    + {{ deletable_objects|unordered_list }} +
+

+{% endblock content %} diff --git a/templates/object_detail.html b/templates/object_detail.html index 8d4306e3..8d711129 100644 --- a/templates/object_detail.html +++ b/templates/object_detail.html @@ -12,9 +12,7 @@

Return to list Edit - {% if delete_url %} - Delete - {% endif %} + Delete

From b9adeb5345b1781d1bc0ba362637e17a79a47dd2 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 13:22:13 +0100 Subject: [PATCH 10/13] :sparkles: Add importing URL to the navigation bar. --- templates/menu_bar.html | 1 + 1 file changed, 1 insertion(+) diff --git a/templates/menu_bar.html b/templates/menu_bar.html index a0aed1b1..5aaede24 100644 --- a/templates/menu_bar.html +++ b/templates/menu_bar.html @@ -10,6 +10,7 @@ {% if user.is_authenticated %} Validation + Import data {% endif %} From 5d2305930a56740edd00fe041c60bf69a38761d6 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 13:34:09 +0100 Subject: [PATCH 11/13] :sparkles: Add list of protected objects. --- management/tools.py | 18 +++++++++++++++++- templates/object_delete.html | 6 ++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/management/tools.py b/management/tools.py index 966a00db..44a62923 100644 --- a/management/tools.py +++ b/management/tools.py @@ -7,7 +7,19 @@ def get_deleted_objects( objs: list[models.Model], ) -> tuple[list[str], dict[str, int], list[str]]: - """Return information about related objects to be deleted.""" + """Return information about related objects to be deleted. + + How to do this has been taken from https://stackoverflow.com/a/39533619/3778792 + + Args: + objs (list[models.Model]): List of objects to be deleted. + + Returns: + tuple[list[str], dict[str, int], list[str]]: Tuple containing the following: + - List of strings representing the objects to be deleted. + - Dictionary containing the count of objects to be deleted for each model. + - List of strings representing the objects that are protected from deletion + """ collector = NestedObjects(using="default") collector.collect(objs) @@ -22,4 +34,8 @@ def format_callback(obj): model._meta.verbose_name_plural: len(objs) for model, objs in collector.model_objs.items() } + if len(to_delete) == 0: + to_delete.append("None") + if len(protected) == 0: + protected.append("None") return to_delete, model_count, protected diff --git a/templates/object_delete.html b/templates/object_delete.html index 0f5d09d5..8a9afa34 100644 --- a/templates/object_delete.html +++ b/templates/object_delete.html @@ -37,6 +37,12 @@

+ Individual objects that are protected from deletion: +

+

    + {{ protected|unordered_list }} +
+

Individual objects to be deleted:

    From 831f44bd64e3001c65adca4b38aa26205b3af009 Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Wed, 2 Oct 2024 13:36:58 +0100 Subject: [PATCH 12/13] :memo: Update some docstrings --- management/views.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/management/views.py b/management/views.py index 2ca66b61..c2dfce4e 100755 --- a/management/views.py +++ b/management/views.py @@ -209,6 +209,12 @@ class Meta: fields = self.fields def __init__(self, *args, **kwargs): + """Filter the queryset for foreign key fields based on the user. + + Otherwise, the user would see all objects, even those they don't have + access to. We need to pop the user from the kwargs, as it's not a valid + argument for the form. + """ user = kwargs.pop("user") if "user" in kwargs else None super().__init__(*args, **kwargs) if user: @@ -300,6 +306,12 @@ class Meta: fields = self.fields def __init__(self, *args, **kwargs): + """Filter the queryset for foreign key fields based on the user. + + Otherwise, the user would see all objects, even those they don't have + access to. We need to pop the user from the kwargs, as it's not a valid + argument for the form. + """ user = kwargs.pop("user") if "user" in kwargs else None super().__init__(*args, **kwargs) if user: From 14f3123c7287819ddedab4d25d70929484a3bc4b Mon Sep 17 00:00:00 2001 From: Diego Alonso Alvarez Date: Thu, 3 Oct 2024 14:38:31 +0100 Subject: [PATCH 13/13] :bug: Add logged user as object owner --- management/views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/management/views.py b/management/views.py index c2dfce4e..f65bce0f 100755 --- a/management/views.py +++ b/management/views.py @@ -2,6 +2,7 @@ from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import PermissionDenied from django.db.models import Model +from django.http import HttpResponse from django.shortcuts import get_object_or_404 from django.urls import reverse_lazy from django.views.generic import CreateView, DeleteView, DetailView @@ -321,6 +322,21 @@ def __init__(self, *args, **kwargs): return CustomCreateForm + def form_valid(self, form: forms.ModelForm) -> HttpResponse: + """Set the owner of the object to the current user. + + This is done before saving the object to the database. + + Args: + form (forms.ModelForm): Form with the object data. + + Returns: + HttpResponse: Redirect to the detail view of the created object. + """ + if hasattr(form.instance, "owner"): + form.instance.owner = self.request.user + return super().form_valid(form) + @property def app_label(self) -> str: return self.model._meta.app_label