Fix sonar code smells

backend/src/main/java/ch/puzzle/okr/controller/CompletedController.java

@@ -1,5 +1,7 @@
 package ch.puzzle.okr.controller;
 
+import ch.puzzle.okr.dto.CompletedDto;
+import ch.puzzle.okr.mapper.CompletedMapper;
 import ch.puzzle.okr.models.Completed;
 import ch.puzzle.okr.service.authorization.CompletedAuthorizationService;
 import io.swagger.v3.oas.annotations.Operation;
@@ -16,9 +18,12 @@
 public class CompletedController {
 
     private final CompletedAuthorizationService completedAuthorizationService;
+    private final CompletedMapper completedMapper;
 
-    public CompletedController(CompletedAuthorizationService completedAuthorizationService) {
+    public CompletedController(CompletedAuthorizationService completedAuthorizationService,
+            CompletedMapper completedMapper) {
         this.completedAuthorizationService = completedAuthorizationService;
+        this.completedMapper = completedMapper;
     }
 
     @Operation(summary = "Create Completed", description = "Create a new Completed Reference.")
@@ -28,9 +33,10 @@ public CompletedController(CompletedAuthorizationService completedAuthorizationS
             @ApiResponse(responseCode = "401", description = "Not authorized to create Completed Reference", content = @Content),
             @ApiResponse(responseCode = "404", description = "Could not create Completed Reference", content = @Content) })
     @PostMapping
-    public ResponseEntity<Completed> createCompleted(@RequestBody Completed completed) {
+    public ResponseEntity<CompletedDto> createCompleted(@RequestBody CompletedDto completedDto) {
+        Completed completed = completedMapper.toCompleted(completedDto);
         Completed createdCompleted = completedAuthorizationService.createCompleted(completed);
-        return ResponseEntity.status(HttpStatus.CREATED).body(createdCompleted);
+        return ResponseEntity.status(HttpStatus.CREATED).body(completedMapper.toDto(createdCompleted));
     }
 
     @Operation(summary = "Delete Completed by Objective Id", description = "Delete Completed Reference by Objective Id")

backend/src/main/java/ch/puzzle/okr/dto/CompletedDto.java

@@ -0,0 +1,6 @@
+package ch.puzzle.okr.dto;
+
+import ch.puzzle.okr.models.Objective;
+
+public record CompletedDto(Long id, Objective objective, String comment) {
+}

backend/src/main/java/ch/puzzle/okr/mapper/CompletedMapper.java

@@ -0,0 +1,18 @@
+package ch.puzzle.okr.mapper;
+
+import ch.puzzle.okr.dto.CompletedDto;
+import ch.puzzle.okr.models.Completed;
+import org.springframework.stereotype.Component;
+
+@Component
+public class CompletedMapper {
+
+    public CompletedDto toDto(Completed completed) {
+        return new CompletedDto(completed.getId(), completed.getObjective(), completed.getComment());
+    }
+
+    public Completed toCompleted(CompletedDto completedDto) {
+        return Completed.Builder.builder().withId(completedDto.id()).withObjective(completedDto.objective())
+                .withComment(completedDto.comment()).build();
+    }
+}

backend/src/test/java/ch/puzzle/okr/TestHelper.java

@@ -15,13 +15,18 @@ public class TestHelper {
     private TestHelper() {
     }
 
+    private static final String FIRSTNAME = "Bob";
+    private static final String LASTNAME = "Kaufmann";
+    private static final String USERNAME = "bkaufmann";
+    private static final String EMAIL = "[email protected]";
+
     public static User defaultUser(Long id) {
-        return User.Builder.builder().withId(id).withFirstname("Bob").withLastname("Kaufmann").withUsername("bkaufmann")
-                .withEmail("[email protected]").build();
+        return User.Builder.builder().withId(id).withFirstname(FIRSTNAME).withLastname(LASTNAME).withUsername(USERNAME)
+                .withEmail(EMAIL).build();
     }
 
     public static AuthorizationUser defaultAuthorizationUser() {
-        return mockAuthorizationUser(1L, "bkaufmann", "Bob", "Kaufmann", "[email protected]", List.of(5L), 5L,
+        return mockAuthorizationUser(1L, USERNAME, FIRSTNAME, LASTNAME, EMAIL, List.of(5L), 5L,
                 List.of(READ_ALL_PUBLISHED, READ_ALL_DRAFT, WRITE_ALL));
     }
 
@@ -38,7 +43,7 @@ public static AuthorizationUser mockAuthorizationUser(Long id, String username,
     }
 
     public static Jwt defaultJwtToken() {
-        return mockJwtToken("bkaufmann", "Bob", "Kaufmann", "[email protected]", List.of("org_gl"));
+        return mockJwtToken(USERNAME, FIRSTNAME, LASTNAME, EMAIL, List.of("org_gl"));
     }
 
     public static Jwt mockJwtToken(String username, String firstname, String lastname, String email) {

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

@@ -1,9 +1,12 @@
 package ch.puzzle.okr.controller;
 
+import ch.puzzle.okr.dto.CompletedDto;
+import ch.puzzle.okr.mapper.CompletedMapper;
 import ch.puzzle.okr.models.Completed;
 import ch.puzzle.okr.models.Objective;
 import ch.puzzle.okr.service.authorization.CompletedAuthorizationService;
 import org.hamcrest.core.Is;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.BDDMockito;
@@ -40,22 +43,34 @@ class CompletedControllerIT {
             """;
     @MockBean
     CompletedAuthorizationService completedAuthorizationService;
+    @MockBean
+    private CompletedMapper completedMapper;
+    private static final String WELL_DONE = "Wir haben es gut geschafft";
+
     Completed successfulCompleted = Completed.Builder.builder().withId(1L)
             .withObjective(Objective.Builder.builder().withId(3L).withTitle("Gute Lernende").build())
-            .withComment("Wir haben es gut geschafft").build();
+            .withComment(WELL_DONE).build();
+    CompletedDto completedDto = new CompletedDto(22L,
+            Objective.Builder.builder().withId(3L).withTitle("Gute Lernende").build(), WELL_DONE);
     String baseUrl = "/api/v2/completed";
     @Autowired
     private MockMvc mvc;
 
+    @BeforeEach
+    void setUp() {
+        BDDMockito.given(completedMapper.toDto(any())).willReturn(completedDto);
+        BDDMockito.given(completedMapper.toCompleted(any())).willReturn(successfulCompleted);
+    }
+
     @Test
     void createSuccessfulCompleted() throws Exception {
         BDDMockito.given(this.completedAuthorizationService.createCompleted(any())).willReturn(successfulCompleted);
 
         mvc.perform(post(baseUrl).content(SUCCESSFUL_CREATE_BODY).contentType(MediaType.APPLICATION_JSON)
                 .with(SecurityMockMvcRequestPostProcessors.csrf()))
-                .andExpect(MockMvcResultMatchers.status().is2xxSuccessful()).andExpect(jsonPath(JSON_PATH_ID, Is.is(1)))
-                .andExpect(jsonPath("$.objective.id", Is.is(3)))
-                .andExpect(jsonPath("$.comment", Is.is("Wir haben es gut geschafft")));
+                .andExpect(MockMvcResultMatchers.status().is2xxSuccessful())
+                .andExpect(jsonPath(JSON_PATH_ID, Is.is(22))).andExpect(jsonPath("$.objective.id", Is.is(3)))
+                .andExpect(jsonPath("$.comment", Is.is(WELL_DONE)));
     }
 
     @Test

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

@@ -38,8 +38,6 @@
 @ExtendWith(MockitoExtension.class)
 @WebMvcTest(ObjectiveController.class)
 class ObjectiveControllerIT {
-
-    private static final AuthorizationUser authorizationUser = defaultAuthorizationUser();
     private static final String OBJECTIVE_TITLE_1 = "Objective 1";
     private static final String OBJECTIVE_TITLE_2 = "Objective 2";
     private static final String DESCRIPTION = "This is our description";

backend/src/test/java/ch/puzzle/okr/converter/JwtOrganisationConverterTest.java

@@ -22,14 +22,14 @@ public void setup() {
     }
 
     @Test
-    void convert_shouldReturnOrganisations_whenValidJwt() {
+    void convertShouldReturnOrganisationsWhenValidJwt() {
         List<String> organisations = converter.convert(jwt);
 
         assertThat(List.of("org_gl")).hasSameElementsAs(organisations);
     }
 
     @Test
-    void convert_shouldReturnEmptyList_whenNoClaimRealmSection() {
+    void convertShouldReturnEmptyListWhenNoClaimRealmSection() {
         setClaimRealm("foo");
 
         List<String> organisations = converter.convert(jwt);
@@ -38,7 +38,7 @@ void convert_shouldReturnEmptyList_whenNoClaimRealmSection() {
     }
 
     @Test
-    void convert_shouldReturnEmptyList_whenNoClaimOrganisationsSection() {
+    void convertShouldReturnEmptyListWhenNoClaimOrganisationsSection() {
         setClaimOrganisations("bar");
 
         List<String> organisations = converter.convert(jwt);
@@ -47,7 +47,7 @@ void convert_shouldReturnEmptyList_whenNoClaimOrganisationsSection() {
     }
 
     @Test
-    void convert_shouldReturnEmptyList_whenNoRoleNameMatch() {
+    void convertShouldReturnEmptyListWhenNoRoleNameMatch() {
         setOrganisationNamePrefix("foo_");
 
         List<String> organisations = converter.convert(jwt);

backend/src/test/java/ch/puzzle/okr/converter/JwtUserConverterTest.java

@@ -25,15 +25,15 @@ public void setup() {
     }
 
     @Test
-    void convert_shouldReturnUser_whenValidJwt() {
+    void convertShouldReturnUserWhenValidJwt() {
         User user = converter.convert(jwt);
 
         assertEquals(User.Builder.builder().withUsername("bkaufmann").withFirstname("Bob").withLastname("Kaufmann")
                 .withEmail("[email protected]").build(), user);
     }
 
     @Test
-    void convert_shouldThrowException_whenClaimNameDoesNotMatch() {
+    void convertShouldThrowExceptionWhenClaimNameDoesNotMatch() {
         setUsername("foo_name");
 
         ResponseStatusException exception = assertThrows(ResponseStatusException.class, () -> converter.convert(jwt));

backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceIT.java

@@ -103,7 +103,7 @@ void tearDown() {
     }
 
     @Test
-    void updateEntity_shouldUpdateKeyResultWithSameTypeMetric() {
+    void updateEntityShouldUpdateKeyResultWithSameTypeMetric() {
         createdKeyResult = keyResultBusinessService.createEntity(createKeyResultMetric(null), authorizationUser);
         createdKeyResult.setTitle(KEY_RESULT_UPDATED);
 
@@ -114,7 +114,7 @@ void updateEntity_shouldUpdateKeyResultWithSameTypeMetric() {
     }
 
     @Test
-    void updateEntity_shouldUpdateKeyResultWithSameTypeOrdinal() {
+    void updateEntityShouldUpdateKeyResultWithSameTypeOrdinal() {
         createdKeyResult = keyResultBusinessService.createEntity(createKeyResultOrdinal(null), authorizationUser);
         createdKeyResult.setTitle(KEY_RESULT_UPDATED);
 

backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java

@@ -327,7 +327,7 @@ void shouldDeleteKeyResultAndAssociatedCheckInsAndActions() {
     }
 
     @Test
-    void shouldReturnImUsedProperly_False1() {
+    void shouldReturnImUsedProperlyFalse1() {
         when(checkInBusinessService.getCheckInsByKeyResultId(any())).thenReturn(Collections.emptyList());
 
         boolean returnValue = this.keyResultBusinessService.isImUsed(1L, this.metricKeyResult);
@@ -336,7 +336,7 @@ void shouldReturnImUsedProperly_False1() {
     }
 
     @Test
-    void shouldReturnImUsedProperly_False2() {
+    void shouldReturnImUsedProperlyFalse2() {
         when(checkInBusinessService.getCheckInsByKeyResultId(any())).thenReturn(Collections.emptyList());
 
         boolean returnValue = this.keyResultBusinessService.isImUsed(1L, this.ordinalKeyResult);
@@ -345,7 +345,7 @@ void shouldReturnImUsedProperly_False2() {
     }
 
     @Test
-    void shouldReturnImUsedProperly_False3() {
+    void shouldReturnImUsedProperlyFalse3() {
         when(keyResultPersistenceService.findById(any())).thenReturn(this.metricKeyResult);
         when(checkInBusinessService.getCheckInsByKeyResultId(any())).thenReturn(checkIns);
 
@@ -355,7 +355,7 @@ void shouldReturnImUsedProperly_False3() {
     }
 
     @Test
-    void shouldReturnImUsedProperly_True1() {
+    void shouldReturnImUsedProperlyTrue1() {
         when(keyResultPersistenceService.findById(any())).thenReturn(this.metricKeyResult);
         when(checkInBusinessService.getCheckInsByKeyResultId(any())).thenReturn(checkIns);
 

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

@@ -13,7 +13,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @SpringIntegrationTest
-public class AlignmentSelectionPersistenceServiceIT {
+class AlignmentSelectionPersistenceServiceIT {
     @Autowired
     private AlignmentSelectionPersistenceService alignmentSelectionPersistenceService;
 

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

@@ -15,8 +15,7 @@
 
 @SpringIntegrationTest
 class AuthorizationCriteriaIT {
-    private final String reason = "not authorized to read objective";
-    private final AuthorizationUser authorizationUser = defaultAuthorizationUser();
+    private static final String REASON = "not authorized to read objective";
 
     @Autowired
     ObjectivePersistenceService objectivePersistenceService;
@@ -27,7 +26,7 @@ class AuthorizationCriteriaIT {
     void appendObjectiveShouldReturnObjectiveWhenFirstLevelRole() {
         Long objectiveId = 5L;
         AuthorizationUser authorizationUser = defaultAuthorizationUser();
-        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, reason);
+        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, REASON);
 
         assertEquals(objectiveId, objective.getId());
     }
@@ -37,7 +36,7 @@ void appendObjectiveShouldReturnObjectiveWhenSecondLevelRole() {
         Long objectiveId = 6L;
         AuthorizationUser authorizationUser = mockAuthorizationUser(defaultUser(null), List.of(), 5L,
                 List.of(READ_ALL_PUBLISHED, READ_TEAMS_DRAFT));
-        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, reason);
+        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, REASON);
 
         assertEquals(objectiveId, objective.getId());
     }
@@ -47,7 +46,7 @@ void appendObjectiveShouldReturnObjectiveWhenMemberRole() {
         Long objectiveId = 6L;
         AuthorizationUser authorizationUser = mockAuthorizationUser(defaultUser(null), List.of(), 5L,
                 List.of(READ_ALL_PUBLISHED, READ_TEAM_DRAFT));
-        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, reason);
+        Objective objective = objectivePersistenceService.findObjectiveById(objectiveId, authorizationUser, REASON);
 
         assertEquals(objectiveId, objective.getId());
     }

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

@@ -28,6 +28,8 @@ private static CheckIn createCheckIn(Long id) {
         return createCheckIn(id, 1);
     }
 
+    private static final String UPDATED_CHECKIN = "Updated CheckIn";
+
     private static CheckIn createCheckIn(Long id, int version) {
         return CheckInMetric.Builder.builder().withValue(30D).withId(id).withVersion(version)
                 .withCreatedBy(User.Builder.builder().withId(1L).withFirstname("Frank").build())
@@ -71,20 +73,20 @@ void saveCheckInShouldSaveNewCheckIn() {
     void updateKeyResultShouldUpdateKeyResult() {
         createdCheckIn = checkInPersistenceService.save(createCheckIn(null));
         CheckIn updateCheckIn = createCheckIn(createdCheckIn.getId(), createdCheckIn.getVersion());
-        updateCheckIn.setChangeInfo("Updated CheckIn");
+        updateCheckIn.setChangeInfo(UPDATED_CHECKIN);
 
         CheckIn updatedCheckIn = checkInPersistenceService.save(updateCheckIn);
 
         assertEquals(createdCheckIn.getId(), updatedCheckIn.getId());
         assertEquals(createdCheckIn.getVersion() + 1, updatedCheckIn.getVersion());
-        assertEquals("Updated CheckIn", updatedCheckIn.getChangeInfo());
+        assertEquals(UPDATED_CHECKIN, updatedCheckIn.getChangeInfo());
     }
 
     @Test
     void updateKeyResultShouldThrowExceptionWhenAlreadyUpdated() {
         createdCheckIn = checkInPersistenceService.save(createCheckIn(null));
         CheckIn updateCheckIn = createCheckIn(createdCheckIn.getId(), 0);
-        updateCheckIn.setChangeInfo("Updated CheckIn");
+        updateCheckIn.setChangeInfo(UPDATED_CHECKIN);
 
         ResponseStatusException exception = assertThrows(ResponseStatusException.class,
                 () -> checkInPersistenceService.save(updateCheckIn));

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

@@ -47,6 +47,7 @@ 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";
 
     @AfterEach
     void tearDown() {
@@ -120,7 +121,7 @@ void recreateEntityShouldUpdateKeyResultNoTypeChange() {
         // Should delete the old KeyResult
         ResponseStatusException exception = assertThrows(ResponseStatusException.class, this::execute);
         assertEquals(HttpStatus.NOT_FOUND, exception.getStatus());
-        assertEquals("KeyResult with id " + createdKeyResult.getId() + " not found", exception.getReason());
+        assertEquals("KeyResult with id " + keyResultId + " not found", exception.getReason());
 
         // delete re-created key result in tearDown()
         createdKeyResult = recreatedKeyResult;
@@ -148,9 +149,9 @@ void recreateEntityShouldUpdateKeyResultWithTypeChange() {
         Long keyResultId = createdKeyResult.getId();
         // Should delete the old KeyResult
         ResponseStatusException exception = assertThrows(ResponseStatusException.class,
-                () -> keyResultPersistenceService.findById(createdKeyResult.getId()));
+                () -> keyResultPersistenceService.findById(keyResultId));
         assertEquals(HttpStatus.NOT_FOUND, exception.getStatus());
-        assertEquals("KeyResult with id " + createdKeyResult.getId() + " not found", exception.getReason());
+        assertEquals("KeyResult with id " + keyResultId + " not found", exception.getReason());
 
         // delete re-created key result in tearDown()
         createdKeyResult = recreatedKeyResult;
@@ -162,14 +163,14 @@ void updateEntityShouldUpdateKeyResult() {
         createdKeyResult = keyResultPersistenceService.save(keyResult);
         KeyResult updateKeyResult = createKeyResultOrdinal(createdKeyResult.getId(), createdKeyResult.getVersion());
         updateKeyResult.setTitle(KEY_RESULT_UPDATED);
-        updateKeyResult.setDescription("This is a new description");
+        updateKeyResult.setDescription(THIS_IS_DESCRIPTION);
 
         KeyResult updatedKeyResult = keyResultPersistenceService.updateEntity(updateKeyResult);
 
         assertEquals(createdKeyResult.getId(), updatedKeyResult.getId());
         assertEquals(createdKeyResult.getVersion() + 1, updatedKeyResult.getVersion());
         assertEquals(KEY_RESULT_UPDATED, updatedKeyResult.getTitle());
-        assertEquals("This is a new description", updatedKeyResult.getDescription());
+        assertEquals(THIS_IS_DESCRIPTION, updatedKeyResult.getDescription());
         assertEquals(createdKeyResult.getOwner().getId(), updatedKeyResult.getOwner().getId());
         assertEquals(createdKeyResult.getObjective().getId(), updatedKeyResult.getObjective().getId());
         assertEquals(createdKeyResult.getModifiedOn(), updatedKeyResult.getModifiedOn());
@@ -181,7 +182,7 @@ void updateEntityShouldThrowExceptionWhenAlreadyUpdated() {
         createdKeyResult = keyResultPersistenceService.save(keyResult);
         KeyResult updateKeyResult = createKeyResultOrdinal(createdKeyResult.getId(), 0);
         updateKeyResult.setTitle(KEY_RESULT_UPDATED);
-        updateKeyResult.setDescription("This is a new description");
+        updateKeyResult.setDescription(THIS_IS_DESCRIPTION);
 
         ResponseStatusException exception = assertThrows(ResponseStatusException.class,
                 () -> keyResultPersistenceService.updateEntity(updateKeyResult));