django_reverse_admin
django_reverse_admin copied to clipboard
Fix incorrect calling of get_fieldsets
get_fieldsets
should be called with 2 arguments. All other instances of self.get_fieldsets
are correctly passing 2 arguments. Not passing the obj
argument can cause weird behavior when the fieldsets change between adding models and editing models, one such case is with the django UserAdmin implementation that returns different fieldsets whether or not obj
is present
Fixes #226
I just ran into a similar issue with our own code that behaves differently if obj
is set in get_fields()
and I can confirm that this also fixes my issue :+1:
@daniyalzade can you check this PR? My team is using our own fork with the fix but we would like to see it merged upstream
@daniyalzade Just a friendly reminder :slightly_smiling_face: Would be so great if this fix could get merged and a new release published.
@daniyalzade
another friendly reminder
could we get this merged, please? its an excellent change
i would also point out that:
this, needs to be changed to:
def get_inline_instances(self, request, obj=None):
own = list(filter(
lambda inline: inline.has_view_or_change_permission(request, obj) or
inline.has_add_permission(request, obj) or
inline.has_delete_permission(request, obj), self.tmp_inline_instances))
return super(ReverseModelAdmin, self).get_inline_instances(request,
obj) + own
the own inline instances need to be at the bottom/end otherwise, it's never gonna match the right fields when building formsets, if no one responds in a while ill make another PR with the change based on this one, there are several other places where an object is not passed to self.get_inline_*
functions
I added basic test coverage and am merging this patch.
@abdul-hamid-achik Can we consider this a separate bug? I believe you mentioned this in another ISSUE. I'm going to close this one, but considering that it needs improvement. And your PR is welcome, if you can cover it with tests so that we have this "documented" scenario, even better.
I added basic test coverage and am merging this patch.
@abdul-hamid-achik Can we consider this a separate bug? I believe you mentioned this in another ISSUE. I'm going to close this one, but considering that it needs improvement. And your PR is welcome, if you can cover it with tests so that we have this "documented" scenario, even better.
amazing thank you! i'm no longer working on that project where i was using that but ill take a look in the weekend
thanks a lot @gutierri !!!