Org: Moves contact inherit functionality to base ContactExtension

This means resources, forms and directories can now inherit their
contact information from a topic as well.

TYPE: Feature
LINK: OGC-2049

src/onegov/org/models/extensions.py

@@ -239,77 +239,64 @@ class VisibleOnHomepageForm(form_class):  # type:ignore
         return VisibleOnHomepageForm
 
 
-class ContactExtensionBase:
-    """ Common base class for extensions that add a contact field.
+class ContactExtension(ContentExtension):
+    """ Extends any class that has a content dictionary field with a simple
+    contacts field, that can optionally be inherited from another topic.
 
     """
 
     contact: dict_property[str | None] = content_property()
 
     @contact.setter  # type:ignore[no-redef]
     def contact(self, value: str | None) -> None:
-        self.content['contact'] = value  # type:ignore[attr-defined]
+        self.content['contact'] = value
+        if self.inherit_contact:
+            # no need to update the cache
+            return
+
         # update cache
         self.__dict__['contact_html'] = to_html_ul(
-            self.contact, convert_dashes=True, with_title=True
-        ) if self.contact is not None else None
-
-    @cached_property
-    def contact_html(self) -> Markup | None:
-        if self.contact is None:
-            return None
-        return to_html_ul(self.contact, convert_dashes=True, with_title=True)
-
-    def get_contact_html(self, request: OrgRequest) -> Markup | None:
-        return self.contact_html
-
-    def extend_form(
-        self,
-        form_class: type[FormT],
-        request: OrgRequest
-    ) -> type[FormT]:
-
-        class ContactPageForm(form_class):  # type:ignore
-            contact = TextAreaField(
-                label=_('Address'),
-                fieldset=_('Contact'),
-                render_kw={'rows': 5},
-                description=_("- '-' will be converted to a bulleted list\n"
-                              "- Urls will be transformed into links\n"
-                              "- Emails and phone numbers as well")
-            )
-
-        return ContactPageForm
+            value, convert_dashes=True, with_title=True
+        ) if value is not None else None
 
+    inherit_contact: dict_property[bool] = content_property(default=False)
 
-class ContactExtension(ContactExtensionBase, ContentExtension):
-    """ Extends any class that has a content dictionary field with a simple
-    contacts field.
+    @inherit_contact.setter  # type:ignore[no-redef]
+    def inherit_contact(self, value: bool) -> None:
+        self.content['inherit_contact'] = value
 
-    """
+        # clear cache (don't update eagerly since it involves a query)
+        if 'contact_html' in self.__dict__:
+            del self.__dict__['contact_html']
 
+    contact_inherited_from: dict_property[int | None] = content_property()
 
-class InheritableContactExtension(ContactExtensionBase, ContentExtension):
-    """ Extends any class that has a content dictionary field with a simple
-    contacts field, that can optionally be inherited from another topic.
+    @contact_inherited_from.setter  # type:ignore[no-redef]
+    def contact_inherited_from(self, value: int | None) -> None:
+        self.content['contact_inherited_from'] = value
+        if not self.inherit_contact:
+            # no need to clear the cache
+            return
 
-    """
+        # clear cache (don't update eagerly since it involves a query)
+        if 'contact_html' in self.__dict__:
+            del self.__dict__['contact_html']
 
-    inherit_contact: dict_property[bool] = content_property(default=False)
-    contact_inherited_from: dict_property[int | None] = content_property()
-
-    # TODO: If we end up calling this more than once per request
-    #       we may want to cache this
-    def get_contact_html(self, request: OrgRequest) -> Markup | None:
+    @cached_property
+    def contact_html(self) -> Markup | None:
         if self.inherit_contact:
             if self.contact_inherited_from is None:
                 return None
 
-            pages = PageCollection(request.session)
+            pages = PageCollection(object_session(self))
             page = pages.by_id(self.contact_inherited_from)
-            return getattr(page, 'contact_html', None)
+            contact = getattr(page, 'contact', None)
+        else:
+            contact = self.contact
 
-        return self.contact_html
+        if contact is None:
+            return None
+        return to_html_ul(contact, convert_dashes=True, with_title=True)
 
     def extend_form(
         self,
@@ -343,16 +330,14 @@ def extend_form(
             for choice in reversed(pinned.items()):
                 choices.insert(0, choice)
 
-        class InheritableContactPageForm(form_class):  # type:ignore
-
+        class ContactPageForm(form_class):  # type:ignore
             contact = TextAreaField(
                 label=_('Address'),
                 fieldset=_('Contact'),
                 render_kw={'rows': 5},
                 description=_("- '-' will be converted to a bulleted list\n"
                               "- Urls will be transformed into links\n"
-                              "- Emails and phone numbers as well"),
-                depends_on=('inherit_contact', '!y')
+                              "- Emails and phone numbers as well")
             )
 
             inherit_contact = BooleanField(
@@ -370,7 +355,7 @@ class InheritableContactPageForm(form_class):  # type:ignore
                 validators=[InputRequired()]
             )
 
-        return InheritableContactPageForm
+        return ContactPageForm
 
 
 class ContactHiddenOnPageExtension(ContentExtension):

src/onegov/org/models/page.py

@@ -9,7 +9,7 @@
 from onegov.org.forms import LinkForm, PageForm, IframeForm
 from onegov.org.models.atoz import AtoZ
 from onegov.org.models.extensions import (
-    InheritableContactExtension, ContactHiddenOnPageExtension,
+    ContactExtension, ContactHiddenOnPageExtension,
     PeopleShownOnMainPageExtension, ImageExtension,
     NewsletterExtension, PublicationExtension, DeletableContentExtension,
     InlinePhotoAlbumExtension
@@ -40,7 +40,7 @@
 
 class Topic(Page, TraitInfo, SearchableContent, AccessExtension,
             PublicationExtension, VisibleOnHomepageExtension,
-            InheritableContactExtension, ContactHiddenOnPageExtension,
+            ContactExtension, ContactHiddenOnPageExtension,
             PeopleShownOnMainPageExtension, PersonLinkExtension,
             CoordinatesExtension, ImageExtension,
             GeneralFileLinkExtension, SidebarLinksExtension,
@@ -143,7 +143,7 @@ def get_form_class(
 
 class News(Page, TraitInfo, SearchableContent, NewsletterExtension,
            AccessExtension, PublicationExtension, VisibleOnHomepageExtension,
-           InheritableContactExtension, ContactHiddenOnPageExtension,
+           ContactExtension, ContactHiddenOnPageExtension,
            PeopleShownOnMainPageExtension, PersonLinkExtension,
            CoordinatesExtension, ImageExtension, GeneralFileLinkExtension,
            DeletableContentExtension, InlinePhotoAlbumExtension):
@@ -207,7 +207,7 @@ def is_supported_trait(self, trait: str) -> bool:
     def get_root_page_form_class(self, request: OrgRequest) -> type[Form]:
         return self.with_content_extensions(
             Form, request, extensions=(
-                InheritableContactExtension, ContactHiddenOnPageExtension,
+                ContactExtension, ContactHiddenOnPageExtension,
                 PersonLinkExtension, AccessExtension
             )
         )

src/onegov/org/templates/news.pt

@@ -35,6 +35,6 @@
         </div>
 
         <tal:b metal:use-macro="layout.macros.page_content"
-         tal:define="lead page.content.get('lead');text page.text|None; people page.people; contact page.get_contact_html(request); coordinates page.coordinates; files page.files; sidepanel_links page.sidepanel_links|False;" />
+         tal:define="lead page.content.get('lead');text page.text|None; people page.people; contact page.contact_html; coordinates page.coordinates; files page.files; sidepanel_links page.sidepanel_links|False;" />
     </tal:b>
 </div>

src/onegov/org/templates/topic.pt

@@ -11,7 +11,7 @@
         <tal:b condition="page.trait == 'page'">
 
             <tal:b metal:use-macro="layout.macros.page_content"
-             tal:define="lead layout.linkify(page.content.get('lead'));text page.text|None; people page.people; contact page.get_contact_html(request); coordinates page.coordinates; files page.files; sidepanel_links page.sidepanel_links;">
+             tal:define="lead layout.linkify(page.content.get('lead'));text page.text|None; people page.people; contact page.contact_html; coordinates page.coordinates; files page.files; sidepanel_links page.sidepanel_links;">
                 <tal:b metal:fill-slot="after-text">
                      <div class="row" tal:condition="children">
                         <div class="small-12 columns">

src/onegov/town6/layout.py

@@ -258,7 +258,7 @@ class PageLayout(OrgPageLayout, AdjacencyListLayout):
 
     @cached_property
     def contact_html(self) -> str:
-        return self.model.get_contact_html(self.request) or to_html_ul(
+        return self.model.contact_html or to_html_ul(
             self.org.contact
         )
 
@@ -270,7 +270,7 @@ class NewsLayout(OrgNewsLayout, AdjacencyListLayout):
 
     @cached_property
     def contact_html(self) -> str:
-        return self.model.get_contact_html(self.request) or to_html_ul(
+        return self.model.contact_html or to_html_ul(
             self.org.contact, convert_dashes=False
         )
 

tests/onegov/org/test_extensions.py

@@ -339,19 +339,22 @@ class TopicForm(Form):
     ]
 
 
-def test_contact_extension():
-    class Topic(ContactExtension):
-        content = {}
+def test_contact_extension(org_app):
+    from onegov.org.models import Topic
 
     class TopicForm(Form):
         pass
 
-    topic = Topic()
+    topic = Topic('test')
     assert topic.contact is None
     assert topic.contact_html is None
 
-    request = Bunch(**{'app.settings.org.disabled_extensions': []})
-    form_class = topic.with_content_extensions(TopicForm, request=request)
+    request = Bunch(app=org_app, session=org_app.session())
+    form_class = topic.with_content_extensions(
+        TopicForm,
+        request=request,
+        extensions=(ContactExtension,)
+    )
     form = form_class()
 
     assert 'contact' in form._fields
@@ -379,7 +382,11 @@ class TopicForm(Form):
         '</p>'
     )
 
-    form_class = topic.with_content_extensions(TopicForm, request=request)
+    form_class = topic.with_content_extensions(
+        TopicForm,
+        request=request,
+        extensions=(ContactExtension,)
+    )
     form = form_class()
 
     form.process(obj=topic)
@@ -391,21 +398,25 @@ class TopicForm(Form):
     )
 
 
-def test_contact_extension_with_top_level_domain_agency():
-    class Topic(ContactExtension):
-        content = {}
+def test_contact_extension_with_top_level_domain_agency(org_app):
+    from onegov.org.models import Topic
 
     class TopicForm(Form):
         pass
 
-    topic = Topic()
+    topic = Topic('test')
 
     assert topic.contact is None
     assert topic.contact_html is None
 
-    request = Bunch(**{'app.settings.org.disabled_extensions': []})
-    form_class = topic.with_content_extensions(TopicForm, request=request)
+    request = Bunch(app=org_app, session=org_app.session())
+    form_class = topic.with_content_extensions(
+        TopicForm,
+        request=request,
+        extensions=(ContactExtension,)
+    )
     form = form_class()
+    form.request = request
 
     assert 'contact' in form._fields