Merge branch '#343_suggest_change_and_log' into #341_grscicoll_permis…

…sions
gbif · May 28, 2021 · a8ffa9d · a8ffa9d
2 parents c0655e2 + f6eac0b
commit a8ffa9d
Show file tree

Hide file tree

Showing 14 changed files with 87 additions and 51 deletions.
diff --git a/pom.xml b/pom.xml
@@ -86,7 +86,7 @@
     <spring-cloud-sleuth.version>2.2.8.RELEASE</spring-cloud-sleuth.version>
 
     <!-- GBIF -->
-    <gbif-api.version>0.152-SNAPSHOT</gbif-api.version>
+    <gbif-api.version>0.153-SNAPSHOT</gbif-api.version>
     <gbif-common.version>0.50</gbif-common.version>
     <gbif-common-mybatis.version>1.1</gbif-common-mybatis.version>
     <gbif-common-ws.version>1.15</gbif-common-ws.version>

diff --git a/...rg/gbif/registry/ws/it/collections/service/suggestions/BaseChangeSuggestionServiceIT.java b/...rg/gbif/registry/ws/it/collections/service/suggestions/BaseChangeSuggestionServiceIT.java
@@ -17,9 +17,9 @@
 
 import org.gbif.api.model.collections.Address;
 import org.gbif.api.model.collections.Collection;
-import org.gbif.api.model.collections.CollectionEntity;
 import org.gbif.api.model.collections.Contactable;
 import org.gbif.api.model.collections.Institution;
+import org.gbif.api.model.collections.PrimaryCollectionEntity;
 import org.gbif.api.model.collections.suggestions.ChangeSuggestion;
 import org.gbif.api.model.collections.suggestions.ChangeSuggestionService;
 import org.gbif.api.model.collections.suggestions.Status;
@@ -51,7 +51,8 @@
 
 /** Tests the {@link ChangeSuggestionService}. */
 public abstract class BaseChangeSuggestionServiceIT<
-        T extends CollectionEntity & Contactable & LenientEquals<T>, R extends ChangeSuggestion<T>>
+        T extends PrimaryCollectionEntity & Contactable & LenientEquals<T>,
+        R extends ChangeSuggestion<T>>
     extends BaseServiceIT {
 
   protected static final String PROPOSER = "proposer@test.com";
@@ -94,7 +95,8 @@ public void newEntitySuggestionTest() {
     suggestion = changeSuggestionService.getChangeSuggestion(suggKey);
     assertCreatedSuggestion(suggestion);
     assertEquals(Type.CREATE, suggestion.getType());
-    assertEquals(Country.DENMARK, suggestion.getCountry());
+    assertNull(suggestion.getEntityCountry());
+    assertNull(suggestion.getEntityName());
     assertTrue(suggestion.getChanges().isEmpty());
 
     // When - update the suggestion (e.g.: the reviewer does some changes)
@@ -155,7 +157,8 @@ public void updateEntityChangeSuggestionTest() {
     suggestion = changeSuggestionService.getChangeSuggestion(suggKey);
     assertCreatedSuggestion(suggestion);
     assertEquals(Type.UPDATE, suggestion.getType());
-    assertEquals(Country.DENMARK, suggestion.getCountry());
+    assertEquals(Country.DENMARK, suggestion.getEntityCountry());
+    assertEquals(entity.getName(), suggestion.getEntityName());
     assertEquals(address.getCity(), suggestion.getSuggestedEntity().getAddress().getCity());
     assertTrue(entity.lenientEquals(suggestion.getSuggestedEntity()));
     assertEquals(numberChanges, suggestion.getChanges().size());
@@ -186,6 +189,11 @@ public void updateEntityChangeSuggestionTest() {
   public void deleteInstitutionSuggestionTest() {
     // State
     T entity = createEntity();
+
+    Address address = new Address();
+    address.setCountry(Country.DENMARK);
+    entity.setAddress(address);
+
     UUID entityKey = crudService.create(entity);
 
     R suggestion = createEmptyChangeSuggestion();
@@ -201,6 +209,8 @@ public void deleteInstitutionSuggestionTest() {
     // Then
     suggestion = changeSuggestionService.getChangeSuggestion(suggKey);
     assertCreatedSuggestion(suggestion);
+    assertEquals(Country.DENMARK, suggestion.getEntityCountry());
+    assertEquals(entity.getName(), suggestion.getEntityName());
     assertEquals(Type.DELETE, suggestion.getType());
 
     // When
@@ -219,6 +229,11 @@ public void deleteInstitutionSuggestionTest() {
   public void mergeInstitutionSuggestionTest() {
     // State
     T entity = createEntity();
+
+    Address address = new Address();
+    address.setCountry(Country.DENMARK);
+    entity.setAddress(address);
+
     UUID entityKey = crudService.create(entity);
 
     T entity2 = createEntity();
@@ -238,6 +253,8 @@ public void mergeInstitutionSuggestionTest() {
     // Then
     suggestion = changeSuggestionService.getChangeSuggestion(suggKey);
     assertCreatedSuggestion(suggestion);
+    assertEquals(Country.DENMARK, suggestion.getEntityCountry());
+    assertEquals(entity.getName(), suggestion.getEntityName());
     assertEquals(Type.MERGE, suggestion.getType());
 
     // When

diff --git a/.../registry/ws/it/collections/service/suggestions/InstitutionChangeSuggestionServiceIT.java b/.../registry/ws/it/collections/service/suggestions/InstitutionChangeSuggestionServiceIT.java
@@ -15,6 +15,7 @@
  */
 package org.gbif.registry.ws.it.collections.service.suggestions;
 
+import org.gbif.api.model.collections.Address;
 import org.gbif.api.model.collections.AlternativeCode;
 import org.gbif.api.model.collections.Collection;
 import org.gbif.api.model.collections.Institution;
@@ -23,6 +24,7 @@
 import org.gbif.api.model.registry.Identifier;
 import org.gbif.api.service.collections.CollectionService;
 import org.gbif.api.service.collections.InstitutionService;
+import org.gbif.api.vocabulary.Country;
 import org.gbif.api.vocabulary.IdentifierType;
 import org.gbif.registry.service.collections.suggestions.InstitutionChangeSuggestionService;
 import org.gbif.ws.client.filter.SimplePrincipalProvider;
@@ -62,6 +64,11 @@ public void convertInstitutionToCollectionSuggestionTest() {
     Institution i1 = new Institution();
     i1.setCode("i1");
     i1.setName("institution 1");
+
+    Address address = new Address();
+    address.setCountry(Country.DENMARK);
+    i1.setAddress(address);
+
     UUID i1Key = institutionService.create(i1);
 
     InstitutionChangeSuggestion suggestion = new InstitutionChangeSuggestion();
@@ -78,6 +85,8 @@ public void convertInstitutionToCollectionSuggestionTest() {
     // Then
     suggestion = institutionChangeSuggestionService.getChangeSuggestion(suggKey);
     assertCreatedSuggestion(suggestion);
+    assertEquals(Country.DENMARK, suggestion.getEntityCountry());
+    assertEquals(i1.getName(), suggestion.getEntityName());
     assertEquals(Type.CONVERSION_TO_COLLECTION, suggestion.getType());
 
     // When

diff --git a/...rc/main/java/org/gbif/registry/persistence/mapper/collections/ChangeSuggestionMapper.java b/...rc/main/java/org/gbif/registry/persistence/mapper/collections/ChangeSuggestionMapper.java
@@ -26,7 +26,7 @@ List<ChangeSuggestionDto> list(
       @Param("status") Status status,
       @Param("type") Type type,
       @Param("entityType") CollectionEntityType entityType,
-      @Param("country") Country country,
+      @Param("entityCountry") Country entityCountry,
       @Param("proposerEmail") String proposerEmail,
       @Param("entityKey") UUID entityKey,
       @Nullable @Param("page") Pageable page);
@@ -35,7 +35,7 @@ long count(
       @Param("status") Status status,
       @Param("type") Type type,
       @Param("entityType") CollectionEntityType entityType,
-      @Param("country") Country country,
+      @Param("entityCountry") Country entityCountry,
       @Param("proposerEmail") String proposerEmail,
       @Param("entityKey") UUID entityKey);
 

diff --git a/...c/main/java/org/gbif/registry/persistence/mapper/collections/dto/ChangeSuggestionDto.java b/...c/main/java/org/gbif/registry/persistence/mapper/collections/dto/ChangeSuggestionDto.java
@@ -20,6 +20,8 @@ public class ChangeSuggestionDto {
   private Integer key;
   private CollectionEntityType entityType;
   private UUID entityKey;
+  private String entityName;
+  private Country entityCountry;
   private Type type;
   private Status status;
   private String proposedBy;
@@ -29,7 +31,6 @@ public class ChangeSuggestionDto {
   private Date applied;
   private String discardedBy;
   private Date discarded;
-  private Country country;
   private String suggestedEntity;
   private Set<ChangeDto> changes = new HashSet<>();
   private List<String> comments = new ArrayList<>();

diff --git a/registry-persistence/src/main/resources/liquibase/094-grscicoll-change-suggestions.xml b/registry-persistence/src/main/resources/liquibase/094-grscicoll-change-suggestions.xml
@@ -23,7 +23,8 @@
           applied_by varchar CHECK (assert_min_length(applied_by, 3)),
           discarded timestamptz,
           discarded_by varchar CHECK (assert_min_length(discarded_by, 3)),
-          country varchar(2) CHECK (length(country) = 2),
+          entity_country varchar(2) CHECK (length(entity_country) = 2),
+          entity_name varchar,
           suggested_entity jsonb,
           comments text[],
           merge_target_key uuid,

diff --git a/...ain/resources/org/gbif/registry/persistence/mapper/collections/ChangeSuggestionMapper.xml b/...ain/resources/org/gbif/registry/persistence/mapper/collections/ChangeSuggestionMapper.xml
@@ -10,15 +10,16 @@
   </resultMap>
 
   <sql id="SUGGESTION_WRITE_FIELDS">
-    entity_type, entity_key, type, status, proposed, proposed_by, proposer_email, country, changes, comments, suggested_entity,
-    merge_target_key, institution_converted_collection, name_new_institution_converted_collection, modified, modified_by
+    entity_type, entity_key, type, status, proposed, proposed_by, proposer_email, entity_country, changes, comments,
+    suggested_entity, merge_target_key, institution_converted_collection, name_new_institution_converted_collection,
+    modified, modified_by, entity_name
   </sql>
 
   <sql id="SUGGESTION_READ_FIELDS">
     cs.key, cs.entity_type, cs.entity_key, cs.type, cs.status, cs.proposed, cs.proposed_by, cs.proposer_email, cs. applied,
-    cs.applied_by, cs.discarded_by, cs.discarded, cs.country, cs.suggested_entity, cs.comments,
+    cs.applied_by, cs.discarded_by, cs.discarded, cs.entity_country, cs.suggested_entity, cs.comments,
     cs.merge_target_key, cs.changes, cs.institution_converted_collection, cs.name_new_institution_converted_collection,
-    cs.modified, cs.modified_by
+    cs.modified, cs.modified_by, cs.entity_name
   </sql>
 
   <sql id="SUGGESTION_PARAMS_CREATE">
@@ -29,15 +30,16 @@
     now(), <!-- proposed -->
     #{proposedBy,jdbcType=VARCHAR},
     #{proposerEmail,jdbcType=VARCHAR},
-    #{country,jdbcType=VARCHAR},
+    #{entityCountry,jdbcType=VARCHAR},
     #{changes,jdbcType=OTHER,typeHandler=SuggestedChangesTypeHandler}::jsonb,
     #{comments,jdbcType=OTHER,typeHandler=StringArrayTypeHandler},
     #{suggestedEntity,jdbcType=OTHER}::jsonb,
     #{mergeTargetKey,jdbcType=OTHER},
     #{institutionConvertedCollection,jdbcType=OTHER},
     #{nameNewInstitutionConvertedCollection,jdbcType=VARCHAR},
     now(), <!-- modified -->
-    #{modifiedBy,jdbcType=VARCHAR}
+    #{modifiedBy,jdbcType=VARCHAR},
+    #{entityName,jdbcType=VARCHAR}
   </sql>
 
   <sql id="SUGGESTION_PARAMS_UPDATE">
@@ -81,8 +83,8 @@
       <if test="entityType != null">
         AND cs.entity_type = #{entityType,jdbcType=OTHER}
       </if>
-      <if test="country != null">
-        AND cs.country = #{country,jdbcType=VARCHAR}
+      <if test="entityCountry != null">
+        AND cs.entity_country = #{entityCountry,jdbcType=VARCHAR}
       </if>
       <if test="proposerEmail != null">
         AND cs.proposer_email = #{proposerEmail,jdbcType=OTHER}
@@ -110,8 +112,8 @@
       <if test="entityType != null">
         AND cs.entity_type = #{entityType,jdbcType=OTHER}
       </if>
-      <if test="country != null">
-        AND cs.country = #{country,jdbcType=VARCHAR}
+      <if test="entityCountry != null">
+        AND cs.entity_country = #{entityCountry,jdbcType=VARCHAR}
       </if>
       <if test="proposerEmail != null">
         AND cs.proposer_email = #{proposerEmail,jdbcType=OTHER}

diff --git a/...y-service/src/main/java/org/gbif/registry/service/collections/merge/BaseMergeService.java b/...y-service/src/main/java/org/gbif/registry/service/collections/merge/BaseMergeService.java
@@ -185,7 +185,10 @@ protected boolean containsOccurrenceMapping(T entity, OccurrenceMapping occurren
         .anyMatch(om -> om.lenientEquals(occurrenceMapping));
   }
 
-  protected void setNullFields(T target, T source) {
+  /**
+   * Sets the fields that are null in target with the value of the source.
+   */
+  protected void setNullFieldsInTarget(T target, T source) {
     Class<T> clazz = (Class<T>) target.getClass();
     Arrays.stream(clazz.getDeclaredFields())
         .filter(f -> !f.getType().isAssignableFrom(List.class))

diff --git a/...ice/src/main/java/org/gbif/registry/service/collections/merge/CollectionMergeService.java b/...ice/src/main/java/org/gbif/registry/service/collections/merge/CollectionMergeService.java
@@ -55,7 +55,7 @@ void checkMergeExtraPreconditions(Collection entityToReplace, Collection replace
 
   @Override
   Collection mergeEntityFields(Collection entityToReplace, Collection replacement) {
-    setNullFields(replacement, entityToReplace);
+    setNullFieldsInTarget(replacement, entityToReplace);
     replacement.setEmail(mergeLists(entityToReplace.getEmail(), replacement.getEmail()));
     replacement.setPhone(mergeLists(entityToReplace.getPhone(), replacement.getPhone()));
     replacement.setContentTypes(

diff --git a/...ce/src/main/java/org/gbif/registry/service/collections/merge/InstitutionMergeService.java b/...ce/src/main/java/org/gbif/registry/service/collections/merge/InstitutionMergeService.java
@@ -15,6 +15,7 @@
  */
 package org.gbif.registry.service.collections.merge;
 
+import org.gbif.api.model.collections.Address;
 import org.gbif.api.model.collections.AlternativeCode;
 import org.gbif.api.model.collections.Collection;
 import org.gbif.api.model.collections.Institution;
@@ -106,8 +107,17 @@ && isIDigBioRecord(institutionToConvert)) {
     newCollection.setHomepage(institutionToConvert.getHomepage());
     newCollection.setCatalogUrl(institutionToConvert.getCatalogUrl());
     newCollection.setApiUrl(institutionToConvert.getApiUrl());
-    newCollection.setAddress(institutionToConvert.getAddress());
-    newCollection.setMailingAddress(institutionToConvert.getMailingAddress());
+
+    if (institutionToConvert.getAddress() != null) {
+      Address address = institutionToConvert.getAddress();
+      address.setKey(null);
+      newCollection.setAddress(address);
+    }
+    if (institutionToConvert.getMailingAddress() != null) {
+      Address address = institutionToConvert.getMailingAddress();
+      address.setKey(null);
+      newCollection.setMailingAddress(address);
+    }
 
     // if there is no institution passed we need to create a new institution
     if (institutionKeyForNewCollection == null) {
@@ -190,7 +200,7 @@ Institution mergeEntityFields(Institution entityToReplace, Institution replaceme
     replacement.getAlternativeCodes().addAll(entityToReplace.getAlternativeCodes());
 
     // Copy over information that would be lost when removing these duplicates
-    setNullFields(replacement, entityToReplace);
+    setNullFieldsInTarget(replacement, entityToReplace);
     replacement.setEmail(mergeLists(entityToReplace.getEmail(), replacement.getEmail()));
     replacement.setPhone(mergeLists(entityToReplace.getPhone(), replacement.getPhone()));
     replacement.setDisciplines(