fix: Do not show edit action for version objects where editing is not possible (#405)

* Fix: Do not show edit action for version objects where editing is not possible

* fix ruff errors

* Fix tests

* Fix linting

Author: fsbraun

.github/workflows/lint.yml

@@ -14,4 +14,4 @@ jobs:
     - run: python -Im pip install --user ruff
 
     - name: Run ruff
-      run: ruff --output-format=github djangocms_versioning tests
+      run: ruff check --output-format=github djangocms_versioning tests

djangocms_versioning/admin.py

@@ -491,7 +491,7 @@ def get_actions_list(self):
         actions = [
             self._get_preview_link,
             self._get_edit_link,
-         ]
+        ]
         if "state_indicator" not in self.versioning_list_display:
             # State indicator mixin loaded?
             actions.append(self._get_manage_versions_link)
@@ -729,6 +729,10 @@ def _get_unpublish_link(self, obj, request, disabled=False):
     def _get_edit_link(self, obj, request, disabled=False):
         """Helper function to get the html link to the edit action
         """
+
+        if not obj.check_edit_redirect.as_bool(request.user):
+            return ""
+
         # Only show if no draft exists
         if obj.state == PUBLISHED:
             pks_for_grouper = obj.versionable.for_content_grouping_values(
@@ -758,7 +762,7 @@ def _get_edit_link(self, obj, request, disabled=False):
             title=_("Edit") if icon == "pencil" else _("New Draft"),
             name="edit",
             action="post",
-            disabled=not obj.check_edit_redirect.as_bool(request.user) or disabled,
+            disabled=disabled,
             keepsideframe=keepsideframe,
         )
 

tests/test_admin.py

@@ -528,9 +528,9 @@ def test_revert_action_link_enable_state(self):
             'cms-action-revert '
             'js-action '
             'js-keep-sideframe" '
-            'href="%s" '
+            f'href="{draft_revert_url}" '
             'title="Revert">'
-        ) % draft_revert_url
+        )
         self.assertIn(expected_enabled_state, actual_enabled_control.replace("\n", ""))
 
     def test_revert_action_link_for_draft_state(self):
@@ -599,9 +599,9 @@ def test_discard_action_link_enabled_state(self):
             'cms-action-discard '
             'js-action '
             'js-keep-sideframe" '
-            'href="%s" '
+            f'href="{draft_discard_url}" '
             'title="Discard">'
-        ) % draft_discard_url
+        )
         self.assertIn(expected_enabled_state, actual_enabled_control.replace("\n", ""))
 
     def test_discard_action_link_for_archive_state(self):
@@ -664,11 +664,11 @@ def test_revert_action_link_for_archive_state(self):
             'cms-action-revert '
             'js-action '
             'js-keep-sideframe" '
-            'href="%s" '
+            f'href="{draft_revert_url}" '
             'title="Revert">'
             '<span class="cms-icon cms-icon-undo"></span>'
             '</a>'
-        ) % draft_revert_url
+        )
 
         self.assertIn(
             expected_disabled_control, actual_disabled_control.replace("\n", "")

tests/test_extensions.py

@@ -117,7 +117,7 @@ def test_title_extension_admin_monkey_patch_save(self):
         poll_extension = PollTitleExtensionFactory(extended_object=self.version.content)
         model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollPageContentExtension)
         test_url = admin_reverse("extended_polls_pollpagecontentextension_change", args=(poll_extension.pk,))
-        test_url += "?extended_object=%s" % self.version.content.pk
+        test_url += f"?extended_object={self.version.content.pk}"
         request = RequestFactory().post(path=test_url)
         request.user = self.get_superuser()
 
@@ -137,7 +137,7 @@ def test_title_extension_admin_monkey_patch_save_date_modified_updated(self):
         model_site = PollExtensionAdmin(admin_site=admin.AdminSite(), model=PollPageContentExtension)
         pre_changes_date_modified = Version.objects.get(id=self.version.pk).modified
         test_url = admin_reverse("extended_polls_pollpagecontentextension_change", args=(poll_extension.pk,))
-        test_url += "?extended_object=%s" % self.version.content.pk
+        test_url += f"?extended_object={self.version.content.pk}"
 
         request = RequestFactory().post(path=test_url)
         request.user = self.get_superuser()
@@ -155,7 +155,7 @@ def test_title_extension_admin_monkeypatch_add_view(self):
         with self.login_user_context(self.get_superuser()):
             response = self.client.get(
                 admin_reverse("extended_polls_pollpagecontentextension_add") +
-                "?extended_object=%s" % self.version.content.pk,
+                f"?extended_object={self.version.content.pk}",
                 follow=True
             )
             self.assertEqual(response.status_code, 200)

tests/test_handlers.py

@@ -21,6 +21,7 @@ def test_modified_date(self):
     def test_add_plugin(self):
         version = factories.PageVersionFactory()
         placeholder = factories.PlaceholderFactory(source=version.content)
+        placeholder.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
         poll = factories.PollFactory()
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -45,6 +46,7 @@ def test_change_plugin(self):
         plugin = add_plugin(
             placeholder, "PollPlugin", version.content.language, poll=poll
         )
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -61,6 +63,7 @@ def test_change_plugin(self):
     def test_clear_placeholder(self):
         version = factories.PageVersionFactory()
         placeholder = factories.PlaceholderFactory(source=version.content)
+        placeholder.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -81,6 +84,7 @@ def test_delete_plugin(self):
         plugin = add_plugin(
             placeholder, "PollPlugin", version.content.language, poll=poll
         )
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -103,6 +107,7 @@ def test_add_plugins_from_placeholder(self):
         plugin = add_plugin(
             source_placeholder, "PollPlugin", version.content.language, poll=poll
         )
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -165,7 +170,7 @@ def test_paste_plugin(self):
         plugin = add_plugin(
             source_placeholder, "PollPlugin", version.content.language, poll=poll
         )
-
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
             endpoint = self.get_move_plugin_uri(plugin)
@@ -197,6 +202,7 @@ def test_cut_plugin(self):
         plugin = add_plugin(
             placeholder, "PollPlugin", version.content.language, poll=poll
         )
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):
@@ -223,6 +229,7 @@ def test_move_plugin(self):
         plugin = add_plugin(
             source_placeholder, "PollPlugin", version.content.language, poll=poll
         )
+        plugin.page.get_absolute_url = lambda *args, **kwargs: "/test_page/"  # Fake URL needed for URI
 
         dt = datetime(2016, 6, 6)
         with freeze_time(dt):

tests/test_locking.py

@@ -394,12 +394,12 @@ def test_edit_action_link_disabled_state(self):
         author_request.user = self.user_author
         otheruser_request = RequestFactory()
         otheruser_request.user = self.superuser
-        expected_disabled_state = "inactive"
+        expected_disabled_state = ""
 
         actual_disabled_state = self.version_admin._get_edit_link(version, otheruser_request)
 
         self.assertFalse(version.check_edit_redirect.as_bool(self.superuser))
-        self.assertIn(expected_disabled_state, actual_disabled_state)
+        self.assertEqual(expected_disabled_state, actual_disabled_state)
 
 
 @override_settings(DJANGOCMS_VERSIONING_LOCK_VERSIONS=True)

tests/test_toolbars.py

@@ -341,6 +341,13 @@ def test_view_published_in_toolbar_in_edit_mode_for_published_page(self):
         are published
         """
         published_version = PageVersionFactory(content__language="en", state=PUBLISHED)
+        # Create URL
+        PageUrlFactory(
+            page=published_version.content.page,
+            language=published_version.content.language,
+            path=slugify("test_page"),
+            slug=slugify("test_page"),
+        )
         toolbar = get_toolbar(published_version.content, edit_mode=True)
 
         toolbar.post_template_populate()
@@ -353,6 +360,13 @@ def test_view_published_in_toolbar_in_preview_mode_for_published_page(self):
         are published
         """
         published_version = PageVersionFactory(content__language="en", state=PUBLISHED)
+        # Create URL
+        PageUrlFactory(
+            page=published_version.content.page,
+            language=published_version.content.language,
+            path=slugify("test_page"),
+            slug=slugify("test_page"),
+        )
         toolbar = get_toolbar(published_version.content, preview_mode=True)
 
         toolbar.post_template_populate()