launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> 
> === modified file 'lib/lp/code/interfaces/gitrepository.py'
> --- lib/lp/code/interfaces/gitrepository.py	2016-10-05 09:54:42 +0000
> +++ lib/lp/code/interfaces/gitrepository.py	2016-10-13 12:29:37 +0000
> @@ -946,14 +951,24 @@
>          return identities
>  
>  
> -def user_has_special_git_repository_access(user):
> +def user_has_special_git_repository_access(user, repository=None):
>      """Admins have special access.
>  
>      :param user: An `IPerson` or None.
> +    :param repository: An `IGitRepository` or None when checking collection
> +        access.
>      """
>      if user is None:
>          return False
>      roles = IPersonRoles(user)
>      if roles.in_admin:
>          return True
> -    return False
> +    if repository is None:
> +        return False
> +    code_import = repository.code_import
> +    if code_import is None:
> +        return False
> +    return (
> +        roles.in_vcs_imports
> +        or (IPersonRoles(repository.owner).in_vcs_imports
> +            and user.inTerm(code_import.registrant)))

Granted that the Git side at least isn't tested, and the typo you noticed.  As for why it's there, there used to be a comment about this but Curtis removed it during the last refactoring:

        # It used to be the case that all import branches were owned by the
        # special, restricted team ~vcs-imports. For these legacy code import
        # branches, we still want the code import registrant to be able to
        # edit them. Similarly, we still want vcs-imports members to be able
        # to edit those branches.

This case is tested (lp.code.tests.test_branch.TestWriteToBranch.test_code_import_registrant_can_edit).

There are still 34 distinct registrants of vcs-imports-owned branches on dogfood, quite a few of whom aren't privileged, so I think I should still leave this code there for the Bazaar case.  However, I'll put the comment back so that it's less mystifying, and I'll drop this from the Git side since we have no legacy concerns there.



-- 
https://code.launchpad.net/~cjwatson/launchpad/git-code-import-security-deletion/+merge/308319
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.