digitalfabrik / integreat-cms

Simplified content management back end for the Integreat App - a multilingual information platform for newcomers
https://digitalfabrik.github.io/integreat-cms/
Apache License 2.0
55 stars 33 forks source link

`ValueError` when creating a brand new page #2806

Open MizukiTemma opened 1 month ago

MizukiTemma commented 1 month ago

Describe the Bug

When an user tries to create a new page and opens the page form, ValueError occurs in background, although it looks working perfect in the user interface.

Steps to Reproduce

  1. Go to "Create page"
  2. See ValueError in the console

Expected Behavior

No error

Actual Behavior

ValueError

Additional Information

Traceback Traceback (most recent call last): ``` File "/home/micucu/integreat-cms/integreat_cms/cms/models/abstract_base_model.py", line 46, in __repr__ return self.get_repr() ^^^^^^^^^^^^^^^ File "/home/micucu/integreat-cms/integreat_cms/cms/models/pages/page.py", line 377, in get_repr f", slug: {self.best_translation.slug}" if self.best_translation else "" ^^^^^^^^^^^^^^^^^^^^^ File "/home/micucu/integreat-cms/.venv/lib/python3.11/site-packages/django/utils/functional.py", line 57, in __get__ res = instance.__dict__[self.name] = self.func(instance) ^^^^^^^^^^^^^^^^^^^ File "/home/micucu/integreat-cms/integreat_cms/cms/models/abstract_content_model.py", line 437, in best_translation or self.translations.first() ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/micucu/integreat-cms/.venv/lib/python3.11/site-packages/django/db/models/manager.py", line 87, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^ File "/home/micucu/integreat-cms/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 718, in get_queryset raise ValueError( ValueError: 'Page' instance needs to have a primary key value before this relationship can be used. ```
david-venhoff commented 1 month ago

This error is caused by our debug toolbar, which probably tries to access the page before it is ready. Disabling the templates option fixes it image

PeterNerlich commented 1 month ago

A similar issue also happens when a page (or anything inheriting from AbstractContentModel) is logged, or printed during a test that is not using the database. In the latter case the lookup for best_translation fails because database access is disallowed. However, logging this exception leads to it attempting to get the string representation of the object where it happened, which runs into the same thing again, producing a huge trace of the same stuff over and over.

I previously whipped up a quick patch for it but didn't test whether it worked properly or as intended at all. Maybe it is still useful:

diff --git a/integreat_cms/cms/models/abstract_content_model.py b/integreat_cms/cms/models/abstract_content_model.py
index 6963adb6c..f54dc4d44 100644
--- a/integreat_cms/cms/models/abstract_content_model.py
+++ b/integreat_cms/cms/models/abstract_content_model.py
@@ -496,6 +496,8 @@ class AbstractContentModel(AbstractBaseModel):

         :return: A readable string representation of the content object
         """
+        if self.best_translation is None:
+            return super().__str__()
         return self.best_translation.title

     def get_repr(self) -> str:
@@ -506,7 +508,7 @@ class AbstractContentModel(AbstractBaseModel):
         :return: The canonical string representation of the content object
         """
         class_name = type(self).__name__
-        return f"<{class_name} (id: {self.id}, region: {self.region.slug}, slug: {self.best_translation.slug})>"
+        return f"<{class_name} (id: {self.id}, region: {self.region.slug}, slug: {self.best_translation.slug if self.best_translation is not None else None})>"

     class Meta:
         #: This model is an abstract base class