fix: Remove ordering on account organizations list (#900)

Author: spalmurray-codecov

graphql_api/helpers/mutation.py

@@ -75,5 +75,22 @@ def authenticated_resolver(queried_owner, info, *args, **kwargs):
     return authenticated_resolver
 
 
+def require_shared_account_or_part_of_org(resolver):
+    def authenticated_resolver(queried_owner, info, *args, **kwargs):
+        current_user = info.context["request"].user
+        if (
+            current_user
+            and current_user.is_authenticated
+            and queried_owner
+            and queried_owner.account
+            and current_user in queried_owner.account.users.all()
+        ):
+            return resolver(queried_owner, info, *args, **kwargs)
+
+        return require_part_of_org(resolver)(queried_owner, info, *args, **kwargs)
+
+    return authenticated_resolver
+
+
 def resolve_union_error_type(error, *_):
     return error.get_graphql_type()

graphql_api/tests/test_account.py

@@ -126,55 +126,9 @@ def test_fetch_organizations(self) -> None:
             for node in result["owner"]["account"]["organizations"]["edges"]
         ]
 
-        assert orgs == ["owner-2", "owner-1", "owner-0"]
-
-    def test_fetch_organizations_order_by_activated_users_asc(self) -> None:
-        account = AccountFactory(name="account")
-        owner = OwnerFactory(
-            username="owner-0",
-            plan_activated_users=[],
-            account=account,
-        )
-        OwnerFactory(
-            username="owner-1",
-            plan_activated_users=[0],
-            account=account,
-        )
-        OwnerFactory(
-            username="owner-2",
-            plan_activated_users=[0, 1],
-            account=account,
-        )
-
-        query = """
-            query {
-                owner(username: "%s") {
-                    account {
-                        organizations(first: 20, ordering: ACTIVATED_USERS, orderingDirection: ASC) {
-                            edges {
-                                node {
-                                    username
-                                    activatedUserCount
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        """ % (owner.username)
-
-        result = self.gql_request(query, owner=owner)
-
-        assert "errors" not in result
-
-        orgs = [
-            node["node"]["username"]
-            for node in result["owner"]["account"]["organizations"]["edges"]
-        ]
-
         assert orgs == ["owner-0", "owner-1", "owner-2"]
 
-    def test_fetch_organizations_order_by_name(self) -> None:
+    def test_fetch_organizations_desc(self) -> None:
         account = AccountFactory(name="account")
         owner = OwnerFactory(
             username="owner-0",
@@ -196,7 +150,7 @@ def test_fetch_organizations_order_by_name(self) -> None:
             query {
                 owner(username: "%s") {
                     account {
-                        organizations(first: 20, ordering: NAME) {
+                        organizations(first: 20, orderingDirection: DESC) {
                             edges {
                                 node {
                                     username
@@ -220,52 +174,6 @@ def test_fetch_organizations_order_by_name(self) -> None:
 
         assert orgs == ["owner-2", "owner-1", "owner-0"]
 
-    def test_fetch_organizations_order_by_name_asc(self) -> None:
-        account = AccountFactory(name="account")
-        owner = OwnerFactory(
-            username="owner-0",
-            plan_activated_users=[],
-            account=account,
-        )
-        OwnerFactory(
-            username="owner-1",
-            plan_activated_users=[0],
-            account=account,
-        )
-        OwnerFactory(
-            username="owner-2",
-            plan_activated_users=[0, 1],
-            account=account,
-        )
-
-        query = """
-            query {
-                owner(username: "%s") {
-                    account {
-                        organizations(first: 20, ordering: NAME, orderingDirection: ASC) {
-                            edges {
-                                node {
-                                    username
-                                    activatedUserCount
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        """ % (owner.username)
-
-        result = self.gql_request(query, owner=owner)
-
-        assert "errors" not in result
-
-        orgs = [
-            node["node"]["username"]
-            for node in result["owner"]["account"]["organizations"]["edges"]
-        ]
-
-        assert orgs == ["owner-0", "owner-1", "owner-2"]
-
     def test_fetch_organizations_pagination(self) -> None:
         account = AccountFactory(name="account")
         owner = OwnerFactory(
@@ -318,7 +226,7 @@ def test_fetch_organizations_pagination(self) -> None:
             for node in result["owner"]["account"]["organizations"]["edges"]
         ]
 
-        assert orgs == ["owner-2", "owner-1"]
+        assert orgs == ["owner-0", "owner-1"]
 
         hasNextPage = result["owner"]["account"]["organizations"]["pageInfo"][
             "hasNextPage"

graphql_api/tests/test_owner.py

@@ -7,7 +7,10 @@
 from freezegun import freeze_time
 from graphql import GraphQLError
 from prometheus_client import REGISTRY
-from shared.django_apps.codecov_auth.tests.factories import OktaSettingsFactory
+from shared.django_apps.codecov_auth.tests.factories import (
+    AccountsUsersFactory,
+    OktaSettingsFactory,
+)
 from shared.django_apps.reports.models import ReportType
 from shared.upload.utils import UploaderType, insert_coverage_measurement
 
@@ -1027,3 +1030,23 @@ def test_fetch_activated_user_count_returns_null_if_not_in_org(self):
         """ % (owner.username)
         data = self.gql_request(query, owner=user)
         assert data["owner"]["activatedUserCount"] is None
+
+    def test_fetch_activated_user_count_when_not_in_org_but_has_shared_account(self):
+        owner = OwnerFactory(username="sample-user")
+        AccountsUsersFactory(user=owner.user, account=self.account)
+        user2 = OwnerFactory(username="sample-user-2")
+        user3 = OwnerFactory(username="sample-user-3")
+        other_owner = OwnerFactory(
+            username="sample-org",
+            plan_activated_users=[user2.ownerid, user3.ownerid],
+            account=self.account,
+        )
+
+        query = """{
+            owner(username: "%s") {
+                activatedUserCount
+            }
+        }
+        """ % (other_owner.username)
+        data = self.gql_request(query, owner=owner)
+        assert data["owner"]["activatedUserCount"] == 2

graphql_api/types/account/account.graphql

@@ -4,7 +4,6 @@ type Account {
   totalSeatCount: Int!
   activatedUserCount: Int!
   organizations(
-    ordering: AccountOrganizationOrdering
     orderingDirection: OrderingDirection
     first: Int
     after: String

graphql_api/types/account/account.py

@@ -8,7 +8,7 @@
     build_connection_graphql,
     queryset_to_connection,
 )
-from graphql_api.types.enums.enums import AccountOrganizationOrdering, OrderingDirection
+from graphql_api.types.enums.enums import OrderingDirection
 
 account = ariadne_load_local_graphql(__file__, "account.graphql")
 account = account + build_connection_graphql("AccountOrganizationConnection", "Owner")
@@ -43,13 +43,12 @@ def resolve_activated_user_count(account: Account, info: GraphQLResolveInfo) ->
 def resolve_organizations(
     account: Account,
     info: GraphQLResolveInfo,
-    ordering=AccountOrganizationOrdering.ACTIVATED_USERS,
-    ordering_direction=OrderingDirection.DESC,
+    ordering_direction=OrderingDirection.ASC,
     **kwargs,
 ):
     return queryset_to_connection(
         account.organizations,
-        ordering=(ordering,),
+        ordering=("username",),
         ordering_direction=ordering_direction,
         **kwargs,
     )

graphql_api/types/enums/account_organization_ordering.graphql

@@ -11,7 +11,6 @@
 from timeseries.models import MeasurementName
 
 from .enums import (
-    AccountOrganizationOrdering,
     AssetOrdering,
     BundleLoadTypes,
     CoverageLine,
@@ -57,5 +56,4 @@
     EnumType("TestResultsOrderingParameter", TestResultsOrderingParameter),
     EnumType("TestResultsFilterParameter", TestResultsFilterParameter),
     EnumType("AssetOrdering", AssetOrdering),
-    EnumType("AccountOrganizationOrdering", AccountOrganizationOrdering),
 ]

graphql_api/types/enums/enum_types.py

@@ -41,11 +41,6 @@ class RepositoryOrdering(enum.Enum):
     NAME = "name"
 
 
-class AccountOrganizationOrdering(enum.Enum):
-    NAME = "username"
-    ACTIVATED_USERS = "plan_activated_users"
-
-
 class OrderingDirection(enum.Enum):
     ASC = "ascending"
     DESC = "descending"

graphql_api/types/enums/enums.py

@@ -31,7 +31,10 @@
     build_connection_graphql,
     queryset_to_connection,
 )
-from graphql_api.helpers.mutation import require_part_of_org
+from graphql_api.helpers.mutation import (
+    require_part_of_org,
+    require_shared_account_or_part_of_org,
+)
 from graphql_api.types.enums import OrderingDirection, RepositoryOrdering
 from graphql_api.types.errors.errors import NotFoundError, OwnerNotActivatedError
 from plan.constants import FREE_PLAN_REPRESENTATIONS, PlanData, PlanName
@@ -389,6 +392,6 @@ def resolve_upload_token_required(
 
 @owner_bindable.field("activatedUserCount")
 @sync_to_async
-@require_part_of_org
+@require_shared_account_or_part_of_org
 def resolve_activated_user_count(owner: Owner, info: GraphQLResolveInfo) -> int:
     return owner.activated_user_count