PMT-4: Mitarbeiter zu einem projekt hinzufügen #14

Merged
SZUT-Dominik merged 7 commits from story/PMT-4-mitarbeiter-zu-einem-projekt into trunk 2024-10-21 14:11:15 +00:00
8 changed files with 392 additions and 12 deletions

View file

@ -68,6 +68,15 @@ components:
plannedEnd:
type: string
format: date-time
AddEmployeeDTO:
type: object
properties:
employeeId:
type: integer
format: int64
qualificationId:
type: integer
format: int64
responses:
Unauthorized:
description: "Unauthorized"
@ -147,6 +156,37 @@ paths:
/project/{id}:
post:
operationId: "addEmployee"
description: "Adds an employee to a specific Project"
parameters:
- in: path
name: id
schema:
type: integer
format: int64
required: true
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/AddEmployeeDTO"
responses:
204:
description: "Employee successfully added to the specific Project"
401:
$ref: "#/components/responses/Unauthorized"
404:
$ref: "#/components/responses/NotFound"
409:
$ref: "#/components/responses/Conflict"
422:
$ref: "#/components/responses/UnprocessableContent"
500:
$ref: "#/components/responses/InternalError"
503:
SZUT-Rajbir marked this conversation as resolved Outdated

Ich habe nochmal nachgeschaut, ein Not Acceptable ist was anderes und hat mit der Browser aushandlung vom Content Format zu tuhen.

wir wollen statdessen einen 422 Unprocessable Content schmeisen (der Schema eintrag exestiert bereits, den hier verlinkten aber bitte löschen)

Ich habe nochmal nachgeschaut, ein Not Acceptable ist was anderes und hat mit der Browser aushandlung vom Content Format zu tuhen. wir wollen statdessen einen 422 Unprocessable Content schmeisen (der Schema eintrag exestiert bereits, den hier verlinkten aber bitte löschen)
$ref: "#/components/responses/ServiceUnavailable"
delete:
operationId: "deleteProject"
description: "Delete a specific Project"
@ -170,4 +210,3 @@ paths:
type: string
500:
$ref: "#/components/responses/InternalError"
SZUT-Rajbir marked this conversation as resolved Outdated

Newline bitte wieder an das Ende tuhen.

Newline bitte wieder an das Ende tuhen.

View file

@ -1,13 +1,13 @@
package de.hmmh.pmt;
import com.fasterxml.jackson.databind.ObjectMapper;
import de.hmmh.pmt.db.Allocation;
import de.hmmh.pmt.db.AllocationRepository;
import de.hmmh.pmt.db.Project;
import de.hmmh.pmt.db.ProjectRepository;
import de.hmmh.pmt.dtos.CreateProjectDTO;
import de.hmmh.pmt.dtos.CreatedProjectDTO;
import de.hmmh.pmt.dtos.GetAllProjectsDTO;
import de.hmmh.pmt.dtos.ProjectInfo;
import de.hmmh.pmt.dtos.*;
import de.hmmh.pmt.employee.ApiClientFactory;
import de.hmmh.pmt.employee.dtos.EmployeeResponseDTO;
import de.hmmh.pmt.oas.DefaultApi;
import de.hmmh.pmt.util.Mapper;
import jakarta.servlet.http.HttpServletRequest;
@ -19,6 +19,8 @@ import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestClientException;
import java.time.ZoneOffset;
import java.util.List;
import java.util.Optional;
@Controller
@ -30,6 +32,8 @@ public class ApiController implements DefaultApi {
private ApiClientFactory apiClientFactory;
@Autowired
private ProjectRepository projectRepository;
@Autowired
AllocationRepository allocationRepository;
@Override
public Optional<ObjectMapper> getObjectMapper() {
@ -92,4 +96,51 @@ public class ApiController implements DefaultApi {
CreatedProjectDTO response = mapper.map(project);
return new ResponseEntity<>(response, HttpStatus.CREATED);
}
@Override
public ResponseEntity<Void> addEmployee(Long id, AddEmployeeDTO body) {
Optional<Project> optionalProject = projectRepository.findById(id);
if (optionalProject.isEmpty()) {
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
SZUT-Rajbir marked this conversation as resolved Outdated

du kannst einfach direkt repository.existsById(id) nutzen wenn du nichtmer auf as Projekt zugreifen möchtest danach, anosnten bitte direkt nach dem If check aus dem Optional rausziehen in eine neue Variable

du kannst einfach direkt `repository.existsById(id)` nutzen wenn du nichtmer auf as Projekt zugreifen möchtest danach, anosnten bitte direkt nach dem If check aus dem Optional rausziehen in eine neue Variable
}
SZUT-Rajbir marked this conversation as resolved Outdated

Doppelte Nutzung von findById, was einen 2ten Datenbank Query auslöst, hast doch schon den Optional

Doppelte Nutzung von `findById`, was einen 2ten Datenbank Query auslöst, hast doch schon den Optional
Project project = optionalProject.get();
SZUT-Rajbir marked this conversation as resolved Outdated

Newline um eine Logische Trennung zu machen

Newline um eine Logische Trennung zu machen
EmployeeResponseDTO employee;
try {
employee = apiClientFactory.getEmployeeApi().findById(body.getEmployeeId());
} catch (HttpClientErrorException exception) {
return new ResponseEntity<>(exception.getStatusCode().equals(HttpStatus.NOT_FOUND)
? HttpStatus.NOT_FOUND
: HttpStatus.SERVICE_UNAVAILABLE);
} catch (RestClientException exception) {
return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR);
}
SZUT-Rajbir marked this conversation as resolved Outdated

Der Happy Path sollte möglichsts zu letzt kommen, du hast viel zu viel im try catch

Der Happy Path sollte möglichsts zu letzt kommen, du hast viel zu viel im try catch
if (employee.getSkillSet()
SZUT-Rajbir marked this conversation as resolved Outdated

unnötige zwischen Variable, tu es doch direkt in das if

unnötige zwischen Variable, tu es doch direkt in das if
.stream()
.noneMatch(qualification -> qualification.getId().equals(body.getQualificationId()))) {
return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY);
}
long start = project.getStart().toEpochSecond(ZoneOffset.UTC);
long plannedEnd = project.getPlannedEnd().toEpochSecond(ZoneOffset.UTC);
List<Allocation> allocations = allocationRepository.findAllByEmployeeId(body.getEmployeeId());
if (allocations.stream()
.map(Allocation::getProject)
.anyMatch(allocatedProject -> {
long allocatedStart = allocatedProject.getStart().toEpochSecond(ZoneOffset.UTC);
SZUT-Rajbir marked this conversation as resolved Outdated

Direkt in das If tun

Direkt in das If tun
long allocatedPlannedEnd = allocatedProject.getPlannedEnd().toEpochSecond(ZoneOffset.UTC);
return Math.max(start, allocatedStart) <= Math.min(plannedEnd, allocatedPlannedEnd);
SZUT-Rajbir marked this conversation as resolved Outdated

Einmalige methode zum enscheiden von etwas hat hier nix zu suchen, gehört inline

Einmalige methode zum enscheiden von etwas hat hier nix zu suchen, gehört inline
})) {
return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY);
}
Allocation allocation = new Allocation();
allocation.setEmployeeId(employee.getId());
allocation.setRole(body.getQualificationId());
allocation.setProject(project);
allocationRepository.save(allocation);
return new ResponseEntity<>(HttpStatus.NO_CONTENT);
}
}

View file

@ -0,0 +1,35 @@
package de.hmmh.pmt.db;
import jakarta.persistence.*;
import jakarta.validation.constraints.NotNull;
import lombok.*;
@NoArgsConstructor
@AllArgsConstructor
@Getter
@Setter
@Entity
@IdClass(AllocationId.class)
@Table(name = "allocation")
public class Allocation {
@Id
@Column(name = "project_id")
@Setter(AccessLevel.NONE)
private Long projectId;
@ManyToOne
@JoinColumn(name = "project_id", referencedColumnName = "id", insertable = false, updatable = false)
private Project project;
@Id
private Long employeeId;
@NotNull
private Long role; // This is a QualificationId
public void setProject(Project project) {
this.project = project;
this.projectId = project.getId();
}
}

View file

@ -0,0 +1,18 @@
package de.hmmh.pmt.db;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import java.io.Serializable;
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
public class AllocationId implements Serializable {
private static final long serialVersionUID = 1L;
private Long projectId;
private Long employeeId;
}

View file

@ -0,0 +1,10 @@
package de.hmmh.pmt.db;
import org.springframework.data.jpa.repository.JpaRepository;
import java.util.List;
public interface AllocationRepository extends JpaRepository<Allocation, AllocationId> {
List<Allocation> findAllByEmployeeId(Long employeeId);
}
SZUT-Rajbir marked this conversation as resolved Outdated

Raw SQL, bitte die features von Spring Repositories direkt nutzen, siehe andere Repository Klassen

würde ungefähr so ausehen:

List<Allocation> findAllocationsByEmployeeId(Long employeeId);

Raw SQL, bitte die features von Spring Repositories direkt nutzen, siehe andere Repository Klassen würde ungefähr so ausehen: `List<Allocation> findAllocationsByEmployeeId(Long employeeId);`

ist es eine Vorgab von Heidemann das wir kein raw sql nutzen dürfen ?

ist es eine Vorgab von Heidemann das wir kein raw sql nutzen dürfen ?

View file

@ -4,6 +4,10 @@
<Class name="de.hmmh.pmt.employee.ApiClientFactory"/>
<Bug code="M,V,EI"/>
</Match>
<Match>
<Class name="de.hmmh.pmt.db.Allocation"/>
<Bug code="M,V,EI,EI2"/>
</Match>
<Match>
<!--Ignore Auto Generated Code -->
<Source name="~.*build/.*"/>

View file

@ -1,6 +1,8 @@
package de.hmmh.pmt;
import com.fasterxml.jackson.databind.ObjectMapper;
import de.hmmh.pmt.db.Allocation;
import de.hmmh.pmt.db.AllocationRepository;
import de.hmmh.pmt.db.Project;
import de.hmmh.pmt.db.ProjectRepository;
import de.hmmh.pmt.employee.api.EmployeeControllerApi;
@ -24,13 +26,19 @@ import java.util.Map;
public abstract class IntegrationTest {
protected final static String baseUri = "/api/v1";
protected final static long TEST_EMPLOYEE_A_ID = 1L;
protected final static long TEST_QUALIFICATION_A_ID = 10L;
protected final static long TEST_QUALIFICATION_B_ID = 11L;
@Autowired
protected MockMvc mvc;
@Autowired
protected ObjectMapper objectMapper;
@Autowired
protected ProjectRepository projectRepository;
@Autowired
protected AllocationRepository allocationRepository;
@MockBean
protected EmployeeControllerApi mockEmployeeApi;
@ -39,11 +47,13 @@ public abstract class IntegrationTest {
@BeforeEach
void setUp() {
allocationRepository.deleteAll();
projectRepository.deleteAll();
}
@AfterEach
void cleanUp() {
allocationRepository.deleteAll();
projectRepository.deleteAll();
}
@ -54,7 +64,7 @@ public abstract class IntegrationTest {
researchLabProject.setName("Underwater Research Lab");
researchLabProject.setGoal( "Discover new marine species!");
researchLabProject.setCustomerId(73L);
researchLabProject.setAdministratorId(202L);
researchLabProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
researchLabProject.setStart(LocalDateTime.of(2025, 5, 22, 8, 45));
researchLabProject.setPlannedEnd(LocalDateTime.of(2026, 5, 22, 8, 45));
projects.put("research-lab", researchLabProject);
@ -63,7 +73,7 @@ public abstract class IntegrationTest {
spaceStationProject.setName("International Space Station Expansion");
spaceStationProject.setGoal("Build new modules for extended research.");
spaceStationProject.setCustomerId(85L);
spaceStationProject.setAdministratorId(150L);
spaceStationProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
spaceStationProject.setStart(LocalDateTime.of(2024, 10, 15, 10, 0));
spaceStationProject.setPlannedEnd(LocalDateTime.of(2025, 12, 15, 10, 0));
projects.put("space-station", spaceStationProject);
@ -72,7 +82,7 @@ public abstract class IntegrationTest {
aiResearchProject.setName("AI Human Interaction Study");
aiResearchProject.setGoal("Study AI interactions in healthcare.");
aiResearchProject.setCustomerId(63L);
aiResearchProject.setAdministratorId(180L);
aiResearchProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
aiResearchProject.setStart(LocalDateTime.of(2023, 11, 3, 9, 30));
aiResearchProject.setPlannedEnd(LocalDateTime.of(2024, 6, 3, 9, 30));
projects.put("ai-research", aiResearchProject);
@ -81,7 +91,7 @@ public abstract class IntegrationTest {
renewableEnergyProject.setName("Renewable Energy Storage System");
renewableEnergyProject.setGoal("Develop new battery tech for solar power.");
renewableEnergyProject.setCustomerId(90L);
renewableEnergyProject.setAdministratorId(175L);
renewableEnergyProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
renewableEnergyProject.setStart(LocalDateTime.of(2024, 1, 10, 11, 0));
renewableEnergyProject.setPlannedEnd(LocalDateTime.of(2025, 1, 10, 11, 0));
projects.put("renewable-energy", renewableEnergyProject);
@ -90,22 +100,72 @@ public abstract class IntegrationTest {
climateChangeProject.setName("Climate Change Impact Analysis");
climateChangeProject.setGoal("Study global warming effects on ecosystems.");
climateChangeProject.setCustomerId(72L);
climateChangeProject.setAdministratorId(190L);
climateChangeProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
climateChangeProject.setStart(LocalDateTime.of(2025, 3, 12, 8, 0));
climateChangeProject.setPlannedEnd(LocalDateTime.of(2026, 3, 12, 8, 0));
projects.put("climate change", climateChangeProject);
Project medicalResearchProject = new Project();
medicalResearchProject.setName("Cancer Treatment Innovation");
medicalResearchProject.setGoal("Develop new gene therapy techniques.");
medicalResearchProject.setCustomerId(95L);
medicalResearchProject.setAdministratorId(155L);
medicalResearchProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
medicalResearchProject.setStart(LocalDateTime.of(2024, 8, 20, 9, 15));
medicalResearchProject.setPlannedEnd(LocalDateTime.of(2026, 8, 20, 9, 15));
projects.put("medical-research", medicalResearchProject);
Project overlappingProjectA = new Project();
overlappingProjectA.setName("Overlap A");
overlappingProjectA.setGoal("A Project That Overlaps with another one for Testing");
overlappingProjectA.setCustomerId(19L);
overlappingProjectA.setAdministratorId(TEST_EMPLOYEE_A_ID);
overlappingProjectA.setStart(LocalDateTime.of(1970, 6, 20, 9, 15));
overlappingProjectA.setPlannedEnd(LocalDateTime.of(2025, 8, 20, 9, 15));
projects.put("overlap-a", overlappingProjectA);
Project overlappingProjectB = new Project();
overlappingProjectB.setName("Overlap B");
overlappingProjectB.setGoal("B Project That Overlaps with another one for Testing");
overlappingProjectB.setCustomerId(23L);
overlappingProjectB.setAdministratorId(TEST_EMPLOYEE_A_ID);
overlappingProjectB.setStart(LocalDateTime.of(2024, 7, 20, 9, 15));
overlappingProjectB.setPlannedEnd(LocalDateTime.of(2026, 12, 20, 9, 15));
projects.put("overlap-b", overlappingProjectB);
Project overlappingProjectC = new Project();
overlappingProjectC.setName("Overlap C");
overlappingProjectC.setGoal("C Project That Overlaps with another one for Testing");
overlappingProjectC.setCustomerId(19L);
overlappingProjectC.setAdministratorId(TEST_EMPLOYEE_A_ID);
overlappingProjectC.setStart(LocalDateTime.of(2024, 6, 20, 9, 15));
overlappingProjectC.setPlannedEnd(LocalDateTime.of(2026, 8, 20, 9, 15));
projects.put("overlap-c", overlappingProjectC);
Project nonOverlappingProject = new Project();
nonOverlappingProject.setName("Non Overlapping to Overlap a-c");
nonOverlappingProject.setGoal("Project That doesnt overlap with another one of the Overlap ones for Testing");
nonOverlappingProject.setCustomerId(19L);
nonOverlappingProject.setAdministratorId(TEST_EMPLOYEE_A_ID);
nonOverlappingProject.setStart(LocalDateTime.of(1970, 1, 20, 9, 15));
nonOverlappingProject.setPlannedEnd(LocalDateTime.of(1970, 2, 20, 9, 15));
projects.put("non-overlap", nonOverlappingProject);
projectRepository.saveAllAndFlush(projects.values());
return projects;
}
protected Map<String, Allocation> createTestAllocationData(Map<String, Project> allProjects) {
Map<String, Allocation> allocations = new HashMap<>();
Allocation allocation1ToOverlapA = new Allocation();
allocation1ToOverlapA.setProject(allProjects.get("overlap-a"));
allocation1ToOverlapA.setEmployeeId(TEST_EMPLOYEE_A_ID);
allocation1ToOverlapA.setRole(TEST_QUALIFICATION_A_ID);
allocations.put("1>overlap-a", allocation1ToOverlapA);
allocationRepository.saveAllAndFlush(allocations.values());
return allocations;
}
}

View file

@ -0,0 +1,163 @@
package de.hmmh.pmt.project;
import de.hmmh.pmt.IntegrationTest;
import de.hmmh.pmt.db.Allocation;
import de.hmmh.pmt.db.Project;
import de.hmmh.pmt.dtos.AddEmployeeDTO;
import de.hmmh.pmt.employee.dtos.EmployeeResponseDTO;
import de.hmmh.pmt.employee.dtos.QualificationGetDTO;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.RequestBuilder;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestClientException;
import java.util.List;
import java.util.Map;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
public class AddEmployeeTest extends IntegrationTest {
@Test
void addValidEmployee() throws Exception {
when(mockEmployeeApi.findById(anyLong()))
.thenReturn(getEmployee());
Map<String, Project> allProjects = createTestProjectData();
this.mvc
.perform(getRequest(allProjects.get("research-lab").getId(), getAddEmployeeDTO()))
.andExpect(status().isNoContent());
}
@Test
void shouldNotAddEmployeeWhenProjectIsNotFound() throws Exception {
this.mvc
.perform(getRequest(0L, getAddEmployeeDTO()))
.andExpect(status().isNotFound());
}
@Test
void shouldNotAddEmployeeWhenEmployeeDoesNotExist() throws Exception {
when(this.mockEmployeeApi.findById(Mockito.anyLong()))
.thenThrow(new HttpClientErrorException(HttpStatus.NOT_FOUND));
Map<String, Project> allProjects = createTestProjectData();
this.mvc
.perform(getRequest(allProjects.get("research-lab").getId(), getAddEmployeeDTO()))
.andExpect(status().isNotFound());
}
@Test
void shouldReturnUnavailableWhenEmployeeApiIsDown() throws Exception {
when(this.mockEmployeeApi.findById(Mockito.anyLong()))
.thenThrow(new HttpClientErrorException(HttpStatus.INTERNAL_SERVER_ERROR));
Map<String, Project> allProjects = createTestProjectData();
this.mvc
.perform(getRequest(allProjects.get("research-lab").getId(), getAddEmployeeDTO()))
.andExpect(status().isServiceUnavailable());
}
@Test
void shouldReturnInternalServerErrorOnApiClientCrash() throws Exception {
when(this.mockEmployeeApi.findById(Mockito.anyLong()))
.thenThrow(new RestClientException("Api Client crash"));
Map<String, Project> allProjects = createTestProjectData();
this.mvc
.perform(getRequest(allProjects.get("research-lab").getId(), getAddEmployeeDTO()))
.andExpect(status().isInternalServerError());
}
@Test
void shouldReturnUnprocessableWhenEmployeeDoesNotHaveTheQualification() throws Exception {
when(mockEmployeeApi.findById(anyLong()))
.thenReturn(getEmployee());
Map<String, Project> allProjects = createTestProjectData();
AddEmployeeDTO addEmployeeDTO = getAddEmployeeDTO();
addEmployeeDTO.setQualificationId(TEST_QUALIFICATION_B_ID);
this.mvc
.perform(getRequest(allProjects.get("research-lab").getId(), addEmployeeDTO))
.andExpect(status().isUnprocessableEntity());
}
@Test
void shouldReturnUnprocessableWhenEmployeesProjectTimeRangesOverlapsBackwards() throws Exception {
when(mockEmployeeApi.findById(anyLong()))
.thenReturn(getEmployee());
Map<String, Project> allProjects = createTestProjectData();
createTestAllocationData(allProjects);
this.mvc
.perform(getRequest(allProjects.get("overlap-b").getId(), getAddEmployeeDTO()))
.andExpect(status().isUnprocessableEntity());
}
@Test
void shouldReturnUnprocessableWhenEmployeesProjectTimeRangesOverlapsForwards() throws Exception {
when(mockEmployeeApi.findById(anyLong()))
.thenReturn(getEmployee());
Map<String, Project> allProjects = createTestProjectData();
createTestAllocationData(allProjects);
this.mvc
.perform(getRequest(allProjects.get("overlap-c").getId(), getAddEmployeeDTO()))
.andExpect(status().isUnprocessableEntity());
}
@Test
void shouldNotReturnUnprocessableWhenAllocationsDontOverlap() throws Exception {
when(mockEmployeeApi.findById(anyLong()))
.thenReturn(getEmployee());
Map<String, Project> allProjects = createTestProjectData();
createTestAllocationData(allProjects);
this.mvc
.perform(getRequest(allProjects.get("non-overlap").getId(), getAddEmployeeDTO()))
.andExpect(status().isNoContent());
}
private QualificationGetDTO newQualification(Long id) {
QualificationGetDTO qualificationGetDTO = new QualificationGetDTO();
qualificationGetDTO.setId(id);
return qualificationGetDTO;
}
private EmployeeResponseDTO getEmployee() {
EmployeeResponseDTO employee = new EmployeeResponseDTO();
employee.setId(TEST_EMPLOYEE_A_ID);
employee.setSkillSet(List.of(newQualification(TEST_QUALIFICATION_A_ID)));
return employee;
}
private AddEmployeeDTO getAddEmployeeDTO() {
AddEmployeeDTO addEmployeeDTO = new AddEmployeeDTO();
addEmployeeDTO.setEmployeeId(TEST_EMPLOYEE_A_ID);
addEmployeeDTO.setQualificationId(TEST_QUALIFICATION_A_ID);
return addEmployeeDTO;
}
private RequestBuilder getRequest(Long projectId, AddEmployeeDTO addEmployeeDTO) throws Exception {
return MockMvcRequestBuilders
.post(baseUri + "/project/" + projectId)
.content(this.objectMapper.writeValueAsString(addEmployeeDTO))
.contentType(MediaType.APPLICATION_JSON);
}
}