django_reverse_admin icon indicating copy to clipboard operation
django_reverse_admin copied to clipboard

Fix incorrect calling of get_fieldsets

Open augustobmoura opened this issue 3 years ago • 5 comments

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

augustobmoura avatar Nov 04 '21 05:11 augustobmoura

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:

daniel-k avatar Nov 04 '21 16:11 daniel-k

@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

augustobmoura avatar Nov 04 '21 18:11 augustobmoura

@daniyalzade Just a friendly reminder :slightly_smiling_face: Would be so great if this fix could get merged and a new release published.

daniel-k avatar Mar 03 '22 15:03 daniel-k

@daniyalzade

another friendly reminder

could we get this merged, please? its an excellent change

abdul-hamid-achik avatar Jul 06 '22 18:07 abdul-hamid-achik

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

abdul-hamid-achik avatar Jul 12 '22 06:07 abdul-hamid-achik

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.

gutierri avatar May 01 '24 22:05 gutierri

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 !!!

abdul-hamid-achik avatar May 01 '24 23:05 abdul-hamid-achik