feat(circles): add Visitor role to teams (#2968)

lib/Api/v1/Circles.php

@@ -42,6 +42,7 @@ class Circles {
 
 	public const LEVEL_NONE = Member::LEVEL_NONE;
 	public const LEVEL_MEMBER = Member::LEVEL_MEMBER;
+	public const LEVEL_VISITOR = Member::LEVEL_VISITOR;
 	public const LEVEL_MODERATOR = Member::LEVEL_MODERATOR;
 	public const LEVEL_ADMIN = Member::LEVEL_ADMIN;
 	public const LEVEL_OWNER = Member::LEVEL_OWNER;

lib/AppInfo/Capabilities.php

@@ -145,6 +145,7 @@ private function getCapabilitiesCircleConfig(): array {
 	private function getCapabilitiesMemberConstants(): array {
 		return [
 			'level' => [
+				Member::LEVEL_VISITOR => $this->l10n->t('Visitor'),
 				Member::LEVEL_MEMBER => $this->l10n->t('Member'),
 				Member::LEVEL_MODERATOR => $this->l10n->t('Moderator'),
 				Member::LEVEL_ADMIN => $this->l10n->t('Admin'),

lib/CirclesQueryHelper.php

@@ -127,10 +127,14 @@ public function limitToMemberships(
 		string $alias,
 		string $field,
 		IFederatedUser $federatedUser,
+		string $levelAlias = '',
 	): void {
 		$this->queryBuilder->setDefaultSelectAlias($alias);
 		$expr = $this->queryBuilder->expr();
 		$aliasMembership = $this->queryBuilder->generateAlias(CoreQueryBuilder::HELPER, CoreQueryBuilder::MEMBERSHIPS, $options);
+		if ($levelAlias !== '') {
+			$this->queryBuilder->selectAlias($aliasMembership . '.level', $levelAlias);
+		}
 
 		$this->queryBuilder->innerJoin(
 			$alias,

lib/Controller/LocalController.php

@@ -154,9 +154,7 @@ public function memberAdd(string $circleId, string $userId, int $type): DataResp
 			$this->setCurrentFederatedUser();
 
 			$memberUser = $this->permissionService->userMustBeMember($this->userSession->getUser()->getUID(), $circleId);
-			if (!$this->circleService->getCircle($circleId)->isConfig(Circle::CFG_FRIEND)) {
-				$this->permissionService->memberMustBeAtLeastModerator($memberUser);
-			}
+			$this->mustBeAllowedToAddMembers($memberUser, $circleId);
 
 			if ($type === Member::TYPE_CONTACT) {
 				$currentUser = $this->federatedUserService->getCurrentUser();
@@ -192,9 +190,7 @@ public function membersAdd(string $circleId, array $members): DataResponse {
 			$this->setCurrentFederatedUser();
 
 			$memberUser = $this->permissionService->userMustBeMember($this->userSession->getUser()->getUID(), $circleId);
-			if (!$this->circleService->getCircle($circleId)->isConfig(Circle::CFG_FRIEND)) {
-				$this->permissionService->memberMustBeAtLeastModerator($memberUser);
-			}
+			$this->mustBeAllowedToAddMembers($memberUser, $circleId);
 
 			$federatedUsers = [];
 			foreach ($members as $member) {
@@ -308,9 +304,7 @@ public function memberConfirm(string $circleId, string $memberId): DataResponse
 			$this->setCurrentFederatedUser();
 
 			$memberUser = $this->permissionService->userMustBeMember($this->userSession->getUser()->getUID(), $circleId);
-			if (!$this->circleService->getCircle($circleId)->isConfig(Circle::CFG_FRIEND)) {
-				$this->permissionService->memberMustBeAtLeastModerator($memberUser);
-			}
+			$this->mustBeAllowedToAddMembers($memberUser, $circleId);
 
 			$member = $this->memberService->getMemberById($memberId, $circleId);
 			$federatedUser = new FederatedUser();
@@ -712,6 +706,13 @@ public function joinInvitation(string $invitationCode): DataResponse {
 		}
 	}
 
+	private function mustBeAllowedToAddMembers(Member $memberUser, string $circleId): void {
+		if (!$this->circleService->getCircle($circleId)->isConfig(Circle::CFG_FRIEND)
+			|| $memberUser->getLevel() === Member::LEVEL_VISITOR) {
+			$this->permissionService->memberMustBeAtLeastModerator($memberUser);
+		}
+	}
+
 	/**
 	 * @throws FederatedUserException
 	 * @throws FederatedUserNotFoundException

lib/Db/CircleRequest.php

@@ -168,7 +168,7 @@ public function getCircles(?IFederatedUser $initiator, CircleProbe $probe): arra
 		$qb->filterCircles(CoreQueryBuilder::CIRCLE, $probe);
 		if (!is_null($initiator)) {
 			$qb->limitToInitiator(CoreQueryBuilder::CIRCLE, $initiator);
-			$qb->orderBy($qb->generateAlias(CoreQueryBuilder::CIRCLE, CoreQueryBuilder::INITIATOR) . '.level', 'desc');
+			$qb->orderByMemberLevel($qb->generateAlias(CoreQueryBuilder::CIRCLE, CoreQueryBuilder::INITIATOR) . '.level');
 			$qb->addOrderBy(CoreQueryBuilder::CIRCLE . '.display_name', 'asc');
 			$qb->leftJoinCircleInvitation(CoreQueryBuilder::CIRCLE);
 		}

lib/Db/CoreQueryBuilder.php

@@ -223,6 +223,21 @@ public function __construct() {
 		$this->configService = Server::get(ConfigService::class);
 	}
 
+	public function orderByMemberLevel(string $column): void {
+		$ranks = Member::LEVEL_RANKS;
+		arsort($ranks);
+
+		$cases = [];
+		foreach ($ranks as $level => $rank) {
+			$cases[] = 'WHEN ' . $level . ' THEN ' . $rank;
+		}
+
+		$this->orderBy(
+			$this->createFunction('CASE ' . $column . ' ' . implode(' ', $cases) . ' ELSE 0 END'),
+			'desc'
+		);
+	}
+
 	/**
 	 * @param IFederatedModel $federatedModel
 	 *
@@ -693,10 +708,16 @@ public function filterDirectMembership(string $aliasMember, Member $member): voi
 
 		$this->limitToInstance($this->getInstance($member));
 
-		if ($member->getLevel() > 0) {
+		$minLevel = $member->getLevel();
+		if ($minLevel === Member::LEVEL_VISITOR) {
+			// Member ranks above Visitor despite having a lower numeric value.
+			$minLevel = Member::LEVEL_MEMBER;
+		}
+
+		if ($minLevel > 0) {
 			$andX[] = $expr->gte(
 				$aliasMember . '.level',
-				$this->createNamedParameter($member->getLevel(), IQueryBuilder::PARAM_INT)
+				$this->createNamedParameter($minLevel, IQueryBuilder::PARAM_INT)
 			);
 		}
 
@@ -885,6 +906,10 @@ public function innerJoinMembership(
 
 		// limit on membership level if requested
 		$minLevel = $probe?->getMinimumLevel() ?? 0;
+		if ($minLevel === Member::LEVEL_VISITOR) {
+			// Member ranks above Visitor despite having a lower numeric value.
+			$minLevel = Member::LEVEL_MEMBER;
+		}
 		if ($minLevel > Member::LEVEL_MEMBER) {
 			$on->add($this->exprGt('level', $minLevel, true, $aliasMembership));
 		}
@@ -1010,6 +1035,10 @@ public function limitToMembersByInheritance(string $alias, string $singleId, int
 		$expr = $this->expr();
 		$aliasMembership = $this->generateAlias($alias, self::MEMBERSHIPS);
 		$this->andWhere($expr->eq($aliasMembership . '.circle_id', $this->createNamedParameter($singleId)));
+		if ($level === Member::LEVEL_VISITOR) {
+			// Member ranks above Visitor despite having a lower numeric value.
+			$level = Member::LEVEL_MEMBER;
+		}
 		if ($level > 1) {
 			$this->andWhere(
 				$expr->gte(

lib/Db/MemberRequest.php

@@ -303,7 +303,7 @@ public function getMembers(
 			$qb->andWhere($qb->expr()->eq($qb->getDefaultSelectAlias() . '.level', $qb->createNamedParameter($role)));
 		}
 
-		$qb->orderBy($qb->getDefaultSelectAlias() . '.level', 'desc');
+		$qb->orderByMemberLevel($qb->getDefaultSelectAlias() . '.level');
 		$qb->addOrderBy($qb->getDefaultSelectAlias() . '.cached_name', 'asc');
 
 		return $this->getItemsFromRequest($qb);

lib/Db/MembershipRequest.php

@@ -119,6 +119,10 @@ public function getInherited(string $singleId, int $level = 0): array {
 		$qb->limitToCircleId($singleId);
 		$qb->leftJoinCircleConfig(self::TABLE_MEMBERSHIP);
 
+		if ($level === Member::LEVEL_VISITOR) {
+			// Member ranks above Visitor despite having a lower numeric value.
+			$level = Member::LEVEL_MEMBER;
+		}
 		if ($level > 1) {
 			$expr = $qb->expr();
 			$qb->andWhere($expr->gte('level', $qb->createNamedParameter($level, IQueryBuilder::PARAM_INT)));
@@ -137,6 +141,10 @@ public function getInheritedExcludingCircles(string $singleId, int $level = 0):
 		$qb = $this->getMembershipSelectSql();
 		$qb->limitToCircleId($singleId);
 
+		if ($level === Member::LEVEL_VISITOR) {
+			// Member ranks above Visitor despite having a lower numeric value.
+			$level = Member::LEVEL_MEMBER;
+		}
 		$expr = $qb->expr();
 		if ($level > 1) {
 			$qb->andWhere($expr->gte('level', $qb->createNamedParameter($level, IQueryBuilder::PARAM_INT)));

lib/FederatedItems/CircleConfig.php

@@ -10,13 +10,15 @@
 namespace OCA\Circles\FederatedItems;
 
 use OCA\Circles\Db\CircleRequest;
+use OCA\Circles\Db\MemberRequest;
 use OCA\Circles\Exceptions\FederatedItemBadRequestException;
 use OCA\Circles\Exceptions\FederatedItemException;
 use OCA\Circles\Exceptions\RequestBuilderException;
 use OCA\Circles\IFederatedItem;
 use OCA\Circles\Model\Circle;
 use OCA\Circles\Model\Federated\FederatedEvent;
 use OCA\Circles\Model\Helpers\MemberHelper;
+use OCA\Circles\Model\Member;
 use OCA\Circles\Service\ConfigService;
 use OCA\Circles\Service\PermissionService;
 use OCA\Circles\Tools\Traits\TDeserialize;
@@ -41,6 +43,7 @@ public function __construct(
 		private CircleRequest $circleRequest,
 		private PermissionService $permissionService,
 		private ConfigService $configService,
+		private MemberRequest $memberRequest,
 	) {
 	}
 
@@ -121,6 +124,14 @@ public function verify(FederatedEvent $event): void {
 			throw new FederatedItemBadRequestException('Configuration value is not valid');
 		}
 
+		if (!$circle->isConfig(Circle::CFG_FEDERATED)
+			&& $circle->isConfig(Circle::CFG_FEDERATED, $config)
+			&& $this->memberRequest->getMembers(
+				$circle->getSingleId(), limit: 1, role: Member::LEVEL_VISITOR
+			) !== []) {
+			throw new FederatedItemBadRequestException('Remove Visitor roles before enabling federation');
+		}
+
 		$new = clone $circle;
 		$new->setConfig($config);
 		$this->permissionService->confirmAllowedCircleTypes($new, $circle);

lib/FederatedItems/CircleLeave.php

@@ -185,9 +185,10 @@ private function selectNewOwner(Circle $circle): Member {
 				continue;
 			}
 
-			if ($member->getLevel() > $newOwner->getLevel()) {
+			$levelComparison = Member::compareLevels($member->getLevel(), $newOwner->getLevel());
+			if ($levelComparison > 0) {
 				$newOwner = $member;
-			} elseif ($member->getLevel() === $newOwner->getLevel()
+			} elseif ($levelComparison === 0
 					  && ($member->getJoined() < $newOwner->getJoined()
 						  || ($this->configService->isLocalInstance($member->getInstance())
 							  && !$this->configService->isLocalInstance($newOwner->getInstance()))

lib/FederatedItems/MassiveMemberAdd.php

@@ -37,7 +37,8 @@ public function verify(FederatedEvent $event): void {
 		$initiator = $circle->getInitiator();
 
 		$initiatorHelper = new MemberHelper($initiator);
-		if (!$circle->isConfig(Circle::CFG_FRIEND)) {
+		if (!$circle->isConfig(Circle::CFG_FRIEND)
+			|| $initiator->getLevel() === Member::LEVEL_VISITOR) {
 			$initiatorHelper->mustBeModerator();
 		}
 

lib/FederatedItems/MemberLevel.php

@@ -13,6 +13,7 @@
 use OCA\Circles\Exceptions\FederatedItemBadRequestException;
 use OCA\Circles\Exceptions\FederatedItemException;
 use OCA\Circles\Exceptions\MemberLevelException;
+use OCA\Circles\Exceptions\ParseMemberLevelException;
 use OCA\Circles\Exceptions\RequestBuilderException;
 use OCA\Circles\IFederatedItem;
 use OCA\Circles\IFederatedItemHighSeverity;
@@ -70,11 +71,18 @@ public function verify(FederatedEvent $event): void {
 			throw new FederatedItemBadRequestException('This level cannot be edited');
 		}
 
-		if ($level === 0) {
-			// TODO check all level
+		try {
+			$level = Member::parseLevelInt($level);
+		} catch (ParseMemberLevelException) {
 			throw new FederatedItemBadRequestException('invalid level');
 		}
 
+		if ($level === Member::LEVEL_VISITOR
+			&& ($circle->isConfig(Circle::CFG_FEDERATED)
+				|| !$this->configService->isLocalInstance($member->getInstance()))) {
+			throw new FederatedItemBadRequestException('Visitor requires a local team and member');
+		}
+
 		if ($member->getLevel() === $level) {
 			throw new FederatedItemBadRequestException('This member already have the selected level');
 		}
@@ -137,7 +145,7 @@ private function verifyMemberLevel(Member $member, Member $initiator, int $level
 		$initiatorHelper = new MemberHelper($initiator);
 		$memberHelper = new MemberHelper($member);
 
-		$memberHelper->mustBeMember();
+		$memberHelper->mustHaveLevelAboveOrEqual(Member::LEVEL_VISITOR);
 		$memberHelper->cannotBeOwner();
 		$initiatorHelper->mustBeModerator();
 
@@ -167,7 +175,7 @@ private function verifySwitchOwner(Member $member, Member $initiator) {
 		$memberHelper = new MemberHelper($member);
 
 		$initiatorHelper->mustBeOwner();
-		$memberHelper->mustBeMember();
+		$memberHelper->mustHaveLevelAboveOrEqual(Member::LEVEL_VISITOR);
 		$memberHelper->cannotBeOwner();
 	}
 }

lib/FederatedItems/SingleMemberAdd.php

@@ -104,7 +104,8 @@ public function verify(FederatedEvent $event): void {
 		$initiator = $circle->getInitiator();
 
 		$initiatorHelper = new MemberHelper($initiator);
-		if (!$circle->isConfig(Circle::CFG_FRIEND)) {
+		if (!$circle->isConfig(Circle::CFG_FRIEND)
+			|| $initiator->getLevel() === Member::LEVEL_VISITOR) {
 			$initiatorHelper->mustBeModerator();
 		}
 

lib/Model/BaseMember.php

@@ -20,6 +20,7 @@
 class BaseMember implements JsonSerializable {
 	public const LEVEL_NONE = 0;
 	public const LEVEL_MEMBER = 1;
+	public const LEVEL_VISITOR = 2;
 	public const LEVEL_MODERATOR = 4;
 	public const LEVEL_ADMIN = 8;
 	public const LEVEL_OWNER = 9;
@@ -386,6 +387,7 @@ public function getLevelString() {
 		return match ($this->getLevel()) {
 			self::LEVEL_NONE => 'Not a member',
 			self::LEVEL_MEMBER => 'Member',
+			self::LEVEL_VISITOR => 'Visitor',
 			self::LEVEL_MODERATOR => 'Moderator',
 			self::LEVEL_ADMIN => 'Admin',
 			self::LEVEL_OWNER => 'Owner',

lib/Model/Helpers/MemberHelper.php

@@ -65,6 +65,8 @@ public function __call(string $name, array $arguments): void {
 	}
 
 	/**
+	 * Rank-based: unlike CircleProbe::mustBeMember(), mustBeMember() here rejects Visitors.
+	 *
 	 * @param string $levelString
 	 * @param array $arguments
 	 *
@@ -95,7 +97,7 @@ private function cannotBe(string $levelString, array $arguments): void {
 			throw new MemberHelperException('method ' . $levelString . ' not found');
 		}
 
-		if ($this->member->getLevel() >= $level) {
+		if (Member::compareLevels($this->member->getLevel(), $level) >= 0) {
 			throw new MemberLevelException('Member cannot be ' . $levelString);
 		}
 	}
@@ -106,7 +108,7 @@ private function cannotBe(string $levelString, array $arguments): void {
 	 * @throws MemberLevelException
 	 */
 	public function mustHaveLevelAbove(int $level) {
-		if ($this->member->getLevel() <= $level) {
+		if (Member::compareLevels($this->member->getLevel(), $level) <= 0) {
 			throw new MemberLevelException('Insufficient rights');
 		}
 	}
@@ -117,7 +119,7 @@ public function mustHaveLevelAbove(int $level) {
 	 * @throws MemberLevelException
 	 */
 	public function mustHaveLevelAboveOrEqual(int $level) {
-		if ($this->member->getLevel() < $level) {
+		if (Member::compareLevels($this->member->getLevel(), $level) < 0) {
 			throw new MemberLevelException('Insufficient rights');
 		}
 	}
@@ -128,7 +130,7 @@ public function mustHaveLevelAboveOrEqual(int $level) {
 	 * @throws MemberLevelException
 	 */
 	public function mustHaveLevelEqualOrAbove(int $level) {
-		if ($this->member->getLevel() < $level) {
+		if (Member::compareLevels($this->member->getLevel(), $level) < 0) {
 			throw new MemberLevelException('Insufficient rights');
 		}
 	}