TrangPham / django-admin-confirm

AdminConfirmMixin is a mixin for ModelAdmin that adds confirmations to changes, additions and actions.
Other
129 stars 16 forks source link

fix(ISSUE-8): try using admin form #16

Closed TrangPham closed 3 years ago

TrangPham commented 3 years ago

Related to #8

Context Previously tried solutions:

  1. initial state used getattr which returned a weird obj for m2m fields (v0.2.2)
  2. next solution used QueryDict.get but that only returns that last value for a multivalue field
  3. then tried to use QueryDict.lists() but I was using list() on the values which broke ImageField and FileField which worked with v0.2.2

Then I tried to unwrap lists and pass to template - this worked for m2m and simple fields. I did not test on ImageField and FileField.

THEN.... I read this: https://stackoverflow.com/questions/26514892/how-to-pass-parameter-from-form-post-to-view-in-django-python

🤦 I didn't know you could do that.. and turns out that this line was already giving us the form HTML....

https://github.com/TrangPham/django-admin-confirm/blob/main/admin_confirm/admin.py#L188

Also see here, this is the important part of the code which confirmation view calls:

https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1575-L1592

TL;DR Anyhow, so this takes the form which the change view does show, but this would have the items that you changed in the request.POST, and then hides it with CSS. This should mean that this should be more compatible with Django and stay more compatible overtime.

TODO [ ] Tests for FileField and ImageField still need to be added. [ ] Tests for modified forms IE custom forms, setting ModelAdmin.form or overriding ModelAdmin.get_form()

TrangPham commented 3 years ago

So... The journey continues.... 🤕

Embedding the form does not work for any uploads. Since we can't set the file data on html forms at all. Since allow this would be a security issue.

So instead of using a hidden form at all, caching the object will be used. https://docs.djangoproject.com/en/2.2/topics/cache/#basic-usage

After a lot of trial and error, it worked via caching. Tests still need to be added