Repository: urlaubsverwaltung/urlaubsverwaltung (java)

Issue:
Initiales anlegen von Negative Überstunden mit Minus (-10 z.B) nicht möglich 
**Describe the bug**

Negative Überstunden können beim initialen Eintragen nicht mehr mit '-10' im Stundenfeld eingegeben werden. Beim Editieren ist dies jedoch weiterhin möglich.

**To Reproduce**

[Screencast from 2024-02-23 13-32-48.webm](https://github.com/urlaubsverwaltung/urlaubsverwaltung/assets/5683677/38cf49ce-6f0a-4325-8305-0c0f9e426098)


**Expected behavior**
Beim initialen Eintragen von Überstunden darf man -10 in das Stundeninput eingeben dürfen und negative Überstunden werden eingetragen.



Pull request:
Allow negative input overtime on initial creation (already allowed on edit)
closes #4528

Edit: damit hebeln wir nun 

![image](https://github.com/user-attachments/assets/0416331c-4547-4cb1-b1d8-0cbb51e80e24)

"Überstundenabbau ohne Antrag aktivieren:" aus. D.h. aktuell könnte man negative Überstunden eintragen ob wohl "Überstundenabbau ohne Antrag aktivieren:" auf deaktiviert ist wenn man erst positiv einträgt und dann negativ editiert.

Edit 2.:

fixen. Tests für OvertimeFormValidator dazu fehlen noch.

<!--

Thanks for contributing to the Urlaubsverwaltung.
Please review the following notes before submitting you pull request.

Please look for other issues or pull requests which already work on this topic. Is somebody already on it? Do you need to synchronize?

# Security Vulnerabilities

🛑 STOP! 🛑 If your contribution fixes a security vulnerability, please do not submit it.
Instead, please open a report of a security vulnerability via
[Report a security vulnerability](https://github.com/urlaubsverwaltung/urlaubsverwaltung/security/advisories/new)


# Describing Your Changes

If, having reviewed the notes above, you're ready to submit your pull request, please
provide a brief description of the proposed changes.

If they:
 🐞 fix a bug, please describe the broken behaviour and how the changes fix it.
    Please label with 'type: bug' and 'status: new'
    
 🎁 make an enhancement, please describe the new functionality and why you believe it's useful.
    Please label with 'type: enhancement' and 'status: new'
 
If your pull request relates to any existing issues,
please reference them by using the issue number prefixed with #.

-->


Patch:
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeForm.java b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeForm.java
index 48d1f3a9b..0c55f38ea 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeForm.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeForm.java
@@ -1,6 +1,5 @@
 package org.synyx.urlaubsverwaltung.overtime.web;
 
-import jakarta.validation.constraints.Min;
 import org.springframework.format.annotation.DateTimeFormat;
 import org.synyx.urlaubsverwaltung.overtime.Overtime;
 import org.synyx.urlaubsverwaltung.person.Person;
@@ -31,10 +30,7 @@ public class OvertimeForm {
     private String comment;
     private boolean reduce;
 
-    @Min(0)
     private BigDecimal hours;
-
-    @Min(0)
     private Integer minutes;
 
     OvertimeForm() {
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidator.java b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidator.java
index 0c0129d60..b83ae6146 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidator.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidator.java
@@ -92,12 +92,11 @@ public class OvertimeFormValidator implements Validator {
         if (hours == null && minutes == null) {
             errors.rejectValue("hours", "overtime.error.hoursOrMinutesRequired");
             errors.rejectValue("minutes", "overtime.error.hoursOrMinutesRequired");
-        } else if (!overtimeReductionEnabled && overtimeForm.isReduce()) {
+        } else if (!overtimeReductionEnabled && (overtimeForm.isReduce() || (hours != null && hours.signum() < 0) || (minutes != null && minutes < 0))) {
             errors.rejectValue("reduce", "overtime.error.overtimeReductionNotAllowed");
         }
     }
 
-
     private void validateMaximumOvertimeNotReached(OvertimeSettings overtimeSettings, OvertimeForm overtimeForm, Errors errors) {
 
         final Duration numberOfHours = overtimeForm.getDuration();
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewController.java b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewController.java
index 96d73556f..2407b2b54 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewController.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewController.java
@@ -67,10 +67,12 @@ public class OvertimeViewController implements HasLaunchpad {
     private final Clock clock;
 
     @Autowired
-    OvertimeViewController(OvertimeService overtimeService, PersonService personService,
-                           OvertimeFormValidator validator, DepartmentService departmentService,
-                           ApplicationService applicationService, VacationTypeViewModelService vacationTypeViewModelService,
-                           SettingsService settingsService, Clock clock) {
+    OvertimeViewController(
+        OvertimeService overtimeService, PersonService personService,
+        OvertimeFormValidator validator, DepartmentService departmentService,
+        ApplicationService applicationService, VacationTypeViewModelService vacationTypeViewModelService,
+        SettingsService settingsService, Clock clock
+    ) {
         this.overtimeService = overtimeService;
         this.personService = personService;
         this.validator = validator;
@@ -196,8 +198,10 @@ public class OvertimeViewController implements HasLaunchpad {
     }
 
     @PostMapping("/overtime")
-    public String recordOvertime(@Valid @ModelAttribute("overtime") OvertimeForm overtimeForm, Errors errors,
-                                 Model model, RedirectAttributes redirectAttributes) {
+    public String recordOvertime(
+        @Valid @ModelAttribute("overtime") OvertimeForm overtimeForm, Errors errors,
+        Model model, RedirectAttributes redirectAttributes
+    ) {
 
         final Person signedInUser = personService.getSignedInUser();
         final Person person = overtimeForm.getPerson();
@@ -223,7 +227,9 @@ public class OvertimeViewController implements HasLaunchpad {
     }
 
     @GetMapping("/overtime/{id}/edit")
-    public String editOvertime(@PathVariable("id") Long id, Model model) throws UnknownOvertimeException {
+    public String editOvertime(
+        @PathVariable("id") Long id, Model model
+    ) throws UnknownOvertimeException {
 
         final Overtime overtime = overtimeService.getOvertimeById(id).orElseThrow(() -> new UnknownOvertimeException(id));
         final Person signedInUser = personService.getSignedInUser();
@@ -241,9 +247,11 @@ public class OvertimeViewController implements HasLaunchpad {
     }
 
     @PostMapping("/overtime/{id}")
-    public String updateOvertime(@PathVariable("id") Long id,
-                                 @ModelAttribute("overtime") OvertimeForm overtimeForm, Errors errors,
-                                 Model model, RedirectAttributes redirectAttributes) throws UnknownOvertimeException {
+    public String updateOvertime(
+        @PathVariable("id") Long id,
+        @Valid @ModelAttribute("overtime") OvertimeForm overtimeForm, Errors errors,
+        Model model, RedirectAttributes redirectAttributes
+    ) throws UnknownOvertimeException {
 
         final Overtime overtime = overtimeService.getOvertimeById(id).orElseThrow(() -> new UnknownOvertimeException(id));
         final Person signedInUser = personService.getSignedInUser();


Test patch:
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidatorTest.java b/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidatorTest.java
index a286a86fa..040380ca1 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidatorTest.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeFormValidatorTest.java
@@ -3,6 +3,8 @@ package org.synyx.urlaubsverwaltung.overtime.web;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.validation.Errors;
@@ -250,8 +252,9 @@ class OvertimeFormValidatorTest {
         verifyNoInteractions(errors);
     }
 
-    @Test
-    void ensureOvertimeReductionIsAllowedWhenFeatureIsEnabled() {
+    @ParameterizedTest
+    @CsvSource({"1, 30, true", "-1, -30, false", "0, -30, false", "-1, 0, false"})
+    void ensureOvertimeReductionIsAllowedWhenFeatureIsEnabled(long hour, int minutes, boolean isReduce) {
 
         final Settings settings = new Settings();
         settings.getOvertimeSettings().setOvertimeActive(true);
@@ -261,17 +264,18 @@ class OvertimeFormValidatorTest {
         when(overtimeService.getLeftOvertimeForPerson(any())).thenReturn(Duration.ofHours(42));
 
         final OvertimeForm overtimeForm = new OvertimeForm(createOvertimeRecord());
-        overtimeForm.setHours(BigDecimal.TEN);
-        overtimeForm.setMinutes(30);
-        overtimeForm.setReduce(true);
+        overtimeForm.setHours(BigDecimal.valueOf(hour));
+        overtimeForm.setMinutes(minutes);
+        overtimeForm.setReduce(isReduce);
 
         sut.validate(overtimeForm, errors);
 
         verifyNoInteractions(errors);
     }
 
-    @Test
-    void ensureOvertimeReductionIsNotAllowedWhenFeatureIsDisabled() {
+    @ParameterizedTest
+    @CsvSource({"1, 30, true", "-1, -30, false", "0, -30, false", "-1, 0, false"})
+    void ensureOvertimeReductionIsNotAllowedWhenFeatureIsDisabled(long hour, int minutes, boolean isReduce) {
 
         final Settings settings = new Settings();
         settings.getOvertimeSettings().setOvertimeActive(true);
@@ -279,9 +283,9 @@ class OvertimeFormValidatorTest {
         when(settingsService.getSettings()).thenReturn(settings);
 
         final OvertimeForm overtimeForm = new OvertimeForm(createOvertimeRecord());
-        overtimeForm.setHours(BigDecimal.TEN);
-        overtimeForm.setMinutes(30);
-        overtimeForm.setReduce(true);
+        overtimeForm.setHours(BigDecimal.valueOf(hour));
+        overtimeForm.setMinutes(minutes);
+        overtimeForm.setReduce(isReduce);
 
         sut.validate(overtimeForm, errors);
 
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewControllerTest.java b/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewControllerTest.java
index 92ba45985..b30f3c4a1 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewControllerTest.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/overtime/web/OvertimeViewControllerTest.java
@@ -36,6 +36,7 @@ import java.util.List;
 import java.util.Optional;
 
 import static java.time.Duration.ofHours;
+import static java.time.Duration.ofMinutes;
 import static java.time.temporal.TemporalAdjusters.lastDayOfYear;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -696,51 +697,51 @@ class OvertimeViewControllerTest {
     }
 
     @Test
-    void ensureOvertimeHoursMustBeGreaterZero() throws Exception {
+    void ensureOvertimeHoursCanBeLessThanZero() throws Exception {
 
         final Person overtimePerson = new Person();
         overtimePerson.setId(4L);
         when(personService.getSignedInUser()).thenReturn(overtimePerson);
         when(overtimeService.isUserIsAllowedToWriteOvertime(overtimePerson, overtimePerson)).thenReturn(true);
 
-        mockSettings();
+        final Overtime overtime = new Overtime(overtimePerson, LocalDate.of(2020, 12, 18), LocalDate.of(2020, 12, 18), ofHours(-10));
+        overtime.setId(2L);
+        when(overtimeService.save(any(Overtime.class), any(Optional.class), any(Person.class))).thenReturn(overtime);
 
-        final ResultActions resultActions = perform(
+        perform(
             post("/web/overtime")
                 .param("person.id", "4")
                 .param("startDate", "18.12.2020")
                 .param("endDate", "18.12.2020")
                 .param("hours", "-8")
-        );
-
-        resultActions
-            .andExpect(status().isOk())
-            .andExpect(model().attributeHasFieldErrors("overtime", "hours"))
-            .andExpect(view().name("overtime/overtime_form"));
+        )
+            .andExpect(status().is3xxRedirection())
+            .andExpect(view().name("redirect:/web/overtime/2"))
+            .andExpect(flash().attribute("overtimeRecord", "CREATED"));
     }
 
     @Test
-    void ensureOvertimeMinutesMustBeGreaterZero() throws Exception {
+    void ensureOvertimeMinutesCanBeLessThanZero() throws Exception {
 
         final Person overtimePerson = new Person();
         overtimePerson.setId(4L);
         when(personService.getSignedInUser()).thenReturn(overtimePerson);
         when(overtimeService.isUserIsAllowedToWriteOvertime(overtimePerson, overtimePerson)).thenReturn(true);
 
-        mockSettings();
+        final Overtime overtime = new Overtime(overtimePerson, LocalDate.of(2020, 12, 18), LocalDate.of(2020, 12, 18), ofMinutes(-30));
+        overtime.setId(2L);
+        when(overtimeService.save(any(Overtime.class), any(Optional.class), any(Person.class))).thenReturn(overtime);
 
-        final ResultActions resultActions = perform(
+        perform(
             post("/web/overtime")
                 .param("person.id", "4")
                 .param("startDate", "18.12.2020")
                 .param("endDate", "18.12.2020")
                 .param("minutes", "-30")
-        );
-
-        resultActions
-            .andExpect(status().isOk())
-            .andExpect(model().attributeHasFieldErrors("overtime", "minutes"))
-            .andExpect(view().name("overtime/overtime_form"));
+        )
+            .andExpect(status().is3xxRedirection())
+            .andExpect(view().name("redirect:/web/overtime/2"))
+            .andExpect(flash().attribute("overtimeRecord", "CREATED"));
     }
 
     @Test