fixing Confluence.remove_page_attachment_keep_version() method #367
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi there,
TL;DR: This patch intends to fix the
remove_page_attachment_keep_version()method in theConfluenceclass, so that it only removes (some) of the versions and not the attachment as a whole.Background: I am faced with a Confluence instance that has been around for some years and for reasons™ had several hundreds of times the exact same versions of most of its attachments uploaded to it by a third party Confluence plugin, over and over again. In order to free up dozens of gigabytes of disk space, I am trying to use this library to iterate over the pages in a particular space and for each attachment found on these pages to delete all versions of the attachments except the most recent ones. The
remove_page_attachment_keep_version()method seems to be intend to do just that.Problem: When using the function with a given attachment and a number of versions to keep (m), it gets a list of all versions (n), then starts a while-loop deleting the version n-m of the attachment, until only m versions remain. Unfortunately the method
Confluence.delete_attachment()seems to ignore theversionparameter and always deletes the whole attachment. This causes an infinite loop, as it doesn't find the new version count, but gets a 403 error instead as the attachment isn't there anymore.Partial solution: I noticed the
Confluence.delete_attachment_by_id()function just below uses a different API call with an explicit version being required. Since we already have the attachment object including ID available inremove_page_attachment_keep_version()we can fix it by simply using that other deletion method.Conclusion: This PR doesn't address the non-working
versionparameter in theConfluence.delete_attachment()method. I didn't touch it, as I don't know what other functionality may rely on it. Maybe it could be rewritten to use theConfluence.delete_attachment_by_id()when a version is provided and otherwise keep using the existing logic? Hope this PR serves to prevent others from deleting their attachments accidentally. Luckily we use testing instances. :-)