From 0574535f53b94e39025cbffe09967a5d0706326d Mon Sep 17 00:00:00 2001 From: Chen Asraf Date: Sun, 1 Mar 2026 22:38:31 +0200 Subject: [PATCH] feat: team-based permissions --- lib/Controller/CategoryController.php | 69 +-- lib/Controller/RoleController.php | 3 +- lib/Controller/TeamController.php | 179 +++++++ lib/Db/CategoryMapper.php | 21 +- lib/Db/CategoryPerm.php | 18 +- lib/Db/CategoryPermMapper.php | 134 ++++- lib/Migration/SeedHelper.php | 24 +- lib/Migration/Version20Date20260301000000.php | 103 ++++ lib/Migration/Version21Date20260301000001.php | 66 +++ lib/Service/PermissionService.php | 70 ++- openapi-full.json | 361 ++++++++++++- openapi.json | 361 ++++++++++++- .../AppNavigation/AppNavigation.vue | 2 +- .../CategoryPermissionsTable.test.ts | 475 ++++++++++++++++++ .../CategoryPermissionsTable.vue | 277 ++++++++++ .../CategoryPermissionsTable/index.ts | 3 + src/router.ts | 4 + src/types/models.ts | 18 + src/views/admin/AdminCategoryEdit.vue | 135 +++-- src/views/admin/AdminRoleEdit.vue | 242 +-------- src/views/admin/AdminRoleList.vue | 101 +++- src/views/admin/AdminTeamEdit.vue | 266 ++++++++++ .../Controller/CategoryControllerTest.php | 48 +- tests/unit/Controller/RoleControllerTest.php | 12 +- tests/unit/Controller/TeamControllerTest.php | 179 +++++++ tests/unit/Service/PermissionServiceTest.php | 8 +- 26 files changed, 2752 insertions(+), 427 deletions(-) create mode 100644 lib/Controller/TeamController.php create mode 100644 lib/Migration/Version20Date20260301000000.php create mode 100644 lib/Migration/Version21Date20260301000001.php create mode 100644 src/components/CategoryPermissionsTable/CategoryPermissionsTable.test.ts create mode 100644 src/components/CategoryPermissionsTable/CategoryPermissionsTable.vue create mode 100644 src/components/CategoryPermissionsTable/index.ts create mode 100644 src/views/admin/AdminTeamEdit.vue create mode 100644 tests/unit/Controller/TeamControllerTest.php diff --git a/lib/Controller/CategoryController.php b/lib/Controller/CategoryController.php index 326a0dc..1306001 100644 --- a/lib/Controller/CategoryController.php +++ b/lib/Controller/CategoryController.php @@ -23,7 +23,6 @@ use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCSController; -use OCP\IGroupManager; use OCP\IRequest; use OCP\IUserSession; use Psr\Log\LoggerInterface; @@ -39,7 +38,6 @@ class CategoryController extends OCSController { private ReadMarkerMapper $readMarkerMapper, private RoleMapper $roleMapper, private IUserSession $userSession, - private IGroupManager $groupManager, private LoggerInterface $logger, ) { parent::__construct($appName, $request); @@ -345,54 +343,33 @@ class CategoryController extends OCSController { return new DataResponse(['hasPermission' => false]); } - // Check if user is in admin group - admins have all permissions - $adminGroup = $this->groupManager->get('admin'); - if ($adminGroup && $adminGroup->inGroup($user)) { - return new DataResponse(['hasPermission' => true]); - } + $getter = 'get' . ucfirst($permission); - // Get user's roles + // Check role-based permissions $roles = $this->roleMapper->findByUserId($user->getUID()); $roleIds = array_map(fn ($role) => $role->getId(), $roles); - if (empty($roleIds)) { - return new DataResponse(['hasPermission' => false]); - } - - // Get category permissions for user's roles - $categoryPerms = $this->categoryPermMapper->findByCategoryAndRoles($id, $roleIds); - - // Check if any role has the requested permission - $hasPermission = false; - foreach ($categoryPerms as $perm) { - switch ($permission) { - case 'canView': - if ($perm->getCanView()) { - $hasPermission = true; - } - break; - case 'canPost': - if ($perm->getCanPost()) { - $hasPermission = true; - } - break; - case 'canReply': - if ($perm->getCanReply()) { - $hasPermission = true; - } - break; - case 'canModerate': - if ($perm->getCanModerate()) { - $hasPermission = true; - } - break; + if (!empty($roleIds)) { + // Admin role has all permissions + foreach ($roles as $role) { + if ($role->getRoleType() === Role::ROLE_TYPE_ADMIN) { + return new DataResponse(['hasPermission' => true]); + } } - if ($hasPermission) { - break; + + $categoryPerms = $this->categoryPermMapper->findByCategoryAndRoles($id, $roleIds); + foreach ($categoryPerms as $perm) { + try { + if ($perm->$getter()) { + return new DataResponse(['hasPermission' => true]); + } + } catch (\BadMethodCallException $e) { + break; + } } } - return new DataResponse(['hasPermission' => $hasPermission]); + return new DataResponse(['hasPermission' => false]); } catch (\Exception $e) { $this->logger->error("Error checking permission {$permission} for category {$id}: " . $e->getMessage()); return new DataResponse(['hasPermission' => false]); @@ -425,7 +402,7 @@ class CategoryController extends OCSController { * Update permissions for a category * * @param int $id Category ID - * @param list $permissions Permissions array + * @param list $permissions Role permissions array * @return DataResponse * * 200: Permissions updated @@ -455,14 +432,14 @@ class CategoryController extends OCSController { } }); - // Insert new permissions + // Insert role permissions foreach ($filteredPermissions as $perm) { $categoryPerm = new CategoryPerm(); $categoryPerm->setCategoryId($id); - $categoryPerm->setRoleId($perm['roleId']); + $categoryPerm->setTargetType(CategoryPerm::TARGET_TYPE_ROLE); + $categoryPerm->setTargetId((string)$perm['roleId']); $categoryPerm->setCanView($perm['canView'] ?? false); // canPost and canReply default to canView value - // This ensures that if a role can view a category, they can also post/reply unless explicitly restricted $categoryPerm->setCanPost($perm['canView'] ?? false); $categoryPerm->setCanReply($perm['canView'] ?? false); diff --git a/lib/Controller/RoleController.php b/lib/Controller/RoleController.php index e89701a..1b63243 100644 --- a/lib/Controller/RoleController.php +++ b/lib/Controller/RoleController.php @@ -277,7 +277,8 @@ class RoleController extends OCSController { foreach ($permissions as $perm) { $categoryPerm = new CategoryPerm(); $categoryPerm->setCategoryId($perm['categoryId']); - $categoryPerm->setRoleId($id); + $categoryPerm->setTargetType(CategoryPerm::TARGET_TYPE_ROLE); + $categoryPerm->setTargetId((string)$id); $categoryPerm->setCanView($perm['canView'] ?? false); // canPost and canReply default to canView value // This ensures that if a role can view a category, they can also post/reply unless explicitly restricted diff --git a/lib/Controller/TeamController.php b/lib/Controller/TeamController.php new file mode 100644 index 0000000..4bb8b36 --- /dev/null +++ b/lib/Controller/TeamController.php @@ -0,0 +1,179 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace OCA\Forum\Controller; + +use OCA\Forum\Attribute\RequirePermission; +use OCA\Forum\Db\CategoryPerm; +use OCA\Forum\Db\CategoryPermMapper; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\ApiRoute; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCSController; +use OCP\IRequest; +use OCP\Server; +use Psr\Log\LoggerInterface; + +class TeamController extends OCSController { + public function __construct( + string $appName, + IRequest $request, + private CategoryPermMapper $categoryPermMapper, + private LoggerInterface $logger, + ) { + parent::__construct($appName, $request); + } + + /** + * Get the CirclesManager instance, or null if the Circles app is not available + */ + private function getCirclesManager(): ?\OCA\Circles\CirclesManager { + if (!class_exists(\OCA\Circles\CirclesManager::class)) { + return null; + } + try { + return Server::get(\OCA\Circles\CirclesManager::class); + } catch (\Exception $e) { + return null; + } + } + + /** + * List all available teams (circles) + * + * @return DataResponse, array{}> + * + * 200: Teams returned + */ + #[NoAdminRequired] + #[RequirePermission('canAccessAdminTools')] + #[ApiRoute(verb: 'GET', url: '/api/teams')] + public function index(): DataResponse { + $circlesManager = $this->getCirclesManager(); + if ($circlesManager === null) { + return new DataResponse(['error' => 'Teams app is not available'], Http::STATUS_SERVICE_UNAVAILABLE); + } + + try { + $circlesManager->startSuperSession(); + + $probe = new \OCA\Circles\Model\Probes\CircleProbe(); + $probe->filterHiddenCircles() + ->filterBackendCircles() + ->filterPersonalCircles() + ->filterSingleCircles(); + + $circles = $circlesManager->getCircles($probe); + + $result = array_map(function ($circle) { + $owner = ''; + $ownerDisplayName = ''; + if ($circle->hasOwner()) { + $owner = $circle->getOwner()->getUserId(); + $ownerDisplayName = $circle->getOwner()->getDisplayName(); + } + return [ + 'id' => $circle->getSingleId(), + 'displayName' => $circle->getDisplayName() ?: $circle->getName(), + 'owner' => $owner, + 'ownerDisplayName' => $ownerDisplayName, + ]; + }, $circles); + + // Sort by owner display name, then by team display name + usort($result, function ($a, $b) { + $ownerCmp = strcasecmp($a['ownerDisplayName'], $b['ownerDisplayName']); + if ($ownerCmp !== 0) { + return $ownerCmp; + } + return strcasecmp($a['displayName'], $b['displayName']); + }); + + return new DataResponse(array_values($result)); + } catch (\Exception $e) { + $this->logger->error('Error fetching teams: ' . $e->getMessage()); + return new DataResponse(['error' => 'Failed to fetch teams'], Http::STATUS_INTERNAL_SERVER_ERROR); + } finally { + $circlesManager->stopSession(); + } + } + + /** + * Get category permissions for a team (circle) + * + * @param string $id Team/circle single ID + * @return DataResponse>, array{}> + * + * 200: Permissions returned + */ + #[NoAdminRequired] + #[RequirePermission('canAccessAdminTools')] + #[ApiRoute(verb: 'GET', url: '/api/teams/{id}/permissions')] + public function getPermissions(string $id): DataResponse { + try { + $permissions = $this->categoryPermMapper->findByTeamId($id); + return new DataResponse(array_map(fn ($perm) => $perm->jsonSerialize(), $permissions)); + } catch (\Exception $e) { + $this->logger->error('Error fetching team permissions: ' . $e->getMessage()); + return new DataResponse(['error' => 'Failed to fetch permissions'], Http::STATUS_INTERNAL_SERVER_ERROR); + } + } + + /** + * Update category permissions for a team (circle) + * + * @param string $id Team/circle single ID + * @param list $permissions Permissions array + * @return DataResponse + * + * 200: Permissions updated + */ + #[NoAdminRequired] + #[RequirePermission('canEditCategories')] + #[ApiRoute(verb: 'POST', url: '/api/teams/{id}/permissions')] + public function updatePermissions(string $id, array $permissions): DataResponse { + $circlesManager = $this->getCirclesManager(); + if ($circlesManager === null) { + return new DataResponse(['error' => 'Teams app is not available'], Http::STATUS_SERVICE_UNAVAILABLE); + } + + try { + // Verify team exists + $circlesManager->startSuperSession(); + try { + $circlesManager->getCircle($id); + } catch (\OCA\Circles\Exceptions\CircleNotFoundException $e) { + return new DataResponse(['error' => 'Team not found'], Http::STATUS_NOT_FOUND); + } finally { + $circlesManager->stopSession(); + } + + // Delete existing permissions for this team + $this->categoryPermMapper->deleteByTeamId($id); + + // Insert new permissions + foreach ($permissions as $perm) { + $categoryPerm = new CategoryPerm(); + $categoryPerm->setCategoryId($perm['categoryId']); + $categoryPerm->setTargetType(CategoryPerm::TARGET_TYPE_TEAM); + $categoryPerm->setTargetId($id); + $categoryPerm->setCanView($perm['canView'] ?? false); + $categoryPerm->setCanPost($perm['canView'] ?? false); + $categoryPerm->setCanReply($perm['canView'] ?? false); + $categoryPerm->setCanModerate($perm['canModerate'] ?? false); + + $this->categoryPermMapper->insert($categoryPerm); + } + + return new DataResponse(['success' => true]); + } catch (\Exception $e) { + $this->logger->error('Error updating team permissions: ' . $e->getMessage()); + return new DataResponse(['error' => 'Failed to update permissions'], Http::STATUS_INTERNAL_SERVER_ERROR); + } + } +} diff --git a/lib/Db/CategoryMapper.php b/lib/Db/CategoryMapper.php index 682f9f4..b038c16 100644 --- a/lib/Db/CategoryMapper.php +++ b/lib/Db/CategoryMapper.php @@ -126,7 +126,7 @@ class CategoryMapper extends QBMapper { // Get all permissions for these categories $qb = $this->db->getQueryBuilder(); - $qb->select('category_id', 'role_id', 'can_view') + $qb->select('category_id', 'target_type', 'target_id', 'can_view') ->from(Application::tableName('forum_category_perms')) ->where($qb->expr()->in('category_id', $qb->createNamedParameter($categoryIds, IQueryBuilder::PARAM_INT_ARRAY))); @@ -138,14 +138,17 @@ class CategoryMapper extends QBMapper { $permissions[$categoryId] = []; } $permissions[$categoryId][] = [ - 'role_id' => (int)$row['role_id'], + 'target_type' => $row['target_type'], + 'target_id' => $row['target_id'], 'can_view' => (bool)$row['can_view'], ]; } $result->closeCursor(); + $roleIdStrings = array_map('strval', $userRoleIds); + // Filter categories based on permissions - return array_values(array_filter($categories, function ($category) use ($permissions, $userRoleIds) { + return array_values(array_filter($categories, function ($category) use ($permissions, $roleIdStrings) { $categoryId = $category->getId(); // If no permissions exist for this category, it's public @@ -153,14 +156,14 @@ class CategoryMapper extends QBMapper { return true; } - // If user has no roles, they can't view restricted categories - if (empty($userRoleIds)) { - return false; - } - // Check if user has any role with can_view permission foreach ($permissions[$categoryId] as $perm) { - if (in_array($perm['role_id'], $userRoleIds) && $perm['can_view']) { + if (!$perm['can_view']) { + continue; + } + + if ($perm['target_type'] === CategoryPerm::TARGET_TYPE_ROLE + && in_array($perm['target_id'], $roleIdStrings, true)) { return true; } } diff --git a/lib/Db/CategoryPerm.php b/lib/Db/CategoryPerm.php index c814d8f..2f67602 100644 --- a/lib/Db/CategoryPerm.php +++ b/lib/Db/CategoryPerm.php @@ -16,8 +16,10 @@ use OCP\AppFramework\Db\Entity; * @method void setId(int $value) * @method int getCategoryId() * @method void setCategoryId(int $value) - * @method int getRoleId() - * @method void setRoleId(int $value) + * @method string getTargetType() + * @method void setTargetType(string $value) + * @method string getTargetId() + * @method void setTargetId(string $value) * @method bool getCanView() * @method void setCanView(bool $value) * @method bool getCanPost() @@ -28,8 +30,12 @@ use OCP\AppFramework\Db\Entity; * @method void setCanModerate(bool $value) */ class CategoryPerm extends Entity implements JsonSerializable { + public const TARGET_TYPE_ROLE = 'role'; + public const TARGET_TYPE_TEAM = 'team'; + protected $categoryId; - protected $roleId; + protected $targetType; + protected $targetId; protected $canView; protected $canPost; protected $canReply; @@ -38,7 +44,8 @@ class CategoryPerm extends Entity implements JsonSerializable { public function __construct() { $this->addType('id', 'integer'); $this->addType('categoryId', 'integer'); - $this->addType('roleId', 'integer'); + $this->addType('targetType', 'string'); + $this->addType('targetId', 'string'); $this->addType('canView', 'boolean'); $this->addType('canPost', 'boolean'); $this->addType('canReply', 'boolean'); @@ -49,7 +56,8 @@ class CategoryPerm extends Entity implements JsonSerializable { return [ 'id' => $this->getId(), 'categoryId' => $this->getCategoryId(), - 'roleId' => $this->getRoleId(), + 'targetType' => $this->getTargetType(), + 'targetId' => $this->getTargetId(), 'canView' => $this->getCanView(), 'canPost' => $this->getCanPost(), 'canReply' => $this->getCanReply(), diff --git a/lib/Db/CategoryPermMapper.php b/lib/Db/CategoryPermMapper.php index 9ad8d9d..f619cba 100644 --- a/lib/Db/CategoryPermMapper.php +++ b/lib/Db/CategoryPermMapper.php @@ -40,6 +40,27 @@ class CategoryPermMapper extends QBMapper { } /** + * Find permissions by team (circle) ID + * + * @return array + */ + public function findByTeamId(string $teamId): array { + /* @var $qb IQueryBuilder */ + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_TEAM, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter($teamId, IQueryBuilder::PARAM_STR)) + ); + return $this->findEntities($qb); + } + + /** + * Find permissions by role ID + * * @return array */ public function findByRoleId(int $roleId): array { @@ -48,7 +69,10 @@ class CategoryPermMapper extends QBMapper { $qb->select('*') ->from($this->getTableName()) ->where( - $qb->expr()->eq('role_id', $qb->createNamedParameter($roleId, IQueryBuilder::PARAM_INT)) + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_ROLE, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter((string)$roleId, IQueryBuilder::PARAM_STR)) ); return $this->findEntities($qb); } @@ -69,23 +93,34 @@ class CategoryPermMapper extends QBMapper { /** * Find permissions for a category, excluding Admin role (which has implicit full access) + * Returns both role-type and team-type permissions. * * @param int $categoryId Category ID * @return array */ public function findByCategoryIdExcludingAdmin(int $categoryId): array { - /* @var $qb IQueryBuilder */ + // Get all perms for this category + $allPerms = $this->findByCategoryId($categoryId); + + // Get admin role IDs to exclude $qb = $this->db->getQueryBuilder(); - $qb->select('cp.*') - ->from($this->getTableName(), 'cp') - ->innerJoin('cp', Application::tableName('forum_roles'), 'r', 'cp.role_id = r.id') - ->where( - $qb->expr()->eq('cp.category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) - ) - ->andWhere( - $qb->expr()->neq('r.role_type', $qb->createNamedParameter(Role::ROLE_TYPE_ADMIN, IQueryBuilder::PARAM_STR)) - ); - return $this->findEntities($qb); + $qb->select('id') + ->from(Application::tableName('forum_roles')) + ->where($qb->expr()->eq('role_type', $qb->createNamedParameter(Role::ROLE_TYPE_ADMIN, IQueryBuilder::PARAM_STR))); + $result = $qb->executeQuery(); + $adminRoleIds = []; + while ($row = $result->fetch()) { + $adminRoleIds[] = (string)$row['id']; + } + $result->closeCursor(); + + // Filter: include all team-type perms, exclude admin role-type perms + return array_values(array_filter($allPerms, function (CategoryPerm $perm) use ($adminRoleIds) { + if ($perm->getTargetType() === CategoryPerm::TARGET_TYPE_TEAM) { + return true; + } + return !in_array($perm->getTargetId(), $adminRoleIds, true); + })); } /** @@ -103,7 +138,10 @@ class CategoryPermMapper extends QBMapper { $qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) ) ->andWhere( - $qb->expr()->eq('role_id', $qb->createNamedParameter($roleId, IQueryBuilder::PARAM_INT)) + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_ROLE, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter((string)$roleId, IQueryBuilder::PARAM_STR)) ); return $this->findEntity($qb); } @@ -120,6 +158,8 @@ class CategoryPermMapper extends QBMapper { return []; } + $roleIdStrings = array_map('strval', $roleIds); + /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') @@ -128,11 +168,57 @@ class CategoryPermMapper extends QBMapper { $qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) ) ->andWhere( - $qb->expr()->in('role_id', $qb->createNamedParameter($roleIds, IQueryBuilder::PARAM_INT_ARRAY)) + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_ROLE, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->in('target_id', $qb->createNamedParameter($roleIdStrings, IQueryBuilder::PARAM_STR_ARRAY)) ); return $this->findEntities($qb); } + /** + * Find permissions for specific category and multiple team (circle) IDs + * + * @param int $categoryId Category ID + * @param array $teamIds Array of team/circle IDs + * @return array + */ + public function findByCategoryAndTeamIds(int $categoryId, array $teamIds): array { + if (empty($teamIds)) { + return []; + } + + /* @var $qb IQueryBuilder */ + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) + ) + ->andWhere( + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_TEAM, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->in('target_id', $qb->createNamedParameter($teamIds, IQueryBuilder::PARAM_STR_ARRAY)) + ); + return $this->findEntities($qb); + } + + /** + * Delete all permissions for a team (circle) + */ + public function deleteByTeamId(string $teamId): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where( + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_TEAM, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter($teamId, IQueryBuilder::PARAM_STR)) + ); + $qb->executeStatement(); + } + /** * Delete all permissions for a role */ @@ -140,7 +226,25 @@ class CategoryPermMapper extends QBMapper { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) ->where( - $qb->expr()->eq('role_id', $qb->createNamedParameter($roleId, IQueryBuilder::PARAM_INT)) + $qb->expr()->eq('target_type', $qb->createNamedParameter(CategoryPerm::TARGET_TYPE_ROLE, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter((string)$roleId, IQueryBuilder::PARAM_STR)) + ); + $qb->executeStatement(); + } + + /** + * Delete permissions by target type and ID + */ + public function deleteByTargetTypeAndId(string $type, string $id): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where( + $qb->expr()->eq('target_type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->eq('target_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_STR)) ); $qb->executeStatement(); } diff --git a/lib/Migration/SeedHelper.php b/lib/Migration/SeedHelper.php index ef14792..4319c9f 100644 --- a/lib/Migration/SeedHelper.php +++ b/lib/Migration/SeedHelper.php @@ -510,7 +510,8 @@ class SeedHelper { $qb = $db->getQueryBuilder(); $qb->select('id') ->from('forum_category_perms') - ->where($qb->expr()->eq('role_id', $qb->createNamedParameter($guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))) + ->where($qb->expr()->eq('target_type', $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('target_id', $qb->createNamedParameter((string)$guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) ->setMaxResults(1); $result = $qb->executeQuery(); $hasPermissions = $result->fetch(); @@ -532,7 +533,8 @@ class SeedHelper { $qb = $db->getQueryBuilder(); $qb->select('category_id') ->from('forum_category_perms') - ->where($qb->expr()->eq('role_id', $qb->createNamedParameter($userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))) + ->where($qb->expr()->eq('target_type', $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('target_id', $qb->createNamedParameter((string)$userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) ->andWhere($qb->expr()->eq('can_view', $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL))); $result = $qb->executeQuery(); $userAccessibleCategories = $result->fetchAll(); @@ -548,7 +550,8 @@ class SeedHelper { $qb->select('id') ->from('forum_category_perms') ->where($qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('role_id', $qb->createNamedParameter($guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('target_type', $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('target_id', $qb->createNamedParameter((string)$guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))); $result = $qb->executeQuery(); $permExists = $result->fetch(); $result->closeCursor(); @@ -559,7 +562,8 @@ class SeedHelper { $qb->insert('forum_category_perms') ->values([ 'category_id' => $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), - 'role_id' => $qb->createNamedParameter($guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), + 'target_type' => $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), + 'target_id' => $qb->createNamedParameter((string)$guestRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), 'can_view' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_post' => $qb->createNamedParameter(false, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_reply' => $qb->createNamedParameter(false, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), @@ -837,7 +841,8 @@ class SeedHelper { $qb->select('id') ->from('forum_category_perms') ->where($qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('role_id', $qb->createNamedParameter($moderatorRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('target_type', $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('target_id', $qb->createNamedParameter((string)$moderatorRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))); $result = $qb->executeQuery(); $exists = $result->fetch(); $result->closeCursor(); @@ -848,7 +853,8 @@ class SeedHelper { $qb->insert('forum_category_perms') ->values([ 'category_id' => $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), - 'role_id' => $qb->createNamedParameter($moderatorRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), + 'target_type' => $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), + 'target_id' => $qb->createNamedParameter((string)$moderatorRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), 'can_view' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_post' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_reply' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), @@ -866,7 +872,8 @@ class SeedHelper { $qb->select('id') ->from('forum_category_perms') ->where($qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('role_id', $qb->createNamedParameter($userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->eq('target_type', $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('target_id', $qb->createNamedParameter((string)$userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR))); $result = $qb->executeQuery(); $exists = $result->fetch(); $result->closeCursor(); @@ -877,7 +884,8 @@ class SeedHelper { $qb->insert('forum_category_perms') ->values([ 'category_id' => $qb->createNamedParameter($categoryId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), - 'role_id' => $qb->createNamedParameter($userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT), + 'target_type' => $qb->createNamedParameter('role', \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), + 'target_id' => $qb->createNamedParameter((string)$userRoleId, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_STR), 'can_view' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_post' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), 'can_reply' => $qb->createNamedParameter(true, \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_BOOL), diff --git a/lib/Migration/Version20Date20260301000000.php b/lib/Migration/Version20Date20260301000000.php new file mode 100644 index 0000000..b03f98c --- /dev/null +++ b/lib/Migration/Version20Date20260301000000.php @@ -0,0 +1,103 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace OCA\Forum\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Version 20 Migration: + * - Add target_type and target_id columns to forum_category_perms + * - Copy role_id values to target_id as strings + * - Drop old indexes + */ +class Version20Date20260301000000 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $db, + ) { + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return ISchemaWrapper|null + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('forum_category_perms')) { + return null; + } + + $table = $schema->getTable('forum_category_perms'); + + // Add target_type column + if (!$table->hasColumn('target_type')) { + $output->info('Forum: Adding target_type column to forum_category_perms...'); + $table->addColumn('target_type', 'string', [ + 'notnull' => true, + 'length' => 16, + 'default' => 'role', + ]); + } + + // Add target_id column (nullable initially for migration) + if (!$table->hasColumn('target_id')) { + $output->info('Forum: Adding target_id column to forum_category_perms...'); + $table->addColumn('target_id', 'string', [ + 'notnull' => false, + 'length' => 256, + ]); + } + + // Drop old unique index + if ($table->hasIndex('forum_cat_perms_unique_idx')) { + $output->info('Forum: Dropping old unique index forum_cat_perms_unique_idx...'); + $table->dropIndex('forum_cat_perms_unique_idx'); + } + + // Drop old role_id index + if ($table->hasIndex('forum_cat_perms_role_idx')) { + $output->info('Forum: Dropping old index forum_cat_perms_role_idx...'); + $table->dropIndex('forum_cat_perms_role_idx'); + } + + return $schema; + } + + /** + * Copy role_id values to target_id as strings + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $output->info('Forum: Copying role_id values to target_id...'); + + // Select all rows and update each one, converting int role_id to string target_id in PHP + // This is cross-platform safe (works on MySQL, PostgreSQL, SQLite) + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'role_id') + ->from('forum_category_perms'); + $result = $qb->executeQuery(); + + while ($row = $result->fetch()) { + $update = $this->db->getQueryBuilder(); + $update->update('forum_category_perms') + ->set('target_type', $update->createNamedParameter('role')) + ->set('target_id', $update->createNamedParameter((string)$row['role_id'])) + ->where($update->expr()->eq('id', $update->createNamedParameter((int)$row['id'], \OCP\DB\QueryBuilder\IQueryBuilder::PARAM_INT))); + $update->executeStatement(); + } + $result->closeCursor(); + + $output->info('Forum: role_id values copied to target_id successfully.'); + } +} diff --git a/lib/Migration/Version21Date20260301000001.php b/lib/Migration/Version21Date20260301000001.php new file mode 100644 index 0000000..94686bf --- /dev/null +++ b/lib/Migration/Version21Date20260301000001.php @@ -0,0 +1,66 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace OCA\Forum\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Version 21 Migration (runs after data migration in Version 20): + * - Make target_id not null + * - Drop role_id column + * - Create new indexes for (category_id, target_type, target_id) + */ +class Version21Date20260301000001 extends SimpleMigrationStep { + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return ISchemaWrapper|null + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + if (!$schema->hasTable('forum_category_perms')) { + return null; + } + + $table = $schema->getTable('forum_category_perms'); + + // Make target_id not null + if ($table->hasColumn('target_id')) { + $output->info('Forum: Making target_id column not null...'); + $column = $table->getColumn('target_id'); + $column->setNotnull(true); + $column->setDefault(''); + } + + // Drop role_id column + if ($table->hasColumn('role_id')) { + $output->info('Forum: Dropping role_id column from forum_category_perms...'); + $table->dropColumn('role_id'); + } + + // Add unique index on (category_id, target_type, target_id) + if (!$table->hasIndex('forum_cat_perms_uniq_idx')) { + $output->info('Forum: Adding unique index forum_cat_perms_uniq_idx...'); + $table->addUniqueIndex(['category_id', 'target_type', 'target_id'], 'forum_cat_perms_uniq_idx'); + } + + // Add index on (target_type, target_id) + if (!$table->hasIndex('forum_cat_perms_target_idx')) { + $output->info('Forum: Adding index forum_cat_perms_target_idx...'); + $table->addIndex(['target_type', 'target_id'], 'forum_cat_perms_target_idx'); + } + + return $schema; + } +} diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index a29eec9..c232e33 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -15,6 +15,7 @@ use OCA\Forum\Db\RoleMapper; use OCA\Forum\Db\ThreadMapper; use OCA\Forum\Db\UserRoleMapper; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IUserManager; use Psr\Log\LoggerInterface; class PermissionService { @@ -25,6 +26,7 @@ class PermissionService { private CategoryMapper $categoryMapper, private ThreadMapper $threadMapper, private PostMapper $postMapper, + private IUserManager $userManager, private LoggerInterface $logger, ) { } @@ -143,6 +145,8 @@ class PermissionService { return true; } + $getter = 'get' . ucfirst($permission); + try { // Handle guest users (null userId) - use guest role if ($userId === null) { @@ -156,6 +160,7 @@ class PermissionService { $roles = $this->roleMapper->findByUserId($userId); } + // Check role-based permissions foreach ($roles as $role) { try { $perm = $this->categoryPermMapper->findByCategoryAndRole( @@ -163,9 +168,6 @@ class PermissionService { $role->getId() ); - // Check permission using getter method - // Note: Nextcloud Entity uses magic methods, so we call directly without method_exists check - $getter = 'get' . ucfirst($permission); try { if ($perm->$getter()) { return true; @@ -180,6 +182,13 @@ class PermissionService { } } + // Check team/circle-based permissions (only for authenticated users) + if ($userId !== null) { + if ($this->hasTeamCategoryPermission($userId, $categoryId, $getter)) { + return true; + } + } + return false; } catch (\Exception $e) { $this->logger->error("Error checking category permission '$permission' on category $categoryId: " . $e->getMessage()); @@ -287,4 +296,59 @@ class PermissionService { return []; } } + + /** + * Check if user has a specific permission on a category via team (circle) membership + * + * @param string $userId Nextcloud user ID + * @param int $categoryId Category ID + * @param string $getter Getter method name (e.g., 'getCanView') + * @return bool True if user has the permission via a team + */ + private function hasTeamCategoryPermission(string $userId, int $categoryId, string $getter): bool { + try { + if (!class_exists(\OCA\Circles\CirclesManager::class)) { + return false; + } + + $circlesManager = \OCP\Server::get(\OCA\Circles\CirclesManager::class); + + $federatedUser = $circlesManager->getFederatedUser($userId, \OCA\Circles\Model\Member::TYPE_USER); + $circlesManager->startSession($federatedUser); + + try { + $probe = new \OCA\Circles\Model\Probes\CircleProbe(); + $probe->mustBeMember() + ->filterHiddenCircles() + ->filterBackendCircles() + ->filterPersonalCircles() + ->filterSingleCircles(); + + $circles = $circlesManager->getCircles($probe); + $circleIds = array_map(fn ($c) => $c->getSingleId(), $circles); + + if (empty($circleIds)) { + return false; + } + + $teamPerms = $this->categoryPermMapper->findByCategoryAndTeamIds($categoryId, $circleIds); + foreach ($teamPerms as $perm) { + try { + if ($perm->$getter()) { + return true; + } + } catch (\BadMethodCallException $e) { + continue; + } + } + } finally { + $circlesManager->stopSession(); + } + } catch (\Exception $e) { + // Circles app not available or other error - skip team permission check + $this->logger->debug('Team permission check skipped: ' . $e->getMessage()); + } + + return false; + } } diff --git a/openapi-full.json b/openapi-full.json index 36d532a..5daecdc 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -3467,7 +3467,7 @@ "properties": { "permissions": { "type": "array", - "description": "Permissions array", + "description": "Role permissions array", "items": { "type": "object", "required": [ @@ -7788,6 +7788,365 @@ } } }, + "/ocs/v2.php/apps/forum/api/teams": { + "get": { + "operationId": "team-index", + "summary": "List all available teams (circles)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Teams returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "displayName", + "owner", + "ownerDisplayName" + ], + "properties": { + "id": { + "type": "string" + }, + "displayName": { + "type": "string" + }, + "owner": { + "type": "string" + }, + "ownerDisplayName": { + "type": "string" + } + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, + "/ocs/v2.php/apps/forum/api/teams/{id}/permissions": { + "get": { + "operationId": "team-get-permissions", + "summary": "Get category permissions for a team (circle)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "id", + "in": "path", + "description": "Team/circle single ID", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Permissions returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + }, + "post": { + "operationId": "team-update-permissions", + "summary": "Update category permissions for a team (circle)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "permissions" + ], + "properties": { + "permissions": { + "type": "array", + "description": "Permissions array", + "items": { + "type": "object", + "required": [ + "categoryId", + "canView", + "canModerate" + ], + "properties": { + "categoryId": { + "type": "integer", + "format": "int64" + }, + "canView": { + "type": "boolean" + }, + "canModerate": { + "type": "boolean" + } + } + } + } + } + } + } + } + }, + "parameters": [ + { + "name": "id", + "in": "path", + "description": "Team/circle single ID", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Permissions updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "success" + ], + "properties": { + "success": { + "type": "boolean" + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/forum/api/threads": { "get": { "operationId": "thread-index", diff --git a/openapi.json b/openapi.json index 82234ae..2b109ff 100644 --- a/openapi.json +++ b/openapi.json @@ -3467,7 +3467,7 @@ "properties": { "permissions": { "type": "array", - "description": "Permissions array", + "description": "Role permissions array", "items": { "type": "object", "required": [ @@ -7788,6 +7788,365 @@ } } }, + "/ocs/v2.php/apps/forum/api/teams": { + "get": { + "operationId": "team-index", + "summary": "List all available teams (circles)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Teams returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "type": "object", + "required": [ + "id", + "displayName", + "owner", + "ownerDisplayName" + ], + "properties": { + "id": { + "type": "string" + }, + "displayName": { + "type": "string" + }, + "owner": { + "type": "string" + }, + "ownerDisplayName": { + "type": "string" + } + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, + "/ocs/v2.php/apps/forum/api/teams/{id}/permissions": { + "get": { + "operationId": "team-get-permissions", + "summary": "Get category permissions for a team (circle)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "parameters": [ + { + "name": "id", + "in": "path", + "description": "Team/circle single ID", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Permissions returned", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + }, + "post": { + "operationId": "team-update-permissions", + "summary": "Update category permissions for a team (circle)", + "tags": [ + "team" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "permissions" + ], + "properties": { + "permissions": { + "type": "array", + "description": "Permissions array", + "items": { + "type": "object", + "required": [ + "categoryId", + "canView", + "canModerate" + ], + "properties": { + "categoryId": { + "type": "integer", + "format": "int64" + }, + "canView": { + "type": "boolean" + }, + "canModerate": { + "type": "boolean" + } + } + } + } + } + } + } + } + }, + "parameters": [ + { + "name": "id", + "in": "path", + "description": "Team/circle single ID", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Permissions updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "required": [ + "success" + ], + "properties": { + "success": { + "type": "boolean" + } + } + } + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/forum/api/threads": { "get": { "operationId": "thread-index", diff --git a/src/components/AppNavigation/AppNavigation.vue b/src/components/AppNavigation/AppNavigation.vue index b807cf0..e4ab5d5 100644 --- a/src/components/AppNavigation/AppNavigation.vue +++ b/src/components/AppNavigation/AppNavigation.vue @@ -275,7 +275,7 @@ export default defineComponent({ navAdminDashboard: t('forum', 'Dashboard'), navAdminSettings: t('forum', 'Forum settings'), navAdminUsers: t('forum', 'Users'), - navAdminRoles: t('forum', 'Roles'), + navAdminRoles: t('forum', 'Roles & Teams'), navAdminCategories: t('forum', 'Categories'), navAdminBBCodes: t('forum', 'BBCodes'), expand: t('forum', 'Expand'), diff --git a/src/components/CategoryPermissionsTable/CategoryPermissionsTable.test.ts b/src/components/CategoryPermissionsTable/CategoryPermissionsTable.test.ts new file mode 100644 index 0000000..1d724d3 --- /dev/null +++ b/src/components/CategoryPermissionsTable/CategoryPermissionsTable.test.ts @@ -0,0 +1,475 @@ +import { describe, it, expect, vi } from 'vitest' +import { mount } from '@vue/test-utils' +import CategoryPermissionsTable from './CategoryPermissionsTable.vue' +import type { CategoryPermission } from './CategoryPermissionsTable.vue' +import type { CategoryHeader } from '@/types' + +vi.mock('@nextcloud/vue/components/NcCheckboxRadioSwitch', () => ({ + default: { + name: 'NcCheckboxRadioSwitch', + template: + '', + props: ['modelValue', 'disabled', 'indeterminate'], + emits: ['update:model-value'], + }, +})) + +function createHeaders(): CategoryHeader[] { + return [ + { + id: 1, + name: 'General', + description: null, + sortOrder: 0, + createdAt: 0, + categories: [ + { + id: 10, + headerId: 1, + name: 'Announcements', + description: 'Important announcements', + slug: 'announcements', + sortOrder: 0, + threadCount: 5, + postCount: 20, + createdAt: 0, + updatedAt: 0, + }, + { + id: 11, + headerId: 1, + name: 'Off-topic', + description: null, + slug: 'off-topic', + sortOrder: 1, + threadCount: 3, + postCount: 10, + createdAt: 0, + updatedAt: 0, + }, + ], + }, + { + id: 2, + name: 'Support', + description: null, + sortOrder: 1, + createdAt: 0, + categories: [ + { + id: 20, + headerId: 2, + name: 'Bug reports', + description: null, + slug: 'bug-reports', + sortOrder: 0, + threadCount: 8, + postCount: 30, + createdAt: 0, + updatedAt: 0, + }, + ], + }, + ] +} + +function createPermissions(): Record { + return { + 10: { canView: true, canModerate: false }, + 11: { canView: false, canModerate: false }, + 20: { canView: true, canModerate: true }, + } +} + +describe('CategoryPermissionsTable', () => { + describe('rendering', () => { + it('should render the permissions table when categories exist', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + expect(wrapper.find('.permissions-table').exists()).toBe(true) + }) + + it('should show empty message when no categories', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: [], + permissions: {}, + }, + }) + expect(wrapper.find('.permissions-table').exists()).toBe(false) + expect(wrapper.text()).toContain('No categories available') + }) + + it('should render header names', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + const headerNames = wrapper.findAll('.header-name') + expect(headerNames).toHaveLength(2) + expect(headerNames[0].text()).toBe('General') + expect(headerNames[1].text()).toBe('Support') + }) + + it('should render category names', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + const categoryNames = wrapper.findAll('.category-name') + expect(categoryNames).toHaveLength(3) + expect(categoryNames[0].text()).toBe('Announcements') + expect(categoryNames[1].text()).toBe('Off-topic') + expect(categoryNames[2].text()).toBe('Bug reports') + }) + + it('should render category descriptions when present', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + const descriptions = wrapper.findAll('.category-desc') + expect(descriptions).toHaveLength(1) + expect(descriptions[0].text()).toBe('Important announcements') + }) + + it('should render table column headers', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + const header = wrapper.find('.table-header') + expect(header.text()).toContain('Category') + expect(header.text()).toContain('Can view') + expect(header.text()).toContain('Can moderate') + }) + }) + + describe('checkbox states', () => { + it('should reflect individual category permissions', () => { + const permissions = createPermissions() + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + const checkboxes = wrapper.findAll('input[type="checkbox"]') + // 3 categories × 2 (view + moderate) + 2 headers × 2 (view + moderate) = 10 + expect(checkboxes).toHaveLength(10) + }) + }) + + describe('disabled states', () => { + it('should disable view checkboxes when disableView is true', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + disableView: true, + }, + }) + // Header view checkboxes and category view checkboxes should be disabled + const disabledLabels = wrapper.findAll('.nc-checkbox.disabled') + // 2 header view + 3 category view = 5 disabled checkboxes + expect(disabledLabels.length).toBe(5) + }) + + it('should disable moderate checkboxes when disableModerate is true', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + disableModerate: true, + }, + }) + const disabledLabels = wrapper.findAll('.nc-checkbox.disabled') + // 2 header moderate + 3 category moderate = 5 disabled checkboxes + expect(disabledLabels.length).toBe(5) + }) + + it('should disable all checkboxes when both disable props are true', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + disableView: true, + disableModerate: true, + }, + }) + const disabledLabels = wrapper.findAll('.nc-checkbox.disabled') + // All 10 checkboxes disabled + expect(disabledLabels.length).toBe(10) + }) + + it('should not disable any checkboxes by default', () => { + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions: createPermissions(), + }, + }) + const disabledLabels = wrapper.findAll('.nc-checkbox.disabled') + expect(disabledLabels.length).toBe(0) + }) + }) + + describe('category permission updates', () => { + it('should update canView when a category view checkbox is toggled', async () => { + const permissions = createPermissions() + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + // Category 11 (Off-topic) currently has canView=false + // Find the category rows, second row's first checkbox (view) + const rows = wrapper.findAll('.table-row') + const offTopicRow = rows[1] // Off-topic is second category row + const viewCheckbox = offTopicRow.findAll('.nc-checkbox')[0] + + await viewCheckbox.trigger('click') + + expect(permissions[11].canView).toBe(true) + expect(wrapper.emitted('update:permissions')).toBeTruthy() + }) + + it('should update canModerate when a category moderate checkbox is toggled', async () => { + const permissions = createPermissions() + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + // Category 10 (Announcements) currently has canModerate=false + const rows = wrapper.findAll('.table-row') + const announcementsRow = rows[0] + const moderateCheckbox = announcementsRow.findAll('.nc-checkbox')[1] + + await moderateCheckbox.trigger('click') + + expect(permissions[10].canModerate).toBe(true) + expect(wrapper.emitted('update:permissions')).toBeTruthy() + }) + }) + + describe('header toggle behavior', () => { + it('should check all categories in header when header view is toggled on', () => { + const permissions: Record = { + 10: { canView: false, canModerate: false }, + 11: { canView: false, canModerate: false }, + 20: { canView: false, canModerate: false }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + toggleHeaderView: (id: number) => void + } + const vm = wrapper.vm as unknown as VM + vm.toggleHeaderView(1) + + // Both categories under "General" should now have canView=true + expect(permissions[10].canView).toBe(true) + expect(permissions[11].canView).toBe(true) + // "Support" category should be unchanged + expect(permissions[20].canView).toBe(false) + }) + + it('should uncheck all categories in header when header view is toggled off', () => { + const permissions: Record = { + 10: { canView: true, canModerate: false }, + 11: { canView: true, canModerate: false }, + 20: { canView: true, canModerate: false }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + toggleHeaderView: (id: number) => void + } + const vm = wrapper.vm as unknown as VM + vm.toggleHeaderView(1) + + // Both categories under "General" should now have canView=false + expect(permissions[10].canView).toBe(false) + expect(permissions[11].canView).toBe(false) + // "Support" category should be unchanged + expect(permissions[20].canView).toBe(true) + }) + + it('should check all categories in header when header moderate is toggled on', () => { + const permissions: Record = { + 10: { canView: true, canModerate: false }, + 11: { canView: true, canModerate: false }, + 20: { canView: true, canModerate: false }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + toggleHeaderModerate: (id: number) => void + } + const vm = wrapper.vm as unknown as VM + vm.toggleHeaderModerate(1) + + expect(permissions[10].canModerate).toBe(true) + expect(permissions[11].canModerate).toBe(true) + expect(permissions[20].canModerate).toBe(false) + }) + }) + + describe('header state computation', () => { + it('should show indeterminate when some categories in header are checked', () => { + const permissions: Record = { + 10: { canView: true, canModerate: false }, + 11: { canView: false, canModerate: false }, + 20: { canView: true, canModerate: true }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + getHeaderViewState: (id: number) => { checked: boolean; indeterminate: boolean } + getHeaderModerateState: (id: number) => { checked: boolean; indeterminate: boolean } + } + const vm = wrapper.vm as unknown as VM + + // General header: 1/2 view checked → indeterminate + const generalView = vm.getHeaderViewState(1) + expect(generalView.checked).toBe(false) + expect(generalView.indeterminate).toBe(true) + + // General header: 0/2 moderate checked → not indeterminate + const generalModerate = vm.getHeaderModerateState(1) + expect(generalModerate.checked).toBe(false) + expect(generalModerate.indeterminate).toBe(false) + }) + + it('should show checked when all categories in header are checked', () => { + const permissions: Record = { + 10: { canView: true, canModerate: true }, + 11: { canView: true, canModerate: true }, + 20: { canView: false, canModerate: false }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + getHeaderViewState: (id: number) => { checked: boolean; indeterminate: boolean } + getHeaderModerateState: (id: number) => { checked: boolean; indeterminate: boolean } + } + const vm = wrapper.vm as unknown as VM + + // General header: 2/2 view checked → checked + const generalView = vm.getHeaderViewState(1) + expect(generalView.checked).toBe(true) + expect(generalView.indeterminate).toBe(false) + + // General header: 2/2 moderate checked → checked + const generalModerate = vm.getHeaderModerateState(1) + expect(generalModerate.checked).toBe(true) + expect(generalModerate.indeterminate).toBe(false) + }) + + it('should show unchecked when no categories in header are checked', () => { + const permissions: Record = { + 10: { canView: false, canModerate: false }, + 11: { canView: false, canModerate: false }, + 20: { canView: true, canModerate: true }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + getHeaderViewState: (id: number) => { checked: boolean; indeterminate: boolean } + } + const vm = wrapper.vm as unknown as VM + + // General header: 0/2 view checked → unchecked + const generalView = vm.getHeaderViewState(1) + expect(generalView.checked).toBe(false) + expect(generalView.indeterminate).toBe(false) + }) + }) + + describe('ensurePermission', () => { + it('should create a default permission entry for unknown category IDs', () => { + const permissions: Record = {} + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + ensurePermission: (id: number) => CategoryPermission + } + const vm = wrapper.vm as unknown as VM + + const result = vm.ensurePermission(999) + expect(result).toEqual({ canView: false, canModerate: false }) + }) + + it('should return existing permission entry when it exists', () => { + const permissions: Record = { + 10: { canView: true, canModerate: true }, + } + const wrapper = mount(CategoryPermissionsTable, { + props: { + categoryHeaders: createHeaders(), + permissions, + }, + }) + + type VM = InstanceType & { + ensurePermission: (id: number) => CategoryPermission + } + const vm = wrapper.vm as unknown as VM + + const result = vm.ensurePermission(10) + expect(result).toEqual({ canView: true, canModerate: true }) + }) + }) +}) diff --git a/src/components/CategoryPermissionsTable/CategoryPermissionsTable.vue b/src/components/CategoryPermissionsTable/CategoryPermissionsTable.vue new file mode 100644 index 0000000..a57d400 --- /dev/null +++ b/src/components/CategoryPermissionsTable/CategoryPermissionsTable.vue @@ -0,0 +1,277 @@ + + + + + diff --git a/src/components/CategoryPermissionsTable/index.ts b/src/components/CategoryPermissionsTable/index.ts new file mode 100644 index 0000000..e4aa861 --- /dev/null +++ b/src/components/CategoryPermissionsTable/index.ts @@ -0,0 +1,3 @@ +import CategoryPermissionsTable from './CategoryPermissionsTable.vue' +export default CategoryPermissionsTable +export type { CategoryPermission } from './CategoryPermissionsTable.vue' diff --git a/src/router.ts b/src/router.ts index 8935cad..afab4b2 100644 --- a/src/router.ts +++ b/src/router.ts @@ -22,6 +22,10 @@ const routes: RouteRecordRaw[] = [ { path: '/admin/roles', component: () => import('@/views/admin/AdminRoleList.vue') }, { path: '/admin/roles/create', component: () => import('@/views/admin/AdminRoleEdit.vue') }, { path: '/admin/roles/:id/edit', component: () => import('@/views/admin/AdminRoleEdit.vue') }, + { + path: '/admin/teams/:id/edit', + component: () => import('@/views/admin/AdminTeamEdit.vue'), + }, { path: '/admin/categories', component: () => import('@/views/admin/AdminCategoryList.vue') }, { path: '/admin/categories/create', diff --git a/src/types/models.ts b/src/types/models.ts index 3a3bef6..abd13fa 100644 --- a/src/types/models.ts +++ b/src/types/models.ts @@ -193,3 +193,21 @@ export interface PostHistoryResponse { current: Post history: PostHistoryEntry[] } + +export interface CategoryPerm { + id: number + categoryId: number + targetType: 'role' | 'team' + targetId: string + canView: boolean + canPost: boolean + canReply: boolean + canModerate: boolean +} + +export interface Team { + id: string + displayName: string + owner: string + ownerDisplayName: string +} diff --git a/src/views/admin/AdminCategoryEdit.vue b/src/views/admin/AdminCategoryEdit.vue index a2a2dec..42c1b9d 100644 --- a/src/views/admin/AdminCategoryEdit.vue +++ b/src/views/admin/AdminCategoryEdit.vue @@ -120,8 +120,8 @@
, - selectedModerateRoles: [] as Array<{ id: number; label: string }>, + selectedViewTargets: [] as Array<{ id: number; label: string }>, + selectedModerateTargets: [] as Array<{ id: number; label: string }>, formData: { headerId: null as number | null, name: '', @@ -281,9 +281,9 @@ export default defineComponent({ 'forum', 'Control which roles can access and moderate this category', ), - viewRoles: t('forum', 'Roles that can view'), + viewRoles: t('forum', 'Can view'), viewRolesHelp: t('forum', 'Select roles that can view this category and its threads'), - moderateRoles: t('forum', 'Roles that can moderate'), + moderateRoles: t('forum', 'Can moderate'), moderateRolesHelp: t( 'forum', 'Select roles that can moderate (edit/delete) content in this category', @@ -312,8 +312,7 @@ export default defineComponent({ label: header.name, })) }, - roleOptions(): Array<{ id: number; label: string }> { - // Filter out Admin role - it has implicit full access to all categories + viewTargetOptions(): Array<{ id: number; label: string }> { return this.roles .filter((role) => !isAdminRole(role)) .map((role) => ({ @@ -321,10 +320,8 @@ export default defineComponent({ label: role.name, })) }, - moderateRoleOptions(): Array<{ id: number; label: string }> { - // Filter out Admin, Guest, and Default roles - // - Admin has implicit full access - // - Guest and Default cannot moderate + moderateTargetOptions(): Array<{ id: number; label: string }> { + // Filter out Admin, Guest, and Default roles for moderation return this.roles .filter((role) => !isAdminRole(role) && !isGuestRole(role) && !isDefaultRole(role)) .map((role) => ({ @@ -403,18 +400,29 @@ export default defineComponent({ // View: Default user role const memberRole = this.roles.find(isDefaultRole) if (memberRole) { - this.selectedViewRoles = [{ id: memberRole.id, label: memberRole.name }] + this.selectedViewTargets = [ + { + id: memberRole.id, + label: memberRole.name, + }, + ] } // Moderate: Admin and Moderator const adminRole = this.roles.find(isAdminRole) const moderatorRole = this.roles.find(isModeratorRole) - this.selectedModerateRoles = [] + this.selectedModerateTargets = [] if (adminRole) { - this.selectedModerateRoles.push({ id: adminRole.id, label: adminRole.name }) + this.selectedModerateTargets.push({ + id: adminRole.id, + label: adminRole.name, + }) } if (moderatorRole) { - this.selectedModerateRoles.push({ id: moderatorRole.id, label: moderatorRole.name }) + this.selectedModerateTargets.push({ + id: moderatorRole.id, + label: moderatorRole.name, + }) } } } catch (e) { @@ -454,41 +462,31 @@ export default defineComponent({ if (!this.categoryId) return try { - const permsResponse = await ocs.get< - Array<{ - id: number - categoryId: number - roleId: number - canView: boolean - canModerate: boolean - }> - >(`/categories/${this.categoryId}/permissions`) + const permsResponse = await ocs.get( + `/categories/${this.categoryId}/permissions`, + ) const perms = permsResponse.data || [] - // Map permissions to role selections - const viewRoleIds = new Set() - const moderateRoleIds = new Set() + // Only handle role-type permissions + const rolePerms = perms.filter((p) => p.targetType === 'role') - perms.forEach((perm) => { - if (perm.canView) { - viewRoleIds.add(perm.roleId) - } - if (perm.canModerate) { - moderateRoleIds.add(perm.roleId) - } - }) + this.selectedViewTargets = rolePerms + .filter((p) => p.canView) + .map((p) => { + const role = this.roles.find((r) => String(r.id) === p.targetId) + return role ? { id: role.id, label: role.name } : null + }) + .filter((o): o is { id: number; label: string } => o !== null) - // Set selected roles - this.selectedViewRoles = this.roles - .filter((role) => viewRoleIds.has(role.id)) - .map((role) => ({ id: role.id, label: role.name })) - - this.selectedModerateRoles = this.roles - .filter( - (role) => moderateRoleIds.has(role.id) && !isGuestRole(role) && !isDefaultRole(role), - ) - .map((role) => ({ id: role.id, label: role.name })) + this.selectedModerateTargets = rolePerms + .filter((p) => p.canModerate) + .map((p) => { + const role = this.roles.find((r) => String(r.id) === p.targetId) + if (!role || isGuestRole(role) || isDefaultRole(role)) return null + return { id: role.id, label: role.name } + }) + .filter((o): o is { id: number; label: string } => o !== null) } catch (e) { console.error('Failed to load category permissions', e) } @@ -537,35 +535,24 @@ export default defineComponent({ }, async updatePermissions(categoryId: number): Promise { - // Build permissions array combining view and moderate roles - const allRoleIds = new Set() - const viewRoleIds = new Set(this.selectedViewRoles.map((r) => r.id)) - // Filter out guest and default roles from moderate roles - const guestRole = this.roles.find(isGuestRole) - const defaultRole = this.roles.find(isDefaultRole) - const moderateRoleIds = new Set( - this.selectedModerateRoles - .filter((r) => r.id !== guestRole?.id && r.id !== defaultRole?.id) - .map((r) => r.id), - ) + const viewRoleIds = new Set(this.selectedViewTargets.map((t) => t.id)) + const moderateRoleIds = new Set(this.selectedModerateTargets.map((t) => t.id)) - // Add all selected role IDs to the set - this.selectedViewRoles.forEach((r) => allRoleIds.add(r.id)) - this.selectedModerateRoles.forEach((r) => { - // Don't add guest or default to moderate permissions - if (r.id !== guestRole?.id && r.id !== defaultRole?.id) { - allRoleIds.add(r.id) - } - }) + // Collect all unique role IDs + const allRoleIds = new Set([...viewRoleIds, ...moderateRoleIds]) - const permissionsData = Array.from(allRoleIds).map((roleId) => ({ - roleId, - canView: viewRoleIds.has(roleId), - canModerate: moderateRoleIds.has(roleId), - })) + const permissions: Array<{ roleId: number; canView: boolean; canModerate: boolean }> = [] + + for (const roleId of allRoleIds) { + permissions.push({ + roleId, + canView: viewRoleIds.has(roleId), + canModerate: moderateRoleIds.has(roleId), + }) + } await ocs.post(`/categories/${categoryId}/permissions`, { - permissions: permissionsData, + permissions, }) }, diff --git a/src/views/admin/AdminRoleEdit.vue b/src/views/admin/AdminRoleEdit.vue index c558f4c..f794759 100644 --- a/src/views/admin/AdminRoleEdit.vue +++ b/src/views/admin/AdminRoleEdit.vue @@ -179,67 +179,12 @@

{{ strings.categoryPermissionsDesc }}

-
-
-
{{ strings.category }}
-
{{ strings.canView }}
-
{{ strings.canModerate }}
-
- - -
-
{{ strings.noCategories }}
+ @@ -271,17 +216,15 @@ import ArrowLeftIcon from '@icons/ArrowLeft.vue' import PageWrapper from '@/components/PageWrapper' import PageHeader from '@/components/PageHeader' import AppToolbar from '@/components/AppToolbar' +import CategoryPermissionsTable, { + type CategoryPermission, +} from '@/components/CategoryPermissionsTable' import { ocs } from '@/axios' import { t } from '@nextcloud/l10n' import { isAdminRole, isGuestRole, isDefaultRole } from '@/constants' import { usePublicSettings } from '@/composables/usePublicSettings' import type { Role, CategoryHeader } from '@/types' -interface CategoryPermission { - canView: boolean - canModerate: boolean -} - export default defineComponent({ name: 'AdminRoleEdit', components: { @@ -297,6 +240,7 @@ export default defineComponent({ ArrowLeftIcon, PageWrapper, AppToolbar, + CategoryPermissionsTable, }, setup() { const { allowGuestAccess, fetchPublicSettings } = usePublicSettings() @@ -356,11 +300,6 @@ export default defineComponent({ canEditCategoriesDesc: t('forum', 'Allow creating, editing and deleting categories'), categoryPermissions: t('forum', 'Category permissions'), categoryPermissionsDesc: t('forum', 'Set which categories this role can access'), - category: t('forum', 'Category'), - canView: t('forum', 'Can view'), - canModerate: t('forum', 'Can moderate'), - allow: t('forum', 'Allow'), - noCategories: t('forum', 'No categories available'), adminAllRolePermissions: t('forum', 'Admin role must have all permissions enabled'), adminFullAccess: t('forum', 'Admin role has full access to all categories'), guestNoRolePermissions: t('forum', 'Guest role cannot have admin permissions'), @@ -410,91 +349,6 @@ export default defineComponent({ this.refresh() }, methods: { - ensurePermission(categoryId: number): CategoryPermission { - if (!this.permissions[categoryId]) { - this.permissions[categoryId] = { - canView: false, - canModerate: false, - } - } - return this.permissions[categoryId] - }, - - getHeaderViewState(headerId: number): { checked: boolean; indeterminate: boolean } { - const header = this.categoryHeaders.find((h) => h.id === headerId) - if (!header || !header.categories || header.categories.length === 0) { - return { checked: false, indeterminate: false } - } - - const checkedCount = header.categories.filter( - (cat) => this.permissions[cat.id]?.canView, - ).length - const totalCount = header.categories.length - - if (checkedCount === 0) { - return { checked: false, indeterminate: false } - } else if (checkedCount === totalCount) { - return { checked: true, indeterminate: false } - } else { - return { checked: false, indeterminate: true } - } - }, - - getHeaderModerateState(headerId: number): { checked: boolean; indeterminate: boolean } { - const header = this.categoryHeaders.find((h) => h.id === headerId) - if (!header || !header.categories || header.categories.length === 0) { - return { checked: false, indeterminate: false } - } - - const checkedCount = header.categories.filter( - (cat) => this.permissions[cat.id]?.canModerate, - ).length - const totalCount = header.categories.length - - if (checkedCount === 0) { - return { checked: false, indeterminate: false } - } else if (checkedCount === totalCount) { - return { checked: true, indeterminate: false } - } else { - return { checked: false, indeterminate: true } - } - }, - - updateCategoryView(categoryId: number, checked: boolean): void { - this.ensurePermission(categoryId).canView = checked - }, - - updateCategoryModerate(categoryId: number, checked: boolean): void { - this.ensurePermission(categoryId).canModerate = checked - }, - - toggleHeaderView(headerId: number): void { - const header = this.categoryHeaders.find((h) => h.id === headerId) - if (!header || !header.categories) return - - const state = this.getHeaderViewState(headerId) - // If all are checked, uncheck all - // If some or none are checked, check all - const newValue = !state.checked - - header.categories.forEach((cat) => { - this.ensurePermission(cat.id).canView = newValue - }) - }, - - toggleHeaderModerate(headerId: number): void { - const header = this.categoryHeaders.find((h) => h.id === headerId) - if (!header || !header.categories) return - - const state = this.getHeaderModerateState(headerId) - // If all are checked, uncheck all - // If some or none are checked, check all - const newValue = !state.checked - - header.categories.forEach((cat) => { - this.ensurePermission(cat.id).canModerate = newValue - }) - }, async refresh(): Promise { try { this.loading = true @@ -856,82 +710,6 @@ export default defineComponent({ } } - .permissions-table { - display: flex; - flex-direction: column; - gap: 1px; - background: var(--color-border); - border-radius: 8px; - overflow: hidden; - - .table-header, - .table-row { - display: grid; - grid-template-columns: 1fr 150px 150px; - gap: 16px; - padding: 16px; - background: var(--color-main-background); - align-items: center; - } - - .table-header { - font-weight: 600; - font-size: 0.85rem; - text-transform: uppercase; - letter-spacing: 0.05em; - color: var(--color-text-maxcontrast); - background: var(--color-background-hover); - } - - .table-header-row { - display: grid; - grid-template-columns: 1fr 150px 150px; - gap: 16px; - padding: 12px 16px; - background: var(--color-background-dark); - align-items: center; - - .header-name { - font-weight: 600; - font-size: 1rem; - color: var(--color-main-text); - text-transform: uppercase; - letter-spacing: 0.05em; - } - - .header-permission { - display: flex; - align-items: center; - } - } - - .table-row { - &:hover { - background: var(--color-background-hover); - } - - .col-category { - display: flex; - flex-direction: column; - gap: 4px; - - .category-name { - font-weight: 500; - color: var(--color-main-text); - } - - .category-desc { - font-size: 0.85rem; - } - } - - .col-permission { - display: flex; - align-items: center; - } - } - } - .form-actions { display: flex; justify-content: flex-end; diff --git a/src/views/admin/AdminRoleList.vue b/src/views/admin/AdminRoleList.vue index f1909d7..86092f1 100644 --- a/src/views/admin/AdminRoleList.vue +++ b/src/views/admin/AdminRoleList.vue @@ -94,6 +94,64 @@ + + + + + +
+ + {{ strings.loadingTeams }} +
+ + + + + + + + + + + + + + + + +
@@ -116,7 +174,7 @@ import PageHeader from '@/components/PageHeader' import AppToolbar from '@/components/AppToolbar' import { ocs } from '@/axios' import { t } from '@nextcloud/l10n' -import type { Role } from '@/types' +import type { Role, Team } from '@/types' export default defineComponent({ name: 'AdminRoleList', @@ -141,6 +199,9 @@ export default defineComponent({ loading: false, roles: [] as Role[], error: null as string | null, + teamsLoading: false, + teams: [] as Team[], + teamsError: null as string | null, strings: { title: t('forum', 'Role management'), @@ -153,6 +214,8 @@ export default defineComponent({ createRole: t('forum', 'Create role'), id: t('forum', 'ID'), name: t('forum', 'Name'), + displayName: t('forum', 'Name'), + owner: t('forum', 'Owner'), description: t('forum', 'Description'), created: t('forum', 'Created'), actions: t('forum', 'Actions'), @@ -166,6 +229,12 @@ export default defineComponent({ { name }, ), systemRoleWarning: t('forum', 'System roles cannot be deleted'), + teamsTitle: t('forum', 'Team permissions'), + teamsSubtitle: t('forum', 'Manage category permissions for Nextcloud Teams'), + loadingTeams: t('forum', 'Loading teams …'), + teamsErrorTitle: t('forum', 'Error loading teams'), + teamsEmptyTitle: t('forum', 'No teams found'), + teamsEmptyDesc: t('forum', 'No Nextcloud Teams are available'), }, } }, @@ -178,9 +247,16 @@ export default defineComponent({ { key: 'created', label: this.strings.created, minWidth: '120px' }, ] }, + teamTableColumns(): TableColumn[] { + return [ + { key: 'displayName', label: this.strings.displayName, minWidth: '200px' }, + { key: 'ownerDisplayName', label: this.strings.owner, minWidth: '150px' }, + ] + }, }, created() { this.refresh() + this.refreshTeams() }, methods: { async refresh(): Promise { @@ -198,6 +274,21 @@ export default defineComponent({ } }, + async refreshTeams(): Promise { + try { + this.teamsLoading = true + this.teamsError = null + + const response = await ocs.get('/teams') + this.teams = response.data || [] + } catch (e) { + console.error('Failed to load teams', e) + this.teamsError = (e as Error).message || t('forum', 'An unexpected error occurred') + } finally { + this.teamsLoading = false + } + }, + createRole(): void { this.$router.push('/admin/roles/create') }, @@ -206,6 +297,10 @@ export default defineComponent({ this.$router.push(`/admin/roles/${roleId}/edit`) }, + editTeam(teamId: string): void { + this.$router.push(`/admin/teams/${teamId}/edit`) + }, + confirmDelete(role: Role): void { if (role.isSystemRole) { alert(this.strings.systemRoleWarning) @@ -241,6 +336,10 @@ export default defineComponent({ margin-top: 16px; } + .mt-32 { + margin-top: 32px; + } + .ml-8 { margin-left: 8px; } diff --git a/src/views/admin/AdminTeamEdit.vue b/src/views/admin/AdminTeamEdit.vue new file mode 100644 index 0000000..93b009e --- /dev/null +++ b/src/views/admin/AdminTeamEdit.vue @@ -0,0 +1,266 @@ + + + + + diff --git a/tests/unit/Controller/CategoryControllerTest.php b/tests/unit/Controller/CategoryControllerTest.php index 083d9a4..4cc6314 100644 --- a/tests/unit/Controller/CategoryControllerTest.php +++ b/tests/unit/Controller/CategoryControllerTest.php @@ -18,8 +18,6 @@ use OCA\Forum\Db\RoleMapper; use OCA\Forum\Db\ThreadMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; -use OCP\IGroup; -use OCP\IGroupManager; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; @@ -43,8 +41,6 @@ class CategoryControllerTest extends TestCase { private RoleMapper $roleMapper; /** @var IUserSession&MockObject */ private IUserSession $userSession; - /** @var IGroupManager&MockObject */ - private IGroupManager $groupManager; /** @var LoggerInterface&MockObject */ private LoggerInterface $logger; /** @var IRequest&MockObject */ @@ -59,7 +55,6 @@ class CategoryControllerTest extends TestCase { $this->readMarkerMapper = $this->createMock(ReadMarkerMapper::class); $this->roleMapper = $this->createMock(RoleMapper::class); $this->userSession = $this->createMock(IUserSession::class); - $this->groupManager = $this->createMock(IGroupManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->controller = new CategoryController( @@ -72,7 +67,6 @@ class CategoryControllerTest extends TestCase { $this->readMarkerMapper, $this->roleMapper, $this->userSession, - $this->groupManager, $this->logger ); } @@ -448,9 +442,6 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - // User is not an admin - $this->groupManager->method('get')->with('admin')->willReturn(null); - // User has a role $role = new Role(); $role->setId(1); @@ -465,7 +456,8 @@ class CategoryControllerTest extends TestCase { $categoryPerm = new CategoryPerm(); $categoryPerm->setId(1); $categoryPerm->setCategoryId($categoryId); - $categoryPerm->setRoleId(1); + $categoryPerm->setTargetType('role'); + $categoryPerm->setTargetId('1'); $categoryPerm->setCanView(true); $categoryPerm->setCanPost(false); $categoryPerm->setCanReply(false); @@ -492,8 +484,6 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - $this->groupManager->method('get')->with('admin')->willReturn(null); - $role = new Role(); $role->setId(1); $role->setName('User'); @@ -506,7 +496,8 @@ class CategoryControllerTest extends TestCase { $categoryPerm = new CategoryPerm(); $categoryPerm->setId(1); $categoryPerm->setCategoryId($categoryId); - $categoryPerm->setRoleId(1); + $categoryPerm->setTargetType('role'); + $categoryPerm->setTargetId('1'); $categoryPerm->setCanView(true); $categoryPerm->setCanPost(false); $categoryPerm->setCanReply(false); @@ -533,10 +524,16 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - // User is in admin group - $adminGroup = $this->createMock(IGroup::class); - $adminGroup->method('inGroup')->with($user)->willReturn(true); - $this->groupManager->method('get')->with('admin')->willReturn($adminGroup); + // User has Admin role + $adminRole = new Role(); + $adminRole->setId(1); + $adminRole->setName('Admin'); + $adminRole->setRoleType(Role::ROLE_TYPE_ADMIN); + + $this->roleMapper->expects($this->once()) + ->method('findByUserId') + ->with($userId) + ->willReturn([$adminRole]); $response = $this->controller->checkPermission($categoryId, $permission); @@ -552,7 +549,8 @@ class CategoryControllerTest extends TestCase { $perm1 = new CategoryPerm(); $perm1->setId(1); $perm1->setCategoryId($categoryId); - $perm1->setRoleId(2); + $perm1->setTargetType('role'); + $perm1->setTargetId('2'); $perm1->setCanView(true); $perm1->setCanPost(true); $perm1->setCanReply(true); @@ -561,7 +559,8 @@ class CategoryControllerTest extends TestCase { $perm2 = new CategoryPerm(); $perm2->setId(2); $perm2->setCategoryId($categoryId); - $perm2->setRoleId(3); + $perm2->setTargetType('role'); + $perm2->setTargetId('3'); $perm2->setCanView(true); $perm2->setCanPost(false); $perm2->setCanReply(false); @@ -578,7 +577,8 @@ class CategoryControllerTest extends TestCase { $data = $response->getData(); $this->assertIsArray($data); $this->assertCount(2, $data); - $this->assertEquals(2, $data[0]['roleId']); + $this->assertEquals('role', $data[0]['targetType']); + $this->assertEquals('2', $data[0]['targetId']); $this->assertTrue($data[0]['canView']); $this->assertFalse($data[0]['canModerate']); } @@ -665,7 +665,7 @@ class CategoryControllerTest extends TestCase { ->method('insert') ->willReturnCallback(function ($perm) { // Verify that Admin role (ID 1) is never inserted - $this->assertNotEquals(1, $perm->getRoleId()); + $this->assertNotEquals('1', $perm->getTargetId()); return $perm; }); @@ -758,7 +758,7 @@ class CategoryControllerTest extends TestCase { ->method('insert') ->willReturnCallback(function ($perm) use ($guestRoleId, $categoryId) { $this->assertEquals($categoryId, $perm->getCategoryId()); - $this->assertEquals($guestRoleId, $perm->getRoleId()); + $this->assertEquals((string)$guestRoleId, $perm->getTargetId()); $this->assertTrue($perm->getCanView()); // Verify guest role never has moderate permission, even if requested $this->assertFalse($perm->getCanModerate()); @@ -808,7 +808,7 @@ class CategoryControllerTest extends TestCase { ->method('insert') ->willReturnCallback(function ($perm) use ($moderatorRoleId, $categoryId) { $this->assertEquals($categoryId, $perm->getCategoryId()); - $this->assertEquals($moderatorRoleId, $perm->getRoleId()); + $this->assertEquals((string)$moderatorRoleId, $perm->getTargetId()); $this->assertTrue($perm->getCanView()); // Verify non-guest role CAN have moderate permission $this->assertTrue($perm->getCanModerate()); @@ -858,7 +858,7 @@ class CategoryControllerTest extends TestCase { ->method('insert') ->willReturnCallback(function ($perm) use ($defaultRoleId, $categoryId) { $this->assertEquals($categoryId, $perm->getCategoryId()); - $this->assertEquals($defaultRoleId, $perm->getRoleId()); + $this->assertEquals((string)$defaultRoleId, $perm->getTargetId()); $this->assertTrue($perm->getCanView()); // Verify default role never has moderate permission, even if requested $this->assertFalse($perm->getCanModerate()); diff --git a/tests/unit/Controller/RoleControllerTest.php b/tests/unit/Controller/RoleControllerTest.php index 4a2b2bd..6cd0b83 100644 --- a/tests/unit/Controller/RoleControllerTest.php +++ b/tests/unit/Controller/RoleControllerTest.php @@ -332,7 +332,8 @@ class RoleControllerTest extends TestCase { $perm1 = new \OCA\Forum\Db\CategoryPerm(); $perm1->setId(1); $perm1->setCategoryId(1); - $perm1->setRoleId($roleId); + $perm1->setTargetType('role'); + $perm1->setTargetId((string)$roleId); $perm1->setCanView(true); $perm1->setCanPost(true); $perm1->setCanReply(true); @@ -341,7 +342,8 @@ class RoleControllerTest extends TestCase { $perm2 = new \OCA\Forum\Db\CategoryPerm(); $perm2->setId(2); $perm2->setCategoryId(2); - $perm2->setRoleId($roleId); + $perm2->setTargetType('role'); + $perm2->setTargetId((string)$roleId); $perm2->setCanView(true); $perm2->setCanPost(false); $perm2->setCanReply(false); @@ -386,7 +388,7 @@ class RoleControllerTest extends TestCase { $this->categoryPermMapper->expects($this->exactly(2)) ->method('insert') ->willReturnCallback(function ($perm) use ($roleId) { - $this->assertEquals($roleId, $perm->getRoleId()); + $this->assertEquals((string)$roleId, $perm->getTargetId()); // Verify canPost and canReply are set based on canView if ($perm->getCategoryId() === 1) { $this->assertTrue($perm->getCanView()); @@ -488,7 +490,7 @@ class RoleControllerTest extends TestCase { $this->categoryPermMapper->expects($this->exactly(2)) ->method('insert') ->willReturnCallback(function ($perm) use ($guestRoleId) { - $this->assertEquals($guestRoleId, $perm->getRoleId()); + $this->assertEquals((string)$guestRoleId, $perm->getTargetId()); // Verify guest role never has moderate permission, even if requested $this->assertFalse($perm->getCanModerate()); return $perm; @@ -522,7 +524,7 @@ class RoleControllerTest extends TestCase { $this->categoryPermMapper->expects($this->exactly(2)) ->method('insert') ->willReturnCallback(function ($perm) use ($defaultRoleId) { - $this->assertEquals($defaultRoleId, $perm->getRoleId()); + $this->assertEquals((string)$defaultRoleId, $perm->getTargetId()); // Verify default role never has moderate permission, even if requested $this->assertFalse($perm->getCanModerate()); return $perm; diff --git a/tests/unit/Controller/TeamControllerTest.php b/tests/unit/Controller/TeamControllerTest.php new file mode 100644 index 0000000..0ad63f1 --- /dev/null +++ b/tests/unit/Controller/TeamControllerTest.php @@ -0,0 +1,179 @@ +request = $this->createMock(IRequest::class); + $this->categoryPermMapper = $this->createMock(CategoryPermMapper::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->controller = new TeamController( + Application::APP_ID, + $this->request, + $this->categoryPermMapper, + $this->logger + ); + } + + public function testGetPermissionsReturnsPermissionsForTeam(): void { + $teamId = 'circle-abc-123'; + + $perm1 = new CategoryPerm(); + $perm1->setId(1); + $perm1->setCategoryId(1); + $perm1->setTargetType(CategoryPerm::TARGET_TYPE_TEAM); + $perm1->setTargetId($teamId); + $perm1->setCanView(true); + $perm1->setCanPost(true); + $perm1->setCanReply(true); + $perm1->setCanModerate(false); + + $perm2 = new CategoryPerm(); + $perm2->setId(2); + $perm2->setCategoryId(2); + $perm2->setTargetType(CategoryPerm::TARGET_TYPE_TEAM); + $perm2->setTargetId($teamId); + $perm2->setCanView(true); + $perm2->setCanPost(true); + $perm2->setCanReply(true); + $perm2->setCanModerate(true); + + $this->categoryPermMapper->expects($this->once()) + ->method('findByTeamId') + ->with($teamId) + ->willReturn([$perm1, $perm2]); + + $response = $this->controller->getPermissions($teamId); + + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + $data = $response->getData(); + $this->assertIsArray($data); + $this->assertCount(2, $data); + $this->assertEquals(1, $data[0]['categoryId']); + $this->assertTrue($data[0]['canView']); + $this->assertFalse($data[0]['canModerate']); + $this->assertEquals(2, $data[1]['categoryId']); + $this->assertTrue($data[1]['canView']); + $this->assertTrue($data[1]['canModerate']); + } + + public function testGetPermissionsReturnsEmptyArrayWhenNoPermissions(): void { + $teamId = 'circle-abc-123'; + + $this->categoryPermMapper->expects($this->once()) + ->method('findByTeamId') + ->with($teamId) + ->willReturn([]); + + $response = $this->controller->getPermissions($teamId); + + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + $data = $response->getData(); + $this->assertIsArray($data); + $this->assertCount(0, $data); + } + + public function testGetPermissionsReturnsErrorOnException(): void { + $teamId = 'circle-abc-123'; + + $this->categoryPermMapper->expects($this->once()) + ->method('findByTeamId') + ->with($teamId) + ->willThrowException(new \Exception('Database error')); + + $this->logger->expects($this->once()) + ->method('error'); + + $response = $this->controller->getPermissions($teamId); + + $this->assertEquals(Http::STATUS_INTERNAL_SERVER_ERROR, $response->getStatus()); + $data = $response->getData(); + $this->assertEquals(['error' => 'Failed to fetch permissions'], $data); + } + + public function testGetPermissionsReturnsCorrectTargetType(): void { + $teamId = 'circle-abc-123'; + + $perm = new CategoryPerm(); + $perm->setId(1); + $perm->setCategoryId(5); + $perm->setTargetType(CategoryPerm::TARGET_TYPE_TEAM); + $perm->setTargetId($teamId); + $perm->setCanView(true); + $perm->setCanPost(false); + $perm->setCanReply(false); + $perm->setCanModerate(false); + + $this->categoryPermMapper->expects($this->once()) + ->method('findByTeamId') + ->with($teamId) + ->willReturn([$perm]); + + $response = $this->controller->getPermissions($teamId); + + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + $data = $response->getData(); + $this->assertEquals(CategoryPerm::TARGET_TYPE_TEAM, $data[0]['targetType']); + $this->assertEquals($teamId, $data[0]['targetId']); + } + + public function testIndexReturnsServiceUnavailableWhenCirclesNotAvailable(): void { + // The controller's getCirclesManager() checks class_exists and Server::get + // Since Circles is not available in the test environment, this should return 503 + $response = $this->controller->index(); + + $this->assertEquals(Http::STATUS_SERVICE_UNAVAILABLE, $response->getStatus()); + $data = $response->getData(); + $this->assertEquals(['error' => 'Teams app is not available'], $data); + } + + public function testUpdatePermissionsReturnsServiceUnavailableWhenCirclesNotAvailable(): void { + $teamId = 'circle-abc-123'; + $permissions = [ + ['categoryId' => 1, 'canView' => true, 'canModerate' => false], + ]; + + // Since Circles is not available in the test environment, this should return 503 + $response = $this->controller->updatePermissions($teamId, $permissions); + + $this->assertEquals(Http::STATUS_SERVICE_UNAVAILABLE, $response->getStatus()); + $data = $response->getData(); + $this->assertEquals(['error' => 'Teams app is not available'], $data); + } + + public function testUpdatePermissionsDoesNotCallMapperWhenCirclesNotAvailable(): void { + $teamId = 'circle-abc-123'; + $permissions = [ + ['categoryId' => 1, 'canView' => true, 'canModerate' => false], + ]; + + // Mapper methods should never be called when Circles is unavailable + $this->categoryPermMapper->expects($this->never()) + ->method('deleteByTeamId'); + $this->categoryPermMapper->expects($this->never()) + ->method('insert'); + + $this->controller->updatePermissions($teamId, $permissions); + } +} diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 7b9601f..9b74dd0 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -18,6 +18,7 @@ use OCA\Forum\Db\UserRole; use OCA\Forum\Db\UserRoleMapper; use OCA\Forum\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -36,6 +37,8 @@ class PermissionServiceTest extends TestCase { private ThreadMapper $threadMapper; /** @var PostMapper&MockObject */ private PostMapper $postMapper; + /** @var IUserManager&MockObject */ + private IUserManager $userManager; /** @var LoggerInterface&MockObject */ private LoggerInterface $logger; @@ -46,6 +49,7 @@ class PermissionServiceTest extends TestCase { $this->categoryMapper = $this->createMock(CategoryMapper::class); $this->threadMapper = $this->createMock(ThreadMapper::class); $this->postMapper = $this->createMock(PostMapper::class); + $this->userManager = $this->createMock(IUserManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->service = new PermissionService( @@ -55,6 +59,7 @@ class PermissionServiceTest extends TestCase { $this->categoryMapper, $this->threadMapper, $this->postMapper, + $this->userManager, $this->logger ); } @@ -653,7 +658,8 @@ class PermissionServiceTest extends TestCase { $perm = new CategoryPerm(); $perm->setId($id); $perm->setCategoryId($categoryId); - $perm->setRoleId($roleId); + $perm->setTargetType('role'); + $perm->setTargetId((string)$roleId); $perm->setCanView($canView); $perm->setCanPost($canPost); $perm->setCanReply($canReply);