Skip to content
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

Fix the error message when deleting a media file in use #2585

Closed
seluianova opened this issue Dec 6, 2023 · 2 comments · Fixed by #2601
Closed

Fix the error message when deleting a media file in use #2585

seluianova opened this issue Dec 6, 2023 · 2 comments · Fixed by #2601
Assignees
Labels
🐛 bug Something isn't working
Milestone

Comments

@seluianova
Copy link
Contributor

seluianova commented Dec 6, 2023

Describe the Bug

When deleting a media file that is in use, a network error is displayed
image

Steps to Reproduce

  1. Go to 'Media Library'
  2. Try to delete a media file that is being used as the organisation icon
  3. See error

Expected Behavior

I think we could display a more understandable (user-friendly) message.
Like: "This media file can't be deleted because it is in use"

Actual Behavior

The following error is displayed:
A network error has occurred. Please try again later.

Additional Information

Traceback
Dec 06 11:32:21 ERROR django.request - 500 Internal Server Error: /ajax/media/file/delete/
Traceback (most recent call last):
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
  response = get_response(request)
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
  response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/views/decorators/http.py", line 40, in inner
  return func(request, *args, **kwargs)
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
  return view_func(request, *args, **kwargs)
File "/home/tory/Integreat/integreat-cms/integreat_cms/api/decorators.py", line 115, in wrap
  return function(request, *args, **kwargs)
File "/home/tory/Integreat/integreat-cms/integreat_cms/cms/views/media/media_actions.py", line 361, in delete_file_ajax
  media_file.delete()
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/base.py", line 966, in delete
  collector.collect([self], keep_parents=keep_parents)
File "/home/tory/Integreat/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/deletion.py", line 302, in collect
  raise ProtectedError(
django.db.models.deletion.ProtectedError: ("Cannot delete some instances of model 'MediaFile' because they are referenced through protected foreign keys: 'Organization.icon'.", {<Organization (id: 1, slug: test-organization, region: augsburg)>})
@seluianova seluianova added 🐛 bug Something isn't working ❓ question Further information is requested labels Dec 6, 2023
@seluianova
Copy link
Contributor Author

seluianova commented Dec 6, 2023

I am not sure about the expected result actually.
Maybe there should be no error in this case? And we should be able to delete used media files?

If so, what do we do with organisations, where the logo is mandatory, and the deleted file was used as a logo?

Or maybe we should disable the "delete" button in this case, and show a tooltip message?

@seluianova seluianova added this to the 24Q1 milestone Dec 6, 2023
@MizukiTemma
Copy link
Member

If so, what do we do with organisations, where the logo is mandatory, and the deleted file was used as a logo?

That's why we want to prevent media files from being deleted when they are used. But I agree with you that just showing an error is not user friendly 😅

Or maybe we should disable the "delete" button in this case, and show a tooltip message?

I think disabling is the best. The reason can be explained like for archiving a mirrored page:

example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants