Repository: urlaubsverwaltung/urlaubsverwaltung (java)

Issue:
Überstunden Menü wird für normalen Benutzenden nicht angezeigt, wenn 'Pflege von Überstunden nur durch privilegierte Benutzer' aktiviert ist
siehe

![grafik](https://github.com/urlaubsverwaltung/urlaubsverwaltung/assets/5683677/7ea2a9fb-1a1f-456e-ab0d-d7738e215da5)

und

![grafik](https://github.com/urlaubsverwaltung/urlaubsverwaltung/assets/5683677/cbaa30e4-ba3c-4177-a69a-77f353b1ed55)

Der normale Benutzende, darf dieses Menü jedoch immer sehen, sofern die Überstunden aktiviert sind.

Pull request:
4745 overtime settings
closes #4745

Patch:
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/web/FrameDataProvider.java b/src/main/java/org/synyx/urlaubsverwaltung/web/FrameDataProvider.java
index 2f2cf23ee..85f300c36 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/web/FrameDataProvider.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/web/FrameDataProvider.java
@@ -10,7 +10,9 @@ import org.springframework.web.servlet.ModelAndView;
 import org.synyx.urlaubsverwaltung.overtime.OvertimeSettings;
 import org.synyx.urlaubsverwaltung.person.Person;
 import org.synyx.urlaubsverwaltung.person.PersonService;
+import org.synyx.urlaubsverwaltung.settings.Settings;
 import org.synyx.urlaubsverwaltung.settings.SettingsService;
+import org.synyx.urlaubsverwaltung.sicknote.settings.SickNoteSettings;
 
 import java.util.ArrayList;
 import java.util.Objects;
@@ -60,61 +62,65 @@ public class FrameDataProvider implements DataProviderInterface {
             modelAndView.addObject("menuGravatarUrl", gravatarUrl);
             modelAndView.addObject("menuHelpUrl", menuProperties.getHelp().getUrl());
 
-            modelAndView.addObject("navigation", createNavigation(user));
-            modelAndView.addObject("navigationRequestPopupEnabled", popupMenuEnabled(user));
-            modelAndView.addObject("navigationSickNoteAddAccess", isAllowedToAddOrSubmitSickNote(user));
-            modelAndView.addObject("navigationOvertimeItemEnabled", overtimeEnabled(user));
-            modelAndView.addObject("gravatarEnabled", settingsService.getSettings().getAvatarSettings().isGravatarEnabled());
+            final Settings settings = settingsService.getSettings();
+            modelAndView.addObject("navigation", createNavigation(user, settings));
+            modelAndView.addObject("navigationRequestPopupEnabled", popupMenuEnabled(user, settings));
+            modelAndView.addObject("navigationSickNoteAddAccess", isAllowedToAddOrSubmitSickNote(user, settings.getSickNoteSettings()));
+            modelAndView.addObject("navigationOvertimeAddAccess", isUserAllowedToWriteOvertime(user, settings.getOvertimeSettings()));
+            modelAndView.addObject("gravatarEnabled", settings.getAvatarSettings().isGravatarEnabled());
         }
     }
 
-    private NavigationDto createNavigation(Person user) {
+    private NavigationDto createNavigation(Person user, Settings settings) {
 
         final ArrayList<NavigationItemDto> elements = new ArrayList<>();
 
         elements.add(new NavigationItemDto("home-link", "/web/overview", "nav.home.title", "home"));
         elements.add(new NavigationItemDto("application-link", "/web/application", "nav.vacation.title", "calendar"));
 
-        final boolean overtime = overtimeEnabled(user);
-        if (overtime) {
+        final boolean overtimeIsEnabled = overtimeEnabled(settings.getOvertimeSettings());
+        if (overtimeIsEnabled) {
             elements.add(new NavigationItemDto("overtime-link", "/web/overtime", "nav.overtime.title", "clock"));
         }
 
-        final boolean sickNote = user.hasRole(OFFICE) || user.hasRole(SICK_NOTE_VIEW);
-        if (sickNote) {
+        final boolean canViewSickNotes = user.hasRole(OFFICE) || user.hasRole(SICK_NOTE_VIEW);
+        if (canViewSickNotes) {
             elements.add(new NavigationItemDto("sicknote-link", "/web/sickdays", "nav.sicknote.title", "medkit", "navigation-sick-notes-link"));
         }
 
-        final boolean person = user.hasRole(OFFICE) || user.hasRole(BOSS) || user.hasRole(DEPARTMENT_HEAD) || user.hasRole(SECOND_STAGE_AUTHORITY);
-        if (person) {
+        final boolean canViewPersons = user.hasRole(OFFICE) || user.hasRole(BOSS) || user.hasRole(DEPARTMENT_HEAD) || user.hasRole(SECOND_STAGE_AUTHORITY);
+        if (canViewPersons) {
             elements.add(new NavigationItemDto("person-link", "/web/person", "nav.person.title", "user"));
         }
 
-        final boolean department = user.hasRole(OFFICE) || user.hasRole(BOSS);
-        if (department) {
+        final boolean canViewDepartments = user.hasRole(OFFICE) || user.hasRole(BOSS);
+        if (canViewDepartments) {
             elements.add(new NavigationItemDto("department-link", "/web/department", "nav.department.title", "users"));
         }
 
-        final boolean settings = user.hasRole(OFFICE);
-        if (settings) {
+        final boolean canViewSettings = user.hasRole(OFFICE);
+        if (canViewSettings) {
             elements.add(new NavigationItemDto("settings-link", "/web/settings", "nav.settings.title", "settings", "navigation-settings-link"));
         }
 
         return new NavigationDto(elements);
     }
 
-    private boolean popupMenuEnabled(Person signedInUser) {
-        return signedInUser.hasRole(OFFICE) || overtimeEnabled(signedInUser) || isAllowedToAddOrSubmitSickNote(signedInUser);
+    private boolean popupMenuEnabled(Person signedInUser, Settings settings) {
+        return signedInUser.hasRole(OFFICE) || isUserAllowedToWriteOvertime(signedInUser, settings.getOvertimeSettings()) || isAllowedToAddOrSubmitSickNote(signedInUser, settings.getSickNoteSettings());
     }
 
-    private boolean overtimeEnabled(Person signedInUser) {
-        final OvertimeSettings overtimeSettings = settingsService.getSettings().getOvertimeSettings();
+    private boolean overtimeEnabled(OvertimeSettings overtimeSettings) {
+        return overtimeSettings.isOvertimeActive();
+    }
+
+    private boolean isUserAllowedToWriteOvertime(Person signedInUser, OvertimeSettings overtimeSettings) {
         boolean userIsAllowedToWriteOvertime = !overtimeSettings.isOvertimeWritePrivilegedOnly() || signedInUser.isPrivileged();
         return overtimeSettings.isOvertimeActive() && userIsAllowedToWriteOvertime;
     }
 
-    private boolean isAllowedToAddOrSubmitSickNote(Person user) {
-        var userIsAllowedToSubmitSickNotes = settingsService.getSettings().getSickNoteSettings().getUserIsAllowedToSubmitSickNotes();
+    private boolean isAllowedToAddOrSubmitSickNote(Person user, SickNoteSettings sickNoteSettings) {
+        var userIsAllowedToSubmitSickNotes = sickNoteSettings.getUserIsAllowedToSubmitSickNotes();
         return user.hasRole(OFFICE) || user.hasRole(SICK_NOTE_ADD) || userIsAllowedToSubmitSickNotes;
     }
 }
diff --git a/src/main/resources/templates/_navigation.html b/src/main/resources/templates/_navigation.html
index 3a9a7661c..f3b82eb1d 100644
--- a/src/main/resources/templates/_navigation.html
+++ b/src/main/resources/templates/_navigation.html
@@ -107,7 +107,7 @@
                               ></svg>
                             </a>
                           </li>
-                          <li th:if="${navigationOvertimeItemEnabled}" class="tw-mt-4 xs:tw-mt-1">
+                          <li th:if="${navigationOvertimeAddAccess}" class="tw-mt-4 xs:tw-mt-1">
                             <a
                               href="#"
                               th:replace="~{::nav-popup-menu-link(href=@{/web/overtime/new}, icon=~{::menu-add-overtime-icon}, text=#{nav.add.overtime}, dataTestId='quick-add-new-overtime')}"


Test patch:
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/web/FrameDataProviderTest.java b/src/test/java/org/synyx/urlaubsverwaltung/web/FrameDataProviderTest.java
index 511d830e3..53f73ea82 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/web/FrameDataProviderTest.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/web/FrameDataProviderTest.java
@@ -106,7 +106,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", false);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -143,7 +143,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", true);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -181,7 +181,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", true);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -217,7 +217,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", false);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -253,7 +253,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", true);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -289,7 +289,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", false);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -325,7 +325,7 @@ class FrameDataProviderTest {
             });
 
         assertThat(modelAndView.getModelMap()).containsEntry("navigationSickNoteAddAccess", true);
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @Test
@@ -384,7 +384,7 @@ class FrameDataProviderTest {
                 );
             });
 
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", true);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", true);
     }
 
     @ParameterizedTest
@@ -412,7 +412,7 @@ class FrameDataProviderTest {
                     .doesNotContain(new NavigationItemDto("overtime-link", "/web/overtime", "nav.overtime.title", "clock"));
             });
 
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", false);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", false);
     }
 
     static Stream<Arguments> modelPropertiesNavigation() {
@@ -478,7 +478,7 @@ class FrameDataProviderTest {
     }
 
     @Test
-    void ensureOvertimeItemIsNotEnabledWhenOvertimeIsRestrictedToPrivileged() {
+    void ensureOvertimeItemIsEnabledWhenOvertimeIsRestrictedToPrivileged() {
         mockSettings(true, true, true, false);
 
         final Person person = new Person();
@@ -498,10 +498,10 @@ class FrameDataProviderTest {
                 final NavigationDto dto = (NavigationDto) navigation;
                 assertThat(dto.getElements())
                     .isNotEmpty()
-                    .doesNotContain(new NavigationItemDto("overtime-link", "/web/overtime", "nav.overtime.title", "clock"));
+                    .contains(new NavigationItemDto("overtime-link", "/web/overtime", "nav.overtime.title", "clock"));
             });
 
-        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeItemEnabled", false);
+        assertThat(modelAndView.getModelMap()).containsEntry("navigationOvertimeAddAccess", false);
     }
 
     @ParameterizedTest