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

Bulk Actions - Select All with customised ListModelView issues #422

Open
SamuelLayNZ opened this issue Mar 1, 2024 · 3 comments
Open

Bulk Actions - Select All with customised ListModelView issues #422

SamuelLayNZ opened this issue Mar 1, 2024 · 3 comments

Comments

@SamuelLayNZ
Copy link

This situation is a little complex, I will do my best to explain with an example I've simplified.

I have a viewset called PhoneNumberViewset
This viewset has list_view_class = ListPhoneNumberView

I have a cutomised ListModelView rather than just using the options within the Viewset definition so that I can control the list view more finely. Some of these things I would do from within the Viewset, but this is not currently possible as far as I can tell.

class ListPhoneNumberView(ListModelView):
    def get_columns(self):
        if self.request.user.is_admin:
            return (
                "phone_number",
                "calling_area",
                "country",
                "status",
            )
        else:
            return (
             "phone_number", 
             "calling_area", 
             )

    def get_queryset(self):
        queryset = super().get_queryset()
        self.account = get_object_or_404(models.Account, pk=self.kwargs["account_id"])
        queryset = queryset.filter(account_id=self.account.id)  # Only show services for this account

    def get_filterset_class(self):
        if self.request.user.is_admin:
            return filters.AdminPhoneNumberListFilterSet
        else:
            return filters.UserPhoneNumberListFilterSet

In the above simplified example, I'm providing a different set of columns and a different FilterSet depending on whether the user is an admin or not.
The queryset is filtered because this list view is used when viewing all phone numbers that belong to a particular billing account.
I also customise get_bulk_actions, get_page_actions and a few others based on the account and user that is viewing.
So far everything is working just fine.

Where it does not work is when using bulk actions with Select All enabled. (Without Select All, it works fine)
For example DeleteBulkActionView I get an attribute error

'NoneType' object has no attribute 'form'
~~~
# Inside delete_action.html
{% if view.objects_count  %}
~~~
# Inside actions.py
    def objects_count(self):
        if self.request.POST.get('select_all') and not self.filterset.form.has_changed():
            return None
        return self.object_list.count()

I figured out to fix this, I need to specify the list_filterset_class inside the viewset, however this removes the ability to control which filterset gets used like I have in the ListPhoneNumberView above.
Plus, it now intends to delete all PhoneNumber in the system, not just the ones specified within get_queryset inside ListPhoneNumberView which would be very bad in my case.
The filters are respected, but not the customised get_queryset.

Until I can figure out how to fix this, I may have to figure out how to hide the Select All option to avoid errors and bulk actions affecting things it shouldn't.
I know it was a bit complex to explain, but I don't think my use case is all that uncommon, perhaps there needs to be a better way of doing some things.

Let me know if you need further information.
Thanks!

@kmmbvnr
Copy link
Contributor

kmmbvnr commented Mar 1, 2024

Thanks for the detailed explanation

Have you tried to customize DeleteBulkActionView with the same initialization for filterset?

@kmmbvnr
Copy link
Contributor

kmmbvnr commented Mar 1, 2024

As from api changes, it seems viewflow FilterableViewMixin need be able to take queryset and filterset from the viewset as other views do. That would reduce code duplication

@SamuelLayNZ
Copy link
Author

Yeah at the moment the Bulk Actions are reliant on the ModelViewset for their kwargs, it does not pull details like queryset or filterset from the ListModelView. This is fine when you are passing it a specific set of objects because it doesn't need to understand the logic behind it. When using Select All however, it needs the filterset options as well as the queryset to know what "all" means in order to apply the bulk update to the right set of objects.

ModelViewset does not have something like get_list_filterset_class or get_list_queryset so that you can tailor the filterset and queryset based on the user or other conditions. If it did have those things, then I'd move all that logic from the ListModelView to the ModelViewset. BaseBulkActionView could then be updated to use the new get_list_filterset_class and get_list_queryset from the viewset.

The other way I can think to do it would be to allow Bulk Actions to get those things from the ListModelView instead, perhaps without even needing a Viewset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants