-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
[admin] Fixed FileNotFoundError #140 #180
Closed
AbhigyaShridhar
wants to merge
6
commits into
openwisp:master
from
AbhigyaShridhar:issues/140-FileNotFound-exception
Closed
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0e78ce8
[admin] Fixed FileNotFoundError #140
AbhigyaShridhar ba7ac15
Merge branch 'openwisp:master' into issues/140-FileNotFound-exception
AbhigyaShridhar c19f1cd
Merge branch 'openwisp:master' into issues/140-FileNotFound-exception
AbhigyaShridhar a3e1263
[admin] fixed FileNotFoundError while deleting a FirmwareImage #140
AbhigyaShridhar 739f0a2
[admin] fixed FileNotFoundError while deleting a FirmwareImage #140
AbhigyaShridhar ba9e1cf
[admin] Fixed FileNotFoundError while deleting a FirmwareImage #140
AbhigyaShridhar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import logging | ||
import os | ||
from datetime import timedelta | ||
|
||
import reversion | ||
|
@@ -7,7 +8,7 @@ | |
from django.conf import settings | ||
from django.contrib import admin, messages | ||
from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME | ||
from django.shortcuts import redirect | ||
from django.shortcuts import HttpResponseRedirect, redirect | ||
from django.template.response import TemplateResponse | ||
from django.urls import resolve, reverse | ||
from django.utils.safestring import mark_safe | ||
|
@@ -84,6 +85,37 @@ def has_change_permission(self, request, obj=None): | |
return False | ||
return True | ||
|
||
def get_parent_object(self, request): | ||
""" | ||
method to get the instance from model' s parent class | ||
in context with the ForeignKey relation | ||
""" | ||
resolved = resolve(request.path_info) | ||
if resolved.kwargs: | ||
return self.parent_model.objects.get(pk=resolved.kwargs["object_id"]) | ||
return None | ||
|
||
def get_queryset(self, request): | ||
""" | ||
overriding queryset to remove any FirmwareImage instances, | ||
where the image file has been manually deleted by the user | ||
from the file system | ||
""" | ||
qs = super(FirmwareImageInline, self).get_queryset(request) | ||
build = self.get_parent_object(request) | ||
qs = qs.filter(build=build) | ||
for imageObject in qs: | ||
if imageObject.file is not None: | ||
path = imageObject.file.path | ||
if not os.path.isfile(path): | ||
try: | ||
type = imageObject.type | ||
imageObject.delete() | ||
logger.warning(f"Image object {type} was removed") | ||
except Exception: | ||
pass | ||
return qs | ||
|
||
|
||
class CategoryFilter(MultitenantOrgFilter): | ||
multitenant_lookup = 'organization_id__in' | ||
|
@@ -192,7 +224,21 @@ def change_view(self, request, object_id, form_url='', extra_context=None): | |
extra_context = extra_context or {} | ||
upgrade_url = f'{app_label}_build_changelist' | ||
extra_context.update({'upgrade_url': upgrade_url}) | ||
return super().change_view(request, object_id, form_url, extra_context) | ||
# preventing change_view to throw an error/exception | ||
try: | ||
return super().change_view(request, object_id, form_url, extra_context) | ||
except Exception as e: | ||
if type(e).__name__ == "FileNotFoundError": | ||
self.message_user( | ||
request, "Please reload the page", level=logging.ERROR | ||
) | ||
else: | ||
self.message_user( | ||
request, | ||
"Image objects have been removed or form data didn't validate", | ||
level=logging.ERROR, | ||
) | ||
return HttpResponseRedirect(request.path) | ||
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.
We want to handle this case without making it obvious to the user. 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. OK, I understand |
||
|
||
|
||
class UpgradeOperationForm(forms.ModelForm): | ||
|
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.
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.
Looping over individual FirmwareImage object will make system very slow. Instead of this, the issue recommended catching the exception when it occurs(on delete operation) and delete the related FirmwareImage.