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.

Sign up for GitHub

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

Jump to bottom

Add filter options to event filter, fixes #992 #1048

Merged
merged 1 commit into from Dec 4, 2021
Merged

Add filter options to event filter, fixes #992 #1048

merged 1 commit into from Dec 4, 2021

Conversation

JoeyStk
Copy link
Contributor

@JoeyStk JoeyStk commented Dec 1, 2021 •
edited by timobrembeck

Short description

This PR changes to only show upcoming events by default and adds filter options to filter upcoming and past events, fixes #992

Resolved issues

Fixes: #992

  • Please make sure, that also the translations have been done properly on my part

@JoeyStk JoeyStk requested a review from a team December 1, 2021 11:11
timobrembeck
timobrembeck reviewed Dec 1, 2021
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your PR! 🎉 💪

In general everything works (except for the translations), see my comments below for a few suggestions.

integreat_cms/locale/de/LC_MESSAGES/django.po Outdated Show resolved Hide resolved
integreat_cms/cms/constants/time_range_events.py Outdated Show resolved Hide resolved
integreat_cms/cms/constants/time_range_events.py Outdated Show resolved Hide resolved
integreat_cms/cms/constants/time_range_events.py Outdated Show resolved Hide resolved
integreat_cms/cms/constants/time_range_events.py Outdated Show resolved Hide resolved
integreat_cms/cms/forms/events/event_filter_form.py Outdated Show resolved Hide resolved
integreat_cms/cms/forms/events/event_filter_form.py Outdated Show resolved Hide resolved
integreat_cms/cms/templates/events/event_list.html Outdated Show resolved Hide resolved
integreat_cms/cms/views/events/event_list_view.py Outdated Show resolved Hide resolved
timobrembeck
timobrembeck reviewed Dec 1, 2021
View reviewed changes
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

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

Oh, and please also have a look at the failed CircleCI workflow and fix the black code style and pylint warnings.

@JoeyStk JoeyStk force-pushed the feature/events branch 3 times, most recently from 72dac6c to b928f3b Compare December 3, 2021 10:16
@JoeyStk @timobrembeck @david-venhoff
Add filter options to event filter
133f6c0 
Filter options for upcoming and past events

Co-authored-by: Timo Ludwig <ludwig@integreat-app.de>
Co-authored-by: David Venhoff <venhoff@integreat-app.de>
@timobrembeck
Copy link
Member

Thanks a lot for addressing my comments! 👍

I noticed a few more problems, but they were due to the fact that I didn't think the feature through when creating the issue. There was a conflict between this filter and the custom time range filter - thus, I now made them mutually exclusive, meaning that you can either filter for upcoming/past events or by a custom time range, not both at the same time.
Also, we didn't take recurring events into account, so I added a custom EventQuerySet which makes filtering a bit easier.
Apart from that, I took the opportunity to implement a few UI improvements and re-arrange the list columns (id and version are no longer required).
Can somebody else have a look at my changes and review the PR again? Thanks in advance!

@timobrembeck timobrembeck requested a review from a team December 4, 2021 00:30
ulliholtgrave
ulliholtgrave approved these changes Dec 4, 2021
View reviewed changes
Copy link
Member

@ulliholtgrave ulliholtgrave left a comment

Choose a reason for hiding this comment

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

Just checked it out and played with it. Everything seems to work accordingly. Good job 👍

@JoeyStk JoeyStk merged commit 066f188 into develop Dec 4, 2021
@JoeyStk JoeyStk deleted the feature/events branch December 4, 2021 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-venhoff david-venhoff david-venhoff left review comments

@ulliholtgrave ulliholtgrave ulliholtgrave approved these changes

@timobrembeck timobrembeck Awaiting requested review from timobrembeck

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show only upcoming events in event list per default
4 participants
@JoeyStk @timobrembeck @ulliholtgrave @david-venhoff