daniyalzade / django_reverse_admin

Django Reverse Admin
BSD 3-Clause "New" or "Revised" License
199 stars 54 forks source link

Fix incorrect calling of get_fieldsets #286

Closed augustobmoura closed 5 months ago

augustobmoura commented 2 years ago

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

daniel-k commented 2 years ago

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:

augustobmoura commented 2 years ago

@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

daniel-k commented 2 years ago

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

abdul-hamid-achik commented 2 years ago

@daniyalzade

another friendly reminder

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

abdul-hamid-achik commented 2 years ago

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

gutierri commented 5 months ago

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.

abdul-hamid-achik commented 5 months ago

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