-
Notifications
You must be signed in to change notification settings - Fork 2
Allow admins to manage media files in Django admin #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9a9ff54
954e198
e1c2364
79e4ad6
0eec276
e858496
3d380de
3a24547
7ef5cbb
73ff5e6
750bede
f75b0d5
fab7cbf
82dd59b
13dff22
e80af4f
ee5baa3
333e809
13a91e9
e6364ee
71e5d7a
f81afbe
decf92e
2b0a270
19e1fb4
e5001f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,269 @@ | ||
| import os | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure either, and have no objections.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works for now. In the future, I recommend borrowing the convention used by the library you're overriding. Since this mostly functions to override components and logic from |
||
| from django.core.files.storage import DefaultStorage | ||
| from django.core.paginator import Paginator, EmptyPage | ||
| from django.template.response import TemplateResponse | ||
| from django.contrib import messages | ||
| from django.urls import reverse | ||
| from django.http import HttpResponseRedirect | ||
| from django.conf import settings | ||
| from filebrowser import signals | ||
| from filebrowser.sites import ( | ||
| FileBrowserSite, | ||
| filebrowser_view, | ||
| get_settings_var, | ||
| get_breadcrumbs, | ||
| admin_site, | ||
| ) | ||
| from filebrowser.decorators import path_exists | ||
| from filebrowser.settings import ( | ||
| DEFAULT_SORTING_BY, | ||
| DEFAULT_SORTING_ORDER, | ||
| VERSIONS_BASEDIR, | ||
| ) | ||
| from filebrowser.templatetags.fb_tags import query_helper | ||
|
|
||
|
|
||
| def media_matches(media_filter, obj): | ||
| """Return True if `obj` satisfies the media-record filter. | ||
| Supported values for media_filter: | ||
| - 'has_record' -> keep objects where obj.has_media_record() is True | ||
| - 'no_record' -> keep objects where obj.has_media_record() is False | ||
| If no media_filter is set, everything matches. | ||
| """ | ||
| if not media_filter: | ||
| return True | ||
| try: | ||
| has_attr = getattr(obj, "has_media_record", None) | ||
| if callable(has_attr): | ||
| has = bool(has_attr()) | ||
| else: | ||
| has = bool(has_attr) | ||
| except AttributeError: | ||
| has = False | ||
| if media_filter == "has_record": | ||
| return has | ||
| if media_filter == "no_record": | ||
| return not has | ||
| return True | ||
|
|
||
|
|
||
| class TekdbFileBrowserSite(FileBrowserSite): | ||
| def files_folders_to_ignore(self): | ||
| """Return list of filenames/folders to ignore in deleting media without record.""" | ||
| return ["__pycache__", "__init__.py", VERSIONS_BASEDIR] | ||
|
|
||
| def get_urls(self): | ||
| from django.urls import re_path | ||
|
|
||
| urls = super().get_urls() | ||
|
|
||
| urls += [ | ||
| re_path( | ||
| r"^delete-media-without-record-confirm/", | ||
| path_exists( | ||
| self, filebrowser_view(self.delete_media_without_record_confirm) | ||
| ), | ||
| name="fb_delete_all_media_without_record_confirm", | ||
| ), | ||
| re_path( | ||
| r"^delete-media-without-record/", | ||
| path_exists(self, filebrowser_view(self.delete_media_without_record)), | ||
| name="fb_delete_all_media_without_record", | ||
| ), | ||
| ] | ||
| return urls | ||
|
|
||
| def browse(self, request): | ||
| """Call upstream browse(), then apply server-side filtering for the | ||
| `filter_media_record` query parameter. This keeps the UI filter backed | ||
| by server logic without changing upstream code. | ||
| """ | ||
| response = super().browse(request) | ||
|
|
||
| # Only proceed if the TemplateResponse has context data we can | ||
| # mutate. | ||
| if not hasattr(response, "context_data"): | ||
| return response | ||
|
|
||
| media_filter = None | ||
| try: | ||
| media_filter = request.GET.get("filter_media_record") | ||
| except AttributeError: | ||
| media_filter = None | ||
|
|
||
| # dict of original paginator -> new paginator | ||
| paginator_replacements = {} | ||
|
|
||
| # loop does the following: | ||
| # 1. filter data if media_filter is set | ||
| # 2. set response.context_data["page"] to filtered page | ||
| # 3. build new paginators dict | ||
| for key, val in list(response.context_data.items()): | ||
| try: | ||
| if ( | ||
| hasattr(val, "paginator") | ||
| and hasattr(val, "number") | ||
| and key == "page" | ||
| ): | ||
| orig_paginator = val.paginator | ||
| try: | ||
| full_list = list(orig_paginator.object_list) | ||
| except (AttributeError, TypeError): | ||
| full_list = list(getattr(val, "object_list", [])) | ||
|
|
||
| filtered_full = [ | ||
| obj for obj in full_list if media_matches(media_filter, obj) | ||
| ] | ||
|
|
||
| max_per_page = getattr(settings, "FILEBROWSER_LIST_PER_PAGE", None) | ||
| if not max_per_page: | ||
| max_per_page = getattr(orig_paginator, "per_page", 20) | ||
|
|
||
| new_paginator = Paginator(filtered_full, max_per_page) | ||
| page_number = getattr(val, "number", 1) | ||
| try: | ||
| new_page = new_paginator.page(page_number) | ||
| except EmptyPage: | ||
| new_page = new_paginator.page(new_paginator.num_pages) | ||
| response.context_data[key] = new_page | ||
| paginator_replacements[orig_paginator] = new_paginator | ||
|
|
||
| except AttributeError: | ||
| # Non-fatal: skip entries we can't process | ||
| pass | ||
|
|
||
| # Replace response.context_data["p"] with new paginator | ||
| if paginator_replacements: | ||
| for key, orig_paginator in list(response.context_data.items()): | ||
| for orig, new_paginator in list(paginator_replacements.items()): | ||
| if orig_paginator is orig: | ||
| response.context_data[key] = new_paginator | ||
|
|
||
| # Update results_current and results_total | ||
| try: | ||
| filelisting = response.context_data.get("filelisting") | ||
| if filelisting is not None: | ||
| # If filelisting exposes a list of current files, try to | ||
| # update results_current/total conservatively. | ||
| try: | ||
| if hasattr(filelisting, "results_current"): | ||
| # get new page set on line 129 | ||
| page = response.context_data.get("page") | ||
|
|
||
| if page is not None and hasattr(page, "object_list"): | ||
| # update results_current from filtered page | ||
| filelisting.results_current = len(list(page.object_list)) | ||
|
|
||
| if hasattr(filelisting, "results_total"): | ||
| # derive results_total from a paginator replacement | ||
| paginator_new = None | ||
| for new_page in paginator_replacements.values(): | ||
| paginator_new = new_page | ||
| break | ||
|
|
||
| if paginator_new is not None: | ||
| try: | ||
| total = len(getattr(paginator_new, "object_list", [])) | ||
| except AttributeError: | ||
| total = 0 | ||
|
|
||
| filelisting.results_total = total | ||
| except AttributeError: | ||
| pass | ||
| except (AttributeError, TypeError): | ||
| pass | ||
|
|
||
| return response | ||
|
|
||
| def delete_media_without_record_confirm(self, request): | ||
| """Confirm page to delete selected files that do not have associated media records.""" | ||
|
|
||
| query = request.GET | ||
| path = "%s" % os.path.join(self.directory, query.get("dir", "")) | ||
|
|
||
| filelisting = self.filelisting_class( | ||
| path, | ||
| filter_func=lambda fo: not fo.has_media_record() | ||
| and fo.filename not in self.files_folders_to_ignore(), | ||
| sorting_by=query.get("o", DEFAULT_SORTING_BY), | ||
| sorting_order=query.get("ot", DEFAULT_SORTING_ORDER), | ||
| site=self, | ||
| ) | ||
| listings = filelisting.files_listing_filtered() | ||
|
|
||
| return TemplateResponse( | ||
| request, | ||
| "filebrowser/delete_no_media_record_confirm.html", | ||
| dict( | ||
| admin_site.each_context(request), | ||
| **{ | ||
| "filelisting": listings, | ||
| "query": query, | ||
| "title": "Confirm Deletion of Unused Media Files", | ||
| "settings_var": get_settings_var(directory=self.directory), | ||
| "breadcrumbs": get_breadcrumbs(query, query.get("dir", "")), | ||
| "breadcrumbs_title": "Delete Unused Media Files", | ||
| "filebrowser_site": self, | ||
| }, | ||
| ), | ||
| ) | ||
|
|
||
| def delete_media_without_record(self, request): | ||
| """Delete selected files that do not have associated media records.""" | ||
| query = request.GET | ||
| path = "%s" % os.path.join(self.directory, query.get("dir", "")) | ||
|
|
||
| filelisting = self.filelisting_class( | ||
| path, | ||
| filter_func=lambda fo: not fo.has_media_record() | ||
| and fo.filename not in self.files_folders_to_ignore(), | ||
| sorting_by=query.get("o", DEFAULT_SORTING_BY), | ||
| sorting_order=query.get("ot", DEFAULT_SORTING_ORDER), | ||
| site=self, | ||
| ) | ||
| listing = filelisting.files_listing_filtered() | ||
|
|
||
| deleted_files = [] | ||
| for fileobject in listing: | ||
| try: | ||
| signals.filebrowser_pre_delete.send( | ||
| sender=request, | ||
| path=fileobject.path, | ||
| name=fileobject.filename, | ||
| site=self, | ||
| ) | ||
| # we have disabled versions for TEKDB, | ||
| # but keep the call here in case that changes in future | ||
| # or in the off chance versions exist | ||
| fileobject.delete_versions() | ||
| fileobject.delete() | ||
| deleted_files.append(fileobject.filename) | ||
| signals.filebrowser_post_delete.send( | ||
| sender=request, | ||
| path=fileobject.path, | ||
| name=fileobject.filename, | ||
| site=self, | ||
| ) | ||
| except OSError: | ||
| messages.add_message( | ||
| request, | ||
| messages.ERROR, | ||
| f"Error deleting file: {fileobject.filename}", | ||
| ) | ||
| break | ||
|
|
||
| messages.add_message( | ||
| request, messages.SUCCESS, f"Deleted files: {', '.join(deleted_files)}" | ||
| ) | ||
|
|
||
| # Redirect back to browse after deletion | ||
| redirect_url = reverse( | ||
| "filebrowser:fb_browse", current_app=self.name | ||
| ) + query_helper(query, "", "filename,filetype") | ||
| return HttpResponseRedirect(redirect_url) | ||
|
|
||
|
|
||
| storage = DefaultStorage() | ||
| site = TekdbFileBrowserSite(name="filebrowser", storage=storage) | ||
| site.directory = "" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| {% extends "admin/base_site.html" %} | ||
|
|
||
| <!-- LOADING --> | ||
| {% load static i18n fb_tags fb_compat %} | ||
|
|
||
| <!-- BREADCRUMBS --> | ||
| {% block breadcrumbs %}{% include "filebrowser/include/breadcrumbs.html" %}{% endblock %} | ||
|
|
||
| <!-- CONTENT --> | ||
| {% block content %} | ||
| <!-- POP-UP BREADCRUMBS --> | ||
| {% if query.pop %} | ||
| {% include "filebrowser/include/breadcrumbs.html" %} | ||
| {% endif %} | ||
| {% if filelisting|length > 0 %} | ||
| <p>Are you sure you want to delete all media files without media records? All of the following related items will be deleted:</p> | ||
| {% if filelisting %} | ||
| <ul> | ||
| {% for fileobject in filelisting %} | ||
| <li>{{ fileobject.filename }}</li> | ||
| {% endfor %} | ||
| </ul> | ||
| {% endif %} | ||
|
|
||
| <form action="{% url 'filebrowser:fb_delete_all_media_without_record' %}{% query_string %}" method="post"> | ||
| {% csrf_token %} | ||
| <div> | ||
| <input type="submit" value="{% trans "Yes, I'm sure" %}" /> | ||
| </div> | ||
| </form> | ||
| {% endif %} | ||
| {% if filelisting|length == 0 %} | ||
| <p>There are no media files without media records to delete.</p> | ||
| {% endif %} | ||
| {% endblock %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the right place for adding methods to
FileObject?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully @rhodges will know better than me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not seen anything like this before, but the more I dig into your code (and the FileBrowser source) this makes a lot of sense.
Ideally we'd want to just override FileObject to include these, but since we don't explicitly define any FileObjects, we'd need to override whichever parent or ancestor that we do touch, and by the end we may have a very difficult to maintain 'stack of crap' (my speciality) just to override one deeply embedded class. There is likely a lower-common-denominator where we could override the view that renders the template
detail.htmlto insert the logic and context we need there, but in the end, if this works, I think it's just as elegant (though maybe harder to track & debug).Knowing about this approach to modifying 3rd Party libraries is something I wish I'd known about long ago. If there are negative consequences, I don't know them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like this is sticking around! 💩 thanks for the feedback!