Fix study center edit

shanoir-ng-front/src/app/studies/study/study.component.html

@@ -283,7 +283,7 @@ <h2 class="header command-zone" i18n="Create study|Title@@studyDetailCreateTitle
             </fieldset>
             <fieldset [class.hidden-tab]="activeTab != 'subjects'" tab="subjects">
                 <legend>Subject tags</legend>
-                <tag-creator ngDefaultControl [mode]="mode" [study]="study" [(ngModel)]="study.tags" (onChange)="onTagListChange()" formControlName="tags"></tag-creator>
+                <tag-creator ngDefaultControl [mode]="mode" [tagsInUse]="subjectTagsInUse" [(ngModel)]="study.tags" (onChange)="onTagListChange()" formControlName="tags"></tag-creator>
                 <subject-study-list [study]="study" [selectableList]="subjects" [(ngModel)]="study.subjectStudyList" formControlName="subjectStudyList" [mode]="mode"></subject-study-list>
             </fieldset>
             <fieldset [class.hidden-tab]="activeTab != 'centers'" tab="centers">
@@ -395,7 +395,7 @@ <h2 class="header command-zone" i18n="Create study|Title@@studyDetailCreateTitle
 			</fieldset>
             <fieldset [class.hidden-tab]="activeTab != 'tags'" class="tags">
                 <legend>Public Study tags</legend>
-                <tag-creator ngDefaultControl [mode]="mode" [study]="study" [(ngModel)]="study.studyTags" (onChange)="onStudyTagListChange()" formControlName="studyTags"></tag-creator>
+                <tag-creator ngDefaultControl [mode]="mode" [(ngModel)]="study.studyTags" (onChange)="onStudyTagListChange()" formControlName="studyTags"></tag-creator>
             </fieldset>
             <fieldset *ngIf="mode == 'view'" [class.hidden-tab]="activeTab != 'bids'" tab="bids">
 				        <legend>BIDS tree</legend>

shanoir-ng-front/src/app/studies/study/study.component.ts

@@ -48,6 +48,7 @@ import { Study } from '../shared/study.model';
 import { StudyService } from '../shared/study.service';
 import {SuperPromise} from "../../utils/super-promise";
 import { Selection } from './tree.service';
+import { Tag } from 'src/app/tags/tag.model';
 
 @Component({
     selector: 'study-detail',
@@ -76,6 +77,7 @@ export class StudyComponent extends EntityComponent<Study> {
     protected hasDownloadRight: boolean;
     accessRequests: AccessRequest[];
     isStudyAdmin: boolean;
+    subjectTagsInUse: Tag[] = [];
 
     public openPrefix: boolean = false;
 
@@ -127,6 +129,7 @@ export class StudyComponent extends EntityComponent<Study> {
 
     public set entity(study: Study) {
         super.entity = study;
+        this.updateSubjectTagsInUse();
     }
 
     public get entity(): Study {
@@ -558,25 +561,25 @@ export class StudyComponent extends EntityComponent<Study> {
     }
 
     onTagListChange() {
-      // hack : force change detection
-      this.study.tags = [].concat(this.study.tags);
+        // hack : force change detection
+        this.study.tags = [].concat(this.study.tags);
+        // hack : force change detection for the subject-study tag list
+        this.study.subjectStudyList.forEach(subjStu => {
+            subjStu.study.tags = this.study.tags;
+        });
+        this.study.subjectStudyList = [].concat(this.study.subjectStudyList);
 
-      // hack : force change detection for the subject-study tag list
-      this.study.subjectStudyList.forEach(subjStu => {
-        subjStu.study.tags = this.study.tags;
-      });
-      this.study.subjectStudyList = [].concat(this.study.subjectStudyList);
+        this.updateSubjectTagsInUse();
     }
 
     onStudyTagListChange() {
-      // hack : force change detection
-      this.study.studyTags = [].concat(this.study.studyTags);
-
-      // hack : force change detection for the subject-study tag list
-      this.study.subjectStudyList.forEach(subjStu => {
-        subjStu.study.studyTags = this.study.studyTags;
-      });
-      this.study.subjectStudyList = [].concat(this.study.subjectStudyList);
+        // hack : force change detection
+        this.study.studyTags = [].concat(this.study.studyTags);
+        // hack : force change detection for the subject-study tag list
+        this.study.subjectStudyList.forEach(subjStu => {
+            subjStu.study.studyTags = this.study.studyTags;
+        });
+        this.study.subjectStudyList = [].concat(this.study.subjectStudyList);
     }
 
     goToAccessRequest(accessRequest : AccessRequest) {
@@ -598,4 +601,12 @@ export class StudyComponent extends EntityComponent<Study> {
     studyCardPolicyStr() {
         return capitalsAndUnderscoresToDisplayable(this.study.studyCardPolicy);
     }
+
+    private updateSubjectTagsInUse() {
+        let tags: Tag[] = [];
+        this.study.subjectStudyList.forEach(ss => {
+            tags = tags.concat(ss.tags);
+        });
+        this.subjectTagsInUse = tags;
+    }
 }

shanoir-ng-front/src/app/tags/tag.creator.component.html

@@ -11,7 +11,7 @@
 You should have received a copy of the GNU General Public License
 along with this program. If not, see https://www.gnu.org/licenses/gpl-3.0.html
 -->
-<span class="subtitle" *ngIf="study.studyTags?.length == 0 || !study.studyTags">There are no public tags for this study.</span>
+<span class="subtitle" *ngIf="model?.length == 0">There are no public tags for this study.</span>
 <div class="tags" *ngIf="displayedTags">
     <span class="tag" [class.dark]="dtag.darkFont" [style.background]="dtag.tag.color" *ngFor="let dtag of displayedTags">
         <span *ngIf="mode != 'view'" class="clickable" (click)="deleteTag(dtag)" [class.used]="dtag.used">

shanoir-ng-front/src/app/tags/tag.creator.component.ts

@@ -40,7 +40,7 @@ export type Mode =  "view" | "edit" | "create";
 
 export class TagCreatorComponent extends AbstractInput<Tag[]> {
     @ViewChild('input', { static: false }) input: any;
-    @Input() study: Study;
+    @Input() tagsInUse: Tag[];
     @Input() mode: Mode;
     @Output() onChange: EventEmitter<any> = new EventEmitter();
     selectedColor: string;
@@ -94,12 +94,7 @@ export class TagCreatorComponent extends AbstractInput<Tag[]> {
     }
 
     private tagUsed(tag: Tag) {
-        for (let subjectStudy of this.study.subjectStudyList) {
-            if (subjectStudy.tags.findIndex(element => element.equals(tag)) != -1) {
-                return true;
-            }
-        }
-        return false;
+        return !!this.tagsInUse?.find(ssTag => ssTag.equals(tag));
     }
 
     writeValue(obj: any): void {

shanoir-ng-ms-common/src/main/java/org/shanoir/ng/utils/EqualCheckInterface.java

@@ -0,0 +1,20 @@
+/**
+ * Shanoir NG - Import, manage and share neuroimaging data
+ * Copyright (C) 2009-2019 Inria - https://www.inria.fr/
+ * Contact us on https://project.inria.fr/shanoir/
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see https://www.gnu.org/licenses/gpl-3.0.html
+ */
+
+package org.shanoir.ng.utils;
+
+@FunctionalInterface
+public interface EqualCheckInterface <T> {
+    boolean check(T a, T b);
+}

shanoir-ng-ms-common/src/main/java/org/shanoir/ng/utils/ListDependencyUpdate.java

@@ -15,10 +15,8 @@
 package org.shanoir.ng.utils;
 
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.shanoir.ng.shared.core.model.AbstractEntity;
 import org.springframework.beans.BeanUtils;
@@ -34,27 +32,61 @@ public class ListDependencyUpdate {
 	 * @param newValues new values
 	 */
 	public static <T extends AbstractEntity> void updateWith(List<T> oldValues, List<T> newValues) {
+		updateWith(oldValues, newValues, null);
+
+	}
+
+	/**
+	 * Updates oldValues by : deleting entities that don't exist in newValues, 
+	 * updating entities that exist in both and deleting entities that exist only in oldValues.
+	 * 
+	 * @param <T> Class of your entities
+	 * @param oldValues values to be updated
+	 * @param newValues new values
+	 */
+	public static <T extends AbstractEntity> void updateWith(List<T> oldValues, List<T> newValues, EqualCheckInterface<T> equalCheck) {
 		if (newValues == null) throw new IllegalArgumentException("newValues cannot be null");
 		if (oldValues == null) throw new IllegalArgumentException("oldValues cannot be null");
 
 		// Find updated ids
-		Set<Long> updatedIds = new HashSet<>();
-		for (T entity : newValues) updatedIds.add(entity.getId());
+		Map<Long, T> updatedMap = new HashMap<>();
+		for (T entity : newValues) updatedMap.put(entity.getId(), entity);
 
 		// remove deleted entities
-		oldValues.removeIf(oldEntity -> !updatedIds.contains(oldEntity.getId()));
+		oldValues.removeIf(oldEntity -> {
+			if (updatedMap.keySet().contains(oldEntity.getId())) {
+				return false;
+			} else if (equalCheck != null) {
+				for (T newEntity : updatedMap.values()) {
+					if (equalCheck.check(oldEntity, newEntity)) {
+						return false;
+					}
+				}
+			}
+			return true;
+		});
 
 		// Index old values by their ids in a map
 		Map<Long, T> oldValuesMap = new HashMap<>();
 		for (T entity : oldValues) oldValuesMap.put(entity.getId(), entity);
 
 		// Iterate over new values
 		for (T newEntity : newValues) {
-			if (newEntity.getId() != null) {
-				T updated = oldValuesMap.get(newEntity.getId());
-				if (updated != null) {
-					BeanUtils.copyProperties(newEntity, updated, newEntity.getClass());					
+			T updated = null;
+			if (equalCheck != null) {
+				for (T old : oldValues) {
+					if (equalCheck.check(old, newEntity)) {
+						updated = old;
+					}
 				}
+			} 
+			if (updated == null && newEntity.getId() != null) {
+				updated = oldValuesMap.get(newEntity.getId());
+			}
+			if (updated != null) {
+				Long id = updated.getId();
+				BeanUtils.copyProperties(newEntity, updated, newEntity.getClass());
+				updated.setId(id);
 			} else {
 				oldValues.add(newEntity);
 			}
@@ -75,10 +107,6 @@ public static <T extends AbstractEntity> void updateWithNoRemove(List<T> oldValu
 		if (oldValues == null) throw new IllegalArgumentException("oldValues cannot be null");
 		if (oldValues == newValues) throw new IllegalStateException("those cannot be the same object");
 
-		// Find updated ids
-		Set<Long> updatedIds = new HashSet<>();
-		for (T entity : newValues) updatedIds.add(entity.getId());
-
 		// remove deleted entities
 		//oldValues.removeIf(oldEntity -> !updatedIds.contains(oldEntity.getId()));
 
@@ -91,7 +119,9 @@ public static <T extends AbstractEntity> void updateWithNoRemove(List<T> oldValu
 			if (newEntity.getId() != null) {
 				T updated = oldValuesMap.get(newEntity.getId());
 				if (updated != null) {
+					Long id = updated.getId();
 					BeanUtils.copyProperties(newEntity, updated, newEntity.getClass());					
+					updated.setId(id);
 				}
 			} else {
 				oldValues.add(newEntity);

shanoir-ng-studies/src/main/java/org/shanoir/ng/study/service/StudyServiceImpl.java

@@ -66,6 +66,7 @@
 import org.shanoir.ng.tag.model.StudyTag;
 import org.shanoir.ng.tag.model.Tag;
 import org.shanoir.ng.tag.repository.TagRepository;
+import org.shanoir.ng.utils.EqualCheckInterface;
 import org.shanoir.ng.utils.KeycloakUtil;
 import org.shanoir.ng.utils.ListDependencyUpdate;
 import org.shanoir.ng.utils.Utils;
@@ -269,6 +270,24 @@ public Study create(final Study study) throws MicroServiceCommunicationException
 		return studyDb;
 	}
 
+	private class StudyEqualCheck implements EqualCheckInterface<StudyCenter> {
+
+        @Override
+        public boolean check(StudyCenter a, StudyCenter b) {
+            boolean result =  a != null && b != null && (
+				a.getId() != null && a.getId().equals(b.getId()) || (
+					   a.getCenter() != null && a.getCenter().getId() != null 
+					&& b.getCenter() != null && b.getCenter().getId() != null 
+					&& a.getCenter().getId().equals(b.getCenter().getId())
+					&& a.getStudy() != null && a.getStudy().getId() != null
+					&& b.getStudy() != null && b.getStudy().getId() != null
+					&& a.getStudy().getId().equals(b.getStudy().getId())
+				) 
+			);
+			return result;
+        }
+	}
+
 	@Override
 	@Transactional(rollbackOn = {ShanoirException.class})
 	public Study update(Study study) throws ShanoirException {
@@ -295,10 +314,13 @@ public Study update(Study study) throws ShanoirException {
 		studyDb.setWithExamination(study.isWithExamination());
 
 		if (study.getStudyCenterList() != null) {
-			ListDependencyUpdate.updateWith(studyDb.getStudyCenterList(), study.getStudyCenterList());
 			for (StudyCenter studyCenter : studyDb.getStudyCenterList()) {
 				studyCenter.setStudy(studyDb);
 			}
+			for (StudyCenter studyCenter : study.getStudyCenterList()) {
+				studyCenter.setStudy(studyDb);
+			}
+			ListDependencyUpdate.updateWith(studyDb.getStudyCenterList(), study.getStudyCenterList(), new StudyEqualCheck());
 		}
 
 		if (study.getTags() != null) {