From 4674ea84a1a96748476f4b43f3a85709b78d694b Mon Sep 17 00:00:00 2001 From: Lias Kleisa Date: Thu, 6 Jun 2024 09:48:03 +0200 Subject: [PATCH] Clean up code --- .../ch/puzzle/okr/mapper/ObjectiveMapper.java | 2 + .../business/AlignmentBusinessService.java | 92 +++++++++++-------- .../business/ObjectiveBusinessService.java | 46 ++++++---- .../AlignmentPersistenceService.java | 9 +- .../KeyResultPersistenceService.java | 9 +- .../AlignmentValidationService.java | 23 ++--- .../okr/controller/ObjectiveControllerIT.java | 21 +++-- .../controller/OrganisationControllerIT.java | 3 +- .../mapper/AlignmentSelectionMapperTest.java | 15 +-- .../puzzle/okr/mapper/OverviewMapperTest.java | 24 ++--- .../ObjectiveAuthorizationServiceTest.java | 5 +- ...AlignmentSelectionBusinessServiceTest.java | 3 +- .../ObjectiveBusinessServiceTest.java | 46 ++++++---- .../AlignmentValidationServiceTest.java | 3 +- .../objective-form.component.html | 8 +- .../objective-form.component.spec.ts | 56 ++++++----- .../objective-form.component.ts | 51 +++++----- 17 files changed, 240 insertions(+), 176 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/mapper/ObjectiveMapper.java b/backend/src/main/java/ch/puzzle/okr/mapper/ObjectiveMapper.java index c72994f56c..74e4208311 100644 --- a/backend/src/main/java/ch/puzzle/okr/mapper/ObjectiveMapper.java +++ b/backend/src/main/java/ch/puzzle/okr/mapper/ObjectiveMapper.java @@ -19,6 +19,8 @@ public ObjectiveMapper(TeamBusinessService teamBusinessService, QuarterBusinessS this.quarterBusinessService = quarterBusinessService; } + // TODO: Adjust Unit Tests of ObjectiveMapper after merge of multitenancy-main + public ObjectiveDto toDto(Objective objective) { return new ObjectiveDto(objective.getId(), objective.getVersion(), objective.getTitle(), objective.getTeam().getId(), objective.getQuarter().getId(), objective.getQuarter().getLabel(), diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java index ef9f954fbc..cf554b9fa7 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/business/AlignmentBusinessService.java @@ -56,36 +56,49 @@ public void updateEntity(Long objectiveId, Objective objective) { Alignment savedAlignment = alignmentPersistenceService.findByAlignedObjectiveId(objectiveId); if (savedAlignment == null) { - Alignment alignment = buildAlignmentModel(objective, 0); - alignmentValidationService.validateOnCreate(alignment); - alignmentPersistenceService.save(alignment); + createEntity(objective); + } else { + handleExistingAlignment(objective, savedAlignment); + } + } + + private void handleExistingAlignment(Objective objective, Alignment savedAlignment) { + if (objective.getAlignedEntityId() == null) { + validateAndDeleteAlignmentById(savedAlignment.getId()); } else { - if (objective.getAlignedEntityId() == null) { - alignmentValidationService.validateOnDelete(savedAlignment.getId()); - alignmentPersistenceService.deleteById(savedAlignment.getId()); - } else { - Alignment alignment = buildAlignmentModel(objective, savedAlignment.getVersion()); - - alignment.setId(savedAlignment.getId()); - alignmentValidationService.validateOnUpdate(savedAlignment.getId(), alignment); - if (isAlignmentTypeChange(alignment, savedAlignment)) { - alignmentPersistenceService.recreateEntity(savedAlignment.getId(), alignment); - } else { - alignmentPersistenceService.save(alignment); - } - } + validateAndUpdateAlignment(objective, savedAlignment); + } + } + + private void validateAndUpdateAlignment(Objective objective, Alignment savedAlignment) { + Alignment alignment = buildAlignmentModel(objective, savedAlignment.getVersion()); + + alignment.setId(savedAlignment.getId()); + alignmentValidationService.validateOnUpdate(savedAlignment.getId(), alignment); + updateAlignment(savedAlignment, alignment); + } + + private void updateAlignment(Alignment savedAlignment, Alignment alignment) { + if (isAlignmentTypeChange(alignment, savedAlignment)) { + alignmentPersistenceService.recreateEntity(savedAlignment.getId(), alignment); + } else { + alignmentPersistenceService.save(alignment); } } public Alignment buildAlignmentModel(Objective alignedObjective, int version) { if (alignedObjective.getAlignedEntityId().startsWith("O")) { - Objective targetObjective = objectivePersistenceService - .findById(Long.valueOf(alignedObjective.getAlignedEntityId().replace("O", ""))); - return ObjectiveAlignment.Builder.builder().withAlignedObjective(alignedObjective) - .withTargetObjective(targetObjective).withVersion(version).build(); + Long entityId = Long.valueOf(alignedObjective.getAlignedEntityId().replace("O", "")); + + Objective targetObjective = objectivePersistenceService.findById(entityId); + return ObjectiveAlignment.Builder.builder() // + .withAlignedObjective(alignedObjective) // + .withTargetObjective(targetObjective) // + .withVersion(version).build(); } else if (alignedObjective.getAlignedEntityId().startsWith("K")) { - KeyResult targetKeyResult = keyResultPersistenceService - .findById(Long.valueOf(alignedObjective.getAlignedEntityId().replace("K", ""))); + Long entityId = Long.valueOf(alignedObjective.getAlignedEntityId().replace("K", "")); + + KeyResult targetKeyResult = keyResultPersistenceService.findById(entityId); return KeyResultAlignment.Builder.builder().withAlignedObjective(alignedObjective) .withTargetKeyResult(targetKeyResult).withVersion(version).build(); } else { @@ -99,9 +112,10 @@ public boolean isAlignmentTypeChange(Alignment alignment, Alignment savedAlignme || (alignment instanceof KeyResultAlignment && savedAlignment instanceof ObjectiveAlignment); } - public void updateKeyResultIdOnIdChange(Long oldId, KeyResult keyResult) { - List alignments = alignmentPersistenceService.findByKeyResultAlignmentId(oldId); - alignments.forEach(alignment -> { + public void updateKeyResultIdOnIdChange(Long oldKeyResultId, KeyResult keyResult) { + List keyResultAlignmentList = alignmentPersistenceService + .findByKeyResultAlignmentId(oldKeyResultId); + keyResultAlignmentList.forEach(alignment -> { alignment.setAlignmentTarget(keyResult); alignmentValidationService.validateOnUpdate(alignment.getId(), alignment); alignmentPersistenceService.save(alignment); @@ -111,21 +125,23 @@ public void updateKeyResultIdOnIdChange(Long oldId, KeyResult keyResult) { public void deleteAlignmentByObjectiveId(Long objectiveId) { Alignment alignment = alignmentPersistenceService.findByAlignedObjectiveId(objectiveId); if (alignment != null) { - alignmentValidationService.validateOnDelete(alignment.getId()); - alignmentPersistenceService.deleteById(alignment.getId()); + validateAndDeleteAlignmentById(alignment.getId()); } - List alignmentList = alignmentPersistenceService.findByObjectiveAlignmentId(objectiveId); - alignmentList.forEach(objectiveAlignment -> { - alignmentValidationService.validateOnDelete(objectiveAlignment.getId()); - alignmentPersistenceService.deleteById(objectiveAlignment.getId()); - }); + List objectiveAlignmentList = alignmentPersistenceService + .findByObjectiveAlignmentId(objectiveId); + objectiveAlignmentList + .forEach(objectiveAlignment -> validateAndDeleteAlignmentById(objectiveAlignment.getId())); } public void deleteAlignmentByKeyResultId(Long keyResultId) { - List alignmentList = alignmentPersistenceService.findByKeyResultAlignmentId(keyResultId); - alignmentList.forEach(keyResultAlignment -> { - alignmentValidationService.validateOnDelete(keyResultAlignment.getId()); - alignmentPersistenceService.deleteById(keyResultAlignment.getId()); - }); + List keyResultAlignmentList = alignmentPersistenceService + .findByKeyResultAlignmentId(keyResultId); + keyResultAlignmentList + .forEach(keyResultAlignment -> validateAndDeleteAlignmentById(keyResultAlignment.getId())); + } + + private void validateAndDeleteAlignmentById(Long alignmentId) { + alignmentValidationService.validateOnDelete(alignmentId); + alignmentPersistenceService.deleteById(alignmentId); } } diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java index 2fc9c40a54..4244d5eb99 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java @@ -57,37 +57,45 @@ public List getAlignmentPossibilities(Long quarterId) { List objectivesByQuarter = objectivePersistenceService.findObjectiveByQuarterId(quarterId); List alignmentDtoList = new ArrayList<>(); - List teamList = objectivesByQuarter.stream().map(Objective::getTeam).distinct() - .sorted(Comparator.comparing(Team::getName)).toList(); + List teamList = objectivesByQuarter.stream() // + .map(Objective::getTeam) // + .distinct() // + .sorted(Comparator.comparing(Team::getName)) // + .toList(); teamList.forEach(team -> { List filteredObjectiveList = objectivesByQuarter.stream() - .filter(objective -> objective.getTeam().equals(team)).toList().stream() + .filter(objective -> objective.getTeam().equals(team)) .sorted(Comparator.comparing(Objective::getTitle)).toList(); - List alignmentObjectDtos = new ArrayList<>(); + List alignmentObjectDtoList = generateAlignmentObjects(filteredObjectiveList); - filteredObjectiveList.forEach(objective -> { - AlignmentObjectDto objectiveDto = new AlignmentObjectDto(objective.getId(), - "O - " + objective.getTitle(), "objective"); - alignmentObjectDtos.add(objectiveDto); - - List keyResults = keyResultBusinessService.getAllKeyResultsByObjective(objective.getId()) - .stream().sorted(Comparator.comparing(KeyResult::getTitle)).toList(); - - keyResults.forEach(keyResult -> { - AlignmentObjectDto keyResultDto = new AlignmentObjectDto(keyResult.getId(), - "KR - " + keyResult.getTitle(), "keyResult"); - alignmentObjectDtos.add(keyResultDto); - }); - }); - AlignmentDto alignmentDto = new AlignmentDto(team.getId(), team.getName(), alignmentObjectDtos); + AlignmentDto alignmentDto = new AlignmentDto(team.getId(), team.getName(), alignmentObjectDtoList); alignmentDtoList.add(alignmentDto); }); return alignmentDtoList; } + private List generateAlignmentObjects(List filteredObjectiveList) { + List alignmentObjectDtoList = new ArrayList<>(); + filteredObjectiveList.forEach(objective -> { + AlignmentObjectDto objectiveDto = new AlignmentObjectDto(objective.getId(), "O - " + objective.getTitle(), + "objective"); + alignmentObjectDtoList.add(objectiveDto); + + List keyResultList = keyResultBusinessService.getAllKeyResultsByObjective(objective.getId()) + .stream().sorted(Comparator.comparing(KeyResult::getTitle)).toList(); + + keyResultList.forEach(keyResult -> { + AlignmentObjectDto keyResultDto = new AlignmentObjectDto(keyResult.getId(), + "KR - " + keyResult.getTitle(), "keyResult"); + alignmentObjectDtoList.add(keyResultDto); + }); + }); + return alignmentObjectDtoList; + } + public List getEntitiesByTeamId(Long id) { validator.validateOnGet(id); diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/AlignmentPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/AlignmentPersistenceService.java index 8a25281006..dc69d6b1fc 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/AlignmentPersistenceService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/persistence/AlignmentPersistenceService.java @@ -5,6 +5,8 @@ import ch.puzzle.okr.models.alignment.ObjectiveAlignment; import ch.puzzle.okr.repository.AlignmentRepository; import jakarta.transaction.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import java.util.List; @@ -13,6 +15,7 @@ @Service public class AlignmentPersistenceService extends PersistenceBase { + private static final Logger logger = LoggerFactory.getLogger(AlignmentPersistenceService.class); protected AlignmentPersistenceService(AlignmentRepository repository) { super(repository); @@ -25,11 +28,11 @@ public String getModelName() { @Transactional public Alignment recreateEntity(Long id, Alignment alignment) { - System.out.println(alignment.toString()); - System.out.println("*".repeat(30)); + logger.debug("Delete and create new Alignment in order to prevent duplicates in case of changed Type"); + logger.debug("{}", alignment); // delete entity in order to prevent duplicates in case of changed keyResultType deleteById(id); - System.out.printf("reached delete entity with %d", id); + logger.debug("Reached delete entity with id {}", id); return save(alignment); } diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java index 14e223bff5..d8a4da87a4 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/persistence/KeyResultPersistenceService.java @@ -3,6 +3,8 @@ import ch.puzzle.okr.models.keyresult.KeyResult; import ch.puzzle.okr.repository.KeyResultRepository; import jakarta.transaction.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; import java.util.List; @@ -11,6 +13,7 @@ @Service public class KeyResultPersistenceService extends PersistenceBase { + private static final Logger logger = LoggerFactory.getLogger(KeyResultPersistenceService.class); protected KeyResultPersistenceService(KeyResultRepository repository) { super(repository); @@ -27,11 +30,11 @@ public List getKeyResultsByObjective(Long objectiveId) { @Transactional public KeyResult recreateEntity(Long id, KeyResult keyResult) { - System.out.println(keyResult.toString()); - System.out.println("*".repeat(30)); + logger.debug("Delete KeyResult in order to prevent duplicates in case of changed keyResultType"); + logger.debug("{}", keyResult); // delete entity in order to prevent duplicates in case of changed keyResultType deleteById(id); - System.out.printf("reached delete entity with %d", id); + logger.debug("Reached delete entity with id {}", id); return save(keyResult); } diff --git a/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java b/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java index 8a656cd679..40bbba8ee5 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/validation/AlignmentValidationService.java @@ -33,10 +33,10 @@ public AlignmentValidationService(AlignmentPersistenceService alignmentPersisten public void validateOnCreate(Alignment model) { throwExceptionWhenModelIsNull(model); throwExceptionWhenIdIsNotNull(model.getId()); - throwExceptionWhenAlignedObjectIsNull(model); + throwExceptionWhenAlignmentObjectIsNull(model); throwExceptionWhenAlignedIdIsSameAsTargetId(model); throwExceptionWhenAlignmentIsInSameTeam(model); - throwExceptionWhenAlignedObjectiveAlreadyExists(model); + throwExceptionWhenAlignmentWithAlignedObjectiveAlreadyExists(model); validate(model); } @@ -44,13 +44,13 @@ public void validateOnCreate(Alignment model) { public void validateOnUpdate(Long id, Alignment model) { throwExceptionWhenModelIsNull(model); throwExceptionWhenIdIsNull(model.getId()); - throwExceptionWhenAlignedObjectIsNull(model); + throwExceptionWhenAlignmentObjectIsNull(model); throwExceptionWhenAlignedIdIsSameAsTargetId(model); throwExceptionWhenAlignmentIsInSameTeam(model); validate(model); } - private void throwExceptionWhenAlignedObjectIsNull(Alignment model) { + private void throwExceptionWhenAlignmentObjectIsNull(Alignment model) { if (model.getAlignedObjective() == null) { throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_NULL, List.of("alignedObjectiveId")); @@ -68,19 +68,20 @@ private void throwExceptionWhenAlignedObjectIsNull(Alignment model) { } private void throwExceptionWhenAlignmentIsInSameTeam(Alignment model) { - Team alignedTeam = teamPersistenceService.findById(model.getAlignedObjective().getTeam().getId()); - Team targetTeam = null; + Team alignedObjectiveTeam = teamPersistenceService.findById(model.getAlignedObjective().getTeam().getId()); + Team targetObjectTeam = null; if (model instanceof ObjectiveAlignment objectiveAlignment) { - targetTeam = teamPersistenceService.findById(objectiveAlignment.getAlignmentTarget().getTeam().getId()); + targetObjectTeam = teamPersistenceService + .findById(objectiveAlignment.getAlignmentTarget().getTeam().getId()); } else if (model instanceof KeyResultAlignment keyResultAlignment) { - targetTeam = teamPersistenceService + targetObjectTeam = teamPersistenceService .findById(keyResultAlignment.getAlignmentTarget().getObjective().getTeam().getId()); } - if (alignedTeam.equals(targetTeam)) { + if (alignedObjectiveTeam.equals(targetObjectTeam)) { throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.NOT_LINK_IN_SAME_TEAM, - List.of("teamId", targetTeam.getId())); + List.of("teamId", targetObjectTeam.getId())); } } @@ -94,7 +95,7 @@ private void throwExceptionWhenAlignedIdIsSameAsTargetId(Alignment model) { } } - private void throwExceptionWhenAlignedObjectiveAlreadyExists(Alignment model) { + private void throwExceptionWhenAlignmentWithAlignedObjectiveAlreadyExists(Alignment model) { if (this.alignmentPersistenceService.findByAlignedObjectiveId(model.getAlignedObjective().getId()) != null) { throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ALIGNMENT_ALREADY_EXISTS, List.of("alignedObjectiveId", model.getAlignedObjective().getId())); diff --git a/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java b/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java index f13c1fe4e4..2143d06632 100644 --- a/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java +++ b/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java @@ -33,6 +33,7 @@ import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static ch.puzzle.okr.TestConstants.*; @WithMockUser(value = "spring") @ExtendWith(MockitoExtension.class) @@ -44,8 +45,8 @@ class ObjectiveControllerIT { private static final String EVERYTHING_FINE_DESCRIPTION = "Everything Fine"; private static final String TITLE = "Hunting"; private static final String URL_BASE_OBJECTIVE = "/api/v2/objectives"; - private static final String URL_OBJECTIVE_5 = "/api/v2/objectives/5"; - private static final String URL_OBJECTIVE_10 = "/api/v2/objectives/10"; + private static final String URL_OBJECTIVE_5 = URL_BASE_OBJECTIVE + "/5"; + private static final String URL_OBJECTIVE_10 = URL_BASE_OBJECTIVE + "/10"; private static final String JSON = """ { "title": "FullObjective", "ownerId": 1, "ownerFirstname": "Bob", "ownerLastname": "Kaufmann", @@ -66,7 +67,7 @@ class ObjectiveControllerIT { "description": "This is our description", "progress": 33.3 } """; - private static final String RESPONSE_NEW_OBJECTIVE = """ + private static final String JSON_RESPONSE_NEW_OBJECTIVE = """ {"id":null,"version":1,"title":"Program Faster","teamId":1,"quarterId":1,"quarterLabel":"GJ 22/23-Q2","description":"Just be faster","state":"DRAFT","createdOn":null,"modifiedOn":null,"writeable":true,"alignedEntityId":null}"""; private static final String JSON_PATH_TITLE = "$.title"; private static final Objective objective1 = Objective.Builder.builder().withId(5L).withTitle(OBJECTIVE_TITLE_1) @@ -91,7 +92,7 @@ class ObjectiveControllerIT { private static final AlignmentObjectDto alignmentObject1 = new AlignmentObjectDto(3L, "KR Title 1", "keyResult"); private static final AlignmentObjectDto alignmentObject2 = new AlignmentObjectDto(1L, "Objective Title 1", "objective"); - private static final AlignmentDto alignmentPossibilities = new AlignmentDto(1L, "Puzzle ITC", + private static final AlignmentDto alignmentPossibilities = new AlignmentDto(1L, TEAM_PUZZLE, List.of(alignmentObject1, alignmentObject2)); @Autowired @@ -123,7 +124,7 @@ void getObjectiveById() throws Exception { void getObjectiveByIdWithAlignmentId() throws Exception { BDDMockito.given(objectiveAuthorizationService.getEntityById(anyLong())).willReturn(objectiveAlignment); - mvc.perform(get("/api/v2/objectives/9").contentType(MediaType.APPLICATION_JSON)) + mvc.perform(get(URL_BASE_OBJECTIVE + "/9").contentType(MediaType.APPLICATION_JSON)) .andExpect(MockMvcResultMatchers.status().isOk()).andExpect(jsonPath("$.id", Is.is(9))) .andExpect(jsonPath(JSON_PATH_TITLE, Is.is("Objective Alignment"))) .andExpect(jsonPath("$.alignedEntityId", Is.is("O42"))); @@ -143,9 +144,9 @@ void getAlignmentPossibilities() throws Exception { BDDMockito.given(objectiveAuthorizationService.getAlignmentPossibilities(anyLong())) .willReturn(List.of(alignmentPossibilities)); - mvc.perform(get("/api/v2/objectives/alignmentPossibilities/5").contentType(MediaType.APPLICATION_JSON)) + mvc.perform(get(URL_BASE_OBJECTIVE + "/alignmentPossibilities/5").contentType(MediaType.APPLICATION_JSON)) .andExpect(MockMvcResultMatchers.status().isOk()).andExpect(jsonPath("$[0].teamId", Is.is(1))) - .andExpect(jsonPath("$[0].teamName", Is.is("Puzzle ITC"))) + .andExpect(jsonPath("$[0].teamName", Is.is(TEAM_PUZZLE))) .andExpect(jsonPath("$[0].alignmentObjectDtos[0].objectId", Is.is(3))) .andExpect(jsonPath("$[0].alignmentObjectDtos[0].objectTitle", Is.is("KR Title 1"))) .andExpect(jsonPath("$[0].alignmentObjectDtos[0].objectType", Is.is("keyResult"))) @@ -167,7 +168,7 @@ void shouldReturnObjectiveWhenCreatingNewObjective() throws Exception { mvc.perform(post(URL_BASE_OBJECTIVE).contentType(MediaType.APPLICATION_JSON) .with(SecurityMockMvcRequestPostProcessors.csrf()).content(CREATE_NEW_OBJECTIVE)) .andExpect(MockMvcResultMatchers.status().is2xxSuccessful()) - .andExpect(MockMvcResultMatchers.content().string(RESPONSE_NEW_OBJECTIVE)); + .andExpect(MockMvcResultMatchers.content().string(JSON_RESPONSE_NEW_OBJECTIVE)); verify(objectiveAuthorizationService, times(1)).createEntity(any()); } @@ -247,7 +248,7 @@ void throwExceptionWhenObjectiveWithIdCantBeFoundWhileDeleting() throws Exceptio doThrow(new ResponseStatusException(HttpStatus.NOT_FOUND, "Objective not found")) .when(objectiveAuthorizationService).deleteEntityById(anyLong()); - mvc.perform(delete("/api/v2/objectives/1000").with(SecurityMockMvcRequestPostProcessors.csrf())) + mvc.perform(delete(URL_BASE_OBJECTIVE + "/1000").with(SecurityMockMvcRequestPostProcessors.csrf())) .andExpect(MockMvcResultMatchers.status().isNotFound()); } @@ -257,7 +258,7 @@ void shouldReturnIsCreatedWhenObjectiveWasDuplicated() throws Exception { BDDMockito.given(objectiveAuthorizationService.getAuthorizationService()).willReturn(authorizationService); BDDMockito.given(objectiveMapper.toDto(objective1)).willReturn(objective1Dto); - mvc.perform(post("/api/v2/objectives/{id}", objective1.getId()).contentType(MediaType.APPLICATION_JSON) + mvc.perform(post(URL_BASE_OBJECTIVE + "/{id}", objective1.getId()).contentType(MediaType.APPLICATION_JSON) .content(JSON).with(SecurityMockMvcRequestPostProcessors.csrf())) .andExpect(MockMvcResultMatchers.status().isCreated()) .andExpect(jsonPath("$.id", Is.is(objective1Dto.id().intValue()))) diff --git a/backend/src/test/java/ch/puzzle/okr/controller/OrganisationControllerIT.java b/backend/src/test/java/ch/puzzle/okr/controller/OrganisationControllerIT.java index 055dcec6dc..d6b663fe39 100644 --- a/backend/src/test/java/ch/puzzle/okr/controller/OrganisationControllerIT.java +++ b/backend/src/test/java/ch/puzzle/okr/controller/OrganisationControllerIT.java @@ -24,6 +24,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static ch.puzzle.okr.TestConstants.*; @WithMockUser(value = "spring") @ExtendWith(MockitoExtension.class) @@ -31,7 +32,7 @@ class OrganisationControllerIT { /* Team test objects */ - private static final Team PUZZLE = Team.Builder.builder().withId(1L).withName("PUZZLE ITC").build(); + private static final Team PUZZLE = Team.Builder.builder().withId(1L).withName(TEAM_PUZZLE).build(); private static final Team BBT = Team.Builder.builder().withId(1L).withName("/BBT").build(); /* Organisation test objects */ diff --git a/backend/src/test/java/ch/puzzle/okr/mapper/AlignmentSelectionMapperTest.java b/backend/src/test/java/ch/puzzle/okr/mapper/AlignmentSelectionMapperTest.java index 26b799ff15..8a7f6fb366 100644 --- a/backend/src/test/java/ch/puzzle/okr/mapper/AlignmentSelectionMapperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/mapper/AlignmentSelectionMapperTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static ch.puzzle.okr.TestConstants.*; class AlignmentSelectionMapperTest { private final AlignmentSelectionMapper alignmentSelectionMapper = new AlignmentSelectionMapper(); @@ -25,7 +26,7 @@ void toDtoShouldReturnEmptyListWhenNoObjectiveFound() { void toDtoShouldReturnOneElementWhenObjectiveFound() { List alignmentSelections = List.of(AlignmentSelection.Builder.builder() .withAlignmentSelectionId(AlignmentSelectionId.Builder.builder().withObjectiveId(1L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").build()); + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").build()); List alignmentObjectiveDtos = alignmentSelectionMapper.toDto(alignmentSelections); assertEquals(1, alignmentObjectiveDtos.size()); @@ -37,7 +38,7 @@ void toDtoShouldReturnOneElementWhenObjectiveWithKeyResultFound() { List alignmentSelections = List.of(AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(1L).withKeyResultId(3L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1") .withKeyResultTitle("Key Result 3").build()); List alignmentObjectiveDtos = alignmentSelectionMapper.toDto(alignmentSelections); @@ -51,12 +52,12 @@ void toDtoShouldReturnOneElementWhenObjectiveWithTwoKeyResultsFound() { AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(1L).withKeyResultId(3L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1") .withKeyResultTitle("Key Result 3").build(), AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(1L).withKeyResultId(5L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1") .withKeyResultTitle("Key Result 5").build()); List alignmentObjectiveDtos = alignmentSelectionMapper.toDto(alignmentSelections); @@ -70,17 +71,17 @@ void toDtoShouldReturnOneElementWhenTwoObjectivesWithKeyResultsFound() { AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(1L).withKeyResultId(3L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1") .withKeyResultTitle("Key Result 3").build(), AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(5L).withKeyResultId(6L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 5") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 5") .withKeyResultTitle("Key Result 6").build(), AlignmentSelection.Builder.builder() .withAlignmentSelectionId( AlignmentSelectionId.Builder.builder().withObjectiveId(1L).withKeyResultId(9L).build()) - .withTeamId(2L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1") + .withTeamId(2L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1") .withKeyResultTitle("Key Result 9").build()); List alignmentObjectiveDtos = alignmentSelectionMapper.toDto(alignmentSelections); diff --git a/backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java b/backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java index 5e4c51ad9b..e17fe68a1d 100644 --- a/backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java +++ b/backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static ch.puzzle.okr.TestConstants.*; @ExtendWith(MockitoExtension.class) class OverviewMapperTest { @@ -42,9 +43,8 @@ void toDtoShouldReturnEmptyListWhenNoTeamFound() { @Test void toDtoShouldReturnEmptyListWhenTeamFound() { - List overviews = List - .of(Overview.Builder.builder().withOverviewId(OverviewId.Builder.builder().withTeamId(2L).build()) - .withTeamName("Puzzle ITC").build()); + List overviews = List.of(Overview.Builder.builder() + .withOverviewId(OverviewId.Builder.builder().withTeamId(2L).build()).withTeamName(TEAM_PUZZLE).build()); List overviewDtos = overviewMapper.toDto(overviews); assertEquals(1, overviewDtos.size()); @@ -55,7 +55,7 @@ void toDtoShouldReturnEmptyListWhenTeamFound() { void toDtoShouldReturnOneElementWhenObjectiveFound() { List overviews = List.of(Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").build()); + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").build()); List overviewDtos = overviewMapper.toDto(overviews); assertEquals(1, overviewDtos.size()); @@ -68,7 +68,7 @@ void toDtoShouldReturnOneElementWhenObjectiveWithKeyResultFound() { List overviews = List.of(Overview.Builder.builder() .withOverviewId( OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L).withKeyResultId(3L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType(KEY_RESULT_TYPE_METRIC).build()); List overviewDtos = overviewMapper.toDto(overviews); @@ -82,7 +82,7 @@ void toDtoShouldReturnOneElementWhenObjectiveWithKeyResultAndCheckInsFound() { List overviews = List.of(Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L).withKeyResultId(3L) .withCheckInId(4L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType(KEY_RESULT_TYPE_METRIC).withCheckInValue(27.5).withConfidence(5).build()); List overviewDtos = overviewMapper.toDto(overviews); @@ -97,12 +97,12 @@ void toDtoShouldReturnOneElementWhenObjectiveWithTwoKeyResultAndCheckInFound() { Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L) .withKeyResultId(3L).withCheckInId(4L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType(KEY_RESULT_TYPE_ORDINAL).withCheckInZone("COMMIT").build(), Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L) .withKeyResultId(5L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 5") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 5") .withKeyResultType(KEY_RESULT_TYPE_METRIC).build()); List overviewDtos = overviewMapper.toDto(overviews); @@ -117,13 +117,13 @@ void toDtoShouldReturnOneElementWhenTwoObjectivesWithKeyResultAndCheckInFound() Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L) .withKeyResultId(3L).withCheckInId(4L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType(KEY_RESULT_TYPE_METRIC).withBaseline(20.0).withStretchGoal(37.0) .withUnit("TCHF").withCheckInValue(27.5).build(), Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(5L).withTeamId(2L) .withKeyResultId(6L).withCheckInId(7L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 5").withKeyResultTitle("Key Result 6") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 5").withKeyResultTitle("Key Result 6") .withKeyResultType(KEY_RESULT_TYPE_ORDINAL).withCommitZone("commit").withTargetZone("target") .withStretchZone("stretch").withCheckInZone("checkIn").build()); List overviewDtos = overviewMapper.toDto(overviews); @@ -155,7 +155,7 @@ void toDtoShouldReturnOneElementWhenTwoTeamsWithObjectivesAndKeyResultsFound() { Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L) .withKeyResultId(3L).withCheckInId(4L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType(KEY_RESULT_TYPE_ORDINAL).withCheckInZone("TARGET").build(), Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(5L).withTeamId(4L) @@ -181,7 +181,7 @@ void toDtoShouldThrowExceptionWhenKeyResultTypeNotSupported() { List overviews = List.of(Overview.Builder.builder() .withOverviewId(OverviewId.Builder.builder().withObjectiveId(1L).withTeamId(2L).withKeyResultId(3L) .withCheckInId(4L).build()) - .withTeamName("Puzzle ITC").withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") + .withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 1").withKeyResultTitle("Key Result 1") .withKeyResultType("unknown").withCheckInZone("TARGET").build()); OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class, diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java index fde8da6881..f6bd954aff 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java @@ -21,6 +21,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.when; import static org.springframework.http.HttpStatus.UNAUTHORIZED; +import static ch.puzzle.okr.TestConstants.*; @ExtendWith(MockitoExtension.class) class ObjectiveAuthorizationServiceTest { @@ -36,7 +37,7 @@ class ObjectiveAuthorizationServiceTest { private static final AlignmentObjectDto alignmentObject1 = new AlignmentObjectDto(3L, "KR Title 1", "keyResult"); private static final AlignmentObjectDto alignmentObject2 = new AlignmentObjectDto(1L, "Objective Title 1", "objective"); - private static final AlignmentDto alignmentPossibilities = new AlignmentDto(1L, "Puzzle ITC", + private static final AlignmentDto alignmentPossibilities = new AlignmentDto(1L, TEAM_PUZZLE, List.of(alignmentObject1, alignmentObject2)); @Test @@ -154,7 +155,7 @@ void getAlignmentPossibilitiesShouldReturnListWhenAuthorized() { when(objectiveBusinessService.getAlignmentPossibilities(anyLong())).thenReturn(List.of(alignmentPossibilities)); List alignmentPossibilities = objectiveAuthorizationService.getAlignmentPossibilities(3L); - assertEquals("Puzzle ITC", alignmentPossibilities.get(0).teamName()); + assertEquals(TEAM_PUZZLE, alignmentPossibilities.get(0).teamName()); assertEquals(1, alignmentPossibilities.get(0).teamId()); assertEquals(3, alignmentPossibilities.get(0).alignmentObjectDtos().get(0).objectId()); assertEquals("KR Title 1", alignmentPossibilities.get(0).alignmentObjectDtos().get(0).objectTitle()); diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentSelectionBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentSelectionBusinessServiceTest.java index 961b1e12cb..f0071bfe61 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentSelectionBusinessServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/business/AlignmentSelectionBusinessServiceTest.java @@ -13,6 +13,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.*; +import static ch.puzzle.okr.TestConstants.*; @ExtendWith(MockitoExtension.class) class AlignmentSelectionBusinessServiceTest { @@ -24,7 +25,7 @@ class AlignmentSelectionBusinessServiceTest { private static AlignmentSelection createAlignmentSelection() { return AlignmentSelection.Builder.builder().withAlignmentSelectionId(AlignmentSelectionId.of(9L, 15L)) - .withTeamId(5L).withTeamName("Puzzle ITC").withObjectiveTitle("Objective 9").withQuarterId(2L) + .withTeamId(5L).withTeamName(TEAM_PUZZLE).withObjectiveTitle("Objective 9").withQuarterId(2L) .withQuarterLabel("GJ 23/24-Q1").withKeyResultTitle("Key Result 15").build(); } diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java index 08d0ceb947..d7a87a4ed8 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java @@ -31,6 +31,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; import static org.springframework.http.HttpStatus.NOT_FOUND; +import static ch.puzzle.okr.TestConstants.*; @ExtendWith(MockitoExtension.class) class ObjectiveBusinessServiceTest { @@ -49,7 +50,12 @@ class ObjectiveBusinessServiceTest { @Mock ObjectiveValidationService validator = Mockito.mock(ObjectiveValidationService.class); - private final Team team1 = Team.Builder.builder().withId(1L).withName("Team1").build(); + private static final String TEAM_1 = "Team1"; + private static final String OBJECTIVE = "objective"; + private static final String FULL_OBJECTIVE_1 = "O - FullObjective1"; + private static final String FULL_OBJECTIVE_2 = "O - FullObjective2"; + + private final Team team1 = Team.Builder.builder().withId(1L).withName(TEAM_1).build(); private final Quarter quarter = Quarter.Builder.builder().withId(1L).withLabel("GJ 22/23-Q2").build(); private final User user = User.Builder.builder().withId(1L).withFirstname("Bob").withLastname("Kaufmann") .withUsername("bkaufmann").withEmail("kaufmann@puzzle.ch").build(); @@ -63,7 +69,7 @@ class ObjectiveBusinessServiceTest { private final Objective fullObjective2 = Objective.Builder.builder().withId(2L).withTitle("FullObjective2") .withCreatedBy(user).withTeam(team1).withQuarter(quarter).withDescription("This is our description") .withModifiedOn(LocalDateTime.MAX).build(); - private final Team team2 = Team.Builder.builder().withId(3L).withName("Puzzle ITC").build(); + private final Team team2 = Team.Builder.builder().withId(3L).withName(TEAM_PUZZLE).build(); private final Objective fullObjective3 = Objective.Builder.builder().withId(3L).withTitle("FullObjective5") .withCreatedBy(user).withTeam(team2).withQuarter(quarter).withDescription("This is our description") .withModifiedOn(LocalDateTime.MAX).build(); @@ -212,14 +218,16 @@ void shouldUpdateAnObjectiveWithAlignmentDelete() { @Test void shouldSaveANewObjectiveWithAlignment() { + // arrange Objective objective = spy(Objective.Builder.builder().withTitle("Received Objective").withTeam(team1) .withQuarter(quarter).withDescription("The description").withModifiedOn(null).withModifiedBy(null) .withState(DRAFT).withAlignedEntityId("O42").build()); - doNothing().when(objective).setCreatedOn(any()); + // act Objective savedObjective = objectiveBusinessService.createEntity(objective, authorizationUser); + // assert verify(objectivePersistenceService, times(1)).save(objective); verify(alignmentBusinessService, times(1)).createEntity(savedObjective); assertEquals(DRAFT, objective.getState()); @@ -229,11 +237,15 @@ void shouldSaveANewObjectiveWithAlignment() { @Test void shouldNotThrowResponseStatusExceptionWhenPuttingNullId() { + // arrange Objective objective1 = Objective.Builder.builder().withId(null).withTitle("Title") .withDescription("Description").withModifiedOn(LocalDateTime.now()).build(); when(objectiveBusinessService.createEntity(objective1, authorizationUser)).thenReturn(fullObjectiveCreate); + // act Objective savedObjective = objectiveBusinessService.createEntity(objective1, authorizationUser); + + // assert assertNull(savedObjective.getId()); assertEquals("FullObjective1", savedObjective.getTitle()); assertEquals("Bob", savedObjective.getCreatedBy().getFirstname()); @@ -309,12 +321,12 @@ void shouldReturnAlignmentPossibilities() { verify(objectivePersistenceService, times(1)).findObjectiveByQuarterId(5L); verify(keyResultBusinessService, times(1)).getAllKeyResultsByObjective(1L); verify(keyResultBusinessService, times(1)).getAllKeyResultsByObjective(2L); - assertEquals("Team1", alignmentsDtos.get(0).teamName()); + assertEquals(TEAM_1, alignmentsDtos.get(0).teamName()); assertEquals(1, alignmentsDtos.get(0).teamId()); assertEquals(6, alignmentsDtos.get(0).alignmentObjectDtos().size()); assertEquals(1, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectId()); - assertEquals("O - FullObjective1", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectTitle()); - assertEquals("objective", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); + assertEquals(FULL_OBJECTIVE_1, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectTitle()); + assertEquals(OBJECTIVE, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); assertEquals(5, alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectId()); assertEquals("KR - Keyresult Ordinal", alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectTitle()); assertEquals("keyResult", alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectType()); @@ -322,8 +334,8 @@ void shouldReturnAlignmentPossibilities() { assertEquals("KR - Keyresult Ordinal", alignmentsDtos.get(0).alignmentObjectDtos().get(2).objectTitle()); assertEquals("keyResult", alignmentsDtos.get(0).alignmentObjectDtos().get(2).objectType()); assertEquals(2, alignmentsDtos.get(0).alignmentObjectDtos().get(3).objectId()); - assertEquals("O - FullObjective2", alignmentsDtos.get(0).alignmentObjectDtos().get(3).objectTitle()); - assertEquals("objective", alignmentsDtos.get(0).alignmentObjectDtos().get(3).objectType()); + assertEquals(FULL_OBJECTIVE_2, alignmentsDtos.get(0).alignmentObjectDtos().get(3).objectTitle()); + assertEquals(OBJECTIVE, alignmentsDtos.get(0).alignmentObjectDtos().get(3).objectType()); assertEquals(5, alignmentsDtos.get(0).alignmentObjectDtos().get(4).objectId()); assertEquals("KR - Keyresult Ordinal", alignmentsDtos.get(0).alignmentObjectDtos().get(4).objectTitle()); assertEquals("keyResult", alignmentsDtos.get(0).alignmentObjectDtos().get(4).objectType()); @@ -347,18 +359,18 @@ void shouldReturnAlignmentPossibilitiesWithMultipleTeams() { verify(keyResultBusinessService, times(1)).getAllKeyResultsByObjective(2L); verify(keyResultBusinessService, times(1)).getAllKeyResultsByObjective(3L); assertEquals(2, alignmentsDtos.size()); - assertEquals("Puzzle ITC", alignmentsDtos.get(0).teamName()); + assertEquals(TEAM_PUZZLE, alignmentsDtos.get(0).teamName()); assertEquals(3, alignmentsDtos.get(0).teamId()); assertEquals(3, alignmentsDtos.get(0).alignmentObjectDtos().size()); - assertEquals("Team1", alignmentsDtos.get(1).teamName()); + assertEquals(TEAM_1, alignmentsDtos.get(1).teamName()); assertEquals(1, alignmentsDtos.get(1).teamId()); assertEquals(6, alignmentsDtos.get(1).alignmentObjectDtos().size()); assertEquals(3, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectId()); assertEquals("O - FullObjective5", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectTitle()); - assertEquals("objective", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); + assertEquals(OBJECTIVE, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); assertEquals(1, alignmentsDtos.get(1).alignmentObjectDtos().get(0).objectId()); - assertEquals("O - FullObjective1", alignmentsDtos.get(1).alignmentObjectDtos().get(0).objectTitle()); - assertEquals("objective", alignmentsDtos.get(1).alignmentObjectDtos().get(0).objectType()); + assertEquals(FULL_OBJECTIVE_1, alignmentsDtos.get(1).alignmentObjectDtos().get(0).objectTitle()); + assertEquals(OBJECTIVE, alignmentsDtos.get(1).alignmentObjectDtos().get(0).objectType()); } @Test @@ -373,11 +385,11 @@ void shouldReturnAlignmentPossibilitiesOnlyObjectives() { verify(keyResultBusinessService, times(1)).getAllKeyResultsByObjective(2L); assertEquals(2, alignmentsDtos.get(0).alignmentObjectDtos().size()); assertEquals(1, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectId()); - assertEquals("O - FullObjective1", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectTitle()); - assertEquals("objective", alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); + assertEquals(FULL_OBJECTIVE_1, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectTitle()); + assertEquals(OBJECTIVE, alignmentsDtos.get(0).alignmentObjectDtos().get(0).objectType()); assertEquals(2, alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectId()); - assertEquals("O - FullObjective2", alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectTitle()); - assertEquals("objective", alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectType()); + assertEquals(FULL_OBJECTIVE_2, alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectTitle()); + assertEquals(OBJECTIVE, alignmentsDtos.get(0).alignmentObjectDtos().get(1).objectType()); } @Test diff --git a/backend/src/test/java/ch/puzzle/okr/service/validation/AlignmentValidationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/validation/AlignmentValidationServiceTest.java index e70413fd23..338a453729 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/validation/AlignmentValidationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/validation/AlignmentValidationServiceTest.java @@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static ch.puzzle.okr.TestConstants.*; @ExtendWith(MockitoExtension.class) class AlignmentValidationServiceTest { @@ -39,7 +40,7 @@ class AlignmentValidationServiceTest { @InjectMocks private AlignmentValidationService validator; - Team team1 = Team.Builder.builder().withId(1L).withName("Puzzle ITC").build(); + Team team1 = Team.Builder.builder().withId(1L).withName(TEAM_PUZZLE).build(); Team team2 = Team.Builder.builder().withId(2L).withName("BBT").build(); Objective objective1 = Objective.Builder.builder().withId(5L).withTitle("Objective 1").withTeam(team1) .withState(DRAFT).build(); diff --git a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.html b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.html index a9a41bbc52..e88271b068 100644 --- a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.html +++ b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.html @@ -70,22 +70,22 @@
- @for (group of filteredOptions$ | async; track group) { + @for (group of filteredAlignmentOptions$ | async; track group) { @for (alignmentObject of group.alignmentObjectDtos; track alignmentObject) { {{ alignmentObject.objectTitle }} diff --git a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.spec.ts b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.spec.ts index 87677017e6..4b8e150f8b 100644 --- a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.spec.ts +++ b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.spec.ts @@ -509,7 +509,7 @@ describe('ObjectiveDialogComponent', () => { }); expect(alignmentPossibilities).toStrictEqual([alignmentPossibility1, alignmentPossibility2]); - expect(component.filteredOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); expect(component.objectiveForm.getRawValue().alignment).toEqual(null); }); @@ -522,15 +522,15 @@ describe('ObjectiveDialogComponent', () => { }); expect(alignmentPossibilities).toStrictEqual([alignmentPossibility2]); - expect(component.filteredOptions$.getValue()).toEqual([alignmentPossibility2]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([alignmentPossibility2]); expect(component.objectiveForm.getRawValue().alignment).toEqual(null); }); it('should return team and objective with same text in alignment possibilities', async () => { - component.input.nativeElement.value = 'puzzle'; + component.alignmentInput.nativeElement.value = 'puzzle'; component.alignmentPossibilities$ = of([alignmentPossibility1, alignmentPossibility2]); component.filter(); - expect(component.filteredOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); }); it('should load existing objective alignment to objectiveForm', async () => { @@ -542,7 +542,7 @@ describe('ObjectiveDialogComponent', () => { }); expect(alignmentPossibilities).toStrictEqual([alignmentPossibility1, alignmentPossibility2]); - expect(component.filteredOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); expect(component.objectiveForm.getRawValue().alignment).toEqual(alignmentObject2); }); @@ -556,13 +556,13 @@ describe('ObjectiveDialogComponent', () => { }); expect(alignmentPossibilities).toStrictEqual([alignmentPossibility1, alignmentPossibility2]); - expect(component.filteredOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([alignmentPossibility1, alignmentPossibility2]); expect(component.objectiveForm.getRawValue().alignment).toEqual(alignmentObject3); }); it('should filter correct alignment possibilities', async () => { // Search for one title - component.input.nativeElement.value = 'palm'; + component.alignmentInput.nativeElement.value = 'palm'; component.alignmentPossibilities$ = of([alignmentPossibility1, alignmentPossibility2]); component.filter(); let modifiedAlignmentPossibility: AlignmentPossibility = { @@ -570,10 +570,10 @@ describe('ObjectiveDialogComponent', () => { teamName: 'Puzzle ITC', alignmentObjectDtos: [alignmentObject3], }; - expect(component.filteredOptions$.getValue()).toEqual([modifiedAlignmentPossibility]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([modifiedAlignmentPossibility]); // Search for team name - component.input.nativeElement.value = 'Puzzle IT'; + component.alignmentInput.nativeElement.value = 'Puzzle IT'; component.alignmentPossibilities$ = of([alignmentPossibility1, alignmentPossibility2]); component.filter(); modifiedAlignmentPossibility = { @@ -581,10 +581,10 @@ describe('ObjectiveDialogComponent', () => { teamName: 'Puzzle ITC', alignmentObjectDtos: [alignmentObject2, alignmentObject3], }; - expect(component.filteredOptions$.getValue()).toEqual([modifiedAlignmentPossibility]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([modifiedAlignmentPossibility]); // Search for two objects - component.input.nativeElement.value = 'buy'; + component.alignmentInput.nativeElement.value = 'buy'; component.alignmentPossibilities$ = of([alignmentPossibility1, alignmentPossibility2]); component.filter(); let modifiedAlignmentPossibilities = [ @@ -599,18 +599,18 @@ describe('ObjectiveDialogComponent', () => { alignmentObjectDtos: [alignmentObject1], }, ]; - expect(component.filteredOptions$.getValue()).toEqual(modifiedAlignmentPossibilities); + expect(component.filteredAlignmentOptions$.getValue()).toEqual(modifiedAlignmentPossibilities); // No match - component.input.nativeElement.value = 'findus'; + component.alignmentInput.nativeElement.value = 'findus'; component.alignmentPossibilities$ = of([alignmentPossibility1, alignmentPossibility2]); component.filter(); - expect(component.filteredOptions$.getValue()).toEqual([]); + expect(component.filteredAlignmentOptions$.getValue()).toEqual([]); }); it('should find correct alignment object', () => { // objective - let alignmentObject = component.findAlignmentObject( + let alignmentObject = component.findAlignmentPossibilityObject( [alignmentPossibility1, alignmentPossibility2], 1, 'objective', @@ -619,38 +619,46 @@ describe('ObjectiveDialogComponent', () => { expect(alignmentObject!.objectTitle).toEqual('We want to increase the income puzzle buy'); // keyResult - alignmentObject = component.findAlignmentObject([alignmentPossibility1, alignmentPossibility2], 1, 'keyResult'); + alignmentObject = component.findAlignmentPossibilityObject( + [alignmentPossibility1, alignmentPossibility2], + 1, + 'keyResult', + ); expect(alignmentObject!.objectId).toEqual(1); expect(alignmentObject!.objectTitle).toEqual('We buy 3 palms'); // no match - alignmentObject = component.findAlignmentObject([alignmentPossibility1, alignmentPossibility2], 133, 'keyResult'); + alignmentObject = component.findAlignmentPossibilityObject( + [alignmentPossibility1, alignmentPossibility2], + 133, + 'keyResult', + ); expect(alignmentObject).toEqual(null); }); it('should display kein alignment vorhanden when no alignment possibility', () => { - component.filteredOptions$.next([alignmentPossibility1, alignmentPossibility2]); + component.filteredAlignmentOptions$.next([alignmentPossibility1, alignmentPossibility2]); fixture.detectChanges(); - expect(component.input.nativeElement.getAttribute('placeholder')).toEqual('Bezug wählen'); + expect(component.alignmentInput.nativeElement.getAttribute('placeholder')).toEqual('Bezug wählen'); - component.filteredOptions$.next([]); + component.filteredAlignmentOptions$.next([]); fixture.detectChanges(); - expect(component.input.nativeElement.getAttribute('placeholder')).toEqual('Kein Alignment vorhanden'); + expect(component.alignmentInput.nativeElement.getAttribute('placeholder')).toEqual('Kein Alignment vorhanden'); }); it('should update alignments on quarter change', () => { objectiveService.getAlignmentPossibilities.mockReturnValue(of([alignmentPossibility1, alignmentPossibility2])); component.updateAlignments(); - expect(component.input.nativeElement.value).toEqual(''); + expect(component.alignmentInput.nativeElement.value).toEqual(''); expect(component.objectiveForm.getRawValue().alignment).toEqual(null); expect(objectiveService.getAlignmentPossibilities).toHaveBeenCalled(); }); it('should return correct displayedValue', () => { - component.input.nativeElement.value = 'O - Objective 1'; + component.alignmentInput.nativeElement.value = 'O - Objective 1'; expect(component.displayedValue).toEqual('O - Objective 1'); - component.input = new ElementRef(document.createElement('input')); + component.alignmentInput = new ElementRef(document.createElement('input')); expect(component.displayedValue).toEqual(''); }); }); diff --git a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts index 767716d530..7a54524ce3 100644 --- a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts +++ b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts @@ -25,8 +25,7 @@ import { AlignmentPossibilityObject } from '../../types/model/AlignmentPossibili changeDetection: ChangeDetectionStrategy.OnPush, }) export class ObjectiveFormComponent implements OnInit { - @ViewChild('input') input!: ElementRef; - filteredOptions$: BehaviorSubject = new BehaviorSubject([]); + @ViewChild('alignmentInput') alignmentInput!: ElementRef; objectiveForm = new FormGroup({ title: new FormControl('', [Validators.required, Validators.minLength(2), Validators.maxLength(250)]), @@ -40,6 +39,7 @@ export class ObjectiveFormComponent implements OnInit { quarters: Quarter[] = []; teams$: Observable = of([]); alignmentPossibilities$: Observable = of([]); + filteredAlignmentOptions$: BehaviorSubject = new BehaviorSubject([]); currentTeam$: BehaviorSubject = new BehaviorSubject(null); state: string | null = null; version!: number; @@ -69,9 +69,9 @@ export class ObjectiveFormComponent implements OnInit { const value = this.objectiveForm.getRawValue(); const state = this.data.objective.objectiveId == null ? submitType : this.state; - let alignmentEntity: AlignmentPossibilityObject | null = value.alignment; - let alignment: string | null = alignmentEntity - ? (alignmentEntity.objectType == 'objective' ? 'O' : 'K') + alignmentEntity.objectId + let alignment: AlignmentPossibilityObject | null = value.alignment; + let alignmentEntity: string | null = alignment + ? (alignment.objectType == 'objective' ? 'O' : 'K') + alignment.objectId : null; let objectiveDTO: Objective = { @@ -82,7 +82,7 @@ export class ObjectiveFormComponent implements OnInit { title: value.title, teamId: value.team, state: state, - alignedEntityId: alignment, + alignedEntityId: alignmentEntity, } as unknown as Objective; const submitFunction = this.getSubmitFunction(objectiveDTO.id, objectiveDTO); @@ -253,24 +253,28 @@ export class ObjectiveFormComponent implements OnInit { } if (objective) { - let alignment: string | null = objective.alignedEntityId; - if (alignment) { - let alignmentType: string = alignment.charAt(0); - let alignmentId: number = parseInt(alignment.substring(1)); + let alignmentEntity: string | null = objective.alignedEntityId; + if (alignmentEntity) { + let alignmentType: string = alignmentEntity.charAt(0); + let alignmentId: number = parseInt(alignmentEntity.substring(1)); alignmentType = alignmentType == 'O' ? 'objective' : 'keyResult'; - let element: AlignmentPossibilityObject | null = this.findAlignmentObject(value, alignmentId, alignmentType); + let alignmentPossibilityObject: AlignmentPossibilityObject | null = this.findAlignmentPossibilityObject( + value, + alignmentId, + alignmentType, + ); this.objectiveForm.patchValue({ - alignment: element, + alignment: alignmentPossibilityObject, }); } } - this.filteredOptions$.next(value.slice()); + this.filteredAlignmentOptions$.next(value.slice()); this.alignmentPossibilities$ = of(value); }); } - findAlignmentObject( + findAlignmentPossibilityObject( alignmentPossibilities: AlignmentPossibility[], objectId: number, objectType: string, @@ -288,8 +292,8 @@ export class ObjectiveFormComponent implements OnInit { } updateAlignments() { - this.input.nativeElement.value = ''; - this.filteredOptions$.next([]); + this.alignmentInput.nativeElement.value = ''; + this.filteredAlignmentOptions$.next([]); this.objectiveForm.patchValue({ alignment: null, }); @@ -297,7 +301,7 @@ export class ObjectiveFormComponent implements OnInit { } filter() { - let filterValue: string = this.input.nativeElement.value.toLowerCase(); + let filterValue: string = this.alignmentInput.nativeElement.value.toLowerCase(); this.alignmentPossibilities$.subscribe((alignmentPossibilities: AlignmentPossibility[]) => { let matchingTeams: AlignmentPossibility[] = alignmentPossibilities.filter((possibility: AlignmentPossibility) => possibility.teamName.toLowerCase().includes(filterValue), @@ -319,7 +323,7 @@ export class ObjectiveFormComponent implements OnInit { matchingPossibilities = [...new Set(matchingPossibilities)]; - let optionList = matchingPossibilities.map((possibility: AlignmentPossibility) => ({ + let alignmentOptionList = matchingPossibilities.map((possibility: AlignmentPossibility) => ({ ...possibility, alignmentObjectDtos: possibility.alignmentObjectDtos.filter( (alignmentPossibilityObject: AlignmentPossibilityObject) => @@ -327,8 +331,9 @@ export class ObjectiveFormComponent implements OnInit { ), })); - let finalArray: AlignmentPossibility[] = filterValue == '' ? matchingTeams : matchingTeams.concat(optionList); - this.filteredOptions$.next([...new Set(finalArray)]); + let concatAlignmentOptionList: AlignmentPossibility[] = + filterValue == '' ? matchingTeams : matchingTeams.concat(alignmentOptionList); + this.filteredAlignmentOptions$.next([...new Set(concatAlignmentOptionList)]); }); } @@ -339,15 +344,15 @@ export class ObjectiveFormComponent implements OnInit { } get displayedValue(): string { - if (this.input) { - return this.input.nativeElement.value; + if (this.alignmentInput) { + return this.alignmentInput.nativeElement.value; } else { return ''; } } scrollLeft() { - this.input.nativeElement.scrollLeft = 0; + this.alignmentInput.nativeElement.scrollLeft = 0; } protected readonly getQuarterLabel = getQuarterLabel;