~netlandish/django-wiki

0f3ad18d1e3a3fa91447c0b23c892137062fd2e8 — Gert-Jan Braas 4 years ago 1178e41
Bugfix/fix delete image (#1034)

* intermediate commit

* extended test for purging image with multiple revisions.

https://github.com/django-wiki/django-wiki/issues/936

* made same implentation as in attachments.model

* cleanup, and a little comment

* cleanup

* Update src/wiki/plugins/images/models.py

Co-Authored-By: Benjamin Balder Bach <benjaoming@gmail.com>

* implemented review request changes

* as per review request

* updated releasenotes for 0.6

* one assignment is enough

Co-authored-by: Benjamin Balder Bach <benjaoming@gmail.com>
M docs/release_notes.rst => docs/release_notes.rst +1 -1
@@ 27,7 27,7 @@ Fixed
* Python 3.7 issue with notifications plugin main view ``/_plugin/notifications/`` :url-issue:`1000` (Mads Jensen)
* Broken Delete and Deleted pages :url-issue:`976` (Benjamin Bach)
* Can't delete article with ``USE_THOUSAND_SEPARATOR = True`` :url-issue:`1014` (tim3towers)

* Deleting images fails :url-issue:'936' (Gert-Jan Braas, Steckelfisch)

Changed
~~~~~~~

M src/wiki/plugins/images/models.py => src/wiki/plugins/images/models.py +14 -4
@@ 119,13 119,23 @@ def on_image_revision_delete(instance, *args, **kwargs):
    if not instance.image:
        return

    # Remove image file
    instance.image.delete(save=False)

    path = None
    try:
        path = instance.image.path.split("/")[:-1]
        path = os.path.dirname(instance.image.path)
    except NotImplementedError:
        # This backend storage doesn't implement 'path' so there is no path to delete
        pass
    except ValueError:
        # in case of Value error
        # https://github.com/django-wiki/django-wiki/issues/936
        pass
    finally:
        # Remove image file
        instance.image.delete(save=False)

    if path is None:
        # This backend storage doesn't implement 'path' so there is no path to delete
        # or some other error (ValueError)
        return

    # Clean up empty directories

M testproject/testproject/urls.py => testproject/testproject/urls.py +12 -0
@@ 22,6 22,18 @@ if settings.DEBUG:
        ),
    ]

if settings.DEBUG:
    try:
        import debug_toolbar
        urlpatterns = [
            re_path('__debug__/', include(debug_toolbar.urls)),

            # For django versions before 2.0:
            # url(r'^__debug__/', include(debug_toolbar.urls)),

        ] + urlpatterns
    except ImportError as ie:
        pass

urlpatterns += [
    re_path(r"^notify/", include("django_nyt.urls")),

M tests/plugins/images/test_views.py => tests/plugins/images/test_views.py +30 -0
@@ 242,6 242,36 @@ class ImageTests(RequireRootArticleMixin, ArticleWebTestUtils, DjangoClientTestB
        self.assertEqual(models.Image.objects.count(), 0)
        self.assertIs(os.path.exists(f_path), False)

    def test_add_revision_purge_image(self):
        """
        Tests that an image with more than one revision is really purged
        """
        # use another test to stage this one
        self.test_add_revision()

        image = models.Image.objects.get()
        image_revision = image.current_revision.imagerevision
        f_path = image_revision.image.file.name

        self.assertIs(os.path.exists(f_path), True)

        response = self.client.post(
            reverse(
                "wiki:images_purge",
                kwargs={
                    "article_id": self.root_article,
                    "image_id": image.pk,
                    "path": "",
                },
            ),
            data={"confirm": True},
        )
        self.assertRedirects(
            response, reverse("wiki:images_index", kwargs={"path": ""})
        )
        self.assertEqual(models.Image.objects.count(), 0)
        self.assertIs(os.path.exists(f_path), False)

    @wiki_override_settings(ACCOUNT_HANDLING=True)
    def test_login_on_revision_add(self):
        self._create_test_image(path="")