Fix session filters is None from request session

[ChandlerBent]

If get session filters is None from request session, the search form will be raise no attribute 'cleaned_data' exception.

[rafalp]

Please format code with black.

[rafalp]

Can you provide test demonstrating the issue and fixing the crash? Or instruction how this crash can be reproduced from admin UI?

[ChandlerBent]

I set any field in User Accounts menu of search list, then I clear the search list. This is OK.
But I set again that I will get this exception.

[rafalp]

Can you add test to misago/admin/tests/test_generic_admin_list_view.py that does what you've described above and confirms no error is raised?

[ChandlerBent]

Yes, I will add the test later.

[ChandlerBent]

Done.

[ChandlerBent]

I try my best to fix this bug.
But make more trouble.
Sorry to you.

My english not good.
The test function name so bad.
:(

[rafalp]

You are doing great, don't worry! 👍

I am just wondering if you know which line in list.py changes session[filters] to None?

[ChandlerBent]

Here

[rafalp]

I saw this line, but I am trying to wrap my mind around it. Where context["active_filters"] becomes Null instead of empty {}? If we find such place it may be better to fix it instead of making sure filters read from session are cast to dict if they are None.

[ChandlerBent]

The get_filtering_method_to_use function is key.

    def get_filtering_method_to_use(self, methods):
        for method in ("GET", "session"):
            if methods.get(method):
                return methods.get(method)

The methods context is {'GET': {}, 'session': {}}. The for part code either get GET or session is {}. So the function return None. It will be influence context["active_filters"] value.

Maybe if methods.get(method): should be correct as if methods.get(method) is not None:.
I don't know that is the correct good idea.

[rafalp]

Hello @ChandlerBent

I've cloned your PR locally, investigated it and found where the bug lies ;)

1. Misago admin calls get_filtering_method_to_use()expecting it to return dict with filters that should be used to filter.
2. When querystring contains no valid filters, both GET and session filtering methods will return no filters, and get_filtering_method_to_use() will return None.
3. Because url has set_filters, this None will then be assigned as session filter.
4. On next request this None will be read from session, failing for and crashing admin.

Actual fix is adding the return {} at the end of get_filtering_method_to_use():

def get_filtering_method_to_use(self, methods):
    for method in ("GET", "session"):
        if methods.get(method):
            return methods.get(method)
    return {}

[rafalp]

Thank you for making Misago better!

Your commits will land as #1190, I'll also backport the fix to Misago 0.19 today or tomorrow!

[rafalp]

Heads up @ChandlerBent I've noticed this bug was introduced on master branch when I was fixing pylint compliance errors in Misago: 67838a3#diff-97d5f581d1c4a4c697b0801d4065bab6L217

If you are running Misago master on production, please don't - we don't support that.