launchpad-reviewers team mailing list archive

[Guruprasad <mp+413581@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
> index 1a0c380..1902f8a 100644
> --- a/lib/lp/registry/model/person.py
> +++ b/lib/lp/registry/model/person.py
> @@ -1342,6 +1342,48 @@ class Person(
>          self._inTeam_cache[team.id] = in_team
>          return in_team
>  
> +    def inAnyTeam(self, teams):
> +        """See `IPerson`."""
> +        if not teams:
> +            return False
> +
> +        # Translate each team name to an ITeam if we were passed
> +        # a list of teams.
> +        if all(isinstance(team, str) for team in teams):
> +            teams = PersonSet.getByNames(teams)
> +            if not teams:
> +                return False
> +
> +        team_ids = {team.id for team in teams if team.is_team}
> +        if self.id in team_ids:
> +            # A team is always a member of itself.
> +            return True
> +
> +        if self._inTeam_cache is None:
> +            # Initialize cache
> +            self._inTeam_cache = {}
> +        else:
> +            if any(self._inTeam_cache.get(team_id) for team_id in team_ids):
> +                return True
> +
> +        team_participations = IStore(TeamParticipation).find(
> +            TeamParticipation,
> +            And(
> +                TeamParticipation.teamID.is_in(team_ids),
> +                TeamParticipation.personID == self.id
> +            )
> +        )
> +
> +        in_any_team = False
> +
> +        for team_participation in team_participations:
> +            in_any_team = True
> +            if team_participation.team.id not in self._inTeam_cache:
> +                self._inTeam_cache[team_participation.team.id] = True

@cjwatson,
>  1) find elements of `team_ids` that aren't in `_inTeam_cache`, and query for just those `TeamParticipation` rows

Shouldn't this be done only if none of the 'team_ids' in the cache are in the list of team memberships that we want to check? Otherwise, it would make sense to just return 'True', no?

> +
> +        return in_any_team
> +
> +
>      def hasParticipationEntryFor(self, team):
>          """See `IPerson`."""
>          return bool(TeamParticipation.selectOneBy(person=self, team=team))


-- 
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/413581
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:optimize-has-any-bug-role-bulk-team-membership-checks into launchpad:master.