[WPB-27169] Do not count apps and collaborators as members in get-team-size.

changelog.d/1-api-changes/WPB-27169-do-not-count-apps-and-collaborators-as-members-in-get-team-size

@@ -0,0 +1 @@
+Do not count apps and collaborators as members in get-team-size.  New schema: `{"teamSize": num, "apps": num, "collaborators": num}` (non-overlapping).  `teamSize` has been the label since the dawn of time, only the other two have changed.  The protobuf schema for TeamEvents changed accordingly.

integration/test/Test/Apps.hs

@@ -68,6 +68,13 @@ testCreateGetApp sameOrOtherDomain = do
         void $ assertNoEvent 5 wsRegularMember
       pure (appId, cookie)
 
+  -- team size counts apps separately.  (they are not members.)
+  bindResponse (getTeamSize owner tid) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "teamSize" `shouldMatchInt` 2
+    resp.json %. "apps" `shouldMatchInt` 1
+    resp.json %. "collaborators" `shouldMatchInt` 0
+
   -- Verify that the team.member-join event is in the team notifications queue
   bindResponse (getTeamNotifications regularMember (Just lastTeamNotif)) $ \resp -> do
     resp.status `shouldMatchInt` 200
@@ -189,6 +196,7 @@ testDeleteAppFromTeam = do
   appId <- bindResponse (createApp owner tid new) $ \resp -> do
     resp.status `shouldMatchInt` 200
     resp.json %. "user.id" & asString
+  BrigI.refreshIndex domain
 
   let appIdObject = object ["domain" .= domain, "id" .= appId]
 
@@ -572,30 +580,29 @@ testAppReceivesMemberJoinNotification = do
 testTeamSizeWithApps :: (HasCallStack) => TaggedBool "test internal api" -> App ()
 testTeamSizeWithApps (TaggedBool testInternalApi) = do
   domain <- make OwnDomain
-  numRegulars <- liftIO $ randomRIO (1 :: Int, 3)
+  numRegulars <- liftIO $ randomRIO (2 :: Int, 4)
   numApps <- liftIO $ randomRIO (1 :: Int, 3)
 
-  (owner, tid, extraMembers) <- createTeam domain (numRegulars + 1)
+  (owner, tid, extraMembers) <- createTeam domain numRegulars
 
   apps <- replicateM numApps $ bindResponse (createApp owner tid def) $ \resp -> do
     resp.status `shouldMatchInt` 200
     resp.json %. "user"
 
   let checkSize :: (HasCallStack) => Int -> Int -> App ()
-      checkSize wantRegulars wantApps =
-        (if testInternalApi then BrigI.getTeamSize else Brig.getTeamSize) owner tid `bindResponse` \resp -> do
-          resp.status `shouldMatchInt` 200
-          resp.json %. "teamSize" `shouldMatchInt` (1 + wantRegulars + wantApps)
-          resp.json %. "teamSizeRegulars" `shouldMatchInt` (1 + wantRegulars)
-          resp.json %. "teamSizeApps" `shouldMatchInt` wantApps
+      checkSize wantRegulars wantApps = do
+        BrigI.refreshIndex domain
+        eventually $ do
+          (if testInternalApi then BrigI.getTeamSize else Brig.getTeamSize) owner tid `bindResponse` \resp -> do
+            resp.status `shouldMatchInt` 200
+            resp.json %. "teamSize" `shouldMatchInt` wantRegulars
+            resp.json %. "apps" `shouldMatchInt` wantApps
+            resp.json %. "collaborators" `shouldMatchInt` 0
 
-  BrigI.refreshIndex domain
-  eventually $ do
-    checkSize numRegulars numApps
+  checkSize numRegulars numApps
 
   deleteTeamMember tid owner (head apps) >>= assertSuccess
-  deleteTeamMember tid owner (head extraMembers) >>= assertSuccess
+  checkSize numRegulars (numApps - 1)
 
-  BrigI.refreshIndex domain
-  eventually $ do
-    checkSize (numRegulars - 1) (numApps - 1)
+  deleteTeamMember tid owner (head extraMembers) >>= assertSuccess
+  checkSize (numRegulars - 1) (numApps - 1)

integration/test/Test/TeamCollaborators.hs

@@ -19,7 +19,7 @@
 
 module Test.TeamCollaborators where
 
-import qualified API.Brig as BrigP
+import API.Brig as BrigP
 import qualified API.BrigInternal as BrigI
 import API.Common (randomName)
 import API.Galley
@@ -63,6 +63,14 @@ testCreateTeamCollaborator = do
     res %. "team" `shouldMatch` team
     res %. "permissions" `shouldMatch` ["create_team_conversation", "implicit_connection"]
 
+  -- team size counts collaborators separately
+  BrigI.refreshIndex OwnDomain
+  bindResponse (getTeamSize owner team) $ \resp -> do
+    resp.status `shouldMatchInt` 200
+    resp.json %. "teamSize" `shouldMatchInt` 2
+    resp.json %. "apps" `shouldMatchInt` 0
+    resp.json %. "collaborators" `shouldMatchInt` 1
+
 testTeamCollaboratorEndpointsForbiddenForOtherTeams :: (HasCallStack) => App ()
 testTeamCollaboratorEndpointsForbiddenForOtherTeams = do
   (owner, _team, _members) <- createTeam OwnDomain 2

libs/types-common-journal/proto/TeamEvents.proto

@@ -22,10 +22,11 @@ message TeamEvent {
         // are guaranteed to be present).
         //
         // for backwards compatibility, clients should make these
-        // fields optional, and fall back to using `member_count` if
-        // they are missing.
-        required int32  member_count_regular = 4;
-        required int32  member_count_app = 5;
+        // fields optional, and assume '0' if missing.  wire-server
+        // always sets these fields and never receives team events
+        // from clients, so we make the fields as "required".
+        required int32  apps = 4;
+        required int32  collaborators = 5;
     }
 
     enum EventType {

libs/wire-api/src/Wire/API/Routes/Public/Galley/TeamMember.hs

@@ -213,6 +213,7 @@ type TeamMemberAPI =
            "add-team-collaborator"
            ( Summary "Add a collaborator to the team."
                :> From 'V10
+               :> CanThrow 'TooManyTeamMembersOnTeamWithLegalhold
                :> ZLocalUser
                :> "teams"
                :> Capture "tid" TeamId

libs/wire-api/src/Wire/API/Team/Size.hs

@@ -17,66 +17,33 @@
 
 module Wire.API.Team.Size
   ( TeamSize (..),
-    teamSizeTotal,
-    updateTeamSize,
   )
 where
 
 import Control.Lens ((?~))
 import Data.Aeson qualified as A
-import Data.Aeson.Types qualified as A
 import Data.OpenApi qualified as S
 import Data.Schema
 import Imports
 import Numeric.Natural
 import Test.QuickCheck (arbitrarySizedNatural)
-import Wire.API.User.Search
 import Wire.Arbitrary
 
 data TeamSize = TeamSize
-  { regulars :: Natural,
-    apps :: Natural
+  { teamSize :: Natural,
+    apps :: Natural,
+    collaborators :: Natural
   }
   deriving (Show, Eq)
   deriving (A.ToJSON, A.FromJSON, S.ToSchema) via (Schema TeamSize)
 
--- | Total team members (regulars + apps).
-teamSizeTotal :: TeamSize -> Natural
-teamSizeTotal ts = ts.regulars + ts.apps
-
--- Increase or decrease a team size component, depending on user type.
-
--- If the result of a decrease is <0, it is set to 1 (regulars) or 0
--- (apps).  This handles corner cases where ES reports lower numbers
--- from the past.
-updateTeamSize :: UserTypeFilter -> TeamSize -> Int -> TeamSize
-updateTeamSize = go
-  where
-    go :: UserTypeFilter -> TeamSize -> Int -> TeamSize
-    go UserTypeFilterRegular (TeamSize rs as) n = TeamSize (upd 1 rs n) as
-    go UserTypeFilterApp (TeamSize rs as) n = TeamSize rs (upd 0 as n)
-
-    upd :: Int -> Natural -> Int -> Natural
-    upd low n i = fromIntegral . max low $ fromIntegral n + i
-
 instance ToSchema TeamSize where
   schema =
-    objectWithDocModifier (description ?~ "Team member counts broken down by user type.") $
-      fromTeamSize .= tripleSchema `withParser` validate
-    where
-      fromTeamSize :: TeamSize -> (Natural, Natural, Maybe Natural)
-      fromTeamSize ts = (ts.regulars, ts.apps, Just (teamSizeTotal ts))
-      tripleSchema :: ObjectSchema SwaggerDoc (Natural, Natural, Maybe Natural)
-      tripleSchema =
-        (,,)
-          <$> (\(r, _, _) -> r) .= fieldWithDocModifier "teamSizeRegulars" (description ?~ "Number of regular users in team.") schema
-          <*> (\(_, a, _) -> a) .= fieldWithDocModifier "teamSizeApps" (description ?~ "Number of apps in team.") schema
-          <*> (\(_, _, t) -> t) .= maybe_ (optFieldWithDocModifier "teamSize" (description ?~ "Total team members (teamSizeRegulars + teamSizeApps).") schema)
-      validate :: (Natural, Natural, Maybe Natural) -> A.Parser TeamSize
-      validate (r, a, Nothing) = pure TeamSize {regulars = r, apps = a}
-      validate (r, a, Just t)
-        | r + a == t = pure TeamSize {regulars = r, apps = a}
-        | otherwise = fail $ "teamSize (" <> show t <> ") != regulars + apps (" <> show (r + a) <> ")"
+    objectWithDocModifier (description ?~ "Team member counts: paid seats (regular users), apps, and collaborators.") $
+      TeamSize
+        <$> (.teamSize) .= field "teamSize" schema
+        <*> (.apps) .= field "apps" schema
+        <*> (.collaborators) .= field "collaborators" schema
 
 instance Arbitrary TeamSize where
-  arbitrary = TeamSize <$> arbitrarySizedNatural <*> arbitrarySizedNatural
+  arbitrary = TeamSize <$> arbitrarySizedNatural <*> arbitrarySizedNatural <*> arbitrarySizedNatural

libs/wire-api/test/golden/Test/Wire/API/Golden/Manual/TeamSize.hs

@@ -21,10 +21,10 @@ import Imports
 import Wire.API.Team.Size
 
 testObject_TeamSize_1 :: TeamSize
-testObject_TeamSize_1 = TeamSize 0 0
+testObject_TeamSize_1 = TeamSize 0 0 0
 
 testObject_TeamSize_2 :: TeamSize
-testObject_TeamSize_2 = TeamSize 100 400
+testObject_TeamSize_2 = TeamSize 100 400 7
 
 testObject_TeamSize_3 :: TeamSize
-testObject_TeamSize_3 = TeamSize (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64)
+testObject_TeamSize_3 = TeamSize (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64) (fromIntegral $ maxBound @Word64)

libs/wire-api/test/golden/testObject_Event_meeting_create_manual_1.json

@@ -1,9 +1,5 @@
 {
     "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e",
-    "qualified_id": {
-        "domain": "example.com",
-        "id": "00000001-0000-0000-0000-000000000001"
-    },
     "from": "a471447c-aa30-4592-81b0-dec6c1c02bca",
     "qualified_conversation": {
         "domain": "example.com",
@@ -13,6 +9,10 @@
         "domain": "example.com",
         "id": "a471447c-aa30-4592-81b0-dec6c1c02bca"
     },
+    "qualified_id": {
+        "domain": "example.com",
+        "id": "00000001-0000-0000-0000-000000000001"
+    },
     "time": "2018-01-01T00:00:00.000Z",
     "type": "meeting.create",
     "via": "user"

libs/wire-api/test/golden/testObject_Event_meeting_delete_manual_1.json

@@ -1,9 +1,5 @@
 {
     "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e",
-    "qualified_id": {
-        "domain": "example.com",
-        "id": "00000001-0000-0000-0000-000000000001"
-    },
     "from": "a471447c-aa30-4592-81b0-dec6c1c02bca",
     "qualified_conversation": {
         "domain": "example.com",
@@ -13,6 +9,10 @@
         "domain": "example.com",
         "id": "a471447c-aa30-4592-81b0-dec6c1c02bca"
     },
+    "qualified_id": {
+        "domain": "example.com",
+        "id": "00000001-0000-0000-0000-000000000001"
+    },
     "time": "2018-01-01T00:00:00.000Z",
     "type": "meeting.delete",
     "via": "user"

libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_1.json

@@ -1,9 +1,5 @@
 {
     "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e",
-    "qualified_id": {
-        "domain": "example.com",
-        "id": "00000001-0000-0000-0000-000000000001"
-    },
     "from": "a471447c-aa30-4592-81b0-dec6c1c02bca",
     "qualified_conversation": {
         "domain": "example.com",
@@ -13,6 +9,10 @@
         "domain": "example.com",
         "id": "a471447c-aa30-4592-81b0-dec6c1c02bca"
     },
+    "qualified_id": {
+        "domain": "example.com",
+        "id": "00000001-0000-0000-0000-000000000001"
+    },
     "time": "2018-01-01T00:00:00.000Z",
     "type": "meeting.member-add",
     "via": "user"

libs/wire-api/test/golden/testObject_Event_meeting_member_add_manual_2.json

@@ -1,9 +1,5 @@
 {
     "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e",
-    "qualified_id": {
-        "domain": "example.com",
-        "id": "00000001-0000-0000-0000-000000000001"
-    },
     "from": "a471447c-aa30-4592-81b0-dec6c1c02bca",
     "qualified_conversation": {
         "domain": "example.com",
@@ -13,6 +9,10 @@
         "domain": "example.com",
         "id": "a471447c-aa30-4592-81b0-dec6c1c02bca"
     },
+    "qualified_id": {
+        "domain": "example.com",
+        "id": "00000001-0000-0000-0000-000000000001"
+    },
     "team": "00000002-0000-0000-0000-000000000002",
     "time": "2018-01-01T00:00:00.000Z",
     "type": "meeting.member-add",

libs/wire-api/test/golden/testObject_Event_meeting_update_manual_1.json

@@ -1,9 +1,5 @@
 {
     "conversation": "2126ea99-ca79-43ea-ad99-a59616468e8e",
-    "qualified_id": {
-        "domain": "example.com",
-        "id": "00000001-0000-0000-0000-000000000001"
-    },
     "from": "a471447c-aa30-4592-81b0-dec6c1c02bca",
     "qualified_conversation": {
         "domain": "example.com",
@@ -13,6 +9,10 @@
         "domain": "example.com",
         "id": "a471447c-aa30-4592-81b0-dec6c1c02bca"
     },
+    "qualified_id": {
+        "domain": "example.com",
+        "id": "00000001-0000-0000-0000-000000000001"
+    },
     "time": "2018-01-01T00:00:00.000Z",
     "type": "meeting.update",
     "via": "user"

libs/wire-api/test/golden/testObject_TeamSize_1.json

@@ -1,5 +1,5 @@
 {
-    "teamSize": 0,
-    "teamSizeApps": 0,
-    "teamSizeRegulars": 0
+    "apps": 0,
+    "collaborators": 0,
+    "teamSize": 0
 }

libs/wire-api/test/golden/testObject_TeamSize_2.json

@@ -1,5 +1,5 @@
 {
-    "teamSize": 500,
-    "teamSizeApps": 400,
-    "teamSizeRegulars": 100
+    "apps": 400,
+    "collaborators": 7,
+    "teamSize": 100
 }

libs/wire-api/test/golden/testObject_TeamSize_3.json

@@ -1,5 +1,5 @@
 {
-    "teamSize": 3.689348814741910323e19,
-    "teamSizeApps": 1.8446744073709551615e19,
-    "teamSizeRegulars": 1.8446744073709551615e19
+    "apps": 1.8446744073709551615e19,
+    "collaborators": 1.8446744073709551615e19,
+    "teamSize": 1.8446744073709551615e19
 }

libs/wire-subsystems/src/Wire/IndexedUserStore/ElasticSearch.hs

@@ -90,7 +90,7 @@ getTeamSizeImpl cfg tid = do
   result <- either (embed . throwIO . IndexLookupError) pure (r :: Either ES.EsError (ES.SearchResult UserDoc))
   let aggs = fromMaybe mempty (ES.aggregations result)
       getCount name = maybe 0 (.filterDocCount) $ M.lookup name aggs >>= parseMaybe (parseJSON @FilterResult)
-  pure $ TeamSize (getCount "regulars") (getCount "apps")
+  pure $ TeamSize (getCount "teamSize") (getCount "apps") (getCount "collaborators")
   where
     teamQ = termQ "team" (idToText tid)
 
@@ -119,13 +119,18 @@ getTeamSizeImpl cfg tid = do
           { ES.boolQueryMustMatch = [teamQ, termQ "type" "app"]
           }
 
+    -- Collaborators are not members of the team, they are users (of other teams
+    -- or of no team) that collaborate with it.
+    collaboratorQuery = termQ "collaborating_teams" (idToText tid)
+
     search =
       (ES.mkSearch Nothing Nothing)
         { ES.size = ES.Size 0,
           ES.aggBody =
             Just $
-              ES.mkAggregations "regulars" (ES.FilterAgg (ES.FilterAggregation (ES.Filter regularQuery) Nothing))
+              ES.mkAggregations "teamSize" (ES.FilterAgg (ES.FilterAggregation (ES.Filter regularQuery) Nothing))
                 <> ES.mkAggregations "apps" (ES.FilterAgg (ES.FilterAggregation (ES.Filter appQuery) Nothing))
+                <> ES.mkAggregations "collaborators" (ES.FilterAgg (ES.FilterAggregation (ES.Filter collaboratorQuery) Nothing))
         }
 
 upsertImpl ::

libs/wire-subsystems/src/Wire/TeamJournal.hs

@@ -112,14 +112,13 @@ journalEvent typ tid dat tim = do
 -- utils
 
 evData :: TeamSize -> [UserId] -> Maybe Currency.Alpha -> TeamEvent'EventData
-evData teamSize@(TeamSize regulars apps) billingUserIds cur =
-  defMessage
-    & T.memberCount .~ memberCountTotal
-    & T.billingUser .~ (toBytes <$> billingUserIds)
-    & T.maybe'currency .~ (pack . show <$> cur)
-    & T.memberCountRegular .~ memberCountRegulars
-    & T.memberCountApp .~ memberCountApps
-  where
-    memberCountTotal, memberCountRegulars, memberCountApps :: Int32
-    (memberCountTotal, memberCountRegulars, memberCountApps) =
-      (fromIntegral $ teamSizeTotal teamSize, fromIntegral regulars, fromIntegral apps)
+evData
+  (TeamSize (fromIntegral -> teamSize) (fromIntegral -> apps) (fromIntegral -> collaborators))
+  billingUserIds
+  cur =
+    defMessage
+      & T.memberCount .~ teamSize
+      & T.billingUser .~ (toBytes <$> billingUserIds)
+      & T.maybe'currency .~ (pack . show <$> cur)
+      & T.apps .~ apps
+      & T.collaborators .~ collaborators