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

[7261] apps/measures/index page: add pagination and filters #688

Merged
merged 2 commits into from
May 4, 2023

Conversation

fuzzylogic2000
Copy link
Contributor

No description provided.

@fuzzylogic2000 fuzzylogic2000 changed the title [7261] apps/measures/index page: add pagination and filters WIP [7261] apps/measures/index page: add pagination and filters May 2, 2023
@fuzzylogic2000
Copy link
Contributor Author

Help! I went a bit crazy with the filters (which are crazy themselves) in the second commit and now I cannot see the wood for the trees (I looked it up, also works in English 🤣 ) @Rineee @goapunk Can you have a look and point at the wood if there is one? Ehm, I mean point out what can be improved. I will spent the meantime with building the crazy urls in the frontend.

@fuzzylogic2000 fuzzylogic2000 force-pushed the kl-2022-04-pagination-and-filter branch from c541380 to 0a8a605 Compare May 3, 2023 10:11
@fuzzylogic2000 fuzzylogic2000 changed the title WIP [7261] apps/measures/index page: add pagination and filters [7261] apps/measures/index page: add pagination and filters May 3, 2023
Copy link
Contributor

@Rineee Rineee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, it works really well!

Only 2 little questions/suggestions.

And I was trying to write the filtering stuff for the lists a bit shorter, but did not really succeed :) A maybe a little shorter way would be sth like here, but yeah, not a super great improvement

apps/measures/models.py Outdated Show resolved Hide resolved
apps/measures/models.py Outdated Show resolved Hide resolved
@Rineee
Copy link
Contributor

Rineee commented May 3, 2023

Cool, it works really well!

Only 2 little questions/suggestions.

And I was trying to write the filtering stuff for the lists a bit shorter, but did not really succeed :) A maybe a little shorter way would be sth like here, but yeah, not a super great improvement

Couldnt help but trying out a bit and came up with this MASSIVE improvement 🤣

def _q_filter(self, filter_list, field_name):
        kwargs = [('{}__contains'.format(field_name),element) for element in filter_list]
        return Q(*kwargs, _connector=Q.OR)

But yeah, for readibility probably yours is better, so I am totally fine with both!

@fuzzylogic2000 fuzzylogic2000 force-pushed the kl-2022-04-pagination-and-filter branch from 0a8a605 to ba4e2f0 Compare May 4, 2023 07:36
@fuzzylogic2000
Copy link
Contributor Author

I added a docstring now, and already find my code quite hard to read, so @Rineee yours is not much worse IMHO. But Readability counts. Ah, don't know. @goapunk please decide!

@goapunk
Copy link
Contributor

goapunk commented May 4, 2023

I added a docstring now, and already find my code quite hard to read, so @Rineee yours is not much worse IMHO. But Readability counts. Ah, don't know. @goapunk please decide!

I'd say lets go for the more readable version :)

@fuzzylogic2000
Copy link
Contributor Author

I added a docstring now, and already find my code quite hard to read, so @Rineee yours is not much worse IMHO. But Readability counts. Ah, don't know. @goapunk please decide!

I'd say lets go for the more readable version :)

Alright, thanks! I will merge then.

@Rineee And I did change your other suggestions. :)

@fuzzylogic2000 fuzzylogic2000 merged commit 44bade4 into main May 4, 2023
@fuzzylogic2000 fuzzylogic2000 deleted the kl-2022-04-pagination-and-filter branch May 4, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants