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

Optimize /api/v2/events by changing is_organiser logic #3883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ColonelPhantom
Copy link
Contributor

@ColonelPhantom ColonelPhantom commented Jan 11, 2025

Closes #3586.

Summary

Changes the is_organiser logic so that we need fewer (duplicate) queries by doing more work Python-side.

This does require using @cached_property for member.get_member_groups(), which can be a little error-prone but should be fine in practice.

How to test

  1. Go to /api/v2/events
  2. See the same results (especially for permissions)
  3. Observe fewer queries when signed in as activemember-but-not-superuser

@ColonelPhantom ColonelPhantom changed the title Replace complicated query with two queries and process Python-side Optimize /api/v2/events by changing is_organiser logic Jan 11, 2025
@ColonelPhantom ColonelPhantom marked this pull request as ready for review January 15, 2025 18:53
@ColonelPhantom ColonelPhantom force-pushed the optimize-events-api-activemember branch from e7aa227 to 0dca0d9 Compare January 15, 2025 19:25
@ColonelPhantom ColonelPhantom requested a review from DeD1rk January 15, 2025 19:30
@ColonelPhantom
Copy link
Contributor Author

By the way, it might be beneficial to use the cached member_groups in other places too? Unsure about that, though.

Cache member_groups

Make logic in is_organiser more explicit

Do explicit cache invalidation in test

invalidate cache in more places

fix
@ColonelPhantom ColonelPhantom force-pushed the optimize-events-api-activemember branch from 0dca0d9 to 3caf65e Compare January 23, 2025 00:59
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.

Optimize api/v2/events/ N+1
1 participant