Skip to content

Optimize subgroups permission checking in EpicsFinder

Currently we check all subgroups if user is not member of "top-level" group. We could optimize this further and skip checking permission for some subgroups if we update order in which we check these subgroups.

The following discussion from !59360 (merged) should be addressed:

  • @acroitor started a discussion: (+4 comments)

    I wonder if we can adopt this strategy and check permission on group nodes and exit early if true, thus skipping checking permissions on a bunch of sub-groups. So let's say we have the following simple graph:

    graph TD;
      A-->B;
      A-->C;
      B-->D;
      B-->E;
      C-->F;
      C-->G;

    As in above check if user has permission to read epics in A, no need to check the tree we just return true, similarly if false we can check B and C, so if B is true no need to check D and E and if C is true no need to check permissions on F and G.

    So basically we would start at the top of the tree and go through each level, by retaining the branches where Ability.allowed?(user, :read_epics, group) is true. Worst case scenario would be the same as we do now, check it for all groups in the case where a user has no permissions to read the epics in any of the groups.

    not tested but perhaps smth like:

    def groups_user_can_read_epics(user, group)
      group.children.each do |group|
        next group if Ability.allowed?(user, :read_epics, group)
        recursive_can_read_epics(group)
      end
    end
    
    recursive_can_read_epics(current_user, groups.first.root_ancestor)