Reduce members loaded on new event #2413
Conversation
Only admins or organisers can create new events, so we were loading thousands of students… Not anymore. Fixes codebar#2411
|
|
||
| scope :with_skill, ->(skill_name) { tagged_with(skill_name) } | ||
|
|
||
| scope :admin, -> { with_role(:admin) } |
There was a problem hiding this comment.
@gnclmorais my Rails is so rusty, could you explain why we needed to define the :admin, and :organiser scopes here?
There was a problem hiding this comment.
One reason for having scopes named like this is that it enables the creation of a very legible manager scope: scope :manager, -> { admin.or(organiser).distinct } reads quite clearly.
The manager? method seems to have a fast-path is_admin? first, before querying the more-expensive has_role? call.
Looks nice.
There was a problem hiding this comment.
The main reason was pointed out by Olle: by defining the scopes, we create building blocks that can be reused anywhere and, most importantly for this PR, allows us to express in a clear way that a manager is either an admin or an organiser.
The manager? method came from moving the code that was on app/controllers/application_controller.rb to a more suitable place (i.e. it should be the Member model to own the s_admin? || has_role?(:organiser, :any) code).
There was a problem hiding this comment.
@gnclmorais I think I asked my question wrong. I wanted to know where the :admin and organiser scopes were being called but I went back and re-read the :manager scope definition and answered my own question, I didn't read it closely enough and couldn't see where they were called. Thanks for the update.
3 participants
Fixes #2411 by creating a new
Memberscope and loading only “manager” members, i.e. admins or organisers or a chapter.