Merge pull request #1519 from ae-utbm/page-edit-link

Move the club page edit link to the club main page

club/models.py

@@ -238,7 +238,7 @@ class Club(models.Model):
 
     def can_be_edited_by(self, user: User) -> bool:
         """Method to see if that object can be edited by the given user."""
-        return self.has_rights_in_club(user)
+        return user.has_perm("club.change_club") or self.has_rights_in_club(user)
 
     def can_roles_be_edited_by(self, user: User) -> bool:
         """Return True if the given user can edit the roles of this club"""

club/templates/club/club_detail.jinja

@@ -26,6 +26,15 @@
 {% endblock %}
 
 {% block content %}
+  {% if user.is_authenticated and user.can_edit(club) %}
+    <a
+      href="{{ url("core:page_edit", page_name=club.page.get_full_name()) }}"
+      class="btn btn-blue margin-bottom"
+    >
+      <i class="fa fa-pen-to-square"></i> {% trans %}Edit{% endtrans %}
+    </a>
+  {% endif %}
+
   <h3>{{ club.name }}</h3>
   <div id="club-detail" {% if links %}class="has-links"{% endif %}>
     <div id="club-attributes">

club/tests/test_page.py

@@ -1,8 +1,9 @@
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Callable
 
 import pytest
 from aemark import markdown
 from bs4 import BeautifulSoup
+from django.contrib.auth.models import Permission
 from django.urls import reverse
 from model_bakery import baker
 from pytest_django.asserts import assertHTMLEqual, assertRedirects
@@ -16,13 +17,20 @@ if TYPE_CHECKING:
 
 
 @pytest.mark.django_db
-def test_page_display_on_club_main_page(client: Client):
+@pytest.mark.parametrize("user_factory", [lambda: None, subscriber_user.make])
+def test_page_display_on_club_main_page(
+    client: Client, user_factory: Callable[[], User | None]
+):
     """Test the club Page is properly displayed on the club main view"""
     club = baker.make(Club)
+    # create another previous revision, to test that we display the most recent one
+    baker.make(PageRev, page=club.page, revision=1)
     content = "# foo\nLorem ipsum dolor sit amet"
-    baker.make(PageRev, page=club.page, revision=1, content=content)
-    res = client.get(reverse("club:club_view", kwargs={"club_id": club.id}))
+    baker.make(PageRev, page=club.page, revision=2, content=content)
 
+    if user := user_factory():
+        client.force_login(user)
+    res = client.get(reverse("club:club_view", kwargs={"club_id": club.id}))
     assert res.status_code == 200
     soup = BeautifulSoup(res.text, "lxml")
     detail_html = soup.find(id="club-page").find(class_="markdown")
@@ -30,20 +38,60 @@ def test_page_display_on_club_main_page(client: Client):
 
 
 @pytest.mark.django_db
-def test_club_main_page_without_content(client: Client):
-    """Test the club view works, even if the club page is empty"""
+@pytest.mark.parametrize(
+    ("user_factory", "can_view_tabs", "can_view_edit_button"),
+    [
+        (lambda _: None, False, False),
+        (lambda _: baker.make(User), False, False),
+        (lambda _: subscriber_user.make(), True, False),
+        (
+            # admin user
+            lambda _: subscriber_user.make(
+                user_permissions=[Permission.objects.get(codename="change_club")]
+            ),
+            True,
+            True,
+        ),
+        (
+            # club board member
+            lambda club: (
+                baker.make(
+                    Membership,
+                    club=club,
+                    role=baker.make(ClubRole, is_board=True),
+                    user=subscriber_user.make(),
+                ).user
+            ),
+            True,
+            True,
+        ),
+    ],
+    ids=["anonymous", "public", "subscriber", "admin", "club board"],
+)
+@pytest.mark.django_db
+def test_club_main_page_displayed_elements(
+    client: Client, user_factory, can_view_tabs, can_view_edit_button
+):
+    """Test that the appropriate elements are displayed depending on user status."""
     club = baker.make(Club)
-    club.page.revisions.all().delete()
+    if user := user_factory(club):
+        client.force_login(user)
     res = client.get(reverse("club:club_view", kwargs={"club_id": club.id}))
 
     assert res.status_code == 200
     soup = BeautifulSoup(res.text, "lxml")
-    detail_html = soup.find(id="club-page")
-    assert detail_html.find_all("markdown") == []
+    tabs = soup.find(class_="tool_bar")
+    assert tabs is not None if can_view_tabs else tabs is None
+    edit_btn = soup.find(
+        "a",
+        href=reverse("core:page_edit", kwargs={"page_name": club.page.get_full_name()}),
+    )
+    assert edit_btn is not None if can_view_edit_button else edit_btn is None
 
 
 @pytest.mark.django_db
 def test_page_revision(client: Client):
+    """Test that accessing a previous revision of the club page works."""
     club = baker.make(Club)
     revisions = baker.make(
         PageRev, page=club.page, _quantity=3, content=iter(["foo", "bar", "baz"])

club/views.py

@@ -106,20 +106,19 @@ class ClubTabsMixin(TabedViewMixin):
         return self.object.get_display_name()
 
     def get_list_of_tabs(self):
+        if not self.request.user.has_perm("club.view_club"):
+            # The user will still be able to see the club main page,
+            # but nothing else.
+            # It's useless to display a tab with only one item, so return nothing.
+            return []
         tab_list = [
             {
                 "url": reverse("club:club_view", kwargs={"club_id": self.object.id}),
                 "slug": "infos",
                 "name": _("Infos"),
-            }
-        ]
-        if self.request.user.has_perm("club.view_club"):
-            tab_list.extend(
-                [
+            },
             {
-                        "url": reverse(
-                            "club:club_members", kwargs={"club_id": self.object.id}
-                        ),
+                "url": reverse("club:club_members", kwargs={"club_id": self.object.id}),
                 "slug": "members",
                 "name": _("Members"),
             },
@@ -130,18 +129,12 @@ class ClubTabsMixin(TabedViewMixin):
                 "slug": "elderlies",
                 "name": _("Old members"),
             },
-                ]
-            )
-            if self.object.page:
-                tab_list.append(
             {
-                        "url": reverse(
-                            "club:club_hist", kwargs={"club_id": self.object.id}
-                        ),
+                "url": reverse("club:club_hist", kwargs={"club_id": self.object.id}),
                 "slug": "history",
                 "name": _("History"),
-                    }
-                )
+            },
+        ]
         if self.request.user.can_edit(self.object):
             tab_list.extend(
                 [
@@ -159,21 +152,6 @@ class ClubTabsMixin(TabedViewMixin):
                         "slug": "edit",
                         "name": _("Edit"),
                     },
-                ]
-            )
-            if self.object.page and self.request.user.can_edit(self.object.page):
-                tab_list.append(
-                    {
-                        "url": reverse(
-                            "core:page_edit",
-                            kwargs={"page_name": self.object.page.get_full_name()},
-                        ),
-                        "slug": "page_edit",
-                        "name": _("Edit club page"),
-                    }
-                )
-            tab_list.extend(
-                [
                     {
                         "url": reverse(
                             "club:club_sellings", kwargs={"club_id": self.object.id}

locale/fr/LC_MESSAGES/django.po

@@ -517,6 +517,22 @@ msgstr "Url dupliquée"
 msgid "This link doesn't match with the url base of its type."
 msgstr "Ce lien ne correspond pas à l'url de base de son type."
 
+#: club/templates/club/club_detail.jinja club/views.py
+#: com/templates/com/news_admin_list.jinja com/templates/com/poster_list.jinja
+#: com/templates/com/screen_list.jinja com/templates/com/weekmail.jinja
+#: core/templates/core/file.jinja core/templates/core/group_list.jinja
+#: core/templates/core/page/base.jinja core/templates/core/user_tools.jinja
+#: core/views/user.py counter/templates/counter/cash_summary_list.jinja
+#: counter/templates/counter/counter_list.jinja
+#: election/templates/election/election_detail.jinja
+#: forum/templates/forum/macros.jinja
+#: pedagogy/templates/pedagogy/fragments/ue_detail/comments.jinja
+#: pedagogy/templates/pedagogy/guide.jinja sas/templates/sas/album.jinja
+#: trombi/templates/trombi/detail.jinja
+#: trombi/templates/trombi/edit_profile.jinja
+msgid "Edit"
+msgstr "Éditer"
+
 #: club/templates/club/club_detail.jinja com/templates/com/news_list.jinja
 msgid "Links"
 msgstr "Liens"
@@ -1019,26 +1035,6 @@ msgstr "Historique"
 msgid "Tools"
 msgstr "Outils"
 
-#: club/views.py com/templates/com/news_admin_list.jinja
-#: com/templates/com/poster_list.jinja com/templates/com/screen_list.jinja
-#: com/templates/com/weekmail.jinja core/templates/core/file.jinja
-#: core/templates/core/group_list.jinja core/templates/core/page/base.jinja
-#: core/templates/core/user_tools.jinja core/views/user.py
-#: counter/templates/counter/cash_summary_list.jinja
-#: counter/templates/counter/counter_list.jinja
-#: election/templates/election/election_detail.jinja
-#: forum/templates/forum/macros.jinja
-#: pedagogy/templates/pedagogy/fragments/ue_detail/comments.jinja
-#: pedagogy/templates/pedagogy/guide.jinja sas/templates/sas/album.jinja
-#: trombi/templates/trombi/detail.jinja
-#: trombi/templates/trombi/edit_profile.jinja
-msgid "Edit"
-msgstr "Éditer"
-
-#: club/views.py
-msgid "Edit club page"
-msgstr "Éditer la page de club"
-
 #: club/views.py
 msgid "Sellings"
 msgstr "Vente"