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
Owner

@SZUT-Ole @SZUT-Rajbir kommt mal bitte von !13 her für PMT-4

@SZUT-Ole @SZUT-Rajbir kommt mal bitte von !13 her für PMT-4
SZUT-Dominik reviewed 2024-10-18 21:26:37 +00:00
api/pmt.yml Outdated
@ -169,5 +213,4 @@ paths:
schema:
type: string
500:
$ref: "#/components/responses/InternalError"
Author
Owner

Newline bitte wieder an das Ende tuhen.

Newline bitte wieder an das Ende tuhen.
SZUT-Rajbir marked this conversation as resolved
api/pmt.yml Outdated
@ -77,6 +86,12 @@ components:
text/plain:
schema:
type: string
NotAcceptable:
Author
Owner

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, diesen bitte nicht vergessen zu entfernen)

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, diesen bitte nicht vergessen zu entfernen)
SZUT-Rajbir marked this conversation as resolved
api/pmt.yml Outdated
@ -150,0 +184,4 @@
$ref: "#/components/responses/Unauthorized"
404:
$ref: "#/components/responses/NotFound"
406:
Author
Owner

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)
SZUT-Rajbir marked this conversation as resolved
@ -96,0 +101,4 @@
public ResponseEntity<Void> addEmployee(Long id, AddEmployeeDTO body) {
Optional<Project> optionalProject = projectRepository.findById(id);
if (optionalProject.isEmpty()) {
Author
Owner

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
@ -96,0 +103,4 @@
Optional<Project> optionalProject = projectRepository.findById(id);
if (optionalProject.isEmpty()) {
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}
Author
Owner

Newline um eine Logische Trennung zu machen

Newline um eine Logische Trennung zu machen
SZUT-Rajbir marked this conversation as resolved
@ -96,0 +115,4 @@
if (allocationRepository.hasOverlappingAllocation(employee.getId(), project.getStart(), project.getPlannedEnd())) {
return new ResponseEntity<>(HttpStatus.CONFLICT);
}
Allocation allocation = new Allocation();
Author
Owner

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
SZUT-Rajbir marked this conversation as resolved
@ -96,0 +130,4 @@
}
}
private boolean hasQualification(List<QualificationGetDTO> qualifications, Long neededQualificationId) {
Author
Owner

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
SZUT-Rajbir marked this conversation as resolved
@ -0,0 +7,4 @@
public interface AllocationRepository extends CrudRepository<Allocation, AllocationId> {
@Query("SELECT COUNT(a) > 0 FROM Allocation a " +
Author
Owner

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);`
Member

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 ?
SZUT-Rajbir marked this conversation as resolved
Author
Owner

Das hier wäre meine Etwas aufgeräumte und optimierte lösung, bitte aber erst raufschauen wenn es garnicht mehr geht, habe ja weiter oben einiges an kommentaren hinerlassen :)

Spoiler
@Override
    public ResponseEntity<Void> addEmployee(Long id, AddEmployeeDTO body) {
        Optional<Project> optionalProject = projectRepository.findById(id);
        if (optionalProject.isEmpty()) {
            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
        }
        Project project = optionalProject.get();

        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.INTERNAL_SERVER_ERROR);
        }

        if (employee
                .getSkillSet()
                .stream()
                .noneMatch(qualification -> qualification.getId().equals(body.getQualificationId()))
        ) {
            return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY);
        }

        long start = project.getStart().toEpochSecond(null);
        long plannedEnd = project.getPlannedEnd().toEpochSecond(null);
        List<Allocation> allocations = allocationRepository.findAllocationsByEmployeeId(body.getEmployeeId());
        if(allocations
                .stream()
                .map(Allocation::getProject)
                .anyMatch(allocatedProject -> {
                    long allocatedStart = allocatedProject.getStart().toEpochSecond(null);
                    long allocatedPlannedEnd = allocatedProject.getPlannedEnd().toEpochSecond(null);
                    return Math.max(start, allocatedStart) <= Math.min(plannedEnd, allocatedPlannedEnd);
                })
        ){
            return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY);
        }

        Allocation allocation = new Allocation();
        allocation.setEmployeeId(employee.getId());
        allocation.setRole(body.getQualificationId());
        allocation.setProjectId(project.getId());
        allocationRepository.save(allocation);

        return new ResponseEntity<>(HttpStatus.NO_CONTENT);
    }
Das hier wäre meine Etwas aufgeräumte und optimierte lösung, bitte aber erst raufschauen wenn es garnicht mehr geht, habe ja weiter oben einiges an kommentaren hinerlassen :) <details> <summary>Spoiler</summary> ```java @Override public ResponseEntity<Void> addEmployee(Long id, AddEmployeeDTO body) { Optional<Project> optionalProject = projectRepository.findById(id); if (optionalProject.isEmpty()) { return new ResponseEntity<>(HttpStatus.NOT_FOUND); } Project project = optionalProject.get(); 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.INTERNAL_SERVER_ERROR); } if (employee .getSkillSet() .stream() .noneMatch(qualification -> qualification.getId().equals(body.getQualificationId())) ) { return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY); } long start = project.getStart().toEpochSecond(null); long plannedEnd = project.getPlannedEnd().toEpochSecond(null); List<Allocation> allocations = allocationRepository.findAllocationsByEmployeeId(body.getEmployeeId()); if(allocations .stream() .map(Allocation::getProject) .anyMatch(allocatedProject -> { long allocatedStart = allocatedProject.getStart().toEpochSecond(null); long allocatedPlannedEnd = allocatedProject.getPlannedEnd().toEpochSecond(null); return Math.max(start, allocatedStart) <= Math.min(plannedEnd, allocatedPlannedEnd); }) ){ return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY); } Allocation allocation = new Allocation(); allocation.setEmployeeId(employee.getId()); allocation.setRole(body.getQualificationId()); allocation.setProjectId(project.getId()); allocationRepository.save(allocation); return new ResponseEntity<>(HttpStatus.NO_CONTENT); } ``` </details>
Member

@SZUT-Dominik hab soweit die Änderungen übernommen, jedoch habe ich beim schreiben von den Test noch Schwierigkeiten. Ich schaue mir das trotzdem noch weiter an, falls ich nicht weiterkomme, werde ich Montag weiter machen.

@SZUT-Dominik hab soweit die Änderungen übernommen, jedoch habe ich beim schreiben von den Test noch Schwierigkeiten. Ich schaue mir das trotzdem noch weiter an, falls ich nicht weiterkomme, werde ich Montag weiter machen.
SZUT-Dominik reviewed 2024-10-21 05:55:50 +00:00
@ -96,0 +102,4 @@
Optional<Project> optionalProject = projectRepository.findById(id);
if (projectRepository.findById(id).isEmpty()) {
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
Author
Owner

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
SZUT-Rajbir marked this conversation as resolved
@ -96,0 +116,4 @@
}
boolean hasQualification = employee.getSkillSet()
.stream()
Author
Owner

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

unnötige zwischen Variable, tu es doch direkt in das if
SZUT-Rajbir marked this conversation as resolved
@ -96,0 +128,4 @@
List<Allocation> allocations = allocationRepository.findAllocationsByEmployeeId(body.getEmployeeId());
boolean hasOverlap = allocations.stream()
Author
Owner

Direkt in das If tun

Direkt in das If tun
SZUT-Rajbir marked this conversation as resolved
Author
Owner

Rajbir du must einmal Pullen bevor du was weiter machst, habe einen kleinen Fix für die DB sachen die Ich vorbereitet hatte dazu gepusht.

Die Statische Code Analyse sollte jetzt ohne Probleme durchlaufen, dan musst du nurnoch dein Linting Fixen für die Pipeline

Rajbir du must einmal Pullen bevor du was weiter machst, habe einen kleinen Fix für die DB sachen die Ich vorbereitet hatte dazu gepusht. Die Statische Code Analyse sollte jetzt ohne Probleme durchlaufen, dan musst du nurnoch dein Linting Fixen für die Pipeline
Member

Controller ist soweit fertig. Setze mich jetzt an den Test

Controller ist soweit fertig. Setze mich jetzt an den Test
SZUT-Dominik force-pushed story/PMT-4-mitarbeiter-zu-einem-projekt from 0c3784dd72 to b907501d8b 2024-10-21 11:13:08 +00:00 Compare
SZUT-Dominik force-pushed story/PMT-4-mitarbeiter-zu-einem-projekt from d6e51ffcfe to 5cc1621c02 2024-10-21 12:38:58 +00:00 Compare
SZUT-Dominik requested review from SZUT-Ole 2024-10-21 12:39:58 +00:00
SZUT-Dominik changed title from WIP: story/PMT-4-mitarbeiter-zu-einem-projekt to story/PMT-4-mitarbeiter-zu-einem-projekt 2024-10-21 12:40:00 +00:00
SZUT-Dominik changed title from story/PMT-4-mitarbeiter-zu-einem-projekt to PMT-4: Mitarbeiter zu einem projekt hinzufügen 2024-10-21 12:40:25 +00:00
SZUT-Dominik force-pushed story/PMT-4-mitarbeiter-zu-einem-projekt from 5cc1621c02 to 8fd25a5bed 2024-10-21 13:42:28 +00:00 Compare
SZUT-Dominik force-pushed story/PMT-4-mitarbeiter-zu-einem-projekt from 8fd25a5bed to 32d895c47e 2024-10-21 13:47:27 +00:00 Compare
SZUT-Ole approved these changes 2024-10-21 14:11:07 +00:00
SZUT-Ole left a comment
Member

basst

basst
SZUT-Dominik merged commit 8574e32e34 into trunk 2024-10-21 14:11:15 +00:00
SZUT-Dominik deleted branch story/PMT-4-mitarbeiter-zu-einem-projekt 2024-10-21 14:11:15 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: SZUT/ProjectManagmentTool#14
No description provided.