From 02ed30f710b4693326fb39636ce85005e054751f Mon Sep 17 00:00:00 2001
From: Florian Necas <florian.necas@camptocamp.com>
Date: Wed, 13 Nov 2024 16:49:09 +0100
Subject: [PATCH] fix: editor group not set

---
 .github/PULL_REQUEST_TEMPLATE.md              | 27 -------------------
 .../AbstractGroupSynchronizer.java            |  2 +-
 .../integration/UserSynchronizer.java         | 24 +++++++++--------
 3 files changed, 14 insertions(+), 39 deletions(-)
 delete mode 100644 .github/PULL_REQUEST_TEMPLATE.md

diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
deleted file mode 100644
index e4f7920f8c..0000000000
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ /dev/null
@@ -1,27 +0,0 @@
-
-<!--Include a few sentences describing the overall goals for this Pull Request-->
-  
-<!-- Please help our volunteers reviewing this PR by completing the following items. 
-Ask in a comment if you have troubles with any of them. -->
-
-# Checklist
-
-- [ ] I have read the [contribution guidelines](https://github.com/geonetwork/core-geonetwork/blob/main/CONTRIBUTING.md)
-- [ ] *Pull request* provided for `main` branch, backports managed with label
-- [ ] *Good housekeeping* of code, cleaning up comments, tests, and documentation
-- [ ] *Clean commit history* broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
-- [ ] *Clean commit message*s, longer verbose messages are encouraged
-- [ ] *API Changes* are identified in commit messages
-- [ ] *Testing* provided for features or enhancements using [automatic tests](https://github.com/geonetwork/core-geonetwork/blob/main/software_development/TESTING.md)
-- [ ] *User documentation* provided for new features or enhancements in [manual](https://github.com/geonetwork/core-geonetwork/tree/main/docs/manual)
-- [ ] *Build documentation* provided for development instructions in `README.md` files
-- [ ] *Library management* using `pom.xml` dependency management. Update build documentation with intended library use and library tutorials or documentation
-
-<!--Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or not applicable.-->
-
-<!-- If you can, it's better to give credits to organisation supporting this work:
-- `Funded by NAME`
-- `Funded by URL`
-- `Funded by NAME URL`
--->
-
diff --git a/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/AbstractGroupSynchronizer.java b/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/AbstractGroupSynchronizer.java
index 852178064f..d816bdbbcb 100644
--- a/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/AbstractGroupSynchronizer.java
+++ b/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/AbstractGroupSynchronizer.java
@@ -203,7 +203,7 @@ private Map<String, GroupLink> getExistingGroupLinksById() {
     private void deleteGoneGroups(final Map<String, GroupLink> currentLinks, final Set<String> canonicalGroupIds) {
         final Set<String> deleteCandidateIds = Sets.difference(currentLinks.keySet(), canonicalGroupIds);
         if (deleteCandidateIds.isEmpty()) {
-            log.info("No organizations were deleted.");
+            log.debug("No organizations were deleted.");
             return;
         }
         for (String groupId : deleteCandidateIds) {
diff --git a/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/UserSynchronizer.java b/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/UserSynchronizer.java
index 44a2b79e07..f5e8680cf3 100644
--- a/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/UserSynchronizer.java
+++ b/georchestra-integration/externalized-accounts/src/main/java/org/geonetwork/security/external/integration/UserSynchronizer.java
@@ -20,12 +20,7 @@
 
 import static java.util.Objects.requireNonNull;
 
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.Set;
-import java.util.UUID;
+import java.util.*;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -99,7 +94,7 @@ public void synchronizeAll() {
         log.debug("Fetching canonical user definitions...");
         List<CanonicalUser> canonicalUsers = findCanonicalUsers();
         synchronizeAll(canonicalUsers);
-        log.info("Users synchronization complete.");
+        log.debug("Users synchronization complete.");
     }
 
     @Transactional
@@ -108,7 +103,7 @@ public void synchronizeAll(List<CanonicalUser> canonical) {
         requireNonNull(canonical);
         canonical.forEach(u -> requireNonNull(u, "null references not accepted in user's list"));
 
-        log.debug("Syncrhonizing {} canonical user definitions...", canonical.size());
+        log.debug("Synchronizing {} canonical user definitions...", canonical.size());
         try {
             final Set<String> canonicalIds = canonical.stream().map(CanonicalUser::getId).collect(Collectors.toSet());
             final Map<String, UserLink> currentLinks = getExistingUserLinksById();
@@ -125,7 +120,7 @@ public void synchronizeAll(List<CanonicalUser> canonical) {
      * Ensures a GeoNetwork {@link User} exists matching the {@code canonical}
      * (externally defined) user properties and authorization settings.
      * <p>
-     * 
+     *
      */
     @Transactional
     public UserLink synchronize(CanonicalUser canonical) {
@@ -152,8 +147,15 @@ private void synchronizeUserGroups(User user, List<Privilege> privileges) {
     }
 
     private List<UserGroup> resolveNewPrivileges(User user, List<Privilege> actual) {
-
-        return actual.stream()//
+        List<Privilege> editors = actual.stream().filter(privilege -> privilege.getProfile() == Profile.Reviewer)//
+                .map(privilege -> {
+                    log.debug("User {} is a reviewer of group {}", user.getUsername(),
+                        privilege.getGroup().getName());
+                    return new Privilege(privilege.getGroup(), Profile.Editor);
+                }).collect(Collectors.toList());
+        editors.addAll(actual);
+
+        return editors.stream()//
                 .map(privilege -> newUserGroup(user, privilege))//
                 .collect(Collectors.toList());
     }