Skip to content

perf: use single database query for members_min|max_privacy_level

Bengfort requested to merge privacy-level-field into main

study.members_max_privacy_level() is used in recruitment: a study is not a fit if no one in the study has the required privacy level. Unfortunately, this function is slow because it makes heavy use of django's pluggable permissions system. First, to get the relevant users, and then to check their privacy levels.

To improve this, I had to make some bigger changes:

  • I replaced the privacy_level permissions by a model field. These two options have been on the table since we added privacy levels in the very beginning of castellum. Back then I had argued for permissions to make access control somewhat consistent. Changing this now means that superusers and data protection coordinators no longer implicitly get the maximum privacy level, which is a breaking change.

  • I changed members_max_privacy_level() (and members_min_privacy_level() for consistency) to consider all members, not just recruiters and conductors. This is a downgrade in terms of functionality, but allows us to bypass the permission system completely.

These are quite big changes for a performance optimization. This mostly matters for ParticipationRecruitView which checks recruitment filters for a list of studies. With the current architecture, this will always be an 1+N view. This change makes the N smaller (from O(studies*members) to O(studies)), but that doesn't change much about the general performance characteristics.

So I am a bit on the fence about this. It does improve performance and I also like it on a conceptual level. But I am not convinced this is enough to justify breaking changes. Maybe there is a variation of this that has a better trade-off.

Edited by Bengfort

Merge request reports