Skip to content

Commit

Permalink
Only fetch projects with min access level
Browse files Browse the repository at this point in the history
If we're on an appropriate GitLab version, that is. GitLab doesn't
return the right permissions for projects belonging to the user if the
project is in a subgroup, and Marge is set as a member of the top-level
group[1]. This is rather annoying as Marge will then complain that she
doesn't have permissions for a project when she does.

A new field available in GitLab version >= 11.2 allows us to only fetch
projects for which we have a certain access level. This returns the
right projects even with nested groups, so we can prefer this method if
it is available.
  • Loading branch information
JaimeLennox authored and aschmolck committed Mar 11, 2019
1 parent 826ebe3 commit 2b7531e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
16 changes: 14 additions & 2 deletions marge/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ def filter_by_path_with_namespace(projects):

@classmethod
def fetch_all_mine(cls, api):
projects_kwargs = {'membership': True, 'with_merge_requests_enabled': True}

# GitLab has an issue where projects may not show appropriate permissions in nested groups. Using
# `min_access_level` is known to provide the correct projects, so we'll prefer this method
# if it's available. See #156 for more details.
if api.version().release >= (11, 2):
projects_kwargs["min_access_level"] = int(AccessLevel.developer)

projects_info = api.collect_all_pages(GET(
'/projects',
{'membership': True, 'with_merge_requests_enabled': True},
projects_kwargs,
))

def project_seems_ok(project_info):
Expand All @@ -43,7 +51,11 @@ def project_seems_ok(project_info):

return permissions_ok

return [cls(api, project_info) for project_info in projects_info if project_seems_ok(project_info)]
return [
cls(api, project_info)
for project_info in projects_info
if "min_access_level" in projects_kwargs or project_seems_ok(project_info)
]

@property
def path_with_namespace(self):
Expand Down
8 changes: 3 additions & 5 deletions tests/test_project.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from unittest.mock import Mock
import pytest

from marge.gitlab import Api, GET
from marge.gitlab import Api, GET, Version
from marge.project import AccessLevel, Project


Expand Down Expand Up @@ -67,12 +67,10 @@ def test_fetch_all_mine(self):

api = self.api
api.collect_all_pages = Mock(return_value=[prj1, prj2])
api.version = Mock(return_value=Version.parse("11.2.0-ee"))

result = Project.fetch_all_mine(api)
api.collect_all_pages.assert_called_once_with(GET(
'/projects',
{'membership': True, 'with_merge_requests_enabled': True},
))
api.collect_all_pages.assert_called_once()
assert [prj.info for prj in result] == [prj1, prj2]

def test_properties(self):
Expand Down

0 comments on commit 2b7531e

Please sign in to comment.