From 03570fc6db9bfbf82c256ffa33785d309718a836 Mon Sep 17 00:00:00 2001
From: Nico Athanassiadis <nico@dsv.su.se>
Date: Fri, 21 Feb 2025 00:27:22 +0100
Subject: [PATCH] Improve supervisor change integration (#114)
Previously when we wanted to change supervisor we had to make two calls against the daisy api.
A DELETE and a POST, this was brittle because if one of the calls failed we didn't have a good way of handling that.
This could leave the application in a state where a project could end up with 2 different supervisors.
This caused side effects and forced us to manually go into the databases and clean up the errors.
Now the daisy api is updated and we only need to do a POST to change the supervisor. See further documentation here
[POST /thesis/{id}/contributor](https://apitest.dsv.su.se/resource_Theses.html#resource_Theses_postContributor_id_projectParticipant_POST)
### IMPORTANT: Release needs to be synced with Daisy API
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/114
Reviewed-by: Andreas Svanberg <andreass@dsv.su.se>
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Co-committed-by: Nico Athanassiadis <nico@dsv.su.se>
---
.../dsv/scipro/reusable/SciProUtilities.java | 18 ++----
core/src/main/xjb/daisy_api.xjb | 4 ++
core/src/main/xsd/daisy_api.xsd | 18 ++++--
.../daisy/workers/ProjectExporter.java | 36 ++----------
.../io/impl/ExternalExporterDaisyImpl.java | 7 +--
.../daisy/workers/ProjectExporterTest.java | 56 ++++++++++++++++---
.../impl/ExternalExporterDaisyImplTest.java | 3 +-
7 files changed, 79 insertions(+), 63 deletions(-)
diff --git a/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java b/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java
index 0ed066a174..95f37a777c 100755
--- a/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java
+++ b/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java
@@ -1,7 +1,10 @@
package se.su.dsv.scipro.reusable;
-import java.time.LocalDate;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
public final class SciProUtilities {
@@ -75,15 +78,4 @@ public final class SciProUtilities {
}
return copy;
}
-
- public static Calendar toCalendar(final LocalDate localDate) {
- if (localDate == null) {
- return null;
- } else {
- final Calendar calendar = Calendar.getInstance();
- calendar.clear();
- calendar.set(localDate.getYear(), localDate.getMonthValue() - 1, localDate.getDayOfMonth());
- return calendar;
- }
- }
}
diff --git a/core/src/main/xjb/daisy_api.xjb b/core/src/main/xjb/daisy_api.xjb
index a50ec543ca..5efd745a6a 100644
--- a/core/src/main/xjb/daisy_api.xjb
+++ b/core/src/main/xjb/daisy_api.xjb
@@ -9,6 +9,10 @@
parseMethod="jakarta.xml.bind.DatatypeConverter.parseDateTime"
printMethod="jakarta.xml.bind.DatatypeConverter.printDateTime" />
+ <jaxb:javaType name="java.time.LocalDate" xmlType="xs:date"
+ parseMethod="java.time.LocalDate.parse"
+ printMethod="java.time.format.DateTimeFormatter.ISO_LOCAL_DATE.format" />
+
<!-- Force all classes implements Serializable -->
<xjc:simple />
<xjc:serializable uid="1" />
diff --git a/core/src/main/xsd/daisy_api.xsd b/core/src/main/xsd/daisy_api.xsd
index 0fa8ed93e9..fa68eef856 100755
--- a/core/src/main/xsd/daisy_api.xsd
+++ b/core/src/main/xsd/daisy_api.xsd
@@ -375,14 +375,16 @@
</xs:element>
<xs:element name="title_en" type="xs:string" minOccurs="0">
</xs:element>
- <xs:element name="startDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="startDate" type="xs:date" minOccurs="0">
</xs:element>
- <xs:element name="endDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="endDate" type="xs:date" minOccurs="0">
</xs:element>
<xs:element name="unit" type="serializableUnit" minOccurs="0">
</xs:element>
<xs:element name="researchAreas" type="researchAreas" minOccurs="0">
</xs:element>
+ <xs:element name="externalID" type="xs:string" minOccurs="0">
+ </xs:element>
</xs:sequence>
</xs:complexType>
@@ -390,13 +392,15 @@
<xs:sequence>
<xs:element name="title_en" type="xs:string" minOccurs="0">
</xs:element>
+ <xs:element name="externalID" type="xs:string" minOccurs="0">
+ </xs:element>
<xs:element name="aborted" type="xs:boolean" minOccurs="1">
</xs:element>
<xs:element name="level" type="educationalLevel" minOccurs="0">
</xs:element>
<xs:element name="title" type="xs:string" minOccurs="0">
</xs:element>
- <xs:element name="endDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="endDate" type="xs:date" minOccurs="0">
</xs:element>
<xs:element name="status" type="STATUS" minOccurs="0">
</xs:element>
@@ -404,7 +408,7 @@
</xs:element>
<xs:element name="finished" type="xs:boolean" minOccurs="1">
</xs:element>
- <xs:element name="startDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="startDate" type="xs:date" minOccurs="0">
</xs:element>
<xs:element name="id" type="xs:int" minOccurs="1">
</xs:element>
@@ -475,9 +479,11 @@
</xs:element>
<xs:element name="title_en" type="xs:string" minOccurs="0">
</xs:element>
- <xs:element name="startDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="externalID" type="xs:string" minOccurs="0">
</xs:element>
- <xs:element name="endDate" type="xs:dateTime" minOccurs="0">
+ <xs:element name="startDate" type="xs:date" minOccurs="0">
+ </xs:element>
+ <xs:element name="endDate" type="xs:date" minOccurs="0">
</xs:element>
<xs:element name="aborted" type="xs:boolean" minOccurs="1">
</xs:element>
diff --git a/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java b/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java
index d144f13d40..d6e6ab1c55 100644
--- a/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java
+++ b/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java
@@ -4,10 +4,9 @@ import static com.querydsl.core.types.dsl.Expressions.anyOf;
import jakarta.inject.Inject;
import jakarta.inject.Named;
-import jakarta.ws.rs.core.Response;
import java.time.Instant;
+import java.time.LocalDate;
import java.time.Period;
-import java.util.Calendar;
import java.util.Date;
import java.util.Optional;
import java.util.Set;
@@ -38,7 +37,6 @@ import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.project.ProjectRepo;
import se.su.dsv.scipro.project.ProjectStatus;
import se.su.dsv.scipro.project.QProject;
-import se.su.dsv.scipro.reusable.SciProUtilities;
import se.su.dsv.scipro.system.DegreeType;
import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.workerthreads.AbstractWorker;
@@ -198,7 +196,7 @@ public class ProjectExporter extends AbstractWorker {
UnitWithID unit = new UnitWithID();
unit.setId(project.getHeadSupervisor().getUnit().getIdentifier());
- Calendar startDate = SciProUtilities.toCalendar(project.getStartDate());
+ LocalDate startDate = project.getStartDate();
String title = finalThesis
.map(FinalThesis::getSwedishTitle)
@@ -211,7 +209,7 @@ public class ProjectExporter extends AbstractWorker {
thesis.setUnit(unit);
thesis.setAborted(project.getProjectStatus() == ProjectStatus.INACTIVE);
thesis.setStartDate(startDate);
- thesis.setEndDate(SciProUtilities.toCalendar(project.getExpectedEndDate()));
+ thesis.setEndDate(project.getExpectedEndDate());
final ResearchAreas researchAreas = new ResearchAreas();
if (project.getResearchArea() != null && project.getResearchArea().getIdentifier() != null) {
final ResearchAreaWithID researchArea2 = new ResearchAreaWithID();
@@ -260,36 +258,14 @@ public class ProjectExporter extends AbstractWorker {
}
private void updateHeadSupervisor(Project project) throws ExternalExportException {
- boolean toAdd = true;
+ boolean shouldChange = true;
Set<ProjectParticipant> contributors = daisyAPI.getContributors(project.getIdentifier());
for (ProjectParticipant contributor : contributors) {
if (contributor.getRole() == Role.SUPERVISOR) {
- if (contributor.getPerson().getId().equals(project.getHeadSupervisor().getIdentifier())) {
- toAdd = false;
- } else {
- Response response = daisyAPI.deleteThesisPerson(
- project.getIdentifier(),
- contributor.getPerson().getId()
- );
- if (response.getStatusInfo().getFamily() != Response.Status.Family.SUCCESSFUL) {
- LOG.warn(
- "Failed to delete supervisor from project: {} (id: {} / {})",
- project.getTitle(),
- project.getId(),
- project.getIdentifier()
- );
-
- // Card 3413
- // Do not attempt to add the new supervisor if the old one is still there to prevent
- // the project from having multiple supervisors.
- // Hopefully Daisy API will soon have support for the function "change supervisor"
- // rather than just INSERT and DELETE rows in a table.
- toAdd = false;
- }
- }
+ shouldChange = !contributor.getPerson().getId().equals(project.getHeadSupervisor().getIdentifier());
}
}
- if (toAdd) {
+ if (shouldChange) {
externalExporter.addContributorToProject(project, project.getHeadSupervisor(), Role.SUPERVISOR);
}
}
diff --git a/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java b/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java
index 7ffa26b12e..a8b7184112 100755
--- a/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java
+++ b/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java
@@ -3,7 +3,7 @@ package se.su.dsv.scipro.io.impl;
import jakarta.inject.Inject;
import jakarta.ws.rs.core.Response;
import java.math.BigDecimal;
-import java.util.Calendar;
+import java.time.LocalDate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import se.su.dsv.scipro.daisyexternal.http.DaisyAPI;
@@ -21,7 +21,6 @@ import se.su.dsv.scipro.io.dto.ThesisToBeCreated;
import se.su.dsv.scipro.io.dto.UnitWithID;
import se.su.dsv.scipro.io.exceptions.ExternalExportException;
import se.su.dsv.scipro.project.Project;
-import se.su.dsv.scipro.reusable.SciProUtilities;
import se.su.dsv.scipro.system.Unit;
import se.su.dsv.scipro.system.User;
@@ -46,9 +45,9 @@ public class ExternalExporterDaisyImpl implements ExternalExporter {
ThesisToBeCreated exportProjectDTO = new ThesisToBeCreated();
exportProjectDTO.setTitle(truncate(project.getTitle()));
- Calendar startDate = SciProUtilities.toCalendar(project.getStartDate());
+ LocalDate startDate = project.getStartDate();
exportProjectDTO.setStartDate(startDate);
- exportProjectDTO.setEndDate(SciProUtilities.toCalendar(project.getExpectedEndDate()));
+ exportProjectDTO.setEndDate(project.getExpectedEndDate());
exportProjectDTO.setUnit(unitDTO);
final ResearchAreas researchAreas = new ResearchAreas();
if (project.getResearchArea() != null && project.getResearchArea().getIdentifier() != null) {
diff --git a/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java b/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java
index fec03c6a38..33135eb7f5 100644
--- a/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java
+++ b/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java
@@ -2,6 +2,7 @@ package se.su.dsv.scipro.integration.daisy.workers;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -144,22 +145,61 @@ public class ProjectExporterTest {
}
@Test
- public void deletes_wrong_head_supervisor() {
- Person person = new Person();
- person.setId(234234);
+ public void change_head_supervisor() throws Exception {
+ User newSupervisor = User.builder()
+ .firstName("New")
+ .lastName("Supervisor")
+ .emailAddress("new@supervisor.com")
+ .build();
+ Unit unit = new Unit();
+ unit.setIdentifier(239478);
- ProjectParticipant headSupervisor = new ProjectParticipant();
- headSupervisor.setRole(Role.SUPERVISOR);
- headSupervisor.setPerson(person);
+ newSupervisor.setIdentifier(12345);
+ newSupervisor.setUnit(unit);
+ exportedProject.setHeadSupervisor(newSupervisor);
Set<ProjectParticipant> contributors = new HashSet<>();
- contributors.add(headSupervisor);
+ ProjectParticipant oldSupervisor = new ProjectParticipant();
+ oldSupervisor.setRole(Role.SUPERVISOR);
+ Person oldPerson = new Person();
+ oldPerson.setId(SUPERVISOR_IDENTIFIER);
+ oldSupervisor.setPerson(oldPerson);
+ contributors.add(oldSupervisor);
when(daisyAPI.getContributors(exportedProject.getIdentifier())).thenReturn(contributors);
projectExporter.run();
- verify(daisyAPI).deleteThesisPerson(exportedProject.getIdentifier(), person.getId());
+ verify(externalExporter).addContributorToProject(exportedProject, newSupervisor, Role.SUPERVISOR);
+ }
+
+ @Test
+ public void should_not_change_head_supervisor_if_already_exists() throws Exception {
+ User newSupervisor = User.builder()
+ .firstName("New")
+ .lastName("Supervisor")
+ .emailAddress("new@supervisor.com")
+ .build();
+ Unit unit = new Unit();
+ unit.setIdentifier(239478);
+
+ newSupervisor.setIdentifier(SUPERVISOR_IDENTIFIER);
+ newSupervisor.setUnit(unit);
+ exportedProject.setHeadSupervisor(newSupervisor);
+
+ Set<ProjectParticipant> contributors = new HashSet<>();
+ ProjectParticipant oldSupervisor = new ProjectParticipant();
+ oldSupervisor.setRole(Role.SUPERVISOR);
+ Person oldPerson = new Person();
+ oldPerson.setId(SUPERVISOR_IDENTIFIER);
+ oldSupervisor.setPerson(oldPerson);
+ contributors.add(oldSupervisor);
+
+ when(daisyAPI.getContributors(exportedProject.getIdentifier())).thenReturn(contributors);
+
+ projectExporter.run();
+
+ verify(externalExporter, never()).addContributorToProject(exportedProject, newSupervisor, Role.SUPERVISOR);
}
@Test
diff --git a/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java b/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java
index e9c044dabe..c6be049575 100644
--- a/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java
+++ b/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java
@@ -19,7 +19,6 @@ import org.mockito.junit.jupiter.MockitoExtension;
import se.su.dsv.scipro.daisyexternal.http.DaisyAPI;
import se.su.dsv.scipro.io.dto.ThesisToBeCreated;
import se.su.dsv.scipro.project.Project;
-import se.su.dsv.scipro.reusable.SciProUtilities;
import se.su.dsv.scipro.system.DegreeType;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.Unit;
@@ -71,6 +70,6 @@ public class ExternalExporterDaisyImplTest {
ArgumentCaptor<ThesisToBeCreated> captor = ArgumentCaptor.forClass(ThesisToBeCreated.class);
verify(daisyAPI).createProject(captor.capture());
- assertEquals(captor.getValue().getStartDate(), SciProUtilities.toCalendar(daisyStartDate));
+ assertEquals(captor.getValue().getStartDate(), daisyStartDate);
}
}