From a7696131fde7bdfb6cc361726ca051afbadc2463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 1 Sep 2025 22:28:09 +0200 Subject: [PATCH 01/21] Loading groups with attributes via ReCodEx API. --- app/config/config.neon | 3 + app/helpers/Recodex/RecodexApiHelper.php | 17 +- app/helpers/Recodex/RecodexGroup.php | 279 +++++++++++++++++++++++ app/helpers/Recodex/RecodexUser.php | 2 +- 4 files changed, 296 insertions(+), 5 deletions(-) create mode 100644 app/helpers/Recodex/RecodexGroup.php diff --git a/app/config/config.neon b/app/config/config.neon index 66e262a..549809c 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -24,6 +24,9 @@ parameters: recodex: # just a placeholder, this needs to be overridden in local config apiBase: null extensionId: null + verifySSL: true + sisIdKey: "cas-uk" + sisLoginKey: "ldap-uk" sis: verifySSL: true diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index e3909f8..b2265b2 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -295,12 +295,21 @@ public function removeExternalId(string $id, string $service): RecodexUser } /** - * Get all non-archived groups the user can see. + * Get all non-archived groups with attributes and membership relation to given user. + * @param User $user whose membership relation is being injected + * @return RecodexGroup[] groups indexed by their IDs */ - public function getGroups(): array + public function getGroups(User $user): array { - // TODO: this is just a placeholder, needs finishing - $groups = $this->get('groups'); + $body = $this->get( + "group-attributes", + ['instance' => $user->getInstanceId(), 'service' => $this->extensionId, 'user' => $user->getId()] + ); + $groups = []; + foreach ($body as $groupData) { + $group = new RecodexGroup($groupData, $this->extensionId); + $groups[$group->id] = $group; + } return $groups; } } diff --git a/app/helpers/Recodex/RecodexGroup.php b/app/helpers/Recodex/RecodexGroup.php new file mode 100644 index 0000000..725ca03 --- /dev/null +++ b/app/helpers/Recodex/RecodexGroup.php @@ -0,0 +1,279 @@ + [ titlesBeforeName, firstName, lastName, titlesAfterName, email ] + */ + public array $admins; + + /** + * Group names indexed by locale identifiers + */ + public array $name = []; + + /** + * Group descriptions indexed by locale identifiers + */ + public array $description = []; + + /** + * Indicates whether the group is organizational (does not have assignments) + */ + public bool $organizational; + + /** + * Indicates whether the group is an exam group. + */ + public bool $exam; + + /** + * Indicates whether the group is public. + */ + public bool $public; + + /** + * Indicates whether the group is detaining (students cannot leave on their own). + */ + public bool $detaining; + + /** + * External attributes assigned by this extension. + */ + public array $attributes; + + /** + * Indicates the membership type of the logged in user to the group. + * Possible values are: 'admin', 'supervisor', 'observer', and 'student' + * (and null if there is no relation between the user and the group). + */ + public ?string $membership = null; + + /** + * Validates that admins array contains proper associative arrays with required keys + * @param array $admins + * @throws RecodexApiException if admins structure is invalid + */ + private function validateAdminsStructure(array $admins): void + { + $requiredAdminKeys = ['titlesBeforeName', 'firstName', 'lastName', 'titlesAfterName', 'email']; + + foreach ($admins as $adminId => $adminData) { + if (!is_array($adminData)) { + throw new RecodexApiException( + "Admin with ID '$adminId' must be an associative array, " . gettype($adminData) . ' given' + ); + } + + foreach ($requiredAdminKeys as $key) { + if (!array_key_exists($key, $adminData)) { + throw new RecodexApiException( + "Admin with ID '$adminId' is missing required key '$key'" + ); + } + } + } + } + + /** + * Validates localizedTexts structure and transforms it into name/description arrays + * @param array $localizedTexts + * @return array Returns associative array with 'name' and 'description' keys containing locale-indexed arrays + * @throws RecodexApiException if localizedTexts structure is invalid + */ + private function processLocalizedTexts(array $localizedTexts): array + { + $requiredLocalizedTextKeys = ['locale', 'name', 'description']; + $name = []; + $description = []; + + foreach ($localizedTexts as $index => $localizedTextData) { + if (!is_array($localizedTextData)) { + throw new RecodexApiException( + "LocalizedText at index '$index' must be an associative array, " . + gettype($localizedTextData) . ' given' + ); + } + + foreach ($requiredLocalizedTextKeys as $key) { + if (!array_key_exists($key, $localizedTextData)) { + throw new RecodexApiException( + "LocalizedText at index '$index' is missing required key '$key'" + ); + } + } + + // Transform into locale-indexed arrays + $locale = $localizedTextData['locale']; + $name[$locale] = $localizedTextData['name']; + $description[$locale] = $localizedTextData['description']; + } + + return ['name' => $name, 'description' => $description]; + } + + /** + * @param array $data parsed JSON group view + * @param string $attributesService name of the attributes service (this application's namespace) + */ + public function __construct(array $data, string $attributesService) + { + // Validate all required keys are present in API response + $requiredKeys = [ + 'id', + 'parentGroupId', + 'admins', + 'localizedTexts', + 'organizational', + 'exam', + 'public', + 'detaining', + 'attributes', + 'membership' + ]; + + foreach ($requiredKeys as $key) { + if (!array_key_exists($key, $data)) { + throw new RecodexApiException("Missing required key '$key' in group API response"); + } + } + + // Validate admins array structure + $this->validateAdminsStructure($data['admins']); + + // Process and validate localizedTexts array structure + $localizedData = $this->processLocalizedTexts($data['localizedTexts']); + + // Initialize public members from the associative array (values can be null) + $this->id = $data['id']; + $this->parentGroupId = $data['parentGroupId']; + $this->admins = $data['admins']; + $this->name = $localizedData['name']; + $this->description = $localizedData['description']; + $this->organizational = $data['organizational']; + $this->exam = $data['exam']; + $this->public = $data['public']; + $this->detaining = $data['detaining']; + $this->attributes = $data['attributes'][$attributesService] ?? []; + $this->membership = $data['membership']; + } + + /** + * Serializes the object to a value that can be serialized natively by json_encode(). + * @return array Data which can be serialized by json_encode + */ + public function jsonSerialize(): array + { + return [ + 'id' => $this->id, + 'parentGroupId' => $this->parentGroupId, + 'admins' => $this->admins, + 'name' => $this->name, + 'description' => $this->description, + 'organizational' => $this->organizational, + 'exam' => $this->exam, + 'public' => $this->public, + 'detaining' => $this->detaining, + 'attributes' => $this->attributes, + 'membership' => $this->membership, + ]; + } + + /** + * Make sure all ancestor groups are included in the selection. The selected groups array is updated in place. + * @param RecodexGroup[] $selectedGroups groups selected so far (id => RecodexGroup) + * @param RecodexGroup[] $allGroups all available groups (id => RecodexGroup) + */ + private static function ancestralClosure(array &$selectedGroups, array $allGroups): void + { + $toCheck = array_keys($selectedGroups); + while ($toCheck) { + $currentId = array_shift($toCheck); + if (empty($allGroups[$currentId])) { + continue; + } + + $parentId = $allGroups[$currentId]->parentGroupId; + if ($parentId && empty($selectedGroups[$parentId])) { + $selectedGroups[$parentId] = $allGroups[$parentId]; + $toCheck[] = $parentId; + } + } + } + + /** + * Checks if the group belongs to any SIS group. + * @param RecodexGroup $group The group to check. + * @param array $sisGroupsIndex The index of SIS groups [ sisGroupId => unused value ]. + * @return bool True if the group belongs to any SIS group, false otherwise. + */ + private static function belongsToSisGroup(RecodexGroup $group, array $sisGroupsIndex): bool + { + foreach ($group->attributes[self::ATTR_GROUP_KEY] ?? [] as $sisGrpId) { + if (array_key_exists($sisGrpId, $sisGroupsIndex)) { + return true; + } + } + return false; + } + + /** + * Prunes the group list for students, keeping only relevant groups. + * Relevant are groups that belong to any SIS group or where the student already belongs to + * (the ancestral closure of relevant groups is returned so hierarchical names can be displayed). + * @param RecodexGroup[] $groups The list of groups to prune (indexed by group IDs). + * @param array $sisGroups The list of SIS group IDs. + * @return RecodexGroup[] The pruned list of groups (indexed by group IDs). + */ + public static function pruneForStudent(array $groups, array $sisGroups): array + { + $sisGroupsIndex = array_flip($sisGroups); + $pruned = []; + foreach ($groups as $id => $group) { + if (self::belongsToSisGroup($group, $sisGroupsIndex) || $group->membership === 'student') { + $pruned[$id] = $group; + } + } + + self::ancestralClosure($pruned, $groups); + return $pruned; + } +} diff --git a/app/helpers/Recodex/RecodexUser.php b/app/helpers/Recodex/RecodexUser.php index 40bfc9b..97ec11f 100644 --- a/app/helpers/Recodex/RecodexUser.php +++ b/app/helpers/Recodex/RecodexUser.php @@ -7,7 +7,7 @@ use Nette; /** - * Wraper for User data sent from ReCodEx API. + * Wrapper for User data sent from ReCodEx API. */ class RecodexUser { From 47da88843f10bc2dfc2a754e62b5aa5baf0a1244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Wed, 3 Sep 2025 19:36:06 +0200 Subject: [PATCH 02/21] Making scheduling event parameters nullable, if the event is not fully scheduled yet (placed in a room, or fixed in time). --- app/model/entity/SisScheduleEvent.php | 32 +++++++++++++-------------- migrations/Version20250903173423.php | 31 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 migrations/Version20250903173423.php diff --git a/app/model/entity/SisScheduleEvent.php b/app/model/entity/SisScheduleEvent.php index f2df8eb..9122ae8 100644 --- a/app/model/entity/SisScheduleEvent.php +++ b/app/model/entity/SisScheduleEvent.php @@ -54,31 +54,31 @@ class SisScheduleEvent implements JsonSerializable protected $type; /** - * @ORM\Column(type="integer") + * @ORM\Column(type="integer", nullable=true) * Day of the week (0=Sunday, 1=Monday...6=Saturday) */ protected $dayOfWeek; /** - * @ORM\Column(type="integer") + * @ORM\Column(type="integer", nullable=true) * When the lecture starts (logical weeks of the semester). */ protected $firstWeek; /** - * @ORM\Column(type="integer") + * @ORM\Column(type="integer", nullable=true) * Time of the day when the event starts as minutes from midnight. */ protected $time; /** - * @ORM\Column(type="integer") + * @ORM\Column(type="integer", nullable=true) * Length of the event in minutes. */ protected $length; /** - * @ORM\Column(type="string") + * @ORM\Column(type="string", nullable=true) * Where the event is located. */ protected $room; @@ -143,32 +143,32 @@ public function setType(string $type): void $this->type = $type; } - public function getDayOfWeek(): int + public function getDayOfWeek(): ?int { return $this->dayOfWeek; } - public function getFirstWeek(): int + public function getFirstWeek(): ?int { return $this->firstWeek; } - public function getTime(): int + public function getTime(): ?int { return $this->time; } - public function getLength(): int + public function getLength(): ?int { return $this->length; } - public function setLength(int $length): void + public function setLength(?int $length): void { $this->length = $length; } - public function getRoom(): string + public function getRoom(): ?string { return $this->room; } @@ -179,11 +179,11 @@ public function getFortnight(): bool } public function setSchedule( - int $dayOfWeek, - int $firstWeek, - int $time, - int $length, - string $room, + ?int $dayOfWeek, + ?int $firstWeek, + ?int $time, + ?int $length, + ?string $room, bool $fortnight = false ): void { $this->dayOfWeek = $dayOfWeek; diff --git a/migrations/Version20250903173423.php b/migrations/Version20250903173423.php new file mode 100644 index 0000000..a2282af --- /dev/null +++ b/migrations/Version20250903173423.php @@ -0,0 +1,31 @@ +addSql('ALTER TABLE sis_schedule_event CHANGE day_of_week day_of_week INT DEFAULT NULL, CHANGE first_week first_week INT DEFAULT NULL, CHANGE time time INT DEFAULT NULL, CHANGE length length INT DEFAULT NULL, CHANGE room room VARCHAR(255) DEFAULT NULL'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('ALTER TABLE sis_schedule_event CHANGE day_of_week day_of_week INT NOT NULL, CHANGE first_week first_week INT NOT NULL, CHANGE time time INT NOT NULL, CHANGE length length INT NOT NULL, CHANGE room room VARCHAR(255) NOT NULL'); + } +} From 09c4777eba8958903e757d2580a1bbce4c18a99f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Wed, 3 Sep 2025 20:42:20 +0200 Subject: [PATCH 03/21] Implementing ReCodEx group loading and related functions. Implementing `recodex:groups` command for testing and debugging. --- app/commands/RecodexGroups.php | 149 +++++++++++++++++++++ app/config/config.neon | 7 +- app/helpers/Recodex/RecodexGroup.php | 116 ++++++++++++++-- app/model/repository/SisScheduleEvents.php | 13 +- app/security/TokenScope.php | 4 +- 5 files changed, 274 insertions(+), 15 deletions(-) create mode 100644 app/commands/RecodexGroups.php diff --git a/app/commands/RecodexGroups.php b/app/commands/RecodexGroups.php new file mode 100644 index 0000000..1d4f10a --- /dev/null +++ b/app/commands/RecodexGroups.php @@ -0,0 +1,149 @@ +recodexApi = $recodexApi; + $this->users = $users; + $this->sisEvents = $sisEvents; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Load groups from ReCodEx API for given user.'); + $this->addArgument('ukco', InputArgument::REQUIRED, 'SIS ID of the user whose groups are being loaded.'); + } + + private static function printGroup(OutputInterface $output, RecodexGroup $group, int $level = 0): void + { + $admins = array_map(function ($admin) { + return $admin->lastName; + }, $group->admins); + $admins = ($admins) ? (' \<' . implode(', ', $admins) . '\>') : ''; + + $membership = ($group->membership) ? (' (' . substr($group->membership, 0, 3) . ')') : ''; + + $attributes = []; + foreach ($group->attributes as $key => $values) { + foreach ($values as $value) { + $attributes[] = "$key=$value"; + } + } + $attributes = $attributes ? (' [' . implode(', ', $attributes) . ']') : ''; + + $output->writeln(str_repeat(' ', $level) . ($group->name['en'] ?? $group->name['cs']) + . $admins . $membership . $attributes); + + foreach ($group->children as $child) { + self::printGroup($output, $child, $level + 1); + } + } + + private static function printGroups(OutputInterface $output, array $groups): void + { + $rootGroups = RecodexGroup::populateChildren($groups); + foreach ($rootGroups as $group) { + self::printGroup($output, $group); + } + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $ukco = $input->getArgument('ukco'); + $user = $this->users->findOneBy(['sisId' => $ukco]); + if (!$user) { + $output->writeln('User not found.'); + return Command::FAILURE; + } + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + + $this->recodexApi->setAuthToken($token); + $groups = $this->recodexApi->getGroups($user); + + // Print student's view + if ($user->getRole() === 'student' || $user->getRole() === 'supervisor-student') { + $events = $this->sisEvents->allEventsOfUser($user, null, SisAffiliation::TYPE_STUDENT); + $eventIds = array_map(fn($e) => $e->getSisId(), $events); + $studGroups = RecodexGroup::pruneForStudent($groups, $eventIds); + $output->writeln('Student view:'); + $output->writeln('-------------'); + self::printGroups($output, $studGroups); + } + + if ($user->getRole() === 'supervisor-student') { + $output->writeln("\n\n"); // this role prints both views (students and teachers) + } + + // Print teacher's view + if ($user->getRole() !== 'student') { + $events = $this->sisEvents->allEventsOfUser( + $user, + null, + [SisAffiliation::TYPE_TEACHER, SisAffiliation::TYPE_GUARANTOR] + ); + $courseIds = array_map(fn($e) => $e->getCourse()->getCode(), $events); + $teachGroups = RecodexGroup::pruneForTeacher($groups, $courseIds); + $output->writeln('Teacher view:'); + $output->writeln('-------------'); + self::printGroups($output, $teachGroups); + } + + return Command::SUCCESS; + } +} diff --git a/app/config/config.neon b/app/config/config.neon index 549809c..5002cee 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -28,7 +28,11 @@ parameters: sisIdKey: "cas-uk" sisLoginKey: "ldap-uk" - sis: + sis: # just a placeholder, this needs to be overridden in local config + apiBase: null + faculty: 'here-goes-faculty-id' + secretRozvrhng: "here-goes-secret-api-token-for-rozvrhng-module" + secretKdojekdo: "here-goes-secret-api-token-for-kdojekdo-module" verifySSL: true emails: # common configuration for sending email (addresses and template variables) @@ -106,6 +110,7 @@ services: # commands - App\Console\DoctrineFixtures + - App\Console\RecodexGroups - App\Console\RecodexToken - App\Console\SisGetCourse - App\Console\SisGetUser diff --git a/app/helpers/Recodex/RecodexGroup.php b/app/helpers/Recodex/RecodexGroup.php index 725ca03..b4cdcf5 100644 --- a/app/helpers/Recodex/RecodexGroup.php +++ b/app/helpers/Recodex/RecodexGroup.php @@ -4,6 +4,7 @@ use App\Exceptions\RecodexApiException; use JsonSerializable; +use LogicException; use Nette; /** @@ -21,9 +22,9 @@ class RecodexGroup implements JsonSerializable public const ATTR_COURSE_KEY = 'course'; /** - * Year identifier used for 2nd-level (semester) ReCodEx groups + * Term identifier used for 2nd-level (semester) ReCodEx groups */ - public const ATTR_YEAR_KEY = 'year'; + public const ATTR_TERM_KEY = 'term'; /** * Key used for bindings with SIS student groups (GL identifiers) @@ -41,7 +42,7 @@ class RecodexGroup implements JsonSerializable public ?string $parentGroupId; /** - * List of group primary admins id => [ titlesBeforeName, firstName, lastName, titlesAfterName, email ] + * List of group primary admins id => obj { titlesBeforeName, firstName, lastName, titlesAfterName, email } */ public array $admins; @@ -80,6 +81,12 @@ class RecodexGroup implements JsonSerializable */ public array $attributes; + /** + * List of child groups. + * This field is just a placeholder that needs to be populated (using static function populateChildren). + */ + public array $children = []; + /** * Indicates the membership type of the logged in user to the group. * Possible values are: 'admin', 'supervisor', 'observer', and 'student' @@ -92,7 +99,7 @@ class RecodexGroup implements JsonSerializable * @param array $admins * @throws RecodexApiException if admins structure is invalid */ - private function validateAdminsStructure(array $admins): void + private function processAdminsStructure(array $admins): array { $requiredAdminKeys = ['titlesBeforeName', 'firstName', 'lastName', 'titlesAfterName', 'email']; @@ -110,7 +117,11 @@ private function validateAdminsStructure(array $admins): void ); } } + + $admins[$adminId] = (object)$adminData; } + + return $admins; } /** @@ -176,16 +187,13 @@ public function __construct(array $data, string $attributesService) } } - // Validate admins array structure - $this->validateAdminsStructure($data['admins']); - // Process and validate localizedTexts array structure $localizedData = $this->processLocalizedTexts($data['localizedTexts']); // Initialize public members from the associative array (values can be null) $this->id = $data['id']; $this->parentGroupId = $data['parentGroupId']; - $this->admins = $data['admins']; + $this->admins = $this->processAdminsStructure($data['admins']); $this->name = $localizedData['name']; $this->description = $localizedData['description']; $this->organizational = $data['organizational']; @@ -217,6 +225,10 @@ public function jsonSerialize(): array ]; } + /* + * Private static methods + */ + /** * Make sure all ancestor groups are included in the selection. The selected groups array is updated in place. * @param RecodexGroup[] $selectedGroups groups selected so far (id => RecodexGroup) @@ -255,6 +267,56 @@ private static function belongsToSisGroup(RecodexGroup $group, array $sisGroupsI return false; } + private static function belongsToCourses(RecodexGroup $group, array $coursesIndex): bool + { + foreach ($group->attributes[self::ATTR_COURSE_KEY] ?? [] as $courseId) { + if (array_key_exists($courseId, $coursesIndex)) { + return true; + } + } + return false; + } + + /* + * Public static methods -- array operations for groups + */ + + /** + * Sorts the groups by their name (by English name, Czech name is used as fallback). + * @param RecodexGroup[] $groups The list of groups to sort (in place) + */ + public static function sortGroupsByName(array &$groups): void + { + usort($groups, fn($a, $b) => strcmp($a->name['en'] ?? $a->name['cs'], $b->name['en'] ?? $b->name['cs'])); + } + + /** + * Populates the children array for each group based on the parentGroupId. + * @param RecodexGroup[] $groups The list of groups to populate children for. + * @return RecodexGroup[] The list of root groups (those without a parent). + */ + public static function populateChildren(array $groups): array + { + $rootGroups = []; + foreach ($groups as $group) { + if ($group->parentGroupId) { + if (!isset($groups[$group->parentGroupId])) { + throw new LogicException('Parent group not found'); + } + $groups[$group->parentGroupId]->children[] = $group; + } else { + $rootGroups[] = $group; + } + } + + foreach ($groups as $group) { + self::sortGroupsByName($group->children); + } + self::sortGroupsByName($rootGroups); + + return $rootGroups; + } + /** * Prunes the group list for students, keeping only relevant groups. * Relevant are groups that belong to any SIS group or where the student already belongs to @@ -276,4 +338,42 @@ public static function pruneForStudent(array $groups, array $sisGroups): array self::ancestralClosure($pruned, $groups); return $pruned; } + + /** + * Prunes the group list for teachers, keeping only relevant groups. + * Relevant groups are those that belong to any of the specified courses, + * plus all their descendants (possible targets) and ancestors (for hierarchical naming). + * @param RecodexGroup[] $groups The list of groups to prune (indexed by group IDs). + * @param array $courses The list of course IDs. + * @return RecodexGroup[] The pruned list of groups (indexed by group IDs). + */ + public static function pruneForTeacher(array $groups, array $courses): array + { + $coursesIndex = array_flip($courses); + $pruned = []; + foreach ($groups as $id => $group) { + if (self::belongsToCourses($group, $coursesIndex)) { + $pruned[$id] = $group; + } + } + + // iteratively scan the groups, add children of pruned groups as long as the pruned array grows + // Note: the tree structure is very flat, this takes 3 or 4 iterations at the most + do { + $changed = false; + foreach ($groups as $id => $group) { + if ( + $group->parentGroupId && !array_key_exists($id, $pruned) + && array_key_exists($group->parentGroupId, $pruned) + ) { + // group is not in the result, but its parent is => we must add it as well + $pruned[$id] = $group; + $changed = true; // another run will be required + } + } + } while ($changed); + + self::ancestralClosure($pruned, $groups); + return $pruned; + } } diff --git a/app/model/repository/SisScheduleEvents.php b/app/model/repository/SisScheduleEvents.php index b24fc9b..0fda748 100644 --- a/app/model/repository/SisScheduleEvents.php +++ b/app/model/repository/SisScheduleEvents.php @@ -26,10 +26,10 @@ public function findBySisId(string $sisId): ?SisScheduleEvent * Get all scheduling events for a specific user (optionally filter by term and affiliation). * @param User $user * @param SisTerm|null $term if given, only events of particular term are returned - * @param string|null $affiliation if given, only events with particular affiliation are returned + * @param string|string[]|null $affiliation if given, only events with particular affiliation(s) are returned * @return SisScheduleEvent[] */ - public function allEventsOfUser(User $user, ?SisTerm $term = null, ?string $affiliation = null): array + public function allEventsOfUser(User $user, ?SisTerm $term = null, mixed $affiliation = null): array { $qb = $this->createQueryBuilder('e') ->innerJoin('e.affiliations', 'a') @@ -42,8 +42,13 @@ public function allEventsOfUser(User $user, ?SisTerm $term = null, ?string $affi } if ($affiliation) { - $qb->andWhere('a.type = :affiliation') - ->setParameter('affiliation', $affiliation); + if (is_string($affiliation)) { + $qb->andWhere('a.type = :affiliation') + ->setParameter('affiliation', $affiliation); + } elseif (is_array($affiliation)) { + $qb->andWhere('a.type IN (:affiliation)') + ->setParameter('affiliation', $affiliation); + } } return $qb->getQuery()->getResult(); diff --git a/app/security/TokenScope.php b/app/security/TokenScope.php index 52d8d3d..6a8b274 100644 --- a/app/security/TokenScope.php +++ b/app/security/TokenScope.php @@ -43,9 +43,9 @@ class TokenScope public const EMAIL_VERIFICATION = "email-verification"; /** - * Scope used for 3rd party tools designed to externally manage groups and student memeberships. + * Scope used for 3rd party tools designed to externally manage groups and student memberships. */ - public const GROUP_EXTERNAL_ATTRIBUTES = "group-external-attributes"; + public const GROUP_EXTERNAL = "group-external"; /** * Scope for managing the users. Used in case the user data needs to be updated from an external database. From 632af498297d51882b1f0103b6b5326d4b5ce55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Thu, 4 Sep 2025 18:37:21 +0200 Subject: [PATCH 04/21] Implementing endpoints for retrieving groups (student and teacher). --- app/config/config.neon | 1 + app/config/permissions.neon | 18 ++++- app/exceptions/NotImplementedException.php | 4 +- app/presenters/GroupsPresenter.php | 94 +++++++++++++++++++--- app/router/RouterFactory.php | 3 +- app/security/ACL/IGroupsPermissions.php | 10 +++ 6 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 app/security/ACL/IGroupsPermissions.php diff --git a/app/config/config.neon b/app/config/config.neon index 5002cee..c7dc586 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -81,6 +81,7 @@ mail: # configuration of sending mails acl: config: %appDir%/config/permissions.neon acl: + group: App\Security\ACL\IGroupPermissions term: App\Security\ACL\ITermPermissions user: App\Security\ACL\IUserPermissions policies: diff --git a/app/config/permissions.neon b/app/config/permissions.neon index 26fc7a0..97e55c0 100644 --- a/app/config/permissions.neon +++ b/app/config/permissions.neon @@ -31,7 +31,7 @@ permissions: conditions: - user.isSameUser - + # Term permissions - allow: true resource: term actions: @@ -52,4 +52,18 @@ permissions: actions: - viewTeacherCourses conditions: - - term.isVisibleToTeachers \ No newline at end of file + - term.isVisibleToTeachers + + + # Group permissions + - allow: true + resource: group + role: student + actions: + - viewStudent + + - allow: true + resource: group + role: supervisor-student + actions: + - viewTeacher diff --git a/app/exceptions/NotImplementedException.php b/app/exceptions/NotImplementedException.php index b7b8dc6..39d4ab4 100644 --- a/app/exceptions/NotImplementedException.php +++ b/app/exceptions/NotImplementedException.php @@ -5,7 +5,7 @@ use Nette\Http\IResponse; /** - * Actually not used for debbuging purposes but used in production and thrown + * Actually not used for debugging purposes but used in production and thrown * if user requested non-existing application route. */ class NotImplementedException extends ApiException @@ -16,7 +16,7 @@ class NotImplementedException extends ApiException public function __construct() { parent::__construct( - "This feature is not implemented. Contact the authors of the API for more information about the status of the API.", + "This feature is not implemented (yet).", IResponse::S501_NotImplemented, FrontendErrorMappings::E501_000__NOT_IMPLEMENTED ); diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 6ff513a..48a400f 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -3,12 +3,11 @@ namespace App\Presenters; use App\Exceptions\ForbiddenRequestException; -use App\Exceptions\BadRequestException; -use App\Model\Entity\SisTerm; +use App\Exceptions\NotImplementedException; +use App\Helpers\RecodexApiHelper; +use App\Helpers\RecodexGroup; use App\Model\Repository\SisTerms; -use App\Security\ACL\ITermPermissions; -use Nette\Application\Request; -use DateTime; +use App\Security\ACL\IGroupPermissions; /** * Group management (both for teachers and students). @@ -21,18 +20,93 @@ class GroupsPresenter extends BasePresenter */ public $sisTerms; + /** + * @var RecodexApiHelper + * @inject + */ + public $recodexApi; + + /** + * @var IGroupPermissions + * @inject + */ + public $groupAcl; + + public function checkStudent() + { + if (!$this->groupAcl->canViewStudent()) { + throw new ForbiddenRequestException("You do not have permissions to list student groups."); + } + } - public function checkDefault() + /** + * Proxy to ReCodEx that retrieves all groups relevant for student (joining groups). + * @GET + * @Param(type="query", name="eventIds", validation="array", + * description="List of SIS group IDs that we search for.") + */ + public function actionStudent(array $eventIds) { - // throw new ForbiddenRequestException("You do not have permissions to list terms."); + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $groups = RecodexGroup::pruneForStudent($groups, $eventIds); + $this->sendSuccessResponse($groups); + } + + public function checkTeacher() + { + if (!$this->groupAcl->canViewTeacher()) { + throw new ForbiddenRequestException("You do not have permissions to list teacher groups."); + } } /** + * Proxy to ReCodEx that retrieves all groups relevant for teacher creating groups. * @GET + * @Param(type="query", name="courseIds", validation="array", + * description="List of SIS courses the teacher is involved in.") + */ + public function actionTeacher(array $courseIds) + { + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $groups = RecodexGroup::pruneForTeacher($groups, $courseIds); + $this->sendSuccessResponse($groups); + } + + /** + * Proxy to ReCodEx that creates a new group. + * @POST + */ + public function actionCreate() + { + throw new NotImplementedException(); + } + + /** + * Proxy to ReCodEx that binds a group with schedule event (student group) in SIS. + * This basically sets the 'group' attribute to ReCodEx group entity. + * @POST + */ + public function actionBind(string $id, string $eventId) + { + throw new NotImplementedException(); + } + + /** + * Proxy to ReCodEx that unbinds a group with schedule event (student group) in SIS. + * This basically removes the 'group' attribute from ReCodEx group entity. + * @POST + */ + public function actionUnbind(string $id, string $eventId) + { + throw new NotImplementedException(); + } + + /** + * Proxy to ReCodEx that adds student to a group. + * @POST */ - public function actionDefault() + public function actionJoin(string $id) { - $terms = $this->sisTerms->findAll(); - $this->sendSuccessResponse($terms); + throw new NotImplementedException(); } } diff --git a/app/router/RouterFactory.php b/app/router/RouterFactory.php index 49437fa..5ebe5f1 100644 --- a/app/router/RouterFactory.php +++ b/app/router/RouterFactory.php @@ -73,7 +73,8 @@ private static function createCoursesRoutes(string $prefix): RouteList private static function createGroupsRoutes(string $prefix): RouteList { $router = new RouteList(); - $router[] = new GetRoute("$prefix", "Groups:default"); + $router[] = new GetRoute("$prefix/student", "Groups:student"); + $router[] = new GetRoute("$prefix/teacher", "Groups:teacher"); $router[] = new PostRoute("$prefix", "Groups:create"); $router[] = new PostRoute("$prefix//bind/", "Groups:bind"); $router[] = new DeleteRoute("$prefix//bind/", "Groups:unbind"); diff --git a/app/security/ACL/IGroupsPermissions.php b/app/security/ACL/IGroupsPermissions.php new file mode 100644 index 0000000..48c192b --- /dev/null +++ b/app/security/ACL/IGroupsPermissions.php @@ -0,0 +1,10 @@ + Date: Fri, 5 Sep 2025 17:24:59 +0200 Subject: [PATCH 05/21] Adding tests for group-retrieval endpoints. --- app/helpers/Recodex/RecodexApiHelper.php | 11 +- tests/Presenters/GroupsPresenter.phpt | 175 +++++++++++++++++++++++ 2 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 tests/Presenters/GroupsPresenter.phpt diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index b2265b2..164f57c 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -39,9 +39,9 @@ class RecodexApiHelper /** * @param array $config - * @param GuzzleHttp\HandlerStack|null $handler An optional HTTP handler (mainly for unit testing purposes) + * @param GuzzleHttp\Client|null $client optional injection of HTTP client for testing purposes */ - public function __construct(array $config, ?GuzzleHttp\HandlerStack $handler = null) + public function __construct(array $config, ?GuzzleHttp\Client $client = null) { $this->extensionId = Arrays::get($config, "extensionId", ""); if (!$this->extensionId) { @@ -61,11 +61,10 @@ public function __construct(array $config, ?GuzzleHttp\HandlerStack $handler = n $this->sisIdKey = Arrays::get($config, "sisIdKey", "cas-uk"); $this->sisLoginKey = Arrays::get($config, "sisLoginKey", "ldap-uk"); - $options = ['base_uri' => $this->apiBase]; - if ($handler !== null) { - $options['handler'] = $handler; + if (!$client) { + $client = new GuzzleHttp\Client(['base_uri' => $this->apiBase]); } - $this->client = new GuzzleHttp\Client($options); + $this->client = $client; } public function getSisIdKey(): string diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt new file mode 100644 index 0000000..344894f --- /dev/null +++ b/tests/Presenters/GroupsPresenter.phpt @@ -0,0 +1,175 @@ +container = $container; + $this->em = PresenterTestHelper::getEntityManager($container); + $this->user = $container->getByType(\Nette\Security\User::class); + $this->client = Mockery::mock(Client::class); + + $recodexHelperName = current($this->container->findByType(RecodexApiHelper::class)); + $this->container->removeService($recodexHelperName); + $this->container->addService($recodexHelperName, new RecodexApiHelper( + [ + 'extensionId' => 'sis-cuni', + 'apiBase' => 'https://recodex.example/', + ], + $this->client + )); + } + + protected function setUp() + { + PresenterTestHelper::fillDatabase($this->container); + $this->presenter = PresenterTestHelper::createPresenter( + $this->container, + GroupsPresenter::class + ); + } + + protected function tearDown() + { + Mockery::close(); + + if ($this->user->isLoggedIn()) { + $this->user->logout(true); + } + } + + private static function group( + string $id, + ?string $parentId, + string $name, + bool $org = false, + array $attributes = [], + ?string $membership = null + ): array { + return [ + "id" => $id, + "parentGroupId" => $parentId, + "admins" => [ + "teacher1" => [ + "titlesBeforeName" => "", + "firstName" => "First", + "lastName" => "Teacher", + "titlesAfterName" => "", + "email" => "teacher1@recodex" + ] + ], + "localizedTexts" => [ + [ + "id" => "text1", + "locale" => "en", + "name" => $name, + "description" => "", + "createdAt" => 1738275050 + ] + ], + "organizational" => $org, + "exam" => false, + "public" => false, + "detaining" => false, + "attributes" => $attributes ? ['sis-cuni' => $attributes] : [], + "membership" => $membership + ]; + } + + public function testListGroupsStudent() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('p1', null, 'Parent', true, []), + self::group('p2', null, 'Parent 2', true, []), + self::group('g1', 'p1', 'Group 1', false, ['group' => ['sis1']]), + self::group('g2', 'p1', 'Group 2', false, ['group' => ['sis2']], 'student'), + self::group('g3', 'p1', 'Group 3', false, ['group' => ['sis3']]), + ] + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'GET', + ['action' => 'student', 'eventIds' => ['sis1']] + ); + + Assert::count(3, $payload); + + $ids = array_map(fn($group) => $group->id, $payload); + sort($ids); + Assert::equal(['g1', 'g2', 'p1'], $ids); + Assert::null($payload['p1']->parentGroupId); + Assert::equal('p1', $payload['g1']->parentGroupId); + Assert::equal('p1', $payload['g2']->parentGroupId); + Assert::true($payload['p1']->organizational); + Assert::false($payload['g1']->organizational); + Assert::false($payload['g2']->organizational); + } + + public function testListGroupsTeacher() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('p1', 'r', 'Prg 1', true, ['course' => ['prg1']]), + self::group('p2', 'r', 'Prg 2', true, ['course' => ['prg2']], 'teacher'), + self::group('p3', 'r', 'Prg 1 & 3', true, ['course' => ['prg3', 'prg1']]), + self::group('g1', 'p1', 'Group 1'), + self::group('g2', 'p2', 'Group 2', 'teacher'), + self::group('g3', 'p3', 'Group 3'), + ] + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'GET', + ['action' => 'teacher', 'courseIds' => ['prg1']] + ); + + Assert::count(5, $payload); + + $ids = array_map(fn($group) => $group->id, $payload); + sort($ids); + Assert::equal(['g1', 'g3', 'p1', 'p3', 'r'], $ids); + } +} + +(new TestGroupsPresenter())->run(); From c98564ed5859af16c4f5407432320bbfeca368b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sat, 6 Sep 2025 11:22:50 +0200 Subject: [PATCH 06/21] Adding event IDs to teachers group retrieval endpoint (so we can cover also stranded groups with bound SIS event IDs, but which are no longer under proper 1st level course group). --- app/commands/RecodexGroups.php | 2 +- app/helpers/Recodex/RecodexGroup.php | 24 +++++++++++++++++++----- app/presenters/GroupsPresenter.php | 6 ++++-- tests/Presenters/GroupsPresenter.phpt | 10 ++++++---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/app/commands/RecodexGroups.php b/app/commands/RecodexGroups.php index 1d4f10a..ad5db33 100644 --- a/app/commands/RecodexGroups.php +++ b/app/commands/RecodexGroups.php @@ -138,7 +138,7 @@ protected function execute(InputInterface $input, OutputInterface $output) [SisAffiliation::TYPE_TEACHER, SisAffiliation::TYPE_GUARANTOR] ); $courseIds = array_map(fn($e) => $e->getCourse()->getCode(), $events); - $teachGroups = RecodexGroup::pruneForTeacher($groups, $courseIds); + $teachGroups = RecodexGroup::pruneForTeacher($groups, $courseIds, []); $output->writeln('Teacher view:'); $output->writeln('-------------'); self::printGroups($output, $teachGroups); diff --git a/app/helpers/Recodex/RecodexGroup.php b/app/helpers/Recodex/RecodexGroup.php index b4cdcf5..b4ca0fe 100644 --- a/app/helpers/Recodex/RecodexGroup.php +++ b/app/helpers/Recodex/RecodexGroup.php @@ -257,7 +257,7 @@ private static function ancestralClosure(array &$selectedGroups, array $allGroup * @param array $sisGroupsIndex The index of SIS groups [ sisGroupId => unused value ]. * @return bool True if the group belongs to any SIS group, false otherwise. */ - private static function belongsToSisGroup(RecodexGroup $group, array $sisGroupsIndex): bool + private static function belongsToSisGroups(RecodexGroup $group, array $sisGroupsIndex): bool { foreach ($group->attributes[self::ATTR_GROUP_KEY] ?? [] as $sisGrpId) { if (array_key_exists($sisGrpId, $sisGroupsIndex)) { @@ -322,7 +322,7 @@ public static function populateChildren(array $groups): array * Relevant are groups that belong to any SIS group or where the student already belongs to * (the ancestral closure of relevant groups is returned so hierarchical names can be displayed). * @param RecodexGroup[] $groups The list of groups to prune (indexed by group IDs). - * @param array $sisGroups The list of SIS group IDs. + * @param string[] $sisGroups The list of SIS group IDs. * @return RecodexGroup[] The pruned list of groups (indexed by group IDs). */ public static function pruneForStudent(array $groups, array $sisGroups): array @@ -330,7 +330,7 @@ public static function pruneForStudent(array $groups, array $sisGroups): array $sisGroupsIndex = array_flip($sisGroups); $pruned = []; foreach ($groups as $id => $group) { - if (self::belongsToSisGroup($group, $sisGroupsIndex) || $group->membership === 'student') { + if (self::belongsToSisGroups($group, $sisGroupsIndex) || $group->membership === 'student') { $pruned[$id] = $group; } } @@ -344,17 +344,23 @@ public static function pruneForStudent(array $groups, array $sisGroups): array * Relevant groups are those that belong to any of the specified courses, * plus all their descendants (possible targets) and ancestors (for hierarchical naming). * @param RecodexGroup[] $groups The list of groups to prune (indexed by group IDs). - * @param array $courses The list of course IDs. + * @param string[] $courses The list of course IDs. + * @param string[] $sisGroups The list of SIS group IDs. * @return RecodexGroup[] The pruned list of groups (indexed by group IDs). */ - public static function pruneForTeacher(array $groups, array $courses): array + public static function pruneForTeacher(array $groups, array $courses, array $sisGroups): array { $coursesIndex = array_flip($courses); + $sisGroupsIndex = array_flip($sisGroups); $pruned = []; + $boundGroups = []; foreach ($groups as $id => $group) { if (self::belongsToCourses($group, $coursesIndex)) { $pruned[$id] = $group; } + if (self::belongsToSisGroups($group, $sisGroupsIndex)) { + $boundGroups[$id] = $group; + } } // iteratively scan the groups, add children of pruned groups as long as the pruned array grows @@ -373,6 +379,14 @@ public static function pruneForTeacher(array $groups, array $courses): array } } while ($changed); + // inject directly bound groups before ancestral closure + // this is a rare case, since bound groups should be normally already among course descendant groups + foreach ($boundGroups as $id => $group) { + if (!array_key_exists($id, $pruned)) { + $pruned[$id] = $group; + } + } + self::ancestralClosure($pruned, $groups); return $pruned; } diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 48a400f..0eb5602 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -62,13 +62,15 @@ public function checkTeacher() /** * Proxy to ReCodEx that retrieves all groups relevant for teacher creating groups. * @GET + * @Param(type="query", name="eventIds", validation="array", + * description="List of SIS group IDs the teacher teaches.") * @Param(type="query", name="courseIds", validation="array", * description="List of SIS courses the teacher is involved in.") */ - public function actionTeacher(array $courseIds) + public function actionTeacher(array $eventIds, array $courseIds) { $groups = $this->recodexApi->getGroups($this->getCurrentUser()); - $groups = RecodexGroup::pruneForTeacher($groups, $courseIds); + $groups = RecodexGroup::pruneForTeacher($groups, $courseIds, $eventIds); $this->sendSuccessResponse($groups); } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 344894f..ee87584 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -151,9 +151,11 @@ class TestGroupsPresenter extends Tester\TestCase self::group('p1', 'r', 'Prg 1', true, ['course' => ['prg1']]), self::group('p2', 'r', 'Prg 2', true, ['course' => ['prg2']], 'teacher'), self::group('p3', 'r', 'Prg 1 & 3', true, ['course' => ['prg3', 'prg1']]), + self::group('p4', 'r', 'Prg 2 & 4', true, ['course' => ['prg2', 'prg4']]), self::group('g1', 'p1', 'Group 1'), - self::group('g2', 'p2', 'Group 2', 'teacher'), + self::group('g2', 'p2', 'Group 2', false, ['group' => ['sis2']], 'teacher'), self::group('g3', 'p3', 'Group 3'), + self::group('g4', 'p4', 'Group 4'), ] ]))); @@ -161,14 +163,14 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Terms', 'GET', - ['action' => 'teacher', 'courseIds' => ['prg1']] + ['action' => 'teacher', 'eventIds' => ['sis2'], 'courseIds' => ['prg1']] ); - Assert::count(5, $payload); + Assert::count(7, $payload); $ids = array_map(fn($group) => $group->id, $payload); sort($ids); - Assert::equal(['g1', 'g3', 'p1', 'p3', 'r'], $ids); + Assert::equal(['g1', 'g2', 'g3', 'p1', 'p2', 'p3', 'r'], $ids); } } From 3d9d5136b21af42eeb67b2c1cd94322e2ad853b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Sun, 7 Sep 2025 17:51:09 +0200 Subject: [PATCH 07/21] Implementing ACLs for SIS events and related ReCodEx group operations (create, (un)bind, join). --- app/config/config.neon | 1 + app/config/permissions.neon | 19 +++ app/helpers/Recodex/RecodexGroup.php | 26 ++++ app/model/entity/SisTerm.php | 5 + app/presenters/CoursesPresenter.php | 2 +- app/presenters/GroupsPresenter.php | 128 +++++++++++++++++- app/router/RouterFactory.php | 2 +- app/security/ACL/IEventPermissions.php | 14 ++ ...sPermissions.php => IGroupPermissions.php} | 0 .../Policies/EventPermissionPolicy.php | 49 +++++++ tests/Presenters/TermsPresenter.phpt | 4 +- 11 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 app/security/ACL/IEventPermissions.php rename app/security/ACL/{IGroupsPermissions.php => IGroupPermissions.php} (100%) create mode 100644 app/security/Policies/EventPermissionPolicy.php diff --git a/app/config/config.neon b/app/config/config.neon index c7dc586..0f65e1f 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -82,6 +82,7 @@ acl: config: %appDir%/config/permissions.neon acl: group: App\Security\ACL\IGroupPermissions + event: App\Security\ACL\IEventPermissions term: App\Security\ACL\ITermPermissions user: App\Security\ACL\IUserPermissions policies: diff --git a/app/config/permissions.neon b/app/config/permissions.neon index 97e55c0..5cc61bb 100644 --- a/app/config/permissions.neon +++ b/app/config/permissions.neon @@ -67,3 +67,22 @@ permissions: role: supervisor-student actions: - viewTeacher + + # Event permissions + - allow: true + resource: event + role: student + actions: + - joinGroup + conditions: + - event.isUserEnrolledFor + + - allow: true + resource: event + role: supervisor-student + actions: + - bindGroup + - createGroup + conditions: + - event.isUserTeacherOf + diff --git a/app/helpers/Recodex/RecodexGroup.php b/app/helpers/Recodex/RecodexGroup.php index b4ca0fe..c13000b 100644 --- a/app/helpers/Recodex/RecodexGroup.php +++ b/app/helpers/Recodex/RecodexGroup.php @@ -31,6 +31,14 @@ class RecodexGroup implements JsonSerializable */ public const ATTR_GROUP_KEY = 'group'; + /* + * Known membership constants from ReCodEx + */ + public const MEMBERSHIP_ADMIN = 'admin'; + public const MEMBERSHIP_SUPERVISOR = 'supervisor'; + public const MEMBERSHIP_OBSERVER = 'observer'; + public const MEMBERSHIP_STUDENT = 'student'; + /** * ReCodEx group ID */ @@ -225,6 +233,24 @@ public function jsonSerialize(): array ]; } + /* + * Public helper methods (data getters) + */ + public function hasGroupAttribute($groupId) + { + return in_array($groupId, $this->attributes[self::ATTR_GROUP_KEY] ?? [], true); + } + + public function hasTermAttribute($term) + { + return in_array($term, $this->attributes[self::ATTR_TERM_KEY] ?? [], true); + } + + public function hasCourseAttribute($courseId) + { + return in_array($courseId, $this->attributes[self::ATTR_COURSE_KEY] ?? [], true); + } + /* * Private static methods */ diff --git a/app/model/entity/SisTerm.php b/app/model/entity/SisTerm.php index cc31c2c..14f2ef5 100644 --- a/app/model/entity/SisTerm.php +++ b/app/model/entity/SisTerm.php @@ -132,6 +132,11 @@ public function getTerm(): int return $this->term; } + public function getYearTermKey(): string + { + return sprintf("%d-%d", $this->year, $this->term); + } + public function getBeginning(): ?DateTime { return $this->beginning; diff --git a/app/presenters/CoursesPresenter.php b/app/presenters/CoursesPresenter.php index c995202..23e8032 100644 --- a/app/presenters/CoursesPresenter.php +++ b/app/presenters/CoursesPresenter.php @@ -131,7 +131,7 @@ private function refetchSisCourses(User $user): void // find active terms and create mapping termId => SisTerm $terms = []; foreach ($this->sisTerms->findAllActive() as $term) { - $key = sprintf("%s-%s", $term->getYear(), $term->getTerm()); + $key = $term->getYearTermKey(); $terms[$key] = $term; // we need to clear current affiliations to reflect when students' get unenrolled from courses diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 0eb5602..cc252e9 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -2,11 +2,15 @@ namespace App\Presenters; +use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; +use App\Exceptions\NotFoundException; use App\Exceptions\NotImplementedException; use App\Helpers\RecodexApiHelper; use App\Helpers\RecodexGroup; -use App\Model\Repository\SisTerms; +use App\Model\Entity\SisScheduleEvent; +use App\Model\Repository\SisScheduleEvents; +use App\Security\ACL\IEventPermissions; use App\Security\ACL\IGroupPermissions; /** @@ -15,10 +19,10 @@ class GroupsPresenter extends BasePresenter { /** - * @var SisTerms + * @var SisScheduleEvents * @inject */ - public $sisTerms; + public $sisEvents; /** * @var RecodexApiHelper @@ -26,12 +30,61 @@ class GroupsPresenter extends BasePresenter */ public $recodexApi; + /** + * @var IEventPermissions + * @inject + */ + public $eventAcl; + /** * @var IGroupPermissions * @inject */ public $groupAcl; + private function isGroupSuitableForEvent(array $groups, string $groupId, SisScheduleEvent $event): void + { + if (empty($groups[$groupId])) { + throw new NotFoundException("Group $groupId does not exist or is not accessible by the user."); + } + + $courseId = $event->getCourse()->getCode(); + $term = $event->getTerm()->getYearTermKey(); + $courseCheck = $termCheck = false; + $group = $groups[$groupId]; + while ($group !== null && !$courseCheck && !$termCheck) { + $courseCheck |= $group->hasCourseAttribute($courseId); + $termCheck |= $group->hasTermAttribute($term); + $group = $group->parentGroupId ? ($groups[$group->parentGroupId] ?? null) : null; + } + + if (!$courseCheck || !$termCheck) { + throw new ForbiddenRequestException("Group $groupId is not located under the required course or term."); + } + } + + private function canUserAdministrateGroup(array $groups, string $groupId): void + { + if (empty($groups[$groupId])) { + throw new NotFoundException("Group $groupId does not exist or is not accessible by the user."); + } + + $group = $groups[$groupId]; + if ($group->membership === RecodexGroup::MEMBERSHIP_SUPERVISOR) { + return; // direct supervisor has sufficient rights + } + + // admin of selected group or any ancestor group also has sufficient rights + while ($group !== null) { + if ($group->membership === RecodexGroup::MEMBERSHIP_ADMIN) { + return; + } + $group = $group->parentGroupId ? ($groups[$group->parentGroupId] ?? null) : null; + } + + throw new ForbiddenRequestException("You do not have permissions to administrate group $groupId."); + } + public function checkStudent() { if (!$this->groupAcl->canViewStudent()) { @@ -74,15 +127,43 @@ public function actionTeacher(array $eventIds, array $courseIds) $this->sendSuccessResponse($groups); } + public function checkCreate(string $parentId, string $eventId) + { + $event = $this->sisEvents->findOrThrow($eventId); + if (!$this->eventAcl->canCreateGroup($event)) { + throw new ForbiddenRequestException("You do not have permissions to create groups for selected SIS event."); + } + + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $this->isGroupSuitableForEvent($groups, $parentId, $event); // throws exception if not suitable + } + /** * Proxy to ReCodEx that creates a new group. * @POST + * @param string $parentId ID of the parent group under which the new group is created + * @param string $eventId ID of the SIS event the new group is created for */ - public function actionCreate() + public function actionCreate(string $parentId, string $eventId) { throw new NotImplementedException(); } + public function checkBind(string $id, string $eventId) + { + $event = $this->sisEvents->findOrThrow($eventId); + if (!$this->eventAcl->canBindGroup($event)) { + throw new ForbiddenRequestException("You do not have permissions to bind groups for selected SIS event."); + } + + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $this->isGroupSuitableForEvent($groups, $id, $event); // throws exception if not suitable + + if ($groups[$id]->hasGroupAttribute($event->getSisId())) { + throw new BadRequestException("Group $id is already bound to the selected SIS event."); + } + } + /** * Proxy to ReCodEx that binds a group with schedule event (student group) in SIS. * This basically sets the 'group' attribute to ReCodEx group entity. @@ -93,6 +174,21 @@ public function actionBind(string $id, string $eventId) throw new NotImplementedException(); } + public function checkUnbind(string $id, string $eventId) + { + $event = $this->sisEvents->findOrThrow($eventId); + if (!$this->eventAcl->canBindGroup($event)) { + throw new ForbiddenRequestException("You do not have permissions to unbind groups for selected SIS event."); + } + + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $this->canUserAdministrateGroup($groups, $id); // throws exception if not + + if (!$groups[$id]->hasGroupAttribute($event->getSisId())) { + throw new BadRequestException("Group $id is not bound to the selected SIS event."); + } + } + /** * Proxy to ReCodEx that unbinds a group with schedule event (student group) in SIS. * This basically removes the 'group' attribute from ReCodEx group entity. @@ -103,6 +199,30 @@ public function actionUnbind(string $id, string $eventId) throw new NotImplementedException(); } + public function checkJoin(string $id) + { + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + if (empty($groups[$id])) { + throw new NotFoundException("Group $id does not exist or is not accessible by the user."); + } + + $group = $groups[$id]; + if ($group->membership !== null) { + throw new BadRequestException("User is already a member ($group->membership) of group $id."); + } + + foreach ($group->attributes[RecodexGroup::ATTR_GROUP_KEY] ?? [] as $eventId) { + if (!$this->eventAcl->canJoinGroup($this->sisEvents->findOrThrow($eventId))) { + return; // suitable event was found + } + } + + // no corresponding event found -> deny access + throw new ForbiddenRequestException( + "Group $id does not correspond to any of SIS events you are enrolled for." + ); + } + /** * Proxy to ReCodEx that adds student to a group. * @POST diff --git a/app/router/RouterFactory.php b/app/router/RouterFactory.php index 5ebe5f1..fe0d5f0 100644 --- a/app/router/RouterFactory.php +++ b/app/router/RouterFactory.php @@ -75,7 +75,7 @@ private static function createGroupsRoutes(string $prefix): RouteList $router = new RouteList(); $router[] = new GetRoute("$prefix/student", "Groups:student"); $router[] = new GetRoute("$prefix/teacher", "Groups:teacher"); - $router[] = new PostRoute("$prefix", "Groups:create"); + $router[] = new PostRoute("$prefix//create/", "Groups:create"); $router[] = new PostRoute("$prefix//bind/", "Groups:bind"); $router[] = new DeleteRoute("$prefix//bind/", "Groups:unbind"); $router[] = new PostRoute("$prefix//join", "Groups:join"); diff --git a/app/security/ACL/IEventPermissions.php b/app/security/ACL/IEventPermissions.php new file mode 100644 index 0000000..1754e70 --- /dev/null +++ b/app/security/ACL/IEventPermissions.php @@ -0,0 +1,14 @@ +affiliations = $affiliations; + } + + public function getAssociatedClass() + { + return SisScheduleEvent::class; + } + + public function isUserEnrolledFor(Identity $identity, SisScheduleEvent $event): bool + { + $currentUser = $identity->getUserData(); + if (!$currentUser) { + return false; + } + + $affiliation = $this->affiliations->getAffiliation($event, $currentUser); + return $affiliation !== null && $affiliation->getType() === SisAffiliation::TYPE_STUDENT; + } + + public function isUserTeacherOf(Identity $identity, SisScheduleEvent $event): bool + { + $currentUser = $identity->getUserData(); + if (!$currentUser) { + return false; + } + + $affiliation = $this->affiliations->getAffiliation($event, $currentUser); + return $affiliation !== null && $affiliation->getType() !== SisAffiliation::TYPE_STUDENT; + // (affiliation exists and it's anything but student) + } +} diff --git a/tests/Presenters/TermsPresenter.phpt b/tests/Presenters/TermsPresenter.phpt index 0f2095f..fc94e64 100644 --- a/tests/Presenters/TermsPresenter.phpt +++ b/tests/Presenters/TermsPresenter.phpt @@ -64,7 +64,7 @@ class TestTermsPresenter extends Tester\TestCase ); $terms = array_map(function ($term) { - return $term->getYear() . '-' . $term->getTerm(); + return $term->getYearTermKey(); }, $payload); sort($terms); Assert::equal(['2024-1', '2024-2', '2025-1', '2025-2'], $terms); @@ -335,7 +335,7 @@ class TestTermsPresenter extends Tester\TestCase Assert::equal('OK', $payload); $terms = array_map(function ($term) { - return $term->getYear() . '-' . $term->getTerm(); + return $term->getYearTermKey(); }, $this->presenter->sisTerms->findAll()); sort($terms); Assert::equal(['2024-2', '2025-1', '2025-2'], $terms); From 6ea32640f56ca9b9ad504ee13605d37f875bd20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 8 Sep 2025 00:31:25 +0200 Subject: [PATCH 08/21] Implementing ReCodEx API operations for adding/removing attributes and their debugging CLI commands. --- app/commands/RecodexAddAttribute.php | 70 ++++++++++++++++++++++ app/commands/RecodexRemoveAttribute.php | 74 ++++++++++++++++++++++++ app/config/config.neon | 2 + app/helpers/Recodex/RecodexApiHelper.php | 33 ++++++++++- app/helpers/SisHelper/SisHelper.php | 4 +- tests/Presenters/GroupsPresenter.phpt | 4 ++ 6 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 app/commands/RecodexAddAttribute.php create mode 100644 app/commands/RecodexRemoveAttribute.php diff --git a/app/commands/RecodexAddAttribute.php b/app/commands/RecodexAddAttribute.php new file mode 100644 index 0000000..a8c5714 --- /dev/null +++ b/app/commands/RecodexAddAttribute.php @@ -0,0 +1,70 @@ +recodexApi = $recodexApi; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Add external attribute to ReCodEx group.'); + $this->addArgument('groupId', InputArgument::REQUIRED, 'ID of the group to which the attribute will be added.'); + $this->addArgument('key', InputArgument::REQUIRED, 'The key of the attribute being added.'); + $this->addArgument('value', InputArgument::REQUIRED, 'The value of the attribute being added.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $key = $input->getArgument('key'); + $value = $input->getArgument('value'); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Adding attribute '$key' with value '$value' to group '$groupId'..."); + $this->recodexApi->addAttribute($groupId, $key, $value); + + return Command::SUCCESS; + } +} diff --git a/app/commands/RecodexRemoveAttribute.php b/app/commands/RecodexRemoveAttribute.php new file mode 100644 index 0000000..635b8ae --- /dev/null +++ b/app/commands/RecodexRemoveAttribute.php @@ -0,0 +1,74 @@ +recodexApi = $recodexApi; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Remove external attribute from ReCodEx group.'); + $this->addArgument( + 'groupId', + InputArgument::REQUIRED, + 'ID of the group from which the attribute will be removed.' + ); + $this->addArgument('key', InputArgument::REQUIRED, 'The key of the attribute being removed.'); + $this->addArgument('value', InputArgument::REQUIRED, 'The value of the attribute being removed.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $key = $input->getArgument('key'); + $value = $input->getArgument('value'); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Removing attribute '$key' with value '$value' from group '$groupId'..."); + $this->recodexApi->removeAttribute($groupId, $key, $value); + + return Command::SUCCESS; + } +} diff --git a/app/config/config.neon b/app/config/config.neon index 0f65e1f..45d664b 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -112,7 +112,9 @@ services: # commands - App\Console\DoctrineFixtures + - App\Console\RecodexAddAttribute - App\Console\RecodexGroups + - App\Console\RecodexRemoveAttribute - App\Console\RecodexToken - App\Console\SisGetCourse - App\Console\SisGetUser diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index 164f57c..d303618 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -8,6 +8,7 @@ use Nette; use GuzzleHttp; use Nette\Utils\Arrays; +use Tracy\Debugger; /** * Wrapper for ReCodEx API calls. @@ -126,15 +127,18 @@ private function processJsonBody($response) { $code = $response->getStatusCode(); if ($code !== 200) { + Debugger::log("HTTP request to ReCodEx API failed (response $code).", Debugger::DEBUG); throw new RecodexApiException("HTTP request failed (response $code)."); } $type = $response->getHeaderLine("Content-Type") ?? ''; if (!str_starts_with($type, 'application/json')) { - throw new RecodexApiException("JSON body was expected but '$type' returned instead."); + Debugger::log("JSON response expected from ReCodEx API but '$type' returned instead.", Debugger::DEBUG); + throw new RecodexApiException("JSON response was expected but '$type' returned instead."); } $body = json_decode($response->getBody()->getContents(), true); + Debugger::log($body, Debugger::DEBUG); if (($body['success'] ?? false) !== true) { $code = $body['code']; throw new RecodexApiException($body['error']['message'] ?? "API responded with error code $code."); @@ -218,6 +222,7 @@ public function getTempTokenInstance(string $token): string */ public function getTokenAndUser(): ?array { + Debugger::log('ReCodEx::getTokenAndUser()', Debugger::DEBUG); $body = $this->post('extensions/' . $this->extensionId); if (!is_array($body) || empty($body['accessToken']) || empty($body['user'])) { throw new RecodexApiException("Unexpected ReCodEx API response from extension token endpoint."); @@ -235,6 +240,7 @@ public function getTokenAndUser(): ?array */ public function getUser(string $id): ?RecodexUser { + Debugger::log('ReCodEx::getUser(' . $id . ')', Debugger::DEBUG); $body = $this->get("users/$id"); return $body ? new RecodexUser($body, $this) : null; } @@ -254,6 +260,8 @@ public function updateUser(User $user): RecodexUser 'email' => $user->getEmail(), ]; $id = $user->getId(); + Debugger::log('ReCodEx::updateUser(' . $id . ')', Debugger::INFO); + Debugger::log('New user data: ' . json_encode($body), Debugger::DEBUG); $res = $this->post("users/$id", [], $body); if (!$res || !is_array($res) || empty($res['user']) || ($res['user']['id'] ?? '') !== $id) { throw new RecodexApiException("Unexpected ReCodEx API response from update user's profile endpoint."); @@ -271,6 +279,7 @@ public function updateUser(User $user): RecodexUser */ public function setExternalId(string $id, string $service, string $externalId): RecodexUser { + Debugger::log("ReCodEx::setExternalId('$id', '$service', '$externalId')", Debugger::INFO); $res = $this->post("users/$id/external-login/$service", [], ['externalId' => $externalId]); if (!$res || !is_array($res) || ($res['id'] ?? '') !== $id) { throw new RecodexApiException("Unexpected ReCodEx API response from update user's external ID endpoint."); @@ -286,6 +295,7 @@ public function setExternalId(string $id, string $service, string $externalId): */ public function removeExternalId(string $id, string $service): RecodexUser { + Debugger::log("ReCodEx::removeExternalId('$id', '$service')", Debugger::INFO); $res = $this->delete("users/$id/external-login/$service"); if (!$res || !is_array($res) || ($res['id'] ?? '') !== $id) { throw new RecodexApiException("Unexpected ReCodEx API response from remove user's external ID endpoint."); @@ -300,6 +310,7 @@ public function removeExternalId(string $id, string $service): RecodexUser */ public function getGroups(User $user): array { + Debugger::log('ReCodEx::getGroups(' . $user->getId() . ')', Debugger::DEBUG); $body = $this->get( "group-attributes", ['instance' => $user->getInstanceId(), 'service' => $this->extensionId, 'user' => $user->getId()] @@ -311,4 +322,24 @@ public function getGroups(User $user): array } return $groups; } + + public function addAttribute(string $groupId, string $key, string $value): void + { + Debugger::log("ReCodEx::addAttribute('$groupId', '$key', '$value')", Debugger::INFO); + $this->post("group-attributes/$groupId", [], [ + 'service' => $this->extensionId, + 'key' => $key, + 'value' => $value + ]); + } + + public function removeAttribute(string $groupId, string $key, string $value): void + { + Debugger::log("ReCodEx::removeAttribute('$groupId', '$key', '$value')", Debugger::INFO); + $this->delete("group-attributes/$groupId", [ + 'service' => $this->extensionId, + 'key' => $key, + 'value' => $value + ]); + } } diff --git a/app/helpers/SisHelper/SisHelper.php b/app/helpers/SisHelper/SisHelper.php index 2eb2743..2302e75 100644 --- a/app/helpers/SisHelper/SisHelper.php +++ b/app/helpers/SisHelper/SisHelper.php @@ -110,7 +110,7 @@ public function getUserRecord(string $sisUserId): SisUserRecord 'token' => $salt . '$' . hash('sha256', "$salt,$this->secretKdojekdo"), ]; - Debugger::log("getUserRecord($sisUserId)", Debugger::DEBUG); + Debugger::log("SIS::getUserRecord($sisUserId)", Debugger::DEBUG); try { $response = $this->client->get('kdojekdo/rest.php', $this->prepareOptions($params)); } catch (GuzzleHttp\Exception\ClientException $e) { @@ -165,7 +165,7 @@ public function getCourses(string $sisUserId, mixed $terms = null) $termsStr = 'null'; } - Debugger::log("getCourses($sisUserId, $termsStr)", Debugger::DEBUG); + Debugger::log("SIS::getCourses($sisUserId, $termsStr)", Debugger::DEBUG); try { $response = $this->client->get('rozvrhng/rest.php', $this->prepareOptions($params)); } catch (GuzzleHttp\Exception\ClientException $e) { diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index ee87584..3789879 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -10,6 +10,7 @@ use Doctrine\ORM\EntityManagerInterface; use Tester\Assert; use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; +use Tracy\Debugger; /** * @testCase @@ -107,6 +108,8 @@ class TestGroupsPresenter extends Tester\TestCase public function testListGroupsStudent() { + Debugger::enable(false); + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ 'success' => true, @@ -174,4 +177,5 @@ class TestGroupsPresenter extends Tester\TestCase } } +Debugger::$logDirectory = __DIR__ . '/../../log'; (new TestGroupsPresenter())->run(); From 4b364cb8e88d6a64c8b66d62b737e870be691105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 8 Sep 2025 19:03:56 +0200 Subject: [PATCH 09/21] Implementing bind/unbind endpoints and adding corresponding presenter tests. --- app/config/config.neon | 1 + app/presenters/GroupsPresenter.php | 15 +- tests/Presenters/GroupsPresenter.phpt | 319 ++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 5 deletions(-) diff --git a/app/config/config.neon b/app/config/config.neon index 45d664b..b2d675a 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -87,6 +87,7 @@ acl: user: App\Security\ACL\IUserPermissions policies: _: App\Security\Policies\BasePermissionPolicy + event: App\Security\Policies\EventPermissionPolicy term: App\Security\Policies\TermPermissionPolicy user: App\Security\Policies\UserPermissionPolicy diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index cc252e9..dea6a06 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -52,9 +52,9 @@ private function isGroupSuitableForEvent(array $groups, string $groupId, SisSche $term = $event->getTerm()->getYearTermKey(); $courseCheck = $termCheck = false; $group = $groups[$groupId]; - while ($group !== null && !$courseCheck && !$termCheck) { - $courseCheck |= $group->hasCourseAttribute($courseId); - $termCheck |= $group->hasTermAttribute($term); + while ($group !== null && (!$courseCheck || !$termCheck)) { + $courseCheck = $courseCheck || $group->hasCourseAttribute($courseId); + $termCheck = $termCheck || $group->hasTermAttribute($term); $group = $group->parentGroupId ? ($groups[$group->parentGroupId] ?? null) : null; } @@ -158,6 +158,9 @@ public function checkBind(string $id, string $eventId) $groups = $this->recodexApi->getGroups($this->getCurrentUser()); $this->isGroupSuitableForEvent($groups, $id, $event); // throws exception if not suitable + if ($groups[$id]->organizational) { + throw new BadRequestException("Group $id is organizational, so it cannot be bound to a SIS event."); + } if ($groups[$id]->hasGroupAttribute($event->getSisId())) { throw new BadRequestException("Group $id is already bound to the selected SIS event."); @@ -171,7 +174,8 @@ public function checkBind(string $id, string $eventId) */ public function actionBind(string $id, string $eventId) { - throw new NotImplementedException(); + $this->recodexApi->addAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $eventId); + $this->sendSuccessResponse("OK"); } public function checkUnbind(string $id, string $eventId) @@ -196,7 +200,8 @@ public function checkUnbind(string $id, string $eventId) */ public function actionUnbind(string $id, string $eventId) { - throw new NotImplementedException(); + $this->recodexApi->removeAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $eventId); + $this->sendSuccessResponse("OK"); } public function checkJoin(string $id) diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 3789879..0e7b8b1 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -6,6 +6,8 @@ use App\Helpers\RecodexApiHelper; use App\Presenters\GroupsPresenter; use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; +use App\Helpers\RecodexGroup; +use App\Security\Roles; use Doctrine\ORM\EntityManagerInterface; use Tester\Assert; use GuzzleHttp\Client; @@ -175,6 +177,323 @@ class TestGroupsPresenter extends Tester\TestCase sort($ids); Assert::equal(['g1', 'g2', 'g3', 'p1', 'p2', 'p3', 'r'], $ids); } + + public function testBindGroup() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); + + $this->client->shouldReceive("post")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + + Assert::equal('OK', $payload); + } + + public function testBindTermGroup() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', false, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + ] + ]))); + + $this->client->shouldReceive("post")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 't1', 'eventId' => $event->getId()] + ); + + Assert::equal('OK', $payload); + } + + public function testBundGroupFailWrong1() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testBundGroupFailWrong2() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, []), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testBundGroupFailWrong3() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, []), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testBundGroupFailAlreadyBound() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, BadRequestException::class); + } + + public function testBundGroupFailOrganizational() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 't1', 'eventId' => $event->getId()] + ); + }, BadRequestException::class); + } + + public function testUnbindGroupAdmin() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [], 'admin'), + self::group('g1', 'c1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), + ] + ]))); + + $this->client->shouldReceive("delete")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + + Assert::equal('OK', $payload); + } + + public function testUnbindGroupSupervisor() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'supervisor'), + ] + ]))); + + $this->client->shouldReceive("delete")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + + Assert::equal('OK', $payload); + } + + public function testUnbindGroupFailUnauthorized() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testUnbindGroupFailUnauthorized2() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'student'), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testUnbindGroupFailNotBound() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [], 'admin'), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, BadRequestException::class); + } } Debugger::$logDirectory = __DIR__ . '/../../log'; From 1e6d1e96e9d03bd4cecae8b4efab46af306f8c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 9 Sep 2025 13:15:25 +0200 Subject: [PATCH 10/21] Implementing add/remove student to/from a group API calls and their testing CLI commands. --- app/commands/RecodexAddStudent.php | 77 ++++++++++++++++++++++ app/commands/RecodexRemoveStudent.php | 81 ++++++++++++++++++++++++ app/config/config.neon | 2 + app/helpers/Recodex/RecodexApiHelper.php | 36 +++++++++++ 4 files changed, 196 insertions(+) create mode 100644 app/commands/RecodexAddStudent.php create mode 100644 app/commands/RecodexRemoveStudent.php diff --git a/app/commands/RecodexAddStudent.php b/app/commands/RecodexAddStudent.php new file mode 100644 index 0000000..4320de0 --- /dev/null +++ b/app/commands/RecodexAddStudent.php @@ -0,0 +1,77 @@ +recodexApi = $recodexApi; + $this->users = $users; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Add student into a group as a member.'); + $this->addArgument('groupId', InputArgument::REQUIRED, 'ID of the group to which the student will be added.'); + $this->addArgument('studentId', InputArgument::REQUIRED, 'ID of the student to be added.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $studentId = $input->getArgument('studentId'); + $student = $this->users->findOrThrow($studentId); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Adding student '$studentId' to group '$groupId'..."); + $this->recodexApi->addStudentToGroup($groupId, $student); + + return Command::SUCCESS; + } +} diff --git a/app/commands/RecodexRemoveStudent.php b/app/commands/RecodexRemoveStudent.php new file mode 100644 index 0000000..788dee6 --- /dev/null +++ b/app/commands/RecodexRemoveStudent.php @@ -0,0 +1,81 @@ +recodexApi = $recodexApi; + $this->users = $users; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Remove student from a group.'); + $this->addArgument( + 'groupId', + InputArgument::REQUIRED, + 'ID of the group from which the student will be removed.' + ); + $this->addArgument('studentId', InputArgument::REQUIRED, 'ID of the student to be removed.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $studentId = $input->getArgument('studentId'); + $student = $this->users->findOrThrow($studentId); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Removing student '$studentId' from group '$groupId'..."); + $this->recodexApi->removeStudentFromGroup($groupId, $student); + + return Command::SUCCESS; + } +} diff --git a/app/config/config.neon b/app/config/config.neon index b2d675a..ed106f8 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -114,8 +114,10 @@ services: # commands - App\Console\DoctrineFixtures - App\Console\RecodexAddAttribute + - App\Console\RecodexAddStudent - App\Console\RecodexGroups - App\Console\RecodexRemoveAttribute + - App\Console\RecodexRemoveStudent - App\Console\RecodexToken - App\Console\SisGetCourse - App\Console\SisGetUser diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index d303618..3a47529 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -323,6 +323,12 @@ public function getGroups(User $user): array return $groups; } + /** + * Add external attribute to selected group (service ID is injected automatically). + * @param string $groupId ReCodEx ID of a group for which the attribute is being added + * @param string $key + * @param string $value + */ public function addAttribute(string $groupId, string $key, string $value): void { Debugger::log("ReCodEx::addAttribute('$groupId', '$key', '$value')", Debugger::INFO); @@ -333,6 +339,12 @@ public function addAttribute(string $groupId, string $key, string $value): void ]); } + /** + * Remove external attribute from selected group (service ID is injected automatically). + * @param string $groupId ReCodEx ID of a group from which the attribute is being removed + * @param string $key + * @param string $value + */ public function removeAttribute(string $groupId, string $key, string $value): void { Debugger::log("ReCodEx::removeAttribute('$groupId', '$key', '$value')", Debugger::INFO); @@ -342,4 +354,28 @@ public function removeAttribute(string $groupId, string $key, string $value): vo 'value' => $value ]); } + + /** + * Add student to group. + * @param string $groupId ReCodEx ID of a group to which the student is being added + * @param User $student + */ + public function addStudentToGroup(string $groupId, User $student): void + { + Debugger::log("ReCodEx::addStudentToGroup('$groupId', '{$student->getId()}')", Debugger::INFO); + $studentId = $student->getId(); + $this->post("groups/$groupId/students/$studentId"); + } + + /** + * Remove student from group. + * @param string $groupId ReCodEx ID of a group from which the student is being removed + * @param User $student + */ + public function removeStudentFromGroup(string $groupId, User $student): void + { + Debugger::log("ReCodEx::removeStudentFromGroup('$groupId', '{$student->getId()}')", Debugger::INFO); + $studentId = $student->getId(); + $this->delete("groups/$groupId/students/$studentId"); + } } From c8f323b1fff23643452a1bbf13e697ef657124c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 9 Sep 2025 13:24:50 +0200 Subject: [PATCH 11/21] Add group authorization check for group-bind operation. --- app/presenters/GroupsPresenter.php | 3 ++- tests/Presenters/GroupsPresenter.phpt | 35 ++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index dea6a06..d9db9e1 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -157,7 +157,8 @@ public function checkBind(string $id, string $eventId) } $groups = $this->recodexApi->getGroups($this->getCurrentUser()); - $this->isGroupSuitableForEvent($groups, $id, $event); // throws exception if not suitable + $this->canUserAdministrateGroup($groups, $id); // throws exception if not + $this->isGroupSuitableForEvent($groups, $id, $event); // throws exception if not if ($groups[$id]->organizational) { throw new BadRequestException("Group $id is organizational, so it cannot be bound to a SIS event."); } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 0e7b8b1..267cc92 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -189,7 +189,7 @@ class TestGroupsPresenter extends Tester\TestCase 'code' => 200, 'payload' => [ self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), self::group('g1', 't1', 'Group 1', false, []), ] @@ -223,7 +223,7 @@ class TestGroupsPresenter extends Tester\TestCase 'payload' => [ self::group('r', null, 'Root', true, []), self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), - self::group('t1', 'c1', 'Term group', false, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('t1', 'c1', 'Term group', false, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), ] ]))); @@ -313,6 +313,33 @@ class TestGroupsPresenter extends Tester\TestCase }, ForbiddenRequestException::class); } + public function testBundGroupFailUnauthorized() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'supervisor'), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + public function testBundGroupFailAlreadyBound() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); @@ -324,7 +351,7 @@ class TestGroupsPresenter extends Tester\TestCase 'code' => 200, 'payload' => [ self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), self::group('g1', 't1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), ] @@ -352,7 +379,7 @@ class TestGroupsPresenter extends Tester\TestCase 'payload' => [ self::group('r', null, 'Root', true, []), self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), ] ]))); From 02e7598be60c5fb68f87dd4143e10860759d69f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 9 Sep 2025 14:01:16 +0200 Subject: [PATCH 12/21] Implementing join group endpoint, adding corresponding tests. --- app/presenters/GroupsPresenter.php | 25 +- tests/Presenters/GroupsPresenter.phpt | 409 ++++++++++++++++---------- 2 files changed, 277 insertions(+), 157 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index d9db9e1..b69e38e 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -141,8 +141,10 @@ public function checkCreate(string $parentId, string $eventId) /** * Proxy to ReCodEx that creates a new group. * @POST - * @param string $parentId ID of the parent group under which the new group is created - * @param string $eventId ID of the SIS event the new group is created for + * @Param(type="query", name="parentId", validation="string:1..", + * description="ReCodEx ID of a group that will be the parent group.") + * @Param(type="query", name="eventId", validation="string:1..", + * description="SIS ID of the scheduling event the new group is created for") */ public function actionCreate(string $parentId, string $eventId) { @@ -172,6 +174,10 @@ public function checkBind(string $id, string $eventId) * Proxy to ReCodEx that binds a group with schedule event (student group) in SIS. * This basically sets the 'group' attribute to ReCodEx group entity. * @POST + * @Param(type="query", name="id", validation="string:1..", + * description="ReCodEx ID of a group that will be bound with the event.") + * @Param(type="query", name="eventId", validation="string:1..", + * description="SIS ID of the scheduling event that will be bound with the group.") */ public function actionBind(string $id, string $eventId) { @@ -198,6 +204,10 @@ public function checkUnbind(string $id, string $eventId) * Proxy to ReCodEx that unbinds a group with schedule event (student group) in SIS. * This basically removes the 'group' attribute from ReCodEx group entity. * @POST + * @Param(type="query", name="id", validation="string:1..", + * description="ReCodEx ID of a group from which the event will be unbound.") + * @Param(type="query", name="eventId", validation="string:1..", + * description="SIS ID of the scheduling event that will be unbound from the group.") */ public function actionUnbind(string $id, string $eventId) { @@ -218,7 +228,8 @@ public function checkJoin(string $id) } foreach ($group->attributes[RecodexGroup::ATTR_GROUP_KEY] ?? [] as $eventId) { - if (!$this->eventAcl->canJoinGroup($this->sisEvents->findOrThrow($eventId))) { + $event = $this->sisEvents->findBySisId($eventId); + if ($event && $this->eventAcl->canJoinGroup($event)) { return; // suitable event was found } } @@ -230,11 +241,15 @@ public function checkJoin(string $id) } /** - * Proxy to ReCodEx that adds student to a group. + * Proxy to ReCodEx that adds current user to selected group. * @POST + * @Param(type="query", name="id", validation="string:1..", + * description="ReCodEx ID of a group the user wish to join.") */ public function actionJoin(string $id) { - throw new NotImplementedException(); + $user = $this->getCurrentUser(); + $this->recodexApi->addStudentToGroup($id, $user); + $this->sendSuccessResponse("OK"); } } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 267cc92..eaae56e 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -7,7 +7,7 @@ use App\Presenters\GroupsPresenter; use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; use App\Helpers\RecodexGroup; -use App\Security\Roles; +use App\Model\Repository\Users; use Doctrine\ORM\EntityManagerInterface; use Tester\Assert; use GuzzleHttp\Client; @@ -31,6 +31,9 @@ class TestGroupsPresenter extends Tester\TestCase /** @var Nette\Security\User */ private $user; + /** @var Users */ + private $users; + private $client; public function __construct() @@ -39,6 +42,7 @@ class TestGroupsPresenter extends Tester\TestCase $this->container = $container; $this->em = PresenterTestHelper::getEntityManager($container); $this->user = $container->getByType(\Nette\Security\User::class); + $this->users = $container->getByType(Users::class); $this->client = Mockery::mock(Client::class); $recodexHelperName = current($this->container->findByType(RecodexApiHelper::class)); @@ -113,17 +117,18 @@ class TestGroupsPresenter extends Tester\TestCase Debugger::enable(false); PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('p1', null, 'Parent', true, []), - self::group('p2', null, 'Parent 2', true, []), - self::group('g1', 'p1', 'Group 1', false, ['group' => ['sis1']]), - self::group('g2', 'p1', 'Group 2', false, ['group' => ['sis2']], 'student'), - self::group('g3', 'p1', 'Group 3', false, ['group' => ['sis3']]), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('p1', null, 'Parent', true, []), + self::group('p2', null, 'Parent 2', true, []), + self::group('g1', 'p1', 'Group 1', false, ['group' => ['sis1']]), + self::group('g2', 'p1', 'Group 2', false, ['group' => ['sis2']], 'student'), + self::group('g3', 'p1', 'Group 3', false, ['group' => ['sis3']]), + ] + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -148,21 +153,22 @@ class TestGroupsPresenter extends Tester\TestCase public function testListGroupsTeacher() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('p1', 'r', 'Prg 1', true, ['course' => ['prg1']]), - self::group('p2', 'r', 'Prg 2', true, ['course' => ['prg2']], 'teacher'), - self::group('p3', 'r', 'Prg 1 & 3', true, ['course' => ['prg3', 'prg1']]), - self::group('p4', 'r', 'Prg 2 & 4', true, ['course' => ['prg2', 'prg4']]), - self::group('g1', 'p1', 'Group 1'), - self::group('g2', 'p2', 'Group 2', false, ['group' => ['sis2']], 'teacher'), - self::group('g3', 'p3', 'Group 3'), - self::group('g4', 'p4', 'Group 4'), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('p1', 'r', 'Prg 1', true, ['course' => ['prg1']]), + self::group('p2', 'r', 'Prg 2', true, ['course' => ['prg2']], 'teacher'), + self::group('p3', 'r', 'Prg 1 & 3', true, ['course' => ['prg3', 'prg1']]), + self::group('p4', 'r', 'Prg 2 & 4', true, ['course' => ['prg2', 'prg4']]), + self::group('g1', 'p1', 'Group 1'), + self::group('g2', 'p2', 'Group 2', false, ['group' => ['sis2']], 'teacher'), + self::group('g3', 'p3', 'Group 3'), + self::group('g4', 'p4', 'Group 4'), + ] + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -184,22 +190,24 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), - self::group('g1', 't1', 'Group 1', false, []), - ] - ]))); - - $this->client->shouldReceive("post")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => "OK" - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); + + $this->client->shouldReceive("post")->with('group-attributes/g1', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -217,21 +225,23 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), - self::group('t1', 'c1', 'Term group', false, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), - ] - ]))); - - $this->client->shouldReceive("post")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => "OK" - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', false, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), + ] + ]))); + + $this->client->shouldReceive("post")->with('group-attributes/t1', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -265,16 +275,17 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), - self::group('t1', 'c1', 'Term group', true, []), - self::group('g1', 't1', 'Group 1', false, []), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, []), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -292,16 +303,17 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, []), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), - self::group('g1', 't1', 'Group 1', false, []), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, []), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -319,16 +331,17 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'supervisor'), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), - self::group('g1', 't1', 'Group 1', false, []), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'supervisor'), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, []), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -346,16 +359,17 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), - self::group('g1', 't1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]], 'admin'), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + self::group('g1', 't1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -373,15 +387,16 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), - self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]], 'supervisor'), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -399,21 +414,23 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('c1', 'r', 'Course group', true, [], 'admin'), - self::group('g1', 'c1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), - ] - ]))); - - $this->client->shouldReceive("delete")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => "OK" - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [], 'admin'), + self::group('g1', 'c1', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]]), + ] + ]))); + + $this->client->shouldReceive("delete")->with('group-attributes/g1', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -431,20 +448,22 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'supervisor'), - ] - ]))); - - $this->client->shouldReceive("delete")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => "OK" - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'supervisor'), + ] + ]))); + + $this->client->shouldReceive("delete")->with('group-attributes/g1', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, @@ -478,14 +497,15 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'student'), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'student'), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -503,14 +523,15 @@ class TestGroupsPresenter extends Tester\TestCase $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); Assert::notNull($event); - $this->client->shouldReceive("get")->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ - 'success' => true, - 'code' => 200, - 'payload' => [ - self::group('r', null, 'Root', true, []), - self::group('g1', 'r', 'Group 1', false, [], 'admin'), - ] - ]))); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [], 'admin'), + ] + ]))); Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( @@ -521,6 +542,90 @@ class TestGroupsPresenter extends Tester\TestCase ); }, BadRequestException::class); } + + public function testJoinGroup() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + $studentId = $this->users->findOneBy(['email' => PresenterTestHelper::STUDENT1_LOGIN])?->getId(); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], null), + ] + ]))); + + $this->client->shouldReceive("post")->with("groups/g1/students/$studentId", Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'join', 'id' => 'g1'] + ); + + Assert::equal('OK', $payload); + } + + public function testJoinGroupFailNoEvent() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [], null), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'join', 'id' => 'g1'] + ); + }, ForbiddenRequestException::class); + } + + public function testJoinGroupFailAlreadyMemeber() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('g1', 'r', 'Group 1', false, [RecodexGroup::ATTR_GROUP_KEY => [$event->getSisId()]], 'student'), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Terms', + 'POST', + ['action' => 'join', 'id' => 'g1'] + ); + }, BadRequestException::class); + } } Debugger::$logDirectory = __DIR__ . '/../../log'; From 2bb35775d8d0578b48a634f454e880672991db72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 9 Sep 2025 15:17:57 +0200 Subject: [PATCH 13/21] Implementing ReCodEx API calls to add/remove admin to/from a group and corresponding testing CLI commands. --- app/commands/Recodex/AddAdmin.php | 77 ++++++++++++++++++ .../AddAttribute.php} | 0 .../AddStudent.php} | 0 .../{RecodexGroups.php => Recodex/Groups.php} | 0 app/commands/Recodex/RemoveAdmin.php | 81 +++++++++++++++++++ .../RemoveAttribute.php} | 0 .../RemoveStudent.php} | 0 .../{RecodexToken.php => Recodex/Token.php} | 0 app/config/config.neon | 2 + app/helpers/Recodex/RecodexApiHelper.php | 24 ++++++ 10 files changed, 184 insertions(+) create mode 100644 app/commands/Recodex/AddAdmin.php rename app/commands/{RecodexAddAttribute.php => Recodex/AddAttribute.php} (100%) rename app/commands/{RecodexAddStudent.php => Recodex/AddStudent.php} (100%) rename app/commands/{RecodexGroups.php => Recodex/Groups.php} (100%) create mode 100644 app/commands/Recodex/RemoveAdmin.php rename app/commands/{RecodexRemoveAttribute.php => Recodex/RemoveAttribute.php} (100%) rename app/commands/{RecodexRemoveStudent.php => Recodex/RemoveStudent.php} (100%) rename app/commands/{RecodexToken.php => Recodex/Token.php} (100%) diff --git a/app/commands/Recodex/AddAdmin.php b/app/commands/Recodex/AddAdmin.php new file mode 100644 index 0000000..2864f27 --- /dev/null +++ b/app/commands/Recodex/AddAdmin.php @@ -0,0 +1,77 @@ +recodexApi = $recodexApi; + $this->users = $users; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Add admin into a group as a member.'); + $this->addArgument('groupId', InputArgument::REQUIRED, 'ID of the group to which the admin will be added.'); + $this->addArgument('adminId', InputArgument::REQUIRED, 'ID of the admin to be added.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $adminId = $input->getArgument('adminId'); + $admin = $this->users->findOrThrow($adminId); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Adding admin '$adminId' to group '$groupId'..."); + $this->recodexApi->addAdminToGroup($groupId, $admin); + + return Command::SUCCESS; + } +} diff --git a/app/commands/RecodexAddAttribute.php b/app/commands/Recodex/AddAttribute.php similarity index 100% rename from app/commands/RecodexAddAttribute.php rename to app/commands/Recodex/AddAttribute.php diff --git a/app/commands/RecodexAddStudent.php b/app/commands/Recodex/AddStudent.php similarity index 100% rename from app/commands/RecodexAddStudent.php rename to app/commands/Recodex/AddStudent.php diff --git a/app/commands/RecodexGroups.php b/app/commands/Recodex/Groups.php similarity index 100% rename from app/commands/RecodexGroups.php rename to app/commands/Recodex/Groups.php diff --git a/app/commands/Recodex/RemoveAdmin.php b/app/commands/Recodex/RemoveAdmin.php new file mode 100644 index 0000000..f65b974 --- /dev/null +++ b/app/commands/Recodex/RemoveAdmin.php @@ -0,0 +1,81 @@ +recodexApi = $recodexApi; + $this->users = $users; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Remove admin from a group.'); + $this->addArgument( + 'groupId', + InputArgument::REQUIRED, + 'ID of the group from which the admin will be removed.' + ); + $this->addArgument('adminId', InputArgument::REQUIRED, 'ID of the admin to be removed.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $groupId = $input->getArgument('groupId'); + $adminId = $input->getArgument('adminId'); + $admin = $this->users->findOrThrow($adminId); + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Removing admin '$adminId' from group '$groupId'..."); + $this->recodexApi->removeAdminFromGroup($groupId, $admin); + + return Command::SUCCESS; + } +} diff --git a/app/commands/RecodexRemoveAttribute.php b/app/commands/Recodex/RemoveAttribute.php similarity index 100% rename from app/commands/RecodexRemoveAttribute.php rename to app/commands/Recodex/RemoveAttribute.php diff --git a/app/commands/RecodexRemoveStudent.php b/app/commands/Recodex/RemoveStudent.php similarity index 100% rename from app/commands/RecodexRemoveStudent.php rename to app/commands/Recodex/RemoveStudent.php diff --git a/app/commands/RecodexToken.php b/app/commands/Recodex/Token.php similarity index 100% rename from app/commands/RecodexToken.php rename to app/commands/Recodex/Token.php diff --git a/app/config/config.neon b/app/config/config.neon index ed106f8..7007fe7 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -113,9 +113,11 @@ services: # commands - App\Console\DoctrineFixtures + - App\Console\RecodexAddAdmin - App\Console\RecodexAddAttribute - App\Console\RecodexAddStudent - App\Console\RecodexGroups + - App\Console\RecodexRemoveAdmin - App\Console\RecodexRemoveAttribute - App\Console\RecodexRemoveStudent - App\Console\RecodexToken diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index 3a47529..ad2fb3c 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -378,4 +378,28 @@ public function removeStudentFromGroup(string $groupId, User $student): void $studentId = $student->getId(); $this->delete("groups/$groupId/students/$studentId"); } + + /** + * Add admin to group. + * @param string $groupId ReCodEx ID of a group to which the admin is being added + * @param User $admin + */ + public function addAdminToGroup(string $groupId, User $admin): void + { + Debugger::log("ReCodEx::addAdminToGroup('$groupId', '{$admin->getId()}')", Debugger::INFO); + $adminId = $admin->getId(); + $this->post("groups/$groupId/members/$adminId", [], ['type' => 'admin']); + } + + /** + * Remove admin from group. + * @param string $groupId ReCodEx ID of a group from which the admin is being removed + * @param User $admin + */ + public function removeAdminFromGroup(string $groupId, User $admin): void + { + Debugger::log("ReCodEx::removeAdminFromGroup('$groupId', '{$admin->getId()}')", Debugger::INFO); + $adminId = $admin->getId(); + $this->delete("groups/$groupId/members/$adminId"); + } } From 645dd212d5cd1a6132a716d9e608fd5368fc2c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 9 Sep 2025 23:24:00 +0200 Subject: [PATCH 14/21] Implementing ReCodEx API calls for creating a (sub)group from a SIS scheduling event (and corresponding CLI command for debugging it). --- app/commands/Recodex/CreateGroup.php | 96 ++++++++++++++++++++++++ app/config/config.neon | 2 + app/helpers/NamingHelper.php | 77 +++++++++++++++++++ app/helpers/Recodex/RecodexApiHelper.php | 53 ++++++++++++- tests/Presenters/GroupsPresenter.phpt | 2 + 5 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 app/commands/Recodex/CreateGroup.php create mode 100644 app/helpers/NamingHelper.php diff --git a/app/commands/Recodex/CreateGroup.php b/app/commands/Recodex/CreateGroup.php new file mode 100644 index 0000000..d7f1a18 --- /dev/null +++ b/app/commands/Recodex/CreateGroup.php @@ -0,0 +1,96 @@ +recodexApi = $recodexApi; + $this->sisEvents = $sisEvents; + $this->users = $users; + } + + /** + * Register the command. + */ + protected function configure() + { + $this->setName(self::$defaultName)->setDescription('Create a new group in ReCodEx.'); + $this->addArgument('eventId', InputArgument::REQUIRED, 'The SIS ID of the event associated with the group.'); + $this->addArgument('parentId', InputArgument::REQUIRED, 'ReCodEx ID of the the parent group.'); + $this->addArgument('adminId', InputArgument::REQUIRED, 'ReCodEx ID of the admin of the newly created group.'); + } + + /** + * @param InputInterface $input Console input, not used + * @param OutputInterface $output Console output for logging + * @return int 0 on success, 1 on error + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + + $eventId = $input->getArgument('eventId'); + $event = $this->sisEvents->findBySisId($eventId); + if (!$event) { + $output->writeln("Event with ID $eventId not found."); + return Command::FAILURE; + } + $parentId = $input->getArgument('parentId'); + $adminId = $input->getArgument('adminId'); + $admin = $this->users->get($adminId); + if (!$admin) { + $output->writeln("Admin with ID $adminId not found."); + return Command::FAILURE; + } + + $token = trim($this->prompt('Auth token: ')); + if (!$token) { + $output->writeln('No token given, terminating...'); + return Command::SUCCESS; + } + $this->recodexApi->setAuthToken($token); + + $output->writeln("Creating group for event '$eventId' under parent group '$parentId'..."); + $this->recodexApi->createGroup($event, $parentId, $admin); + + return Command::SUCCESS; + } +} diff --git a/app/config/config.neon b/app/config/config.neon index 7007fe7..cdd73db 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -116,6 +116,7 @@ services: - App\Console\RecodexAddAdmin - App\Console\RecodexAddAttribute - App\Console\RecodexAddStudent + - App\Console\RecodexCreateGroup - App\Console\RecodexGroups - App\Console\RecodexRemoveAdmin - App\Console\RecodexRemoveAttribute @@ -132,6 +133,7 @@ services: - App\Helpers\EmailsConfig(%emails%) # helpers + - App\Helpers\NamingHelper - App\Helpers\RecodexApiHelper(%recodex%) - App\Helpers\SisHelper(%sis%) - App\Helpers\UserActions diff --git a/app/helpers/NamingHelper.php b/app/helpers/NamingHelper.php new file mode 100644 index 0000000..b27f88c --- /dev/null +++ b/app/helpers/NamingHelper.php @@ -0,0 +1,77 @@ +getCourse()->getCaption($locale); + $dayNames = [ + 'en' => ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'], + 'cs' => ['Ne', 'Po', 'Út', 'St', 'Čt', 'Pá', 'So'], + ]; + $fortnight = [ + 'en' => [0 => 'Even weeks', 1 => 'Odd weeks'], + 'cs' => [0 => 'Sudé týdny', 1 => 'Liché týdny'], + ]; + $unscheduled = [ + 'en' => 'unscheduled', + 'cs' => 'nerozvrženo', + ]; + + if (!$courseName || empty($dayNames[$locale])) { + return null; + } + + $info = []; + if ($event->getDayOfWeek() !== null && array_key_exists($event->getDayOfWeek(), $dayNames[$locale])) { + $info[] = $dayNames[$locale][$event->getDayOfWeek()]; + } + if ($event->getTime() !== null) { + $info[] = (int)($event->getTime() / 60) . ':' + . str_pad((string)($event->getTime() % 60), 2, '0', STR_PAD_LEFT); + } + if ($event->getFortnight() && $event->getFirstWeek() !== null) { + $info[] = $fortnight[$locale][$event->getFirstWeek() % 2]; + } + if ($event->getRoom() !== null) { + $info[] = $event->getRoom(); + } + + $info = $info ? implode(', ', $info) : $unscheduled[$locale]; + return "$courseName ($info)"; + } + + /** + * Get the description of the group for a given SIS schedule event. + * @param SisScheduleEvent $event The SIS schedule event. + * @param string $locale The locale to use for translation ('cs' or 'en'). + * @return string|null The group description or null if it cannot be determined.s + */ + public function getGroupDescription(SisScheduleEvent $event, string $locale): ?string + { + $templates = [ + 'en' => 'A group create from SIS scheduling event `%s` for course `%s`.', + 'cs' => 'Skupina vytvořená z rozvrhového lístku SISu `%s` pro předmět `%s`.', + ]; + if (empty($templates[$locale])) { + return null; + } + return sprintf($templates[$locale], $event->getSisId(), $event->getCourse()->getCode()); + } +} diff --git a/app/helpers/Recodex/RecodexApiHelper.php b/app/helpers/Recodex/RecodexApiHelper.php index ad2fb3c..7537ea6 100644 --- a/app/helpers/Recodex/RecodexApiHelper.php +++ b/app/helpers/Recodex/RecodexApiHelper.php @@ -4,6 +4,7 @@ use App\Exceptions\ConfigException; use App\Exceptions\RecodexApiException; +use App\Model\Entity\SisScheduleEvent; use App\Model\Entity\User; use Nette; use GuzzleHttp; @@ -35,6 +36,9 @@ class RecodexApiHelper /** @var string|null Authentication token that is added to headers */ private ?string $authToken = null; + /** @var NamingHelper */ + private NamingHelper $namingHelper; + /** @var GuzzleHttp\Client */ private $client; @@ -42,7 +46,7 @@ class RecodexApiHelper * @param array $config * @param GuzzleHttp\Client|null $client optional injection of HTTP client for testing purposes */ - public function __construct(array $config, ?GuzzleHttp\Client $client = null) + public function __construct(array $config, NamingHelper $namingHelper, ?GuzzleHttp\Client $client = null) { $this->extensionId = Arrays::get($config, "extensionId", ""); if (!$this->extensionId) { @@ -66,6 +70,8 @@ public function __construct(array $config, ?GuzzleHttp\Client $client = null) $client = new GuzzleHttp\Client(['base_uri' => $this->apiBase]); } $this->client = $client; + + $this->namingHelper = $namingHelper; } public function getSisIdKey(): string @@ -402,4 +408,49 @@ public function removeAdminFromGroup(string $groupId, User $admin): void $adminId = $admin->getId(); $this->delete("groups/$groupId/members/$adminId"); } + + /** + * Create a new group and make given user an admin. + * @param SisScheduleEvent $event event for which the group is being created + * @param string $parentGroupId ID of the parent group + * @param User $admin user to be added as admin + * @return string|null ID of the created group or null on failure + */ + public function createGroup(SisScheduleEvent $event, string $parentGroupId, User $admin): ?string + { + Debugger::log("ReCodEx::createGroup('{$event->getSisId()}', '$parentGroupId')", Debugger::INFO); + + $localizedTexts = []; + foreach (['en', 'cs'] as $locale) { + $name = $this->namingHelper->getGroupName($event, $locale); + $description = $this->namingHelper->getGroupDescription($event, $locale); + if ($name !== null) { + $localizedTexts[] = [ + 'locale' => $locale, + 'name' => $name, + 'description' => $description ?? '', + ]; + } + } + + $group = $this->post("groups", [], [ + 'instanceId' => $admin->getInstanceId(), + 'parentGroupId' => $parentGroupId, + 'publicStats' => false, + 'detaining' => true, + 'isPublic' => false, + 'isOrganizational' => false, + 'isExam' => false, + 'noAdmin' => true, + 'localizedTexts' => $localizedTexts, + ]); + + if ($group && !empty($group['id'])) { + $this->addAdminToGroup($group['id'], $admin); + $this->addAttribute($group['id'], RecodexGroup::ATTR_GROUP_KEY, $event->getSisId()); + return $group['id']; + } + + return null; + } } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index eaae56e..6bc6ca3 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -6,6 +6,7 @@ use App\Helpers\RecodexApiHelper; use App\Presenters\GroupsPresenter; use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; +use App\Helpers\NamingHelper; use App\Helpers\RecodexGroup; use App\Model\Repository\Users; use Doctrine\ORM\EntityManagerInterface; @@ -52,6 +53,7 @@ class TestGroupsPresenter extends Tester\TestCase 'extensionId' => 'sis-cuni', 'apiBase' => 'https://recodex.example/', ], + new NamingHelper(), $this->client )); } From 0f1da8fdc0780496a16fec892331ecb74ab2afcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Wed, 10 Sep 2025 13:14:01 +0200 Subject: [PATCH 15/21] Implementing create group endpoint and its tests. --- app/presenters/GroupsPresenter.php | 7 +- tests/Presenters/GroupsPresenter.phpt | 183 ++++++++++++++++++++++---- 2 files changed, 163 insertions(+), 27 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index b69e38e..d460d63 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -136,6 +136,9 @@ public function checkCreate(string $parentId, string $eventId) $groups = $this->recodexApi->getGroups($this->getCurrentUser()); $this->isGroupSuitableForEvent($groups, $parentId, $event); // throws exception if not suitable + + // We are not checking ReCodEx permissions since the T.A.s may have none. + // This is the reason we are creating the groups via this extension (to bypass/extend regular permissions). } /** @@ -148,7 +151,9 @@ public function checkCreate(string $parentId, string $eventId) */ public function actionCreate(string $parentId, string $eventId) { - throw new NotImplementedException(); + $event = $this->sisEvents->findOrThrow($eventId); + $this->recodexApi->createGroup($event, $parentId, $this->getCurrentUser()); + $this->sendSuccessResponse("OK"); } public function checkBind(string $id, string $eventId) diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 6bc6ca3..9b04151 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -35,6 +35,9 @@ class TestGroupsPresenter extends Tester\TestCase /** @var Users */ private $users; + /** @var NamingHelper */ + private $namingHelper; + private $client; public function __construct() @@ -47,13 +50,14 @@ class TestGroupsPresenter extends Tester\TestCase $this->client = Mockery::mock(Client::class); $recodexHelperName = current($this->container->findByType(RecodexApiHelper::class)); + $this->namingHelper = $this->container->getByType(NamingHelper::class); $this->container->removeService($recodexHelperName); $this->container->addService($recodexHelperName, new RecodexApiHelper( [ 'extensionId' => 'sis-cuni', 'apiBase' => 'https://recodex.example/', ], - new NamingHelper(), + $this->namingHelper, $this->client )); } @@ -134,7 +138,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'GET', ['action' => 'student', 'eventIds' => ['sis1']] ); @@ -174,7 +178,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'GET', ['action' => 'teacher', 'eventIds' => ['sis2'], 'courseIds' => ['prg1']] ); @@ -213,7 +217,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -247,7 +251,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 't1', 'eventId' => $event->getId()] ); @@ -255,7 +259,7 @@ class TestGroupsPresenter extends Tester\TestCase Assert::equal('OK', $payload); } - public function testBundGroupFailWrong1() + public function testBindGroupFailWrong1() { PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -264,14 +268,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); }, ForbiddenRequestException::class); } - public function testBundGroupFailWrong2() + public function testBindGroupFailWrong2() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -292,14 +296,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); }, ForbiddenRequestException::class); } - public function testBundGroupFailWrong3() + public function testBindGroupFailWrong3() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -320,14 +324,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); }, ForbiddenRequestException::class); } - public function testBundGroupFailUnauthorized() + public function testBindGroupFailUnauthorized() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -348,14 +352,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); }, ForbiddenRequestException::class); } - public function testBundGroupFailAlreadyBound() + public function testBindGroupFailAlreadyBound() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -376,14 +380,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 'g1', 'eventId' => $event->getId()] ); }, BadRequestException::class); } - public function testBundGroupFailOrganizational() + public function testBindGroupFailOrganizational() { PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -403,7 +407,7 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'bind', 'id' => 't1', 'eventId' => $event->getId()] ); @@ -436,7 +440,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -469,7 +473,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -486,7 +490,7 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -512,7 +516,7 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -538,7 +542,7 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () use ($event) { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'unbind', 'id' => 'g1', 'eventId' => $event->getId()] ); @@ -571,7 +575,7 @@ class TestGroupsPresenter extends Tester\TestCase $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'join', 'id' => 'g1'] ); @@ -596,14 +600,14 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'join', 'id' => 'g1'] ); }, ForbiddenRequestException::class); } - public function testJoinGroupFailAlreadyMemeber() + public function testJoinGroupFailAlreadyMember() { PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); @@ -622,12 +626,139 @@ class TestGroupsPresenter extends Tester\TestCase Assert::exception(function () { PresenterTestHelper::performPresenterRequest( $this->presenter, - 'Terms', + 'Groups', 'POST', ['action' => 'join', 'id' => 'g1'] ); }, BadRequestException::class); } + + public function testCreateGroup() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $user = $this->users->findOneBy(['email' => PresenterTestHelper::TEACHER1_LOGIN]); + Assert::notNull($user); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + ] + ]))); + + $this->client->shouldReceive("post")->with('groups', Mockery::on(function ($arg) use ($user, $event) { + Assert::type('array', $arg); + Assert::type('array', $arg['json'] ?? null); + $body = $arg['json']; + Assert::equal($user->getInstanceId(), $body['instanceId']); + Assert::equal('t1', $body['parentGroupId']); + Assert::false($body['publicStats']); + Assert::true($body['detaining']); + Assert::false($body['isPublic']); + Assert::false($body['isOrganizational']); + Assert::false($body['isExam']); + Assert::true($body['noAdmin']); + Assert::count(2, $body['localizedTexts']); + foreach ($body['localizedTexts'] as $localizedText) { + Assert::type('array', $localizedText); + Assert::count(3, $localizedText); + $locale = $localizedText['locale'] ?? ''; + Assert::contains($locale, ['en', 'cs']); + Assert::equal($this->namingHelper->getGroupName($event, $locale), $localizedText['name'] ?? null); + Assert::equal($this->namingHelper->getGroupDescription($event, $locale), $localizedText['description'] ?? null); + } + return true; + }))->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => ['id' => 'g1'] + ]))); + + $this->client->shouldReceive("post")->with('groups/g1/members/' . $user->getId(), Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $this->client->shouldReceive("post")->with('group-attributes/g1', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'create', 'parentId' => 't1', 'eventId' => $event->getId()] + ); + + Assert::equal("OK", $payload); + } + + public function testCreateGroupFailWrongParent() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, []), + self::group('t1', 'c1', 'Term group', true, [RecodexGroup::ATTR_TERM_KEY => [$event->getTerm()->getYearTermKey()]]), + ] + ]))); + + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'create', 'parentId' => 't1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } + + public function testCreateGroupFailWrongParent2() + { + PresenterTestHelper::login($this->container, PresenterTestHelper::TEACHER1_LOGIN); + $event = $this->presenter->sisEvents->findOneBy(['sisId' => 'gl1p']); + Assert::notNull($event); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('r', null, 'Root', true, []), + self::group('c1', 'r', 'Course group', true, [RecodexGroup::ATTR_COURSE_KEY => [$event->getCourse()->getCode()]]), + self::group('t1', 'c1', 'Term group', true, []), + ] + ]))); + + + Assert::exception(function () use ($event) { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'create', 'parentId' => 't1', 'eventId' => $event->getId()] + ); + }, ForbiddenRequestException::class); + } } Debugger::$logDirectory = __DIR__ . '/../../log'; From 836c2e1457671db36066f2f6db20b91d3064b6ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Thu, 11 Sep 2025 17:08:51 +0200 Subject: [PATCH 16/21] Fixing group presented to have proper ReCodEx API injected. --- app/presenters/GroupsPresenter.php | 10 +--------- app/presenters/base/BasePresenterWithApi.php | 8 +++++--- tests/Presenters/GroupsPresenter.phpt | 5 +++++ 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index d460d63..b62be4e 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -5,8 +5,6 @@ use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; use App\Exceptions\NotFoundException; -use App\Exceptions\NotImplementedException; -use App\Helpers\RecodexApiHelper; use App\Helpers\RecodexGroup; use App\Model\Entity\SisScheduleEvent; use App\Model\Repository\SisScheduleEvents; @@ -16,7 +14,7 @@ /** * Group management (both for teachers and students). */ -class GroupsPresenter extends BasePresenter +class GroupsPresenter extends BasePresenterWithApi { /** * @var SisScheduleEvents @@ -24,12 +22,6 @@ class GroupsPresenter extends BasePresenter */ public $sisEvents; - /** - * @var RecodexApiHelper - * @inject - */ - public $recodexApi; - /** * @var IEventPermissions * @inject diff --git a/app/presenters/base/BasePresenterWithApi.php b/app/presenters/base/BasePresenterWithApi.php index 7a33eb5..bf78dbb 100644 --- a/app/presenters/base/BasePresenterWithApi.php +++ b/app/presenters/base/BasePresenterWithApi.php @@ -25,15 +25,17 @@ class BasePresenterWithApi extends BasePresenter public function startup() { - parent::startup(); - // Initialize ReCodEx auth token (main part is in User entity, suffix is in our auth token's payload). $user = $this->getCurrentUser(); $token = $this->getAccessToken(); - $suffix = $token->getPayload('suffix'); + $suffix = $token->getPayloadOrDefault('suffix', null); if ($user->getRecodexToken() && $suffix) { $this->recodexApi->setAuthToken($user->getRecodexToken() . $suffix); } + + // the parent startup performs authorization checks, so it must be called after we set the token + // (some presenters check permissions via ReCodEx API) + parent::startup(); } } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 9b04151..d057871 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -3,6 +3,7 @@ $container = require_once __DIR__ . "/../bootstrap.php"; use App\Helpers\RecodexApiHelper; +use App\Helpers\SisHelper; use App\Presenters\GroupsPresenter; use App\Exceptions\BadRequestException; use App\Exceptions\ForbiddenRequestException; @@ -60,6 +61,10 @@ class TestGroupsPresenter extends Tester\TestCase $this->namingHelper, $this->client )); + + $sisHelperName = current($this->container->findByType(SisHelper::class)); + $this->container->removeService($sisHelperName); + $this->container->addService($sisHelperName, Mockery::mock(SisHelper::class)); } protected function setUp() From 507d758c94facb7661307ace457863adecdbb7c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 16 Sep 2025 00:17:38 +0200 Subject: [PATCH 17/21] Fixing bugs and polishing. --- app/config/config.neon | 2 +- app/helpers/WebappLinks.php | 8 ++++---- app/presenters/GroupsPresenter.php | 12 +++++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/config/config.neon b/app/config/config.neon index cdd73db..abb72c0 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -50,7 +50,7 @@ parameters: exerciseUrl: "%webapp.address%/app/exercises/{id}" shadowAssignmentUrl: "%webapp.address%/app/shadow-assignment/{id}" solutionUrl: "%webapp.address%/app/assignment/{assignmentId}/solution/{solutionId}" - referenceSolutiontUrl: "%webapp.address%/app/exercises/{exerciseId}/reference-solution/{solutionId}" + referenceSolutionUrl: "%webapp.address%/app/exercises/{exerciseId}/reference-solution/{solutionId}" forgottenPasswordUrl: "%webapp.address%/forgotten-password/change?{token}" # URL of web application where the password can be changed emailVerificationUrl: "%webapp.address%/email-verification?{token}" invitationUrl: "%webapp.address%/accept-invitation?{token}" diff --git a/app/helpers/WebappLinks.php b/app/helpers/WebappLinks.php index f5048fe..5ed5e6a 100644 --- a/app/helpers/WebappLinks.php +++ b/app/helpers/WebappLinks.php @@ -27,7 +27,7 @@ class WebappLinks private $solutionUrl; /** @var string */ - private $referenceSolutiontUrl; + private $referenceSolutionUrl; /** @var string */ private $forgottenPasswordUrl; @@ -69,9 +69,9 @@ public function __construct(string $webappUrl, array $linkTemplates) ["solutionUrl"], "$webappUrl/app/assignment/{assignmentId}/solution/{solutionId}" ); - $this->referenceSolutiontUrl = Arrays::get( + $this->referenceSolutionUrl = Arrays::get( $linkTemplates, - ["referenceSolutiontUrl"], + ["referenceSolutionUrl"], "$webappUrl/app/exercises/{exerciseId}/reference-solution/{solutionId}" ); $this->forgottenPasswordUrl = Arrays::get( @@ -141,7 +141,7 @@ public function getSolutionPageUrl(string $assignmentId, string $solutionId): st public function getReferenceSolutionPageUrl(string $exerciseId, string $solutionId): string { return self::getLink( - $this->referenceSolutiontUrl, + $this->referenceSolutionUrl, ['exerciseId' => $exerciseId, 'solutionId' => $solutionId] ); } diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index b62be4e..9ba6dff 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -139,7 +139,7 @@ public function checkCreate(string $parentId, string $eventId) * @Param(type="query", name="parentId", validation="string:1..", * description="ReCodEx ID of a group that will be the parent group.") * @Param(type="query", name="eventId", validation="string:1..", - * description="SIS ID of the scheduling event the new group is created for") + * description="Internal ID of the scheduling event the new group is created for") */ public function actionCreate(string $parentId, string $eventId) { @@ -174,11 +174,12 @@ public function checkBind(string $id, string $eventId) * @Param(type="query", name="id", validation="string:1..", * description="ReCodEx ID of a group that will be bound with the event.") * @Param(type="query", name="eventId", validation="string:1..", - * description="SIS ID of the scheduling event that will be bound with the group.") + * description="Internal ID of the scheduling event that will be bound with the group.") */ public function actionBind(string $id, string $eventId) { - $this->recodexApi->addAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $eventId); + $event = $this->sisEvents->findOrThrow($eventId); + $this->recodexApi->addAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $event->getSisId()); $this->sendSuccessResponse("OK"); } @@ -204,11 +205,12 @@ public function checkUnbind(string $id, string $eventId) * @Param(type="query", name="id", validation="string:1..", * description="ReCodEx ID of a group from which the event will be unbound.") * @Param(type="query", name="eventId", validation="string:1..", - * description="SIS ID of the scheduling event that will be unbound from the group.") + * description="Internal ID of the scheduling event that will be unbound from the group.") */ public function actionUnbind(string $id, string $eventId) { - $this->recodexApi->removeAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $eventId); + $event = $this->sisEvents->findOrThrow($eventId); + $this->recodexApi->removeAttribute($id, RecodexGroup::ATTR_GROUP_KEY, $event->getSisId()); $this->sendSuccessResponse("OK"); } From c1dfd1464703842c3812980e85ac69b06defafd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Mon, 22 Sep 2025 23:59:45 +0200 Subject: [PATCH 18/21] Adding endpoints for low-level attribute updates. --- app/helpers/Recodex/RecodexGroup.php | 11 +++- app/presenters/GroupsPresenter.php | 76 ++++++++++++++++++++++++++ app/router/RouterFactory.php | 2 + app/security/ACL/IGroupPermissions.php | 2 + 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/app/helpers/Recodex/RecodexGroup.php b/app/helpers/Recodex/RecodexGroup.php index c13000b..54a8a94 100644 --- a/app/helpers/Recodex/RecodexGroup.php +++ b/app/helpers/Recodex/RecodexGroup.php @@ -236,19 +236,24 @@ public function jsonSerialize(): array /* * Public helper methods (data getters) */ + public function hasAttribute(string $key, string $value): bool + { + return in_array($value, $this->attributes[$key] ?? [], true); + } + public function hasGroupAttribute($groupId) { - return in_array($groupId, $this->attributes[self::ATTR_GROUP_KEY] ?? [], true); + return $this->hasAttribute(self::ATTR_GROUP_KEY, $groupId); } public function hasTermAttribute($term) { - return in_array($term, $this->attributes[self::ATTR_TERM_KEY] ?? [], true); + return $this->hasAttribute(self::ATTR_TERM_KEY, $term); } public function hasCourseAttribute($courseId) { - return in_array($courseId, $this->attributes[self::ATTR_COURSE_KEY] ?? [], true); + return $this->hasAttribute(self::ATTR_COURSE_KEY, $courseId); } /* diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 9ba6dff..9eb60d4 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -251,4 +251,80 @@ public function actionJoin(string $id) $this->recodexApi->addStudentToGroup($id, $user); $this->sendSuccessResponse("OK"); } + + public function checkAddAttribute() + { + if (!$this->groupAcl->canEditRawAttributes()) { + throw new ForbiddenRequestException("You do not have permissions to edit raw group attributes."); + } + } + + /** + * Proxy to ReCodEx that adds an attribute to a group. + * This is rather low-level operation for super-admins only (to edit top-level and term groups). + * @POST + * @Param(type="post", name="groupId", validation="string:1..", + * description="ReCodEx ID of a group to which the attribute will be added.") + * @Param(type="post", name="key", validation="string:1..", + * description="Key of the attribute to add.") + * @Param(type="post", name="value", validation="string:1..", + * description="Value of the attribute to add.") + */ + public function actionAddAttribute() + { + $groupId = $this->getRequest()->getPost('groupId'); + $key = $this->getRequest()->getPost('key'); + $value = $this->getRequest()->getPost('value'); + + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $group = $groups[$groupId] ?? null; + if ($group === null) { + throw new BadRequestException("Group $groupId does not exist or is not accessible by the user."); + } + + if ($group->hasAttribute($key, $value)) { + throw new BadRequestException("Group $groupId already has attribute $key=$value."); + } + + $this->recodexApi->addAttribute($groupId, $key, $value); + $this->sendSuccessResponse("OK"); + } + + public function checkRemoveAttribute() + { + if (!$this->groupAcl->canEditRawAttributes()) { + throw new ForbiddenRequestException("You do not have permissions to edit raw group attributes."); + } + } + + /** + * Proxy to ReCodEx that removes an attribute from a group. + * This is rather low-level operation for super-admins only (to edit top-level and term groups). + * @POST + * @Param(type="post", name="groupId", validation="string:1..", + * description="ReCodex ID of a group from which the attribute will be removed.") + * @Param(type="post", name="key", validation="string:1..", + * description="Key of the attribute to remove.") + * @Param(type="post", name="value", validation="string:1..", + * description="Value of the attribute to remove.") + */ + public function actionRemoveAttribute() + { + $groupId = $this->getRequest()->getPost('groupId'); + $key = $this->getRequest()->getPost('key'); + $value = $this->getRequest()->getPost('value'); + + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $group = $groups[$groupId] ?? null; + if ($group === null) { + throw new BadRequestException("Group $groupId does not exist or is not accessible by the user."); + } + + if (!$group->hasAttribute($key, $value)) { + throw new BadRequestException("Group $groupId does not have attribute $key=$value."); + } + + $this->recodexApi->removeAttribute($groupId, $key, $value); + $this->sendSuccessResponse("OK"); + } } diff --git a/app/router/RouterFactory.php b/app/router/RouterFactory.php index fe0d5f0..86c7c74 100644 --- a/app/router/RouterFactory.php +++ b/app/router/RouterFactory.php @@ -79,6 +79,8 @@ private static function createGroupsRoutes(string $prefix): RouteList $router[] = new PostRoute("$prefix//bind/", "Groups:bind"); $router[] = new DeleteRoute("$prefix//bind/", "Groups:unbind"); $router[] = new PostRoute("$prefix//join", "Groups:join"); + $router[] = new PostRoute("$prefix//add-attribute", "Groups:addAttribute"); + $router[] = new PostRoute("$prefix//remove-attribute", "Groups:removeAttribute"); return $router; } } diff --git a/app/security/ACL/IGroupPermissions.php b/app/security/ACL/IGroupPermissions.php index 48c192b..4bc4ca1 100644 --- a/app/security/ACL/IGroupPermissions.php +++ b/app/security/ACL/IGroupPermissions.php @@ -7,4 +7,6 @@ interface IGroupPermissions public function canViewStudent(): bool; public function canViewTeacher(): bool; + + public function canEditRawAttributes(): bool; } From 8066becb3429f8f4b1a0fa70229e3b0ef4efa288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Tue, 23 Sep 2025 00:15:41 +0200 Subject: [PATCH 19/21] Adding tests for add/remove attribute endpoints. --- tests/Presenters/GroupsPresenter.phpt | 174 ++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index d057871..2624209 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -764,6 +764,180 @@ class TestGroupsPresenter extends Tester\TestCase ); }, ForbiddenRequestException::class); } + + public function testAddAttribute() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar']]), + ] + ]))); + + $this->client->shouldReceive("post")->with('group-attributes/g1', Mockery::on(function ($arg) { + Assert::type('array', $arg); + Assert::type('array', $arg['json'] ?? null); + $body = $arg['json']; + Assert::equal('sis-cuni', $body['service'] ?? null); + Assert::equal('foo', $body['key'] ?? null); + Assert::equal('baz', $body['value'] ?? null); + return true; + })) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'addAttribute'], + ['groupId' => 'g1', 'key' => 'foo', 'value' => 'baz'] + ); + + Assert::equal("OK", $payload); + } + + public function testAddAttributeWrongGroup() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar']]), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'addAttribute'], + ['groupId' => 'g2', 'key' => 'foo', 'value' => 'baz'] + ); + }, BadRequestException::class); + } + + public function testAddAttributeAlreadyExists() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar']]), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'addAttribute'], + ['groupId' => 'g1', 'key' => 'foo', 'value' => 'bar'] + ); + }, BadRequestException::class); + } + + public function testRemoveAttribute() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar', 'baz']]), + ] + ]))); + + $this->client->shouldReceive("delete")->with('group-attributes/g1', Mockery::on(function ($arg) { + Assert::type('array', $arg); + Assert::type('array', $arg['query'] ?? null); + $query = $arg['query']; + Assert::equal('sis-cuni', $query['service'] ?? null); + Assert::equal('foo', $query['key'] ?? null); + Assert::equal('bar', $query['value'] ?? null); + return true; + })) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => "OK" + ]))); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'removeAttribute'], + ['groupId' => 'g1', 'key' => 'foo', 'value' => 'bar'] + ); + + Assert::equal("OK", $payload); + } + + public function testRemoveAttributeWrongGroup() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar']]), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'removeAttribute'], + ['groupId' => 'g2', 'key' => 'foo', 'value' => 'bar'] + ); + }, BadRequestException::class); + } + + public function testRemoveAttributeDoesNotExist() + { + PresenterTestHelper::loginDefaultAdmin($this->container); + + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('g1', null, 'Root', true, ['foo' => ['bar']]), + ] + ]))); + + Assert::exception(function () { + PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'POST', + ['action' => 'removeAttribute'], + ['groupId' => 'g1', 'key' => 'foo', 'value' => 'baz'] + ); + }, BadRequestException::class); + } } Debugger::$logDirectory = __DIR__ . '/../../log'; From 056a708e7b4a6747d109ad950d9c7a9fe201e790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Wed, 24 Sep 2025 13:05:47 +0200 Subject: [PATCH 20/21] Adding default group operation that retrieves all (non-archived) groups. --- app/presenters/GroupsPresenter.php | 17 +++++++++++ app/router/RouterFactory.php | 1 + app/security/ACL/IGroupPermissions.php | 2 ++ tests/Presenters/GroupsPresenter.phpt | 42 ++++++++++++++++++++++++-- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 9eb60d4..96d939c 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -77,6 +77,23 @@ private function canUserAdministrateGroup(array $groups, string $groupId): void throw new ForbiddenRequestException("You do not have permissions to administrate group $groupId."); } + public function checkDefault() + { + if (!$this->groupAcl->canViewAll()) { + throw new ForbiddenRequestException("You do not have permissions to list groups."); + } + } + + /** + * Proxy to ReCodEx that retrieves all groups accessible by the user. + * @GET + */ + public function actionDefault() + { + $groups = $this->recodexApi->getGroups($this->getCurrentUser()); + $this->sendSuccessResponse($groups); + } + public function checkStudent() { if (!$this->groupAcl->canViewStudent()) { diff --git a/app/router/RouterFactory.php b/app/router/RouterFactory.php index 86c7c74..c4a5e94 100644 --- a/app/router/RouterFactory.php +++ b/app/router/RouterFactory.php @@ -73,6 +73,7 @@ private static function createCoursesRoutes(string $prefix): RouteList private static function createGroupsRoutes(string $prefix): RouteList { $router = new RouteList(); + $router[] = new GetRoute("$prefix", "Groups:default"); $router[] = new GetRoute("$prefix/student", "Groups:student"); $router[] = new GetRoute("$prefix/teacher", "Groups:teacher"); $router[] = new PostRoute("$prefix//create/", "Groups:create"); diff --git a/app/security/ACL/IGroupPermissions.php b/app/security/ACL/IGroupPermissions.php index 4bc4ca1..ab29fff 100644 --- a/app/security/ACL/IGroupPermissions.php +++ b/app/security/ACL/IGroupPermissions.php @@ -4,6 +4,8 @@ interface IGroupPermissions { + public function canViewAll(): bool; + public function canViewStudent(): bool; public function canViewTeacher(): bool; diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 2624209..0b4b23e 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -123,10 +123,48 @@ class TestGroupsPresenter extends Tester\TestCase ]; } - public function testListGroupsStudent() + public function testListGroupsAll() { - Debugger::enable(false); + PresenterTestHelper::loginDefaultAdmin($this->container); + $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) + ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ + 'success' => true, + 'code' => 200, + 'payload' => [ + self::group('p1', null, 'Parent', true, []), + self::group('p2', null, 'Parent 2', true, []), + self::group('g1', 'p1', 'Group 1', false, ['group' => ['sis1']]), + self::group('g2', 'p1', 'Group 2', false, ['group' => ['sis2']], 'student'), + self::group('g3', 'p1', 'Group 3', false, ['group' => ['sis3']]), + ] + ]))); + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + 'Groups', + 'GET', + ['action' => 'default'] + ); + + Assert::count(5, $payload); + + $ids = array_map(fn($group) => $group->id, $payload); + sort($ids); + Assert::equal(['g1', 'g2', 'g3', 'p1', 'p2'], $ids); + Assert::null($payload['p1']->parentGroupId); + Assert::null($payload['p2']->parentGroupId); + Assert::equal('p1', $payload['g1']->parentGroupId); + Assert::equal('p1', $payload['g2']->parentGroupId); + Assert::equal('p1', $payload['g3']->parentGroupId); + Assert::true($payload['p1']->organizational); + Assert::true($payload['p2']->organizational); + Assert::false($payload['g1']->organizational); + Assert::false($payload['g2']->organizational); + Assert::false($payload['g3']->organizational); + } + + public function testListGroupsStudent() + { PresenterTestHelper::login($this->container, PresenterTestHelper::STUDENT1_LOGIN); $this->client->shouldReceive("get")->with('group-attributes', Mockery::any()) ->andReturn(new Response(200, ['Content-Type' => 'application/json'], json_encode([ From 6959b152e36513bc487fc649e4f844c95301a8bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kruli=C5=A1?= Date: Wed, 24 Sep 2025 14:34:20 +0200 Subject: [PATCH 21/21] Fixing add/remove attribute endpoints to get their group ID from the URL. --- app/presenters/GroupsPresenter.php | 26 ++++++++++++-------------- tests/Presenters/GroupsPresenter.phpt | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/app/presenters/GroupsPresenter.php b/app/presenters/GroupsPresenter.php index 96d939c..26ac3a3 100644 --- a/app/presenters/GroupsPresenter.php +++ b/app/presenters/GroupsPresenter.php @@ -280,30 +280,29 @@ public function checkAddAttribute() * Proxy to ReCodEx that adds an attribute to a group. * This is rather low-level operation for super-admins only (to edit top-level and term groups). * @POST - * @Param(type="post", name="groupId", validation="string:1..", + * @Param(type="query", name="id", validation="string:1..", * description="ReCodEx ID of a group to which the attribute will be added.") * @Param(type="post", name="key", validation="string:1..", * description="Key of the attribute to add.") * @Param(type="post", name="value", validation="string:1..", * description="Value of the attribute to add.") */ - public function actionAddAttribute() + public function actionAddAttribute(string $id) { - $groupId = $this->getRequest()->getPost('groupId'); $key = $this->getRequest()->getPost('key'); $value = $this->getRequest()->getPost('value'); $groups = $this->recodexApi->getGroups($this->getCurrentUser()); - $group = $groups[$groupId] ?? null; + $group = $groups[$id] ?? null; if ($group === null) { - throw new BadRequestException("Group $groupId does not exist or is not accessible by the user."); + throw new BadRequestException("Group $id does not exist or is not accessible by the user."); } if ($group->hasAttribute($key, $value)) { - throw new BadRequestException("Group $groupId already has attribute $key=$value."); + throw new BadRequestException("Group $id already has attribute $key=$value."); } - $this->recodexApi->addAttribute($groupId, $key, $value); + $this->recodexApi->addAttribute($id, $key, $value); $this->sendSuccessResponse("OK"); } @@ -318,30 +317,29 @@ public function checkRemoveAttribute() * Proxy to ReCodEx that removes an attribute from a group. * This is rather low-level operation for super-admins only (to edit top-level and term groups). * @POST - * @Param(type="post", name="groupId", validation="string:1..", + * @Param(type="query", name="id", validation="string:1..", * description="ReCodex ID of a group from which the attribute will be removed.") * @Param(type="post", name="key", validation="string:1..", * description="Key of the attribute to remove.") * @Param(type="post", name="value", validation="string:1..", * description="Value of the attribute to remove.") */ - public function actionRemoveAttribute() + public function actionRemoveAttribute(string $id) { - $groupId = $this->getRequest()->getPost('groupId'); $key = $this->getRequest()->getPost('key'); $value = $this->getRequest()->getPost('value'); $groups = $this->recodexApi->getGroups($this->getCurrentUser()); - $group = $groups[$groupId] ?? null; + $group = $groups[$id] ?? null; if ($group === null) { - throw new BadRequestException("Group $groupId does not exist or is not accessible by the user."); + throw new BadRequestException("Group $id does not exist or is not accessible by the user."); } if (!$group->hasAttribute($key, $value)) { - throw new BadRequestException("Group $groupId does not have attribute $key=$value."); + throw new BadRequestException("Group $id does not have attribute $key=$value."); } - $this->recodexApi->removeAttribute($groupId, $key, $value); + $this->recodexApi->removeAttribute($id, $key, $value); $this->sendSuccessResponse("OK"); } } diff --git a/tests/Presenters/GroupsPresenter.phpt b/tests/Presenters/GroupsPresenter.phpt index 0b4b23e..ed52436 100644 --- a/tests/Presenters/GroupsPresenter.phpt +++ b/tests/Presenters/GroupsPresenter.phpt @@ -835,8 +835,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'addAttribute'], - ['groupId' => 'g1', 'key' => 'foo', 'value' => 'baz'] + ['action' => 'addAttribute', 'id' => 'g1'], + ['key' => 'foo', 'value' => 'baz'] ); Assert::equal("OK", $payload); @@ -860,8 +860,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'addAttribute'], - ['groupId' => 'g2', 'key' => 'foo', 'value' => 'baz'] + ['action' => 'addAttribute', 'id' => 'g2'], + ['key' => 'foo', 'value' => 'baz'] ); }, BadRequestException::class); } @@ -884,8 +884,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'addAttribute'], - ['groupId' => 'g1', 'key' => 'foo', 'value' => 'bar'] + ['action' => 'addAttribute', 'id' => 'g1'], + ['key' => 'foo', 'value' => 'bar'] ); }, BadRequestException::class); } @@ -922,8 +922,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'removeAttribute'], - ['groupId' => 'g1', 'key' => 'foo', 'value' => 'bar'] + ['action' => 'removeAttribute', 'id' => 'g1'], + ['key' => 'foo', 'value' => 'bar'] ); Assert::equal("OK", $payload); @@ -947,8 +947,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'removeAttribute'], - ['groupId' => 'g2', 'key' => 'foo', 'value' => 'bar'] + ['action' => 'removeAttribute', 'id' => 'g2'], + ['key' => 'foo', 'value' => 'bar'] ); }, BadRequestException::class); } @@ -971,8 +971,8 @@ class TestGroupsPresenter extends Tester\TestCase $this->presenter, 'Groups', 'POST', - ['action' => 'removeAttribute'], - ['groupId' => 'g1', 'key' => 'foo', 'value' => 'baz'] + ['action' => 'removeAttribute', 'id' => 'g1'], + ['key' => 'foo', 'value' => 'baz'] ); }, BadRequestException::class); }