Merge pull request #3020 from athenianco/DEV-5150-openapi-align-list-team-members

[DEV-5150] openapi align list team members

Author: vmarkovtsev

server/athenian/api/align/queries/members.py

@@ -32,7 +32,9 @@ async def resolve_members(
 
     team, meta_ids = await gather(
         # teamId 0 means root team
-        get_root_team(accountId, sdb) if teamId == 0 else get_team_from_db(accountId, teamId, sdb),
+        get_root_team(accountId, sdb)
+        if teamId == 0
+        else get_team_from_db(teamId, accountId, None, sdb),
         get_metadata_account_ids(accountId, sdb, cache),
     )
 

server/athenian/api/align/spec

@@ -0,0 +1,48 @@
+from typing import Optional
+
+from aiohttp import web
+
+from athenian.api.internal.account import get_metadata_account_ids
+from athenian.api.internal.team import (
+    fetch_team_members_recursively,
+    get_all_team_members,
+    get_root_team,
+    get_team_from_db,
+)
+from athenian.api.models.state.models import Team
+from athenian.api.models.web import BadRequestError, Contributor
+from athenian.api.request import AthenianWebRequest
+from athenian.api.response import ResponseError, model_response
+
+
+async def list_team_members(
+    request: AthenianWebRequest,
+    team_id: int,
+    recursive: bool,
+    account: Optional[int] = None,
+) -> web.Response:
+    """List the members of a team."""
+    sdb, mdb, cache = request.sdb, request.mdb, request.cache
+
+    if team_id == 0:
+        if account is None:
+            msg = "Parameter account is required with team_id 0"
+            raise ResponseError(BadRequestError(detail=msg))
+        team = await get_root_team(account, sdb)
+    else:
+        team = await get_team_from_db(team_id, account, request.uid, sdb)
+        account = team[Team.owner_id.name]
+
+    meta_ids = await get_metadata_account_ids(account, sdb, cache)
+
+    if recursive:
+        member_ids = await fetch_team_members_recursively(account, sdb, team[Team.id.name])
+    else:
+        member_ids = team[Team.members.name]
+    members = await get_all_team_members(member_ids, account, meta_ids, mdb, sdb, cache)
+
+    def sort_key(member: Contributor) -> tuple[bool, str, str]:
+        # first users with a name
+        return (member.name is None, (member.name or "").lower(), member.login.lower())
+
+    return model_response(sorted(members.values(), key=sort_key))

server/athenian/api/controllers/align/member_controller.py

@@ -13,7 +13,7 @@
 from athenian.api.db import Connection, Database, DatabaseLike, Row, conn_in_transaction
 from athenian.api.internal.jira import load_mapped_jira_users
 from athenian.api.models.metadata.github import User
-from athenian.api.models.state.models import Team
+from athenian.api.models.state.models import Team, UserAccount
 from athenian.api.models.web import BadRequestError, Contributor, GenericError
 from athenian.api.response import ResponseError
 from athenian.api.tracing import sentry_span
@@ -77,9 +77,30 @@ async def get_root_team(account_id: int, sdb_conn: DatabaseLike) -> Row:
 
 
 @sentry_span
-async def get_team_from_db(account_id: int, team_id: int, sdb_conn: DatabaseLike) -> Row:
-    """Return a team owned by an account."""
-    stmt = sa.select(Team).where(sa.and_(Team.owner_id == account_id, Team.id == team_id))
+async def get_team_from_db(
+    team_id: int,
+    account_id: Optional[int],
+    user_id: Optional[str],
+    sdb_conn: DatabaseLike,
+) -> Row:
+    """Return a team owned by an account.
+
+    Team can be filtered by owner account and/or user id with access to team.
+    At least one of the two filtering conditions must be specified.
+
+    """
+    if account_id is None and user_id is None:
+        raise ValueError("At least one between account_id and user_id must be specified")
+
+    where = Team.id == team_id
+    if account_id is not None:
+        where = sa.and_(where, Team.owner_id == account_id)
+        from_ = Team
+    if user_id is not None:
+        from_ = sa.join(Team, UserAccount, Team.owner_id == UserAccount.account_id)
+        where = sa.and_(where, UserAccount.user_id == user_id)
+
+    stmt = sa.select(Team).select_from(from_).where(where)
     team = await sdb_conn.fetch_one(stmt)
     if team is None:
         raise TeamNotFoundError(team_id)

server/athenian/api/internal/team.py

@@ -0,0 +1,191 @@
+from typing import Optional
+
+from athenian.api.db import Database
+from tests.testutils.auth import force_request_auth
+from tests.testutils.db import DBCleaner, models_insert
+from tests.testutils.factory import metadata as md_factory
+from tests.testutils.factory.state import (
+    MappedJIRAIdentityFactory,
+    TeamFactory,
+    UserAccountFactory,
+)
+from tests.testutils.requester import Requester
+
+
+class BaseListTeamMembersTest(Requester):
+    async def _request(
+        self,
+        team_id: int,
+        assert_status: int = 200,
+        recursive: Optional[bool] = None,
+        account: Optional[int] = None,
+        headers: Optional[dict] = None,
+        user_id: Optional[str] = None,
+    ) -> list[dict] | dict:
+        path = f"/private/align/members/{team_id}"
+        params = {}
+        if recursive is not None:
+            params["recursive"] = str(recursive).lower()
+        if account is not None:
+            params["account"] = str(account)
+        headers = self.headers if headers is None else (self.headers | headers)
+
+        with force_request_auth(user_id, headers) as headers:
+            response = await self.client.request(
+                method="GET", path=path, headers=headers, params=params,
+            )
+
+        assert response.status == assert_status
+        return await response.json()
+
+
+class TestListTeamMembersErrors(BaseListTeamMembersTest):
+    async def test_invalid_auth_token(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(id=1, members=[1, 2]))
+        headers = {"Authorization": "Bearer invalid"}
+        res = await self._request(1, 401, headers=headers)
+        assert isinstance(res, dict)
+        assert res["type"] == "/errors/Unauthorized"
+
+    async def test_team_not_found(self) -> None:
+        res = await self._request(999, 404)
+        assert isinstance(res, dict)
+        assert res["detail"] == "Team 999 not found or access denied"
+
+    async def test_team_not_found_recursive(self) -> None:
+        res = await self._request(999, 404, recursive=True)
+        assert isinstance(res, dict)
+        assert res["detail"] == "Team 999 not found or access denied"
+
+    async def test_team_account_mismatch(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(id=10, owner_id=3))
+        res = await self._request(10, 404)
+        assert isinstance(res, dict)
+        assert res["detail"] == "Team 10 not found or access denied"
+
+    async def test_team_correct_account_param_mismatch(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(id=10))
+        res = await self._request(10, 404, account=3)
+        assert isinstance(res, dict)
+        assert res["detail"] == "Team 10 not found or access denied"
+
+    async def test_team_account_mismatch_non_default_user(self, sdb: Database) -> None:
+        await models_insert(
+            sdb, UserAccountFactory(user_id="my-user", account_id=2), TeamFactory(id=10),
+        )
+        await self._request(10, 404, user_id="my-user")
+
+    async def test_root_team_multiple_roots(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(), TeamFactory())
+        res = await self._request(0, 500, account=1)
+        assert isinstance(res, dict)
+        assert res["title"] == "Multiple root teams"
+        assert res["detail"] == "Account 1 has multiple root teams"
+
+    async def test_root_team_no_team_exists(self, sdb: Database) -> None:
+        res = await self._request(0, 404, account=1)
+        assert isinstance(res, dict)
+        assert res["type"] == "/errors/teams/TeamNotFound"
+        assert res["detail"] == "Root team not found or access denied"
+
+    async def test_root_team_missing_account(self, sdb: Database) -> None:
+        res = await self._request(0, 400, account=None)
+        assert isinstance(res, dict)
+        assert res["type"] == "/errors/BadRequest"
+
+
+class TestListTeamMembers(BaseListTeamMembersTest):
+    async def test_explicit_non_root_team(self, sdb: Database) -> None:
+        # members id are from the 6 MB metadata db fixture
+        await models_insert(
+            sdb,
+            TeamFactory(id=1, members=[]),
+            TeamFactory(id=2, members=[40078, 39652900], parent_id=1),
+        )
+
+        res = await self._request(2)
+        assert [m["login"] for m in res] == ["github.com/leantrace", "github.com/reujab"]
+        assert [m["name"] for m in res] == ["Alexander Schamne", "Christopher Knight"]
+
+        assert "email" in res[0]
+        assert "picture" in res[0]
+
+    async def test_explicit_root_team(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(id=1, members=[40078], parent_id=None))
+        res = await self._request(1)
+        assert [m["login"] for m in res] == ["github.com/reujab"]
+
+    async def test_implicit_root_team_empty(self, sdb: Database) -> None:
+        await models_insert(
+            sdb,
+            TeamFactory(id=10, members=[]),
+            TeamFactory(id=20, parent_id=10, members=[3]),
+            TeamFactory(id=30, parent_id=10, members=[3, 4]),
+        )
+        res = await self._request(0, account=1)
+        assert res == []
+
+    async def test_implicit_root_team(self, sdb: Database) -> None:
+        await models_insert(
+            sdb,
+            TeamFactory(id=10, members=[40078]),
+            TeamFactory(id=11, parent_id=10, members=[39652900]),
+        )
+        res = await self._request(0, account=1)
+        assert [m["login"] for m in res] == ["github.com/reujab"]
+
+    async def test_recursive(self, sdb: Database) -> None:
+        await models_insert(
+            sdb,
+            TeamFactory(id=1, members=[39652900]),
+            TeamFactory(id=2, parent_id=1, members=[40078]),
+        )
+
+        res = await self._request(1, recursive=True)
+        assert [m["login"] for m in res] == ["github.com/leantrace", "github.com/reujab"]
+
+        res = await self._request(1, recursive=False)
+        assert [m["login"] for m in res] == ["github.com/leantrace"]
+
+        res = await self._request(1)
+        assert [m["login"] for m in res] == ["github.com/leantrace"]
+
+    async def test_result_ordering(self, sdb: Database, mdb_rw: Database) -> None:
+        await models_insert(
+            sdb,
+            TeamFactory(id=1, members=[100, 101, 102, 103]),
+        )
+
+        async with DBCleaner(mdb_rw) as mdb_cleaner:
+            models = [
+                md_factory.UserFactory(node_id=100, html_url="https://c", name="Foo Bar"),
+                md_factory.UserFactory(node_id=101, html_url="https://a", name=None),
+                md_factory.UserFactory(node_id=102, html_url="https://b", name="zz-Top"),
+                md_factory.UserFactory(node_id=103, html_url="https://d", name="aa"),
+            ]
+            mdb_cleaner.add_models(*models)
+            await models_insert(mdb_rw, *models)
+
+            res = await self._request(1, recursive=True)
+
+        assert [m["login"] for m in res] == ["d", "c", "b", "a"]
+        assert [m.get("name") for m in res] == ["aa", "Foo Bar", "zz-Top", None]
+
+    async def test_jira_user_field(self, sdb: Database, mdb_rw: Database) -> None:
+        await models_insert(
+            sdb,
+            TeamFactory(id=1, members=[100, 101]),
+            MappedJIRAIdentityFactory(github_user_id=100, jira_user_id="200"),
+        )
+
+        async with DBCleaner(mdb_rw) as mdb_cleaner:
+            models = [
+                md_factory.UserFactory(node_id=100),
+                md_factory.UserFactory(node_id=101),
+                md_factory.JIRAUserFactory(id="200", display_name="My JIRA name"),
+            ]
+            mdb_cleaner.add_models(*models)
+            await models_insert(mdb_rw, *models)
+            res = await self._request(1)
+
+        assert [m.get("jira_user") for m in res] == ["My JIRA name", None]

server/tests/controllers/align/__init__.py

@@ -23,7 +23,12 @@
     models_insert,
     transaction_conn,
 )
-from tests.testutils.factory.state import GoalFactory, TeamFactory, TeamGoalFactory
+from tests.testutils.factory.state import (
+    GoalFactory,
+    TeamFactory,
+    TeamGoalFactory,
+    UserAccountFactory,
+)
 
 
 class TestGetRootTeam:
@@ -44,15 +49,52 @@ async def test_multiple_root_teams(self, sdb: Database) -> None:
 
 
 class TestGetTeamFromDB:
-    async def test_found(self, sdb: Database) -> None:
+    async def test_found_by_account(self, sdb: Database) -> None:
         await models_insert(sdb, TeamFactory(id=99, name="TEAM 99"))
-        team = await get_team_from_db(1, 99, sdb)
-        assert team["name"] == "TEAM 99"
+        team = await get_team_from_db(99, 1, None, sdb)
+        assert team[Team.name.name] == "TEAM 99"
 
-    async def test_not_found(self, sdb: Database) -> None:
+    async def test_not_found_by_account(self, sdb: Database) -> None:
         await models_insert(sdb, TeamFactory(id=99, owner_id=2))
         with pytest.raises(TeamNotFoundError):
-            await get_team_from_db(1, 99, sdb)
+            await get_team_from_db(99, 1, None, sdb)
+
+    async def test_found_by_uid(self, sdb: Database) -> None:
+        await models_insert(
+            sdb, TeamFactory(id=99, owner_id=2), UserAccountFactory(user_id="u0", account_id=2),
+        )
+        team = await get_team_from_db(99, None, "u0", sdb)
+        assert team[Team.id.name] == 99
+
+    async def test_not_found_by_uid(self, sdb: Database) -> None:
+        await models_insert(sdb, UserAccountFactory(user_id="u0", account_id=2))
+        with pytest.raises(TeamNotFoundError):
+            await get_team_from_db(99, None, "u0", sdb)
+
+    async def test_not_found_by_uid_not_owned_team(self, sdb: Database) -> None:
+        await models_insert(
+            sdb, TeamFactory(id=99, owner_id=1), UserAccountFactory(user_id="u0", account_id=2),
+        )
+        with pytest.raises(TeamNotFoundError):
+            await get_team_from_db(99, None, "u0", sdb)
+
+    async def test_both_filters(self, sdb: Database) -> None:
+        await models_insert(
+            sdb, TeamFactory(id=9, owner_id=2), UserAccountFactory(user_id="u0", account_id=2),
+        )
+        team = await get_team_from_db(9, 2, "u0", sdb)
+        assert team[Team.id.name] == 9
+
+        with pytest.raises(TeamNotFoundError):
+            await get_team_from_db(9, 1, "u0", sdb)
+
+        with pytest.raises(TeamNotFoundError):
+            await get_team_from_db(9, 2, "u1", sdb)
+
+    async def test_no_filter(self, sdb: Database) -> None:
+        await models_insert(sdb, TeamFactory(id=9))
+        with pytest.raises(ValueError):
+            await get_team_from_db(9, None, None, sdb)
 
 
 class TestGetBotsTeam: