From 0ac2530e6e951e4431321b87a6de098aa4ca99c6 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 12:51:40 +0100 Subject: [PATCH 01/13] Remove redundant version fetch The block above this line fetches the latest matching version, so it's pointless to then use that to get the latest version's URL. Instead, we get the URL of the object we already have. --- cbv/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cbv/views.py b/cbv/views.py index 3497c057..b38024ae 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -94,7 +94,7 @@ def get_redirect_url(self, **kwargs): except Klass.DoesNotExist: raise http.Http404 - return klass.get_latest_version_url() + return klass.get_absolute_url() @attrs.frozen From f8973fa2d763296fab15207c95da85b37a472076 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 12:44:51 +0100 Subject: [PATCH 02/13] Customise QuerySet instead of Manager This will allow us to add custom queryset methods in an upcoming commit. --- cbv/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cbv/models.py b/cbv/models.py index dddae0d3..3de2e787 100644 --- a/cbv/models.py +++ b/cbv/models.py @@ -107,7 +107,7 @@ def get_absolute_url(self) -> str: ) -class KlassManager(models.Manager): +class KlassQuerySet(models.QuerySet): def get_by_natural_key( self, klass_name: str, module_name: str, project_name: str, version_number: str ) -> "Klass": @@ -146,7 +146,7 @@ class Klass(models.Model): # because docs urls differ between Django versions docs_url = models.URLField(max_length=255, default="") - objects = KlassManager() + objects = KlassQuerySet.as_manager() class Meta: unique_together = ("module", "name") From dad158d293b63d06cb17c1c47cfc92dfcc2af43b Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 19 May 2024 14:27:52 +0100 Subject: [PATCH 03/13] Remove direct knowledge of Django URLs from models Django models shouldn't know anything about URLs. This is a step in the direction of removing `get_absolute_url` from the models. --- cbv/models.py | 49 ++++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/cbv/models.py b/cbv/models.py index 3de2e787..34fc12f7 100644 --- a/cbv/models.py +++ b/cbv/models.py @@ -3,6 +3,29 @@ from django.urls import reverse +class DjangoURLService: + def class_detail( + self, class_name: str, module_name: str, version_number: str + ) -> str: + return reverse( + "klass-detail", + kwargs={ + "version": version_number, + "module": module_name, + "klass": class_name, + }, + ) + + def module_detail(self, module_name: str, version_number: str) -> str: + return reverse( + "module-detail", + kwargs={"version": version_number, "module": module_name}, + ) + + def version_detail(self, version_number: str) -> str: + return reverse("version-detail", kwargs={"version": version_number}) + + class ProjectVersionManager(models.Manager): def get_by_natural_key(self, name: str, version_number: str) -> "ProjectVersion": return self.get( @@ -38,12 +61,7 @@ def natural_key(self) -> tuple[str, str]: return ("Django", self.version_number) def get_absolute_url(self) -> str: - return reverse( - "version-detail", - kwargs={ - "version": self.version_number, - }, - ) + return DjangoURLService().version_detail(self.version_number) def generate_sortable_version_number(self) -> str: return "".join(part.zfill(2) for part in self.version_number.split(".")) @@ -98,12 +116,8 @@ def natural_key(self) -> tuple[str, str, str]: natural_key.dependencies = ["cbv.ProjectVersion"] def get_absolute_url(self) -> str: - return reverse( - "module-detail", - kwargs={ - "version": self.project_version.version_number, - "module": self.name, - }, + return DjangoURLService().module_detail( + module_name=self.name, version_number=self.project_version.version_number ) @@ -167,13 +181,10 @@ def is_secondary(self) -> bool: ) def get_absolute_url(self) -> str: - return reverse( - "klass-detail", - kwargs={ - "version": self.module.project_version.version_number, - "module": self.module.name, - "klass": self.name, - }, + return DjangoURLService().class_detail( + class_name=self.name, + module_name=self.module.name, + version_number=self.module.project_version.version_number, ) def get_latest_version_url(self) -> str: From de10f031f6344703f7567091a82beda0d8e0ae2e Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 12:50:17 +0100 Subject: [PATCH 04/13] Add helper for getting latest version of Klass --- cbv/models.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cbv/models.py b/cbv/models.py index 34fc12f7..d568bfbe 100644 --- a/cbv/models.py +++ b/cbv/models.py @@ -147,6 +147,11 @@ def get_latest_for_name(self, klass_name: str) -> "Klass": else: return obj + def get_latest_version(self, module_name: str, class_name: str) -> "Klass": + return self.filter(module__name=module_name, name=class_name).order_by( + "-module__project_version__sortable_version_number" + )[0] + # TODO: quite a few of the methods on here should probably be denormed. class Klass(models.Model): From 26e1cd05a3b58947ade712810b12ba85aa0051d9 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 12:53:41 +0100 Subject: [PATCH 05/13] Separate getting latest version from URL Before this change, we used a model helper method to fetch the URL of the latest version of a matching class. Now we fetch the latest version, and then get its URL. --- cbv/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cbv/views.py b/cbv/views.py index b38024ae..a3211edc 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -43,7 +43,11 @@ def get_context_data(self, **kwargs): except Klass.DoesNotExist: raise http.Http404 - canonical_url_path = klass.get_latest_version_url() + latest_version = Klass.objects.select_related( + "module__project_version" + ).get_latest_version(module_name=klass.module.name, class_name=klass.name) + + canonical_url_path = latest_version.get_absolute_url() best_current_path = klass.get_absolute_url() if best_current_path != self.request.path: push_state_url = best_current_path From f65b2c98ccd12406bc23322fc3881ad75929849d Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 12:57:10 +0100 Subject: [PATCH 06/13] Remove unused Klass().get_latest_version_url() --- cbv/models.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cbv/models.py b/cbv/models.py index d568bfbe..2bc32b46 100644 --- a/cbv/models.py +++ b/cbv/models.py @@ -192,18 +192,6 @@ def get_absolute_url(self) -> str: version_number=self.module.project_version.version_number, ) - def get_latest_version_url(self) -> str: - latest = ( - self._meta.model.objects.filter( - module__name=self.module.name, - name=self.name, - ) - .select_related("module__project_version") - .order_by("-module__project_version__sortable_version_number") - .first() - ) - return latest.get_absolute_url() - def get_source_url(self) -> str: url = "https://github.com/django/django/blob/" version = self.module.project_version.version_number From 1271520518f8154d9f4aed1acb9fc3461ae8b20a Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 13:58:00 +0100 Subject: [PATCH 07/13] Replace some calls to Klass().get_absolute_url() Part of the process of removing the models is ensuring that we don't rely on any methods that live on them. This change removes most references to `get_absolute_url` on `Klass`, leaving behind those in the templates. --- cbv/queries.py | 15 +++++++++++-- cbv/views.py | 61 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/cbv/queries.py b/cbv/queries.py index f36fb290..e89532f2 100644 --- a/cbv/queries.py +++ b/cbv/queries.py @@ -1,6 +1,7 @@ import attrs from cbv import models +from cbv.models import DjangoURLService @attrs.frozen @@ -40,13 +41,18 @@ def _to_module_data( active_module: models.Module | None, active_klass: models.Klass | None, ) -> "NavData.Module": + url_service = DjangoURLService() return NavData.Module( source_name=module.source_name(), short_name=module.short_name(), classes=[ NavData.Klass( name=klass.name, - url=klass.get_absolute_url(), + url=url_service.class_detail( + class_name=klass.name, + module_name=klass.module.name, + version_number=klass.module.project_version.version_number, + ), active=klass == active_klass, ) for klass in module.klass_set.all() @@ -60,6 +66,7 @@ def make_version_switcher( klass: models.Klass | None = None, ) -> VersionSwitcher: other_versions = models.ProjectVersion.objects.exclude(pk=project_version.pk) + url_service = DjangoURLService() if klass: other_versions_of_klass = models.Klass.objects.filter( name=klass.name, @@ -75,7 +82,11 @@ def make_version_switcher( except KeyError: url = other_version.get_absolute_url() else: - url = other_klass.get_absolute_url() + url = url_service.class_detail( + class_name=other_klass.name, + module_name=other_klass.module.name, + version_number=other_version.version_number, + ) versions.append( VersionSwitcher.OtherVersion( diff --git a/cbv/views.py b/cbv/views.py index a3211edc..afcc6f47 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -5,7 +5,7 @@ from django.urls import reverse from django.views.generic import RedirectView, TemplateView, View -from cbv.models import Klass, Module, ProjectVersion +from cbv.models import DjangoURLService, Klass, Module, ProjectVersion from cbv.queries import NavBuilder @@ -46,9 +46,17 @@ def get_context_data(self, **kwargs): latest_version = Klass.objects.select_related( "module__project_version" ).get_latest_version(module_name=klass.module.name, class_name=klass.name) - - canonical_url_path = latest_version.get_absolute_url() - best_current_path = klass.get_absolute_url() + url_service = DjangoURLService() + canonical_url_path = url_service.class_detail( + class_name=latest_version.name, + module_name=latest_version.module.name, + version_number=latest_version.module.project_version.version_number, + ) + best_current_path = url_service.class_detail( + class_name=klass.name, + module_name=klass.module.name, + version_number=klass.module.project_version.version_number, + ) if best_current_path != self.request.path: push_state_url = best_current_path else: @@ -98,7 +106,11 @@ def get_redirect_url(self, **kwargs): except Klass.DoesNotExist: raise http.Http404 - return klass.get_absolute_url() + return DjangoURLService().class_detail( + class_name=klass.name, + module_name=klass.module.name, + version_number=klass.module.project_version.version_number, + ) @attrs.frozen @@ -148,7 +160,18 @@ def get_context_data(self, **kwargs): klasses = Klass.objects.filter(module=module).select_related( "module__project_version" ) - klass_list = [KlassData(name=k.name, url=k.get_absolute_url()) for k in klasses] + url_service = DjangoURLService() + klass_list = [ + KlassData( + name=k.name, + url=url_service.class_detail( + class_name=k.name, + module_name=k.module.name, + version_number=k.module.project_version.version_number, + ), + ) + for k in klasses + ] latest_version = ( Module.objects.filter( @@ -197,6 +220,7 @@ def get_context_data(self, **kwargs): nav_builder = NavBuilder() version_switcher = nav_builder.make_version_switcher(project_version) nav = nav_builder.get_nav_data(project_version) + url_service = DjangoURLService() return { "nav": nav, "object_list": [ @@ -207,7 +231,11 @@ def get_context_data(self, **kwargs): module_long_name=class_.module.long_name, module_name=class_.module.name, module_short_name=class_.module.short_name, - url=class_.get_absolute_url(), + url=url_service.class_detail( + class_name=class_.name, + module_name=class_.module.name, + version_number=project_version.version_number, + ), ) for class_ in Klass.objects.filter( module__project_version=project_version @@ -226,6 +254,7 @@ def get_context_data(self, **kwargs): nav_builder = NavBuilder() version_switcher = nav_builder.make_version_switcher(project_version) nav = nav_builder.get_nav_data(project_version) + url_service = DjangoURLService() return { "nav": nav, "object_list": [ @@ -236,7 +265,11 @@ def get_context_data(self, **kwargs): module_long_name=class_.module.long_name, module_name=class_.module.name, module_short_name=class_.module.short_name, - url=class_.get_absolute_url(), + url=url_service.class_detail( + class_name=class_.name, + module_name=class_.module.name, + version_number=project_version.version_number, + ), ) for class_ in Klass.objects.filter( module__project_version=project_version @@ -259,10 +292,20 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]: "name", ) + url_service = DjangoURLService() urls = [{"location": reverse("home"), "priority": 1.0}] for klass in klasses: priority = 0.9 if klass.module.project_version == latest_version else 0.5 - urls.append({"location": klass.get_absolute_url(), "priority": priority}) + urls.append( + { + "location": url_service.class_detail( + class_name=klass.name, + module_name=klass.module.name, + version_number=klass.module.project_version.version_number, + ), + "priority": priority, + } + ) return {"urlset": urls} From 2d15e45ece401dbba5f8b9531e0776d495601deb Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 17:01:11 +0100 Subject: [PATCH 08/13] Move if statement outside of loop --- cbv/templates/cbv/klass_detail.html | 56 ++-- .../fuzzy-klass-detail-old.html | 300 ++++++++---------- tests/_page_snapshots/fuzzy-klass-detail.html | 300 ++++++++---------- tests/_page_snapshots/klass-detail-old.html | 300 ++++++++---------- tests/_page_snapshots/klass-detail.html | 300 ++++++++---------- 5 files changed, 587 insertions(+), 669 deletions(-) diff --git a/cbv/templates/cbv/klass_detail.html b/cbv/templates/cbv/klass_detail.html index d290f436..55c3d53e 100644 --- a/cbv/templates/cbv/klass_detail.html +++ b/cbv/templates/cbv/klass_detail.html @@ -95,39 +95,37 @@

Descendants

- {% for attribute in attributes %} - {% if forloop.first %} -
-

Attributes

- - - - - - - - - {% endif %} + {% if attributes %} +
+

Attributes

+
 Defined in
+ - - + + - {% if forloop.last %} + + + {% for attribute in attributes %} + + + + + {% endfor %}
- - {{ attribute.name }} = {{ attribute.value }} - - - {% if attribute.klass == klass %} - {{ attribute.klass.name }} - {% else %} - {{ attribute.klass.name }} - {% endif %} -  Defined in
+ + {{ attribute.name }} = {{ attribute.value }} + + + {% if attribute.klass == klass %} + {{ attribute.klass.name }} + {% else %} + {{ attribute.klass.name }} + {% endif %} +
-
- {% endif %} - {% endfor %} +
+ {% endif %}
{% for method in methods %} diff --git a/tests/_page_snapshots/fuzzy-klass-detail-old.html b/tests/_page_snapshots/fuzzy-klass-detail-old.html index fcd531ac..7ff2a6ed 100644 --- a/tests/_page_snapshots/fuzzy-klass-detail-old.html +++ b/tests/_page_snapshots/fuzzy-klass-detail-old.html @@ -477,170 +477,150 @@

Descendants

- -
-

Attributes

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +
+

Attributes

+
 Defined in
- - content_type = None - - - - TemplateResponseMixin - -
- - extra_context = None - - - - ContextMixin - -
- - form_class = None - - - - FormMixin - -
- - http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] - - - - View - -
- - initial = {} - - - - FormMixin - -
- - prefix = None - - - - FormMixin - -
- - response_class = <class 'django.template.response.TemplateResponse'> - - - - TemplateResponseMixin - -
- - success_url = None - - - - FormMixin - -
- - template_engine = None - - - - TemplateResponseMixin - -
+ - - + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
- - template_name = None - - - - TemplateResponseMixin - -  Defined in
+ + content_type = None + + + + TemplateResponseMixin + +
+ + extra_context = None + + + + ContextMixin + +
+ + form_class = None + + + + FormMixin + +
+ + http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] + + + + View + +
+ + initial = {} + + + + FormMixin + +
+ + prefix = None + + + + FormMixin + +
+ + response_class = <class 'django.template.response.TemplateResponse'> + + + + TemplateResponseMixin + +
+ + success_url = None + + + + FormMixin + +
+ + template_engine = None + + + + TemplateResponseMixin + +
+ + template_name = None + + + + TemplateResponseMixin + +
-
- +
diff --git a/tests/_page_snapshots/fuzzy-klass-detail.html b/tests/_page_snapshots/fuzzy-klass-detail.html index c64becb8..a9f8afc4 100644 --- a/tests/_page_snapshots/fuzzy-klass-detail.html +++ b/tests/_page_snapshots/fuzzy-klass-detail.html @@ -477,170 +477,150 @@

Descendants

- -
-

Attributes

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +
+

Attributes

+
 Defined in
- - content_type = None - - - - TemplateResponseMixin - -
- - extra_context = None - - - - ContextMixin - -
- - form_class = None - - - - FormMixin - -
- - http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] - - - - View - -
- - initial = {} - - - - FormMixin - -
- - prefix = None - - - - FormMixin - -
- - response_class = <class 'django.template.response.TemplateResponse'> - - - - TemplateResponseMixin - -
- - success_url = None - - - - FormMixin - -
- - template_engine = None - - - - TemplateResponseMixin - -
+ - - + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
- - template_name = None - - - - TemplateResponseMixin - -  Defined in
+ + content_type = None + + + + TemplateResponseMixin + +
+ + extra_context = None + + + + ContextMixin + +
+ + form_class = None + + + + FormMixin + +
+ + http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] + + + + View + +
+ + initial = {} + + + + FormMixin + +
+ + prefix = None + + + + FormMixin + +
+ + response_class = <class 'django.template.response.TemplateResponse'> + + + + TemplateResponseMixin + +
+ + success_url = None + + + + FormMixin + +
+ + template_engine = None + + + + TemplateResponseMixin + +
+ + template_name = None + + + + TemplateResponseMixin + +
-
- +
diff --git a/tests/_page_snapshots/klass-detail-old.html b/tests/_page_snapshots/klass-detail-old.html index d2d0f4c9..ab5fb8bd 100644 --- a/tests/_page_snapshots/klass-detail-old.html +++ b/tests/_page_snapshots/klass-detail-old.html @@ -473,170 +473,150 @@

Descendants

- -
-

Attributes

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +
+

Attributes

+
 Defined in
- - content_type = None - - - - TemplateResponseMixin - -
- - extra_context = None - - - - ContextMixin - -
- - form_class = None - - - - FormMixin - -
- - http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] - - - - View - -
- - initial = {} - - - - FormMixin - -
- - prefix = None - - - - FormMixin - -
- - response_class = <class 'django.template.response.TemplateResponse'> - - - - TemplateResponseMixin - -
- - success_url = None - - - - FormMixin - -
- - template_engine = None - - - - TemplateResponseMixin - -
+ - - + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
- - template_name = None - - - - TemplateResponseMixin - -  Defined in
+ + content_type = None + + + + TemplateResponseMixin + +
+ + extra_context = None + + + + ContextMixin + +
+ + form_class = None + + + + FormMixin + +
+ + http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] + + + + View + +
+ + initial = {} + + + + FormMixin + +
+ + prefix = None + + + + FormMixin + +
+ + response_class = <class 'django.template.response.TemplateResponse'> + + + + TemplateResponseMixin + +
+ + success_url = None + + + + FormMixin + +
+ + template_engine = None + + + + TemplateResponseMixin + +
+ + template_name = None + + + + TemplateResponseMixin + +
-
- +
diff --git a/tests/_page_snapshots/klass-detail.html b/tests/_page_snapshots/klass-detail.html index dd2b7947..b384374f 100644 --- a/tests/_page_snapshots/klass-detail.html +++ b/tests/_page_snapshots/klass-detail.html @@ -473,170 +473,150 @@

Descendants

- -
-

Attributes

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +
+

Attributes

+
 Defined in
- - content_type = None - - - - TemplateResponseMixin - -
- - extra_context = None - - - - ContextMixin - -
- - form_class = None - - - - FormMixin - -
- - http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] - - - - View - -
- - initial = {} - - - - FormMixin - -
- - prefix = None - - - - FormMixin - -
- - response_class = <class 'django.template.response.TemplateResponse'> - - - - TemplateResponseMixin - -
- - success_url = None - - - - FormMixin - -
- - template_engine = None - - - - TemplateResponseMixin - -
+ - - + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
- - template_name = None - - - - TemplateResponseMixin - -  Defined in
+ + content_type = None + + + + TemplateResponseMixin + +
+ + extra_context = None + + + + ContextMixin + +
+ + form_class = None + + + + FormMixin + +
+ + http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] + + + + View + +
+ + initial = {} + + + + FormMixin + +
+ + prefix = None + + + + FormMixin + +
+ + response_class = <class 'django.template.response.TemplateResponse'> + + + + TemplateResponseMixin + +
+ + success_url = None + + + + FormMixin + +
+ + template_engine = None + + + + TemplateResponseMixin + +
+ + template_name = None + + + + TemplateResponseMixin + +
-
- +
From 46c1fcf3f05f62430e6909f884b87cdea12674d5 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 18:32:56 +0100 Subject: [PATCH 09/13] Move get_prepared_attributes from model to view We're in the process of moving logic off the models. This model method was only used by one view. This moves the logic from the model to the view. --- cbv/models.py | 34 ---------------------------------- cbv/views.py | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/cbv/models.py b/cbv/models.py index 2bc32b46..ba2a9db9 100644 --- a/cbv/models.py +++ b/cbv/models.py @@ -261,40 +261,6 @@ def get_attributes(self) -> models.QuerySet["KlassAttribute"]: self._attributes = attrs return self._attributes - def get_prepared_attributes(self) -> models.QuerySet["KlassAttribute"]: - attributes = self.get_attributes() - # Make a dictionary of attributes based on name - attribute_names: dict[str, list[KlassAttribute]] = {} - for attr in attributes: - try: - attribute_names[attr.name] += [attr] - except KeyError: - attribute_names[attr.name] = [attr] - - ancestors = self.get_all_ancestors() - - # Find overridden attributes - for name, attrs in attribute_names.items(): - # Skip if we have only one attribute. - if len(attrs) == 1: - continue - - # Sort the attributes by ancestors. - def _key(a: KlassAttribute) -> int: - try: - # If ancestor, return the index (>= 0) - return ancestors.index(a.klass) - except ValueError: # Raised by .index if item is not in list. - # else a.klass == self, so return -1 - return -1 - - sorted_attrs = sorted(attrs, key=_key) - - # Mark overriden KlassAttributes - for a in sorted_attrs[1:]: - a.overridden = True - return attributes - def basic_yuml_data(self, first: bool = False) -> list[str]: self._basic_yuml_data: list[str] if hasattr(self, "_basic_yuml_data"): diff --git a/cbv/views.py b/cbv/views.py index afcc6f47..71eda918 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -2,10 +2,11 @@ import attrs from django import http +from django.db import models from django.urls import reverse from django.views.generic import RedirectView, TemplateView, View -from cbv.models import DjangoURLService, Klass, Module, ProjectVersion +from cbv.models import DjangoURLService, Klass, KlassAttribute, Module, ProjectVersion from cbv.queries import NavBuilder @@ -87,7 +88,7 @@ def get_context_data(self, **kwargs): return { "all_ancestors": ancestors, "all_children": children, - "attributes": klass.get_prepared_attributes(), + "attributes": self.get_prepared_attributes(klass), "canonical_url": self.request.build_absolute_uri(canonical_url_path), "klass": klass, "methods": list(klass.get_methods()), @@ -98,6 +99,42 @@ def get_context_data(self, **kwargs): "yuml_url": klass.basic_yuml_url(), } + def get_prepared_attributes( + self, class_: Klass + ) -> models.QuerySet["KlassAttribute"]: + attributes = class_.get_attributes() + # Make a dictionary of attributes based on name + attribute_names: dict[str, list[KlassAttribute]] = {} + for attr in attributes: + try: + attribute_names[attr.name] += [attr] + except KeyError: + attribute_names[attr.name] = [attr] + + ancestors = class_.get_all_ancestors() + + # Find overridden attributes + for name, attrs_ in attribute_names.items(): + # Skip if we have only one attribute. + if len(attrs_) == 1: + continue + + # Sort the attributes by ancestors. + def _key(a: KlassAttribute) -> int: + try: + # If ancestor, return the index (>= 0) + return ancestors.index(a.klass) + except ValueError: # Raised by .index if item is not in list. + # else a.klass == self, so return -1 + return -1 + + sorted_attrs = sorted(attrs_, key=_key) + + # Mark overriden KlassAttributes + for a in sorted_attrs[1:]: + a.overridden = True + return attributes + class LatestKlassRedirectView(RedirectView): def get_redirect_url(self, **kwargs): From 27fbac294741f25e937e2bdbff38f1847db14db9 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 18:57:22 +0100 Subject: [PATCH 10/13] Fetch ancestors outside of loop --- cbv/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cbv/views.py b/cbv/views.py index 71eda918..cce0c9df 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -70,13 +70,14 @@ def get_context_data(self, **kwargs): klass.module.project_version, klass.module, klass ) direct_ancestors = list(klass.get_ancestors()) + all_ancestors = klass.get_all_ancestors() ancestors = [ self.Ancestor( name=ancestor.name, url=ancestor.get_absolute_url(), is_direct=ancestor in direct_ancestors, ) - for ancestor in klass.get_all_ancestors() + for ancestor in all_ancestors ] children = [ self.Child( From 464fcd0742a0b3ddb7957dfd4bb8b3486a1183ca Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 20:18:32 +0100 Subject: [PATCH 11/13] Use a defaultdict for building mapped list We were previously manually doing what defaultdict does for us. --- cbv/views.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cbv/views.py b/cbv/views.py index cce0c9df..ba8f64e0 100644 --- a/cbv/views.py +++ b/cbv/views.py @@ -1,3 +1,4 @@ +from collections import defaultdict from typing import Any import attrs @@ -105,12 +106,9 @@ def get_prepared_attributes( ) -> models.QuerySet["KlassAttribute"]: attributes = class_.get_attributes() # Make a dictionary of attributes based on name - attribute_names: dict[str, list[KlassAttribute]] = {} + attribute_names: dict[str, list[KlassAttribute]] = defaultdict(list) for attr in attributes: - try: - attribute_names[attr.name] += [attr] - except KeyError: - attribute_names[attr.name] = [attr] + attribute_names[attr.name].append(attr) ancestors = class_.get_all_ancestors() From 99df59c855d07aaede5ec3f2f0ebeedda7bcd361 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 21:15:32 +0100 Subject: [PATCH 12/13] Add test for view with overridden attributes Before this change, we didn't have a test which covered rendering overridden attributes. This change adds UpdateView to the snapshot tests, to catch regressions. --- tests/_page_snapshots/updateview.html | 2430 +++++++++++++++++++++++++ tests/test_page_snapshots.py | 12 + 2 files changed, 2442 insertions(+) create mode 100644 tests/_page_snapshots/updateview.html diff --git a/tests/_page_snapshots/updateview.html b/tests/_page_snapshots/updateview.html new file mode 100644 index 00000000..55910bcb --- /dev/null +++ b/tests/_page_snapshots/updateview.html @@ -0,0 +1,2430 @@ + + + + + + UpdateView -- Classy CBV + + + + + + + + + + + + + + + + + + + + + + +
+
+
+ ccbv.co.uk + +
+
+
+
+
+ +

class UpdateView

+ 
from django.views.generic import UpdateView
+
+ + + Hierarchy diagram + + + + Documentation + + Source code +
+ + 
View for updating an object, with a response rendered by a template.
+ + +
+
+
+ +
+

Ancestors (MRO)

+
    +
  1. UpdateView
    2. + +
  2. + + SingleObjectTemplateResponseMixin + +
    3. + +
  3. + + TemplateResponseMixin + +
    4. + +
  4. + + BaseUpdateView + +
    5. + +
  5. + + ModelFormMixin + +
    6. + +
  6. + + FormMixin + +
    7. + +
  7. + + SingleObjectMixin + +
    8. + +
  8. + + ContextMixin + +
    9. + +
  9. + + ProcessFormView + +
    10. + +
  10. + + View + +
    11. + +
+
+ + + +
+ +
+ +
+

Attributes

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
 Defined in
+ + content_type = None + + + + TemplateResponseMixin + +
+ + context_object_name = None + + + + SingleObjectMixin + +
+ + extra_context = None + + + + ContextMixin + +
+ + fields = None + + + + ModelFormMixin + +
+ + form_class = None + + + + FormMixin + +
+ + http_method_names = ['get', 'post', 'put', 'patch', 'delete', 'head', 'options', 'trace'] + + + + View + +
+ + initial = {} + + + + FormMixin + +
+ + model = None + + + + SingleObjectMixin + +
+ + pk_url_kwarg = 'pk' + + + + SingleObjectMixin + +
+ + prefix = None + + + + FormMixin + +
+ + query_pk_and_slug = False + + + + SingleObjectMixin + +
+ + queryset = None + + + + SingleObjectMixin + +
+ + response_class = <class 'django.template.response.TemplateResponse'> + + + + TemplateResponseMixin + +
+ + slug_field = 'slug' + + + + SingleObjectMixin + +
+ + slug_url_kwarg = 'slug' + + + + SingleObjectMixin + +
+ + success_url = None + + + + FormMixin + +
+ + template_engine = None + + + + TemplateResponseMixin + +
+ + template_name = None + + + + TemplateResponseMixin + +
+ + template_name_field = None + + + + SingleObjectTemplateResponseMixin + +
+ + template_name_suffix = '_detail' + + + + SingleObjectTemplateResponseMixin + +
+ + template_name_suffix = '_form' + + + + UpdateView + +
+
+ +
+
+ + +
+
+ Expand + Collapse +
+

Methods

+ + + +
+ +

+ + def + __init__(self, **kwargs): + + + View + + +

+ +
+ + + 
Constructor. Called in the URLconf; can contain helpful extra
+keyword arguments, and other things.
+ 
37
+38
+39
+40
+41
+42
+43
+44
+45
def __init__(self, **kwargs):
+    """
+    Constructor. Called in the URLconf; can contain helpful extra
+    keyword arguments, and other things.
+    """
+    # Go through keyword arguments, and either save their values to our
+    # instance, or raise an error.
+    for key, value in kwargs.items():
+        setattr(self, key, value)
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + _allowed_methods(self): + + + View + + +

+ +
+ + + + 
117
+118
def _allowed_methods(self):
+    return [m.upper() for m in self.http_method_names if hasattr(self, m)]
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + as_view(cls, **initkwargs): + + + View + + +

+ +
+ + + 
Main entry point for a request-response process.
+ 
47
+48
+49
+50
+51
+52
+53
+54
+55
+56
+57
+58
+59
+60
+61
+62
+63
+64
+65
+66
+67
+68
+69
+70
+71
+72
+73
+74
+75
+76
+77
+78
+79
+80
@classonlymethod
+def as_view(cls, **initkwargs):
+    """Main entry point for a request-response process."""
+    for key in initkwargs:
+        if key in cls.http_method_names:
+            raise TypeError(
+                'The method name %s is not accepted as a keyword argument '
+                'to %s().' % (key, cls.__name__)
+            )
+        if not hasattr(cls, key):
+            raise TypeError("%s() received an invalid keyword %r. as_view "
+                            "only accepts arguments that are already "
+                            "attributes of the class." % (cls.__name__, key))
+    def view(request, *args, **kwargs):
+        self = cls(**initkwargs)
+        self.setup(request, *args, **kwargs)
+        if not hasattr(self, 'request'):
+            raise AttributeError(
+                "%s instance has no 'request' attribute. Did you override "
+                "setup() and forget to call super()?" % cls.__name__
+            )
+        return self.dispatch(request, *args, **kwargs)
+    view.view_class = cls
+    view.view_initkwargs = initkwargs
+    # __name__ and __qualname__ are intentionally left unchanged as
+    # view_class should be used to robustly determine the name of the view
+    # instead.
+    view.__doc__ = cls.__doc__
+    view.__module__ = cls.__module__
+    view.__annotations__ = cls.dispatch.__annotations__
+    # Copy possible attributes set by decorators, e.g. @csrf_exempt, from
+    # the dispatch method.
+    view.__dict__.update(cls.dispatch.__dict__)
+    return view
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + dispatch(self, request, *args, **kwargs): + + + View + + +

+ +
+ + + + 
 93
+ 94
+ 95
+ 96
+ 97
+ 98
+ 99
+100
+101
def dispatch(self, request, *args, **kwargs):
+    # Try to dispatch to the right method; if a method doesn't exist,
+    # defer to the error handler. Also defer to the error handler if the
+    # request method isn't on the approved list.
+    if request.method.lower() in self.http_method_names:
+        handler = getattr(self, request.method.lower(), self.http_method_not_allowed)
+    else:
+        handler = self.http_method_not_allowed
+    return handler(request, *args, **kwargs)
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + form_invalid(self, form): + + + FormMixin + + +

+ +
+ + + 
If the form is invalid, render the invalid form.
+ 
61
+62
+63
def form_invalid(self, form):
+    """If the form is invalid, render the invalid form."""
+    return self.render_to_response(self.get_context_data(form=form))
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + form_valid(self, form): + + + +

+ +
+ + +
+ +

ModelFormMixin

+ +
+
+ 
If the form is valid, save the associated model.
+ 
125
+126
+127
+128
def form_valid(self, form):
+    """If the form is valid, save the associated model."""
+    self.object = form.save()
+    return super().form_valid(form)
+
+
+
+
+
+ + + +
+ +

FormMixin

+ +
+
+ 
If the form is valid, redirect to the supplied URL.
+ 
57
+58
+59
def form_valid(self, form):
+    """If the form is valid, redirect to the supplied URL."""
+    return HttpResponseRedirect(self.get_success_url())
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + get(self, request, *args, **kwargs): + + + +

+ +
+ + +
+ +

BaseUpdateView

+ +
+
+ 
Handle GET requests: instantiate a blank version of the form.
+ 
190
+191
+192
def get(self, request, *args, **kwargs):
+    self.object = self.get_object()
+    return super().get(request, *args, **kwargs)
+
+
+
+
+
+ + + +
+ +

ProcessFormView

+ +
+
+ 
Handle GET requests: instantiate a blank version of the form.
+ 
133
+134
+135
def get(self, request, *args, **kwargs):
+    """Handle GET requests: instantiate a blank version of the form."""
+    return self.render_to_response(self.get_context_data())
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + get_context_data(self, **kwargs): + + + +

+ +
+ + +
+ +

FormMixin

+ +
+
+ 
Insert the form into the context dict.
+ 
65
+66
+67
+68
+69
def get_context_data(self, **kwargs):
+    """Insert the form into the context dict."""
+    if 'form' not in kwargs:
+        kwargs['form'] = self.get_form()
+    return super().get_context_data(**kwargs)
+
+
+
+
+
+ + + +
+ +

SingleObjectMixin

+ +
+
+ 
Insert the single object into the context dict.
+ 
 91
+ 92
+ 93
+ 94
+ 95
+ 96
+ 97
+ 98
+ 99
+100
def get_context_data(self, **kwargs):
+    """Insert the single object into the context dict."""
+    context = {}
+    if self.object:
+        context['object'] = self.object
+        context_object_name = self.get_context_object_name(self.object)
+        if context_object_name:
+            context[context_object_name] = self.object
+    context.update(kwargs)
+    return super().get_context_data(**context)
+
+
+
+
+
+ + + +
+ +

ContextMixin

+ +
+
+ + 
22
+23
+24
+25
+26
def get_context_data(self, **kwargs):
+    kwargs.setdefault('view', self)
+    if self.extra_context is not None:
+        kwargs.update(self.extra_context)
+    return kwargs
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + + + + + +
+ +

+ + def + get_context_object_name(self, obj): + + + SingleObjectMixin + + +

+ +
+ + + 
Get the name to use for the object.
+ 
82
+83
+84
+85
+86
+87
+88
+89
def get_context_object_name(self, obj):
+    """Get the name to use for the object."""
+    if self.context_object_name:
+        return self.context_object_name
+    elif isinstance(obj, models.Model):
+        return obj._meta.model_name
+    else:
+        return None
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_form(self, form_class=None): + + + FormMixin + + +

+ +
+ + + 
Return an instance of the form to be used in this view.
+ 
31
+32
+33
+34
+35
def get_form(self, form_class=None):
+    """Return an instance of the form to be used in this view."""
+    if form_class is None:
+        form_class = self.get_form_class()
+    return form_class(**self.get_form_kwargs())
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_form_class(self): + + + +

+ +
+ + +
+ +

ModelFormMixin

+ +
+
+ 
Return the form class to use in this view.
+ 
 76
+ 77
+ 78
+ 79
+ 80
+ 81
+ 82
+ 83
+ 84
+ 85
+ 86
+ 87
+ 88
+ 89
+ 90
+ 91
+ 92
+ 93
+ 94
+ 95
+ 96
+ 97
+ 98
+ 99
+100
+101
def get_form_class(self):
+    """Return the form class to use in this view."""
+    if self.fields is not None and self.form_class:
+        raise ImproperlyConfigured(
+            "Specifying both 'fields' and 'form_class' is not permitted."
+        )
+    if self.form_class:
+        return self.form_class
+    else:
+        if self.model is not None:
+            # If a model has been explicitly provided, use it
+            model = self.model
+        elif getattr(self, 'object', None) is not None:
+            # If this view is operating on a single object, use
+            # the class of that object
+            model = self.object.__class__
+        else:
+            # Try to get a queryset and extract the model class
+            # from that
+            model = self.get_queryset().model
+        if self.fields is None:
+            raise ImproperlyConfigured(
+                "Using ModelFormMixin (base class of %s) without "
+                "the 'fields' attribute is prohibited." % self.__class__.__name__
+            )
+        return model_forms.modelform_factory(model, fields=self.fields)
+
+
+
+
+
+ + + +
+ +

FormMixin

+ +
+
+ 
Return the form class to use.
+ 
27
+28
+29
def get_form_class(self):
+    """Return the form class to use."""
+    return self.form_class
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + get_form_kwargs(self): + + + +

+ +
+ + +
+ +

ModelFormMixin

+ +
+
+ 
Return the keyword arguments for instantiating the form.
+ 
105
+106
+107
+108
+109
+110
def get_form_kwargs(self):
+    """Return the keyword arguments for instantiating the form."""
+    kwargs = super().get_form_kwargs()
+    if hasattr(self, 'object'):
+        kwargs.update({'instance': self.object})
+    return kwargs
+
+
+
+
+
+ + + +
+ +

FormMixin

+ +
+
+ 
Return the keyword arguments for instantiating the form.
+ 
37
+38
+39
+40
+41
+42
+43
+44
+45
+46
+47
+48
def get_form_kwargs(self):
+    """Return the keyword arguments for instantiating the form."""
+    kwargs = {
+        'initial': self.get_initial(),
+        'prefix': self.get_prefix(),
+    }
+    if self.request.method in ('POST', 'PUT'):
+        kwargs.update({
+            'data': self.request.POST,
+            'files': self.request.FILES,
+        })
+    return kwargs
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + get_initial(self): + + + FormMixin + + +

+ +
+ + + 
Return the initial data to use for forms on this view.
+ 
19
+20
+21
def get_initial(self):
+    """Return the initial data to use for forms on this view."""
+    return self.initial.copy()
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_object(self, queryset=None): + + + SingleObjectMixin + + +

+ +
+ + + 
Return the object the view is displaying.
+
+Require `self.queryset` and a `pk` or `slug` argument in the URLconf.
+Subclasses can override this to return any object.
+ 
20
+21
+22
+23
+24
+25
+26
+27
+28
+29
+30
+31
+32
+33
+34
+35
+36
+37
+38
+39
+40
+41
+42
+43
+44
+45
+46
+47
+48
+49
+50
+51
def get_object(self, queryset=None):
+    """
+    Return the object the view is displaying.
+    Require `self.queryset` and a `pk` or `slug` argument in the URLconf.
+    Subclasses can override this to return any object.
+    """
+    # Use a custom queryset if provided; this is required for subclasses
+    # like DateDetailView
+    if queryset is None:
+        queryset = self.get_queryset()
+    # Next, try looking up by primary key.
+    pk = self.kwargs.get(self.pk_url_kwarg)
+    slug = self.kwargs.get(self.slug_url_kwarg)
+    if pk is not None:
+        queryset = queryset.filter(pk=pk)
+    # Next, try looking up by slug.
+    if slug is not None and (pk is None or self.query_pk_and_slug):
+        slug_field = self.get_slug_field()
+        queryset = queryset.filter(**{slug_field: slug})
+    # If none of those are defined, it's an error.
+    if pk is None and slug is None:
+        raise AttributeError(
+            "Generic detail view %s must be called with either an object "
+            "pk or a slug in the URLconf." % self.__class__.__name__
+        )
+    try:
+        # Get the single item from the filtered queryset
+        obj = queryset.get()
+    except queryset.model.DoesNotExist:
+        raise Http404(_("No %(verbose_name)s found matching the query") %
+                      {'verbose_name': queryset.model._meta.verbose_name})
+    return obj
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_prefix(self): + + + FormMixin + + +

+ +
+ + + 
Return the prefix to use for forms.
+ 
23
+24
+25
def get_prefix(self):
+    """Return the prefix to use for forms."""
+    return self.prefix
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_queryset(self): + + + SingleObjectMixin + + +

+ +
+ + + 
Return the `QuerySet` that will be used to look up the object.
+
+This method is called by the default implementation of get_object() and
+may not be called if get_object() is overridden.
+ 
58
+59
+60
+61
+62
+63
+64
+65
+66
+67
+68
+69
+70
+71
+72
+73
+74
+75
def get_queryset(self):
+    """
+    Return the `QuerySet` that will be used to look up the object.
+    This method is called by the default implementation of get_object() and
+    may not be called if get_object() is overridden.
+    """
+    if self.queryset is None:
+        if self.model:
+            return self.model._default_manager.all()
+        else:
+            raise ImproperlyConfigured(
+                "%(cls)s is missing a QuerySet. Define "
+                "%(cls)s.model, %(cls)s.queryset, or override "
+                "%(cls)s.get_queryset()." % {
+                    'cls': self.__class__.__name__
+                }
+            )
+    return self.queryset.all()
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_slug_field(self): + + + SingleObjectMixin + + +

+ +
+ + + 
Get the name of a slug field to be used to look up by slug.
+ 
78
+79
+80
def get_slug_field(self):
+    """Get the name of a slug field to be used to look up by slug."""
+    return self.slug_field
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + get_success_url(self): + + + +

+ +
+ + +
+ +

ModelFormMixin

+ +
+
+ 
Return the URL to redirect to after processing a valid form.
+ 
112
+113
+114
+115
+116
+117
+118
+119
+120
+121
+122
+123
def get_success_url(self):
+    """Return the URL to redirect to after processing a valid form."""
+    if self.success_url:
+        url = self.success_url.format(**self.object.__dict__)
+    else:
+        try:
+            url = self.object.get_absolute_url()
+        except AttributeError:
+            raise ImproperlyConfigured(
+                "No URL to redirect to.  Either provide a url or define"
+                " a get_absolute_url method on the Model.")
+    return url
+
+
+
+
+
+ + + +
+ +

FormMixin

+ +
+
+ 
Return the URL to redirect to after processing a valid form.
+ 
51
+52
+53
+54
+55
def get_success_url(self):
+    """Return the URL to redirect to after processing a valid form."""
+    if not self.success_url:
+        raise ImproperlyConfigured("No URL to redirect to. Provide a success_url.")
+    return str(self.success_url)  # success_url may be lazy
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + get_template_names(self): + + + +

+ +
+ + +
+ +

SingleObjectTemplateResponseMixin

+ +
+
+ 
Return a list of template names to be used for the request. May not be
+called if render_to_response() is overridden. Return the following list:
+
+* the value of ``template_name`` on the view (if provided)
+* the contents of the ``template_name_field`` field on the
+  object instance that the view is operating upon (if available)
+* ``<app_label>/<model_name><template_name_suffix>.html``
+ 
115
+116
+117
+118
+119
+120
+121
+122
+123
+124
+125
+126
+127
+128
+129
+130
+131
+132
+133
+134
+135
+136
+137
+138
+139
+140
+141
+142
+143
+144
+145
+146
+147
+148
+149
+150
+151
+152
+153
+154
+155
+156
def get_template_names(self):
+    """
+    Return a list of template names to be used for the request. May not be
+    called if render_to_response() is overridden. Return the following list:
+    * the value of ``template_name`` on the view (if provided)
+    * the contents of the ``template_name_field`` field on the
+      object instance that the view is operating upon (if available)
+    * ``<app_label>/<model_name><template_name_suffix>.html``
+    """
+    try:
+        names = super().get_template_names()
+    except ImproperlyConfigured:
+        # If template_name isn't specified, it's not a problem --
+        # we just start with an empty list.
+        names = []
+        # If self.template_name_field is set, grab the value of the field
+        # of that name from the object; this is the most specific template
+        # name, if given.
+        if self.object and self.template_name_field:
+            name = getattr(self.object, self.template_name_field, None)
+            if name:
+                names.insert(0, name)
+        # The least-specific option is the default <app>/<model>_detail.html;
+        # only use this if the object in question is a model.
+        if isinstance(self.object, models.Model):
+            object_meta = self.object._meta
+            names.append("%s/%s%s.html" % (
+                object_meta.app_label,
+                object_meta.model_name,
+                self.template_name_suffix
+            ))
+        elif getattr(self, 'model', None) is not None and issubclass(self.model, models.Model):
+            names.append("%s/%s%s.html" % (
+                self.model._meta.app_label,
+                self.model._meta.model_name,
+                self.template_name_suffix
+            ))
+        # If we still haven't managed to find any template names, we should
+        # re-raise the ImproperlyConfigured to alert the user.
+        if not names:
+            raise
+    return names
+
+
+
+
+
+ + + +
+ +

TemplateResponseMixin

+ +
+
+ 
Return a list of template names to be used for the request. Must return
+a list. May not be called if render_to_response() is overridden.
+ 
144
+145
+146
+147
+148
+149
+150
+151
+152
+153
+154
def get_template_names(self):
+    """
+    Return a list of template names to be used for the request. Must return
+    a list. May not be called if render_to_response() is overridden.
+    """
+    if self.template_name is None:
+        raise ImproperlyConfigured(
+            "TemplateResponseMixin requires either a definition of "
+            "'template_name' or an implementation of 'get_template_names()'")
+    else:
+        return [self.template_name]
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + http_method_not_allowed(self, request, *args, **kwargs): + + + View + + +

+ +
+ + + + 
103
+104
+105
+106
+107
+108
def http_method_not_allowed(self, request, *args, **kwargs):
+    logger.warning(
+        'Method Not Allowed (%s): %s', request.method, request.path,
+        extra={'status_code': 405, 'request': request}
+    )
+    return HttpResponseNotAllowed(self._allowed_methods())
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + options(self, request, *args, **kwargs): + + + View + + +

+ +
+ + + 
Handle responding to requests for the OPTIONS HTTP verb.
+ 
110
+111
+112
+113
+114
+115
def options(self, request, *args, **kwargs):
+    """Handle responding to requests for the OPTIONS HTTP verb."""
+    response = HttpResponse()
+    response.headers['Allow'] = ', '.join(self._allowed_methods())
+    response.headers['Content-Length'] = '0'
+    return response
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + post(self, request, *args, **kwargs): + + + +

+ +
+ + +
+ +

BaseUpdateView

+ +
+
+ 
Handle POST requests: instantiate a form instance with the passed
+POST variables and then check if it's valid.
+ 
194
+195
+196
def post(self, request, *args, **kwargs):
+    self.object = self.get_object()
+    return super().post(request, *args, **kwargs)
+
+
+
+
+
+ + + +
+ +

ProcessFormView

+ +
+
+ 
Handle POST requests: instantiate a form instance with the passed
+POST variables and then check if it's valid.
+ 
137
+138
+139
+140
+141
+142
+143
+144
+145
+146
def post(self, request, *args, **kwargs):
+    """
+    Handle POST requests: instantiate a form instance with the passed
+    POST variables and then check if it's valid.
+    """
+    form = self.get_form()
+    if form.is_valid():
+        return self.form_valid(form)
+    else:
+        return self.form_invalid(form)
+
+
+
+
+
+ + +
+
+ + + + + + + + + + + +
+ +

+ + def + put(self, *args, **kwargs): + + + ProcessFormView + + +

+ +
+ + + + 
150
+151
def put(self, *args, **kwargs):
+    return self.post(*args, **kwargs)
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + render_to_response(self, context, **response_kwargs): + + + TemplateResponseMixin + + +

+ +
+ + + 
Return a response, using the `response_class` for this view, with a
+template rendered with the given context.
+
+Pass response_kwargs to the constructor of the response class.
+ 
128
+129
+130
+131
+132
+133
+134
+135
+136
+137
+138
+139
+140
+141
def render_to_response(self, context, **response_kwargs):
+    """
+    Return a response, using the `response_class` for this view, with a
+    template rendered with the given context.
+    Pass response_kwargs to the constructor of the response class.
+    """
+    response_kwargs.setdefault('content_type', self.content_type)
+    return self.response_class(
+        request=self.request,
+        template=self.get_template_names(),
+        context=context,
+        using=self.template_engine,
+        **response_kwargs
+    )
+
+
+ + +
+
+ + + + + + + +
+ +

+ + def + setup(self, request, *args, **kwargs): + + + View + + +

+ +
+ + + 
Initialize attributes shared by all view methods.
+ 
85
+86
+87
+88
+89
+90
+91
def setup(self, request, *args, **kwargs):
+    """Initialize attributes shared by all view methods."""
+    if hasattr(self, 'get') and not hasattr(self, 'head'):
+        self.head = self.get
+    self.request = request
+    self.args = args
+    self.kwargs = kwargs
+
+
+ + +
+
+ + +
+ +
+
+
+
+ + + +
+ + + + + + + + + + + + + diff --git a/tests/test_page_snapshots.py b/tests/test_page_snapshots.py index c0d49f18..4278a33a 100644 --- a/tests/test_page_snapshots.py +++ b/tests/test_page_snapshots.py @@ -43,6 +43,18 @@ }, ), ), + ( + "updateview.html", + 33, + reverse( + "klass-detail", + kwargs={ + "version": "4.0", + "module": "django.views.generic.edit", + "klass": "UpdateView", + }, + ), + ), ( "klass-detail-old.html", 30, From e9f1b795dfd28966f2230fac69832c3fb052ce23 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Sun, 2 Jun 2024 23:31:26 +0100 Subject: [PATCH 13/13] Test get_all_ancestors against UpdateView We previously tested against a couple of simple artificial examples, but I didn't trust it to cover all the possible cases. This change ensures that we're testing against a complicated real-world example, UpdateView, which I believe to represent sufficient complexity. --- tests/test_models.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_models.py b/tests/test_models.py index 6e9280f3..0287c9f1 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,4 +1,8 @@ import pytest +from django.core.management import call_command +from django.views.generic import UpdateView + +from cbv.models import Klass from .factories import InheritanceFactory, KlassFactory @@ -53,3 +57,26 @@ def test_diamond(self) -> None: mro = d.get_all_ancestors() assert mro == [b, c, a] + + def test_real(self) -> None: + """ + Test the MRO of a real class hierarchy, taken from the Django's UpdateView. + """ + # The version of Django that we are using for this test is 3.1 + # because that is the version we have in our dependencies when we run tests. + # If this fails in future, it is probably because the version of Django + # has been updated and the MRO of the UpdateView has changed. + # In that case, feel free to update the version we're loading here. + call_command("loaddata", "3.1.json") + + mro = Klass.objects.get(name="UpdateView").get_all_ancestors() + + # For the sake of comparison, we convert the Klass objects to a tuple of strings, + # where the first element is the module name and the second is the class name. + mro_strings = [(k.module.name, k.name) for k in mro] + + # The first element is the class itself, so we skip it. + # The last element is object, so we skip it. + real_ancestors = UpdateView.__mro__[1:][:-1] + + assert mro_strings == [(c.__module__, c.__name__) for c in real_ancestors]