generate URL Filter

[samialfattani]

Fixes #2703

this PR adds a function to generate a URL with filter arguements including all types of filters (Like, NotLike, Greater....etc) and including both indexed (flt2_3=xx) and named (flt3_email_like=@example.com)

Usage:

    filtered_url = view.url_for(
        search="exam",
        filters=[
            filters.FilterLike(Model1.test1, "Email", url_value="@example.com"),
            filters.BooleanEqualFilter(Model1.bool_field, "active", url_value=False),
            filters.FloatGreaterFilter(Model1.test5, "Salary", url_value=15.3),
        ],
    )

Output:
/admin/user/?search=exam&flt0_0=@example.com&flt1_21=0&flt2_16=15.3

OR with named parameters:
/admin/user/?search=exam&flt0_test1_contains=@example.com&flt1_bool_field_equals=0&flt4_test2_greater_than=15.3

[ElLorans]

You aimed for code simplicity and separation of concerns (and rightly so), but does this risk to have complexity O(n*n) if filters are not in the same order? I think this search can be done with a set difference

[samialfattani]

i don't get it, what do you mean by set difference! anyway i've tried to wrokarround to reduce the complexity but it seems like it won't be a simple code. I'm wondering if you can accept the current algorithm and we raise a new issue that requests to refactor it ?

Analysis: the actual complexity would be O(m*n) where m: is number columns to be filtered, and n: number of filter classes for the type of each column. In regular cases m would be 1 < m < 50 and n = 9 (at most), so we are talking about 500 iteration in regular cases which is not that harmful.

[ElLorans]

why is that a problem?

[samialfattani]

ok i will explain. currently, a parameter value is passed to each of stringify, validate , and clean functions, where the new added url_value field can be used instead in all of these functions and we no longer need to pass value parameter. i.e value should be used as a field and can be shared in all functions.
However, i didn't do this in this PR to avoid breaking changes.

[ElLorans]

Thanks for this titanic effort. I will pause reviewing. Is it feasible to remove the breaking changes?

[samialfattani]

Thanks for this titanic effort. I will pause reviewing. Is it feasible to remove the breaking changes?

Most welcome, actually i've reviewed the code, however I found that column is the first argument in all other Filter sub classes such as BasePeeweeFilter and BasePyMongoFilter. that's why i decided to locate column to be the first arguement as well in BaseSQLAFilter to be consistance with other filters. Yes you are right, it is a breaking change but if I relocate it we will loose the consistancy. So, what do you think ?
`

[ElLorans]

Thanks for this titanic effort. I will pause reviewing. Is it feasible to remove the breaking changes?

Most welcome, actually i've reviewed the code, however I found that column is the first argument in all other Filter sub classes such as BasePeeweeFilter and BasePyMongoFilter. that's why i decided to locate column to be the first arguement as well in BaseSQLAFilter to be consistance with other filters. Yes you are right, it is a breaking change but if I relocate it we will loose the consistancy. So, what do you think ? `

We can plan such a breaking change down the line for 3.0 but I would argue that BasePeeweeFilter and BasePyMongoFilter should be the ones to align to SQLA, not the opposite.

[samialfattani]

Thanks for this titanic effort. I will pause reviewing. Is it feasible to remove the breaking changes?

Most welcome, actually i've reviewed the code, however I found that column is the first argument in all other Filter sub classes such as BasePeeweeFilter and BasePyMongoFilter. that's why i decided to locate column to be the first arguement as well in BaseSQLAFilter to be consistance with other filters. Yes you are right, it is a breaking change but if I relocate it we will loose the consistancy. So, what do you think ? `

We can plan such a breaking change down the line for 3.0 but I would argue that BasePeeweeFilter and BasePyMongoFilter should be the ones to align to SQLA, not the opposite.

ok in this case we should move column in SQLA to last argument for now so that we can merge without any break change. then later on we should plan to relocate column in BasePeeweeFilter and BasePyMongoFilter to match SQLA one.

[samialfattani]

i think this PR is ready to review and merge, it's been so long since it is created.

[samuelhwilliams]

Hey @samialfattani - agree this has been open a while and would be really useful functionality to get merged.

Can you clean up the commits please (I think effectively squash everything into one?).

I still want to review the interface generally - a single view.url_for call feels really good, but something's nagging in the back of my head about the filters interface that I still want to unpick a little bit.

+2500/-1000 PR is quite chunky to review so it may take a bit of time to get this lined up.

Really appreciate your efforts/pushing on this though.

[samialfattani]

Thanks for beatiful words. I've tried to squash this branch but i think i failed to find the right way to do it. I had to create a new branch includes the squashed of all commits if that is helping.

here is the new branch #2882

if this works fine with you, you can close this PR.

[ElLorans]

Did you have any merge conflicts?

[samialfattani]

Did you have any merge conflicts?

No

[samuelhwilliams]

Superceded by #2882