  def user_not_authorized(exception)    message = t('flash.access_denied')    if exception.policy.class.to_s.underscore == 'group_policy' && current_user.students.size != 0        current_user.students.each do |student|          student.memberships.each do |membership|            if membership.group_id == exception.record.id              message = t('flash.pending_acceptance')              break # and I would like to break from the outer loop too            end          end        end    end    flash[:alert] = message    redirect_to request.referrer || root_path  endThis looks ugly to me. Is there a way to somehow compress those two each loops, where one should break both of them conditionally?
Checking user authorization
ruby;ruby on rails;authorization
There's no reason (as far as I can tell) to do string comparison to check a class. #is_a? would presumably work fine too. And it'd be stricter, which is desirable.There's no reason to check students.size. If there are no students, the loop just won't do anything, so why bother checking first? Conversely, if there are any students you're actually doing 2 database queries: One to get the count/size of students, and another to fetch the actual records.Presuming non-ancient Rails, you can simply say current_user.students.memberships to get all the memberships in 1 query (admittedly with a couple of JOINs), rather than loop through students individually, and then getting their memberships.Continuing from #3, you can (presumably) collapse the entire things into 1 database query.I'd probably start with something likedef user_not_authorized(exception)  redirect_path = request.referrer || root_path  if exception.policy.is_a?(GroupPolicy)    if current_user.students.memberships.where(group_id = ?, exception.record.id).any?      redirect_to redirect_path, alert: t('flash.pending_acceptance') and return    end  end  redirect_to redirect_path, alert: t('flash.access_denied')endNote that it doesn't actually load any records at all; it just checks if they're there or not.In practice, I'd consider breaking it into separate methods where possible. It's not that great to have a naked query referencing a specific column name in the middle of your method.Edit: Updated the code to return if the first redirect occurs, since you'd otherwise do two redirects in one action, which is not allowed. Thanks to tokland for catching that. Now the and return fix is about as quick and dirty as is gets, but check the comments for better ideas.