Fix all sonar bugs

backend/src/main/java/ch/puzzle/okr/service/authorization/AuthorizationServiceBase.java

@@ -16,7 +16,7 @@ public abstract class AuthorizationServiceBase<ID, T extends WriteableInterface,
     private final BusinessServiceInterface<ID, T> businessService;
     private final AuthorizationService authorizationService;
 
-    public AuthorizationServiceBase(BusinessServiceInterface<ID, T> businessService,
+    protected AuthorizationServiceBase(BusinessServiceInterface<ID, T> businessService,
             AuthorizationService authorizationService) {
         this.businessService = businessService;
         this.authorizationService = authorizationService;

backend/src/test/java/ch/puzzle/okr/controller/OverviewControllerIT.java

@@ -1,5 +1,7 @@
 package ch.puzzle.okr.controller;
 
+import ch.puzzle.okr.mapper.DashboardMapper;
+import ch.puzzle.okr.mapper.OverviewMapper;
 import ch.puzzle.okr.models.overview.Overview;
 import ch.puzzle.okr.models.overview.OverviewId;
 import ch.puzzle.okr.service.authorization.OverviewAuthorizationService;
@@ -41,6 +43,11 @@ class OverviewControllerIT {
     private OverviewAuthorizationService overviewAuthorizationService;
     @MockBean
     OrganisationBusinessService organisationBusinessService;
+    // Dashboard and OverviewMapper are required for testing
+    @SpyBean
+    private DashboardMapper dashboardMapper;
+    @SpyBean
+    private OverviewMapper overviewMapper;
 
     public static final String PUZZLE = "Puzzle";
     public static final String DESCRIPTION = "This is a description";
@@ -95,7 +102,7 @@ void shouldGetAllTeamsWithObjective() throws Exception {
 
         mvc.perform(get("/api/v2/overview?quarter=2&team=1,2,3,4").contentType(MediaType.APPLICATION_JSON))
                 .andExpect(MockMvcResultMatchers.status().isOk())
-                .andExpect(jsonPath("JSON_PATH_OVERVIEWS", Matchers.hasSize(3)))
+                .andExpect(jsonPath(JSON_PATH_OVERVIEWS, Matchers.hasSize(3)))
                 .andExpect(jsonPath("$.adminAccess", Is.is(true))).andExpect(jsonPath(JSON_PATH_TEAM_ID, Is.is(1)))
                 .andExpect(jsonPath(JSON_PATH_TEAM_NAME, Is.is(PUZZLE)))
                 .andExpect(jsonPath("$.overviews[0].objectives[0].id", Is.is(1)))

backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java

@@ -91,10 +91,11 @@ void registerAuthorizationUserShouldSetTeamOrganisations() {
 
     @Test
     void registerAuthorizationUserShouldSetChampions() {
-        User user = User.Builder.builder().withFirstname("firstname").withLastname("lastname").withUsername("peggimann")
-                .withEmail("[email protected]").build();
-        Jwt token = mockJwtToken(user, List.of(ORGANISATION_FIRST_LEVEL, ORGANISATION_SECOND_LEVEL));
-        AuthorizationUser authorizationUser = authorizationRegistrationService.registerAuthorizationUser(user, token);
+        User testUser = User.Builder.builder().withFirstname("firstname").withLastname("lastname")
+                .withUsername("peggimann").withEmail("[email protected]").build();
+        Jwt token = mockJwtToken(testUser, List.of(ORGANISATION_FIRST_LEVEL, ORGANISATION_SECOND_LEVEL));
+        AuthorizationUser authorizationUser = authorizationRegistrationService.registerAuthorizationUser(testUser,
+                token);
 
         assertRoles(List.of(READ_ALL_DRAFT, READ_ALL_PUBLISHED, WRITE_ALL), authorizationUser);
     }

backend/src/test/java/ch/puzzle/okr/service/persistence/ActionPersistenceServiceIT.java

@@ -36,6 +36,8 @@ private static Action createAction(Long id, int version) {
                 .build();
     }
 
+    private static final String UPDATED_ACTION = "Updated Action";
+
     @AfterEach
     void tearDown() {
         try {
@@ -67,21 +69,21 @@ void saveActionShouldSaveNewAction() {
     void updateActionShouldUpdateAction() {
         Action action = createAction(null);
         createdAction = actionPersistenceService.save(action);
-        createdAction.setAction("Updated Action");
+        createdAction.setAction(UPDATED_ACTION);
         createdAction.setPriority(4);
 
         Action updateAction = actionPersistenceService.save(createdAction);
 
         assertEquals(createdAction.getId(), updateAction.getId());
-        assertEquals("Updated Action", updateAction.getAction());
+        assertEquals(UPDATED_ACTION, updateAction.getAction());
         assertEquals(4, updateAction.getPriority());
     }
 
     @Test
     void updateActionShouldThrowExceptionWhenAlreadyUpdated() {
         createdAction = actionPersistenceService.save(createAction(null));
         Action changedAction = createAction(createdAction.getId(), 0);
-        changedAction.setAction("Updated Action");
+        changedAction.setAction(UPDATED_ACTION);
 
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> actionPersistenceService.save(changedAction));

backend/src/test/java/ch/puzzle/okr/service/persistence/AuthorizationCriteriaIT.java

@@ -1,6 +1,5 @@
 package ch.puzzle.okr.service.persistence;
 
-import ch.puzzle.okr.dto.ErrorDto;
 import ch.puzzle.okr.models.Objective;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
 import ch.puzzle.okr.models.overview.Overview;
@@ -16,7 +15,6 @@
 
 @SpringIntegrationTest
 class AuthorizationCriteriaIT {
-    private static final String REASON = "not authorized to read objective";
 
     @Autowired
     ObjectivePersistenceService objectivePersistenceService;

backend/src/test/java/ch/puzzle/okr/service/persistence/CompletedPersistenceServiceIT.java

@@ -34,6 +34,8 @@ private static Completed createCompleted(Long id, int version) {
                 .withComment("Wir haben es gut geschafft").build();
     }
 
+    private static final String COMPLETED = "Completed";
+
     @AfterEach
     void tearDown() {
         try {
@@ -79,7 +81,7 @@ void updateCompletedShouldThrowExceptionWhenAlreadyUpdated() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> completedPersistenceService.save(updateCompleted));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of("Completed")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of(COMPLETED)));
 
         assertEquals(UNPROCESSABLE_ENTITY, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -104,7 +106,7 @@ void deleteCompletedIdShouldDeleteExistingCompletedByObjectiveId() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> completedPersistenceService.findById(3L));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of("Completed", "3")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of(COMPLETED, "3")));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -120,7 +122,7 @@ void deleteCompletedShouldThrowExceptionWhenCompletedNotFound() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> completedPersistenceService.findById(completedId));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of("Completed", "200")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of(COMPLETED, "200")));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());

backend/src/test/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceServiceIT.java

@@ -50,6 +50,8 @@ private static KeyResult createKeyResultOrdinal(Long id, int version) {
 
     private static final String KEY_RESULT_UPDATED = "Updated Key Result";
     private static final String THIS_IS_DESCRIPTION = "This is a new description";
+    private static final String MODEL_NOT_FOUND = "MODEL_WITH_ID_NOT_FOUND";
+    private static final String KEYRESULT = "KeyResult";
 
     @AfterEach
     void tearDown() {
@@ -92,7 +94,7 @@ void getKeyResultByIdShouldThrowExceptionWhenKeyResultNotFound() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.findById(321L));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("MODEL_WITH_ID_NOT_FOUND", List.of("KeyResult", "321")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto(MODEL_NOT_FOUND, List.of(KEYRESULT, "321")));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -104,7 +106,7 @@ void getKeyResultByIdShouldThrowExceptionWhenKeyResultIdIsNull() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.findById(null));
 
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("ATTRIBUTE_NULL", List.of("ID", "KeyResult")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto("ATTRIBUTE_NULL", List.of("ID", KEYRESULT)));
 
         assertEquals(BAD_REQUEST, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -129,8 +131,7 @@ void recreateEntityShouldUpdateKeyResultNoTypeChange() {
         // Should delete the old KeyResult
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, this::execute);
 
-        List<ErrorDto> expectedErrors = List
-                .of(ErrorDto.of("MODEL_WITH_ID_NOT_FOUND", List.of("KeyResult", keyResultId)));
+        List<ErrorDto> expectedErrors = List.of(ErrorDto.of(MODEL_NOT_FOUND, List.of(KEYRESULT, keyResultId)));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -164,8 +165,7 @@ void recreateEntityShouldUpdateKeyResultWithTypeChange() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.findById(keyResultId));
 
-        List<ErrorDto> expectedErrors = List
-                .of(ErrorDto.of("MODEL_WITH_ID_NOT_FOUND", List.of("KeyResult", keyResultId)));
+        List<ErrorDto> expectedErrors = List.of(ErrorDto.of(MODEL_NOT_FOUND, List.of(KEYRESULT, keyResultId)));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -204,7 +204,7 @@ void updateEntityShouldThrowExceptionWhenAlreadyUpdated() {
 
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.updateEntity(updateKeyResult));
-        List<ErrorDto> expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of("KeyResult")));
+        List<ErrorDto> expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of(KEYRESULT)));
 
         assertEquals(UNPROCESSABLE_ENTITY, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -228,8 +228,7 @@ void deleteKeyResultByIdShouldDeleteExistingKeyResult() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.findById(keyResultId));
 
-        List<ErrorDto> expectedErrors = List
-                .of(ErrorDto.of("MODEL_WITH_ID_NOT_FOUND", List.of("KeyResult", keyResultId)));
+        List<ErrorDto> expectedErrors = List.of(ErrorDto.of(MODEL_NOT_FOUND, List.of(KEYRESULT, keyResultId)));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());
@@ -246,8 +245,7 @@ void deleteKeyResultShouldThrowExceptionWhenKeyResultNotFound() {
         OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
                 () -> keyResultPersistenceService.findById(keyResultId));
 
-        List<ErrorDto> expectedErrors = List
-                .of(ErrorDto.of("MODEL_WITH_ID_NOT_FOUND", List.of("KeyResult", keyResultId)));
+        List<ErrorDto> expectedErrors = List.of(ErrorDto.of(MODEL_NOT_FOUND, List.of(KEYRESULT, keyResultId)));
 
         assertEquals(NOT_FOUND, exception.getStatus());
         assertThat(expectedErrors).hasSameElementsAs(exception.getErrors());

frontend/src/app/shared/dialog/checkin/check-in-form/check-in-form.component.ts

@@ -89,7 +89,7 @@ export class CheckInFormComponent implements OnInit {
     if (this.keyResult.keyResultType === 'metric') {
       checkIn = {
         ...this.dialogForm.value,
-        value: this.parserPipe.transform(this.dialogForm?.controls['value'].value!),
+        value: this.parserPipe.transform(this.dialogForm.controls['value'].value),
         keyResultId: this.keyResult.id,
         id: this.checkIn.id,
         version: this.checkIn.version,

frontend/src/app/shared/dialog/confirm-dialog/confirm-dialog.component.ts

@@ -27,13 +27,16 @@ export class ConfirmDialogComponent implements OnInit {
         }
       } else {
         this.dialogTitle = this.data.title + ' löschen';
-        this.dialogText = this.data.isAction
-          ? 'Möchtest du diese Action wirklich löschen?'
-          : 'Möchtest du dieses ' +
+        if (this.data.isAction) {
+          this.dialogText = 'Möchtest du diese Action wirklich löschen?';
+        } else {
+          this.dialogText =
+            'Möchtest du dieses ' +
             this.data.title +
             ' wirklich löschen? Zugehörige ' +
             (this.data.title == 'Objective' ? 'Key Results' : 'Check-ins') +
             ' werden dadurch ebenfalls gelöscht!';
+        }
       }
     }
   }