feat: Add simplified admin API for circle/team management#2434
feat: Add simplified admin API for circle/team management#2434Sam428-png wants to merge 3 commits intonextcloud:masterfrom
Conversation
Adds /manage/ endpoints that allow admins to fully manage circles
(teams) without needing to be a member. Unlike the existing /admin/
endpoints which require user emulation, these endpoints use super
sessions to operate directly as admin.
New endpoints:
- GET/POST/PUT/DELETE /manage/circles — CRUD for circles
- GET/POST/DELETE /manage/circles/{id}/members — member management
- PUT /manage/circles/{id}/members/{id}/level — set member level
Based on the SURF circlesadmin app (sara-nl/nextcloud-circleadmin-api).
Signed-off-by: Sam428-png <s.d.ditmeijer@hva.nl>
0ae8207 to
eb830b3
Compare
cristianscheid
left a comment
There was a problem hiding this comment.
@Sam428-png thanks a lot for taking the time to implement this PR, really appreciated! I tested it locally and left a couple of suggestions inline. I also have one more to add: looking at how other controllers are named and structured, I think it would make sense to merge CircleApiController and MemberApiController into a single file called AdminApiController, and rename CirclesAdminService to AdminApiService for consistency. Thanks again!
- Merge CircleApiController and MemberApiController into AdminApiController - Rename CirclesAdminService to AdminApiService for consistency - Fix desc → description parameter name in create endpoint - Fix createCircle config logic: only set federated config when federated=true - Simplify getUserTypeName/getLevelName using Member::$TYPE and Member::$DEF_LEVEL - Update routes.php accordingly
Sam428-png
left a comment
There was a problem hiding this comment.
Thanks, all feedback addressed!
|
Hi @cristianscheid, thanks again for the thorough review! While implementing your suggestion for |
Hey @Sam428-png, thank you for taking the time for this PR! Given the situation you just described, maybe we could do something like this then? // instead of
if ($federated) {
$updates['config'] = $federatedConfigValue;
}
// something like
$updates['config'] = $federated ? $federatedConfigValue : 0;Does that make sense to you? |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Ensures the stored config matches the response payload by explicitly setting config to 0 when federated=false, instead of relying on the CFG_PERSONAL default.
|
Thanks for the feedback, and apologies for the late reply! This is now fixed in 9ff06b3 — |
There was a problem hiding this comment.
@Sam428-png thanks for the adjustments! One more thing I caught after re-review: since $updates will now always be non-empty, there's no longer need for the if (!empty($updates)) check anymore.
| $updates = []; | ||
| $updates['config'] = $federated ? $federatedConfigValue : 0; | ||
| if ($description !== null && $description !== '') { | ||
| $updates['description'] = $description; | ||
| } | ||
| if (!empty($updates)) { | ||
| $qb = $this->db->getQueryBuilder(); | ||
| $qb->update('circles_circle') | ||
| ->where($qb->expr()->eq('unique_id', $qb->createNamedParameter($circleId))); | ||
| foreach ($updates as $column => $value) { | ||
| $qb->set($column, $qb->createNamedParameter($value)); | ||
| } | ||
| $qb->executeStatement(); | ||
| } |
There was a problem hiding this comment.
| $updates = []; | |
| $updates['config'] = $federated ? $federatedConfigValue : 0; | |
| if ($description !== null && $description !== '') { | |
| $updates['description'] = $description; | |
| } | |
| if (!empty($updates)) { | |
| $qb = $this->db->getQueryBuilder(); | |
| $qb->update('circles_circle') | |
| ->where($qb->expr()->eq('unique_id', $qb->createNamedParameter($circleId))); | |
| foreach ($updates as $column => $value) { | |
| $qb->set($column, $qb->createNamedParameter($value)); | |
| } | |
| $qb->executeStatement(); | |
| } | |
| $updates = [ | |
| 'config' => $federated ? $federatedConfigValue : 0, | |
| ]; | |
| if ($description !== null && $description !== '') { | |
| $updates['description'] = $description; | |
| } | |
| $qb = $this->db->getQueryBuilder(); | |
| $qb->update('circles_circle') | |
| ->where($qb->expr()->eq('unique_id', $qb->createNamedParameter($circleId))); | |
| foreach ($updates as $column => $value) { | |
| $qb->set($column, $qb->createNamedParameter($value)); | |
| } | |
| $qb->executeStatement(); | |
| } |
Summary
/manage/API endpoints that allow Nextcloud admins to fully manage circles (teams) without needing to be a member of the circle/admin/{emulated}/endpoints which require user emulation per request, these endpoints use super sessions to operate directly as adminNew endpoints
GET/manage/circlesGET/manage/circles/{circleId}POST/manage/circlesname,owner,desc,federated)PUT/manage/circles/{circleId}DELETE/manage/circles/{circleId}GET/manage/circles/{circleId}/membersPOST/manage/circles/{circleId}/membersDELETE/manage/circles/{circleId}/members/{memberId}PUT/manage/circles/{circleId}/members/{memberId}/levelFiles added/changed
lib/Service/CirclesAdminService.php— business logic usingCirclesManagersuper/occ sessionslib/Controller/CircleApiController.php— circle CRUD endpoints (@AdminRequired)lib/Controller/MemberApiController.php— member management endpoints (@AdminRequired)appinfo/routes.php— 9 new OCS routes under/manage/Test plan
GET /manage/circles— returns list of all circlesPOST /manage/circles— create local circle with descriptionPOST /manage/circleswithfederated=1— create federated circle (config=0)GET /manage/circles/{id}— returns circle detail with members arrayPUT /manage/circles/{id}— update name and descriptionDELETE /manage/circles/{id}— delete circlePOST /manage/circles/{id}/members— add member to circleGET /manage/circles/{id}/members— list all membersPUT /manage/circles/{id}/members/{id}/level— change member levelDELETE /manage/circles/{id}/members/{id}— remove memberAll endpoints tested on Nextcloud 32 and Nextcloud 33 instances.