Wrong user count when using teams as team owners

[dgsiegel]

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.

---

Steps to reproduce

1. Create two teams "foo" and "bar"
2. Add team "foo" to "bar" and set "foo" as sole owner of that team
3. The sidebar will now show a user count of 2 instead of 1.
4. When visiting the team, the user count reduces to the correct number of 1

Expected behaviour

Always show the same user count. If you count the team and the single member of "foo" as 2, that would be fine as well.

Server configuration

Database: PostgreSQL

PHP version: 8.3

Nextcloud version: 31.0.8

[jancborchardt]

FYI @ArtificialOwl @susnux not sure if frontend or backend issue.

[dgsiegel]

On further tests it looks like this always happens when a team is a member of another team, regardless of whether it's the owner or not.

[cristianscheid]

Hi @dgsiegel, thanks for reporting this!

I've tested it and also got some inconsistent behaviour.

Steps I tested:

1. Logged as "alice" and created teams "foo" and "bar", both with only "alice" as member/owner (count: 1 each)
2. Added team "foo" as member of "bar"
   • Result: "bar" shows count of 2 (alice + foo team), even when clicking on "bar" team
3. Added user "bob" to "foo"
   • "foo" count updates to 2
   • "bar" count remains at 2 (doesn't reflect the new user in nested team)
4. Removed "foo" from "bar" and re-added it
   • After refreshing page (cliking on contacts app), "bar" shows count of 3
   • Clicking on "bar" team reduces count back to 2

I wonder what the expected behaviour would be. When a team is added to another team, should we:

• Count the team as one member?
• Add all the members inside that team to the count?
• Handle duplicates? For example, if alice is in both "foo" and "bar", and "foo" is inside "bar", should we count alice only once?

[cristianscheid]

@jancborchardt would you happen to know what the expected behavior should be here (based on previous comment)?

[jancborchardt]

@cristianscheid yep, it’s as described in the original issue basically: A team should not be counted as a member on its own.

• If team "Designers" has 1 member
• And that team is added to "Engineering" which otherwise has 0 members
• Both teams should have the number of "1" members shown in the left navigation, even for the team with a nested team inside.

[cristianscheid]

After further investigation on this issue, here's what I found:

My test scenario

• Team "foo": alice, bob, jane, john
• Team "bar": foo (as member), alice, bob

The behavior I'm seeing

When accessing the Contacts app, the count for "bar" on left sidebar shows 5:

• A GET request is made to /ocs/v2.php/apps/circles/circles which returns the population value from the DB (5)

The population count is stored in the database (oc_circles_circle.settings column) and calculated by the MembershipService::updatePopulation() method.

In the DB, the following is stored for "bar": {"population": 5, "populationInherited": 5}

From my understanding, this count includes: alice + bob (direct members) + foo (team as a member) + jane + john (inherited from foo but not in bar) = 5

When clicking on the "bar" team in the sidebar, the count shows 3:

• A GET request is made to /ocs/v2.php/apps/circles/circles/{id}/members which returns the actual members list
• The count appears to be based on the members list length that's returned (alice + bob + foo = 3)

What might be causing this difference

• When loading the Contacts app, the count is fetched from the population field in the database, which is calculated by MembershipService::updatePopulation() when members are added/removed from a team.
• When clicking on a team in the sidebar, the count is calculated using MemberService::getMembers(), which returns the actual member list.

These two approaches count members differently, leading to inconsistent values.

Next steps

Based on @jancborchardt clarification, the expected behavior should be:

• Don't count the nested team itself as a member (only count the users within it)
• Only count users from nested team who are only in the nested team (avoid duplicates)

I'll investigate both approaches and try to align them with this expected behavior.

[cristianscheid]

Looking at the MembershipService::updatePopulation() method as it is now, it appears to me that the original intent was:

• population: Direct members of a given team
• populationInherited: Sum of direct members + inherited members

Current behavior

The oc_circles_membership table stores all memberships of a circle, meaning direct members, teams (as members), and inherited members.

The oc_circles_member table, on the other hand, stores only direct members for a given circle, meaning only direct users and teams added as members.

As it is now, with my test scenario (described in previous comment), updatePopulation() results in both population and populationInherited with the same value in the DB, which doesn't match the expected behavior.

Proposed approach

To try to achieve the desired behaviour, I've tested a different approach locally, updating the method as follows:

public function updatePopulation(Circle $circle): void {
    $local = $inherited = 0;
    $memberships = $this->membershipRequest->getInherited($circle->getSingleId(), Member::LEVEL_MEMBER);
+
+   // First pass: identify circles by extracting their IDs from membership inheritance paths
+   $circleSingleIds = [];
    foreach ($memberships as $membership) {
+       if ($membership->getInheritanceDepth() > 1) {
+           // Last element is excluded as it represents the leaf member, not a circle
+           $parentCircleIds = array_slice($membership->getInheritancePath(), 0, -1);
+           $circleSingleIds = array_merge($circleSingleIds, $parentCircleIds);
+       }
+   }
+   $circleSingleIds = array_unique($circleSingleIds);
+
+   // Second pass: count members, excluding circles identified in first pass
+   foreach ($memberships as $membership) {
+       if (in_array($membership->getSingleId(), $circleSingleIds)) {
+           continue;
+       }
        $inherited++;
-       if ($membership->getCircleId() === $circle->getSingleId()) {
+       if ($membership->getInheritanceDepth() === 1) {
            $local++;
        }
    }
}

Basically what it does is:

1. Check if the member is of type circle, and in that case, skip counting it
2. Only count for population if it's a direct member (inheritance_depth === 1)
3. Count for populationInherited all non-circle members (direct and inherited)

Test results

After my modification, I created the following scenario:

• Team "foo": alice, bob, user1, bar (as member)
• Team "bar": alice, bob, user2, baz (as member)
• Team "baz": alice, bob, user3

Which populated oc_circles_circle.settings column as follows:

• foo: {"population":3,"populationInherited":5}
• bar: {"population":3,"populationInherited":4}
• baz: {"population":3,"populationInherited":3}

I believe this would be the expected behavior.

Additional considerations

That being said, there's a couple more things that I noticed. The count rendering is done in the contacts repository.

Data is fetched from backend here:

• src/store/circles.js
• src/services/circles.ts

Rendered here:

• src/components/AppNavigation/CircleNavigationItem.vue

Backend serialization is done on circles app:

• lib/Model/Circle::jsonSerialize()

The frontend currently has two different ways of calculating the memberCount(), which I believe is causing the inconsistent behavior:

memberCount() {
	const count = Object.keys(this.circle?.members || []).length

	// If member list is empty, let's try the population initial count
	if (count === 0 && this.circle.population > 0) {
		return this.circle.population
	}

	return count
},

Maybe the if the backend returns populationInherited instead of population in Circle::jsonSerialize(), we could always use this.circle.populationInherited for the count instead of the current logic.

And a couple more things to consider:

1. If we change the logic of population/populationInherited, it also affects the occ circles:manage:list command which returns a "Population" column that uses both values.

2. When updatePopulation() is triggered for a team that belongs to other teams, we’d also need to update the population of those parent teams to reflect the change. This would likely require additional DB queries. Is this acceptable performance-wise?

Any thoughts on this?