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

Update the rules for which events get shown where to match expectations #2425

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

r-ferrier
Copy link
Contributor

@r-ferrier r-ferrier commented Apr 23, 2024

Fixes #2407

Description

The list of events we return for any given site is now updated to take in not only events whose location matches the site, but also any events belonging to partners on the site, even if they are happening outside the site's boundaries, and even if they have no address. I've updated the event resolver to support the import of address-less events.

I've also changed how we match events. We now take tags into consideration when we make the first query (some bits of the code were not doing this before leading to some inconsistencies on date filtering) and for tagged events, we specifically check for a partner match on name and address, against a list of approved partners.

I've written a mammoth test to support this, and updated lots of others to reflect the changes in logic.

@aaaaargZombies aaaaargZombies self-assigned this Apr 23, 2024
Copy link
Contributor

@aaaaargZombies aaaaargZombies left a comment

Choose a reason for hiding this comment

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

This LGTM thanks for writing the mega long tests, it helped follow what should be happening.

One thing I'm noticing is that it's hard to know what partner put on this event ("BSL heritage: poetry...") that has now been added

image

image

You can sort of figure it out from contact info. But overall I'm in favor of merging this in in given time left and this looks like really nice event that would have been obscured otherwise. Maybe this relates to showing owned event vs location of event. Might be worth ticketing if you think it's not covered by existing tickets.

@r-ferrier
Copy link
Contributor Author

r-ferrier commented Apr 23, 2024

This LGTM thanks for writing the mega long tests, it helped follow what should be happening.

One thing I'm noticing is that it's hard to know what partner put on this event ("BSL heritage: poetry...") that has now been added

You can sort of figure it out from contact info. But overall I'm in favor of merging this in in given time left and this looks like really nice event that would have been obscured otherwise. Maybe this relates to showing owned event vs location of event. Might be worth ticketing if you think it's not covered by existing tickets.

yeah that is ticketed, just didn't have time to do it. Am tempted to try to squeeze it in as one last thing - it should be a simple addition if it's just on the event itself, and not the index

@r-ferrier r-ferrier merged commit 8c80010 into main Apr 23, 2024
2 checks passed
@r-ferrier r-ferrier deleted the feat/2305-make-event-organiser-clear branch April 23, 2024 12:37
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.

[Bug]: Events without addresses do not display on placeCal instances
2 participants