Skip to content

Commit

Permalink
feat: CARITAS-286
Browse files Browse the repository at this point in the history
* fix all code following PR comments
  • Loading branch information
Leandro13Silva13 committed Nov 7, 2024
1 parent 6a89936 commit 49e7325
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 60 deletions.
4 changes: 2 additions & 2 deletions api/agencyservice.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ paths:
get:
tags:
- agency-controller
summary: 'Returns a list of all the topics assign to all the agencies of a specific tenant'
summary: 'Returns a list of all the topics assigned to all the agencies of a specific tenant'
operationId: getAgenciesTopics
responses:
200:
Expand Down Expand Up @@ -269,7 +269,7 @@ components:
example: 684
name:
type: string
example: "Counselling"
example: "Adoption and fostering a child"

DemographicsDTO:
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,6 @@ public ResponseEntity<List<FullAgencyResponseDTO>> getAgencies(
: new ResponseEntity<>(HttpStatus.NO_CONTENT);
}

/**
* Gets a randomly sorted list of AgencyResponseDTOs (from database) and returns the list and a
* 200 OK on success depending on the post code that is given via query parameter.
*
* @param postcode the postcode for regarding agencies
* @param topicId the (optional) main topicId to filter the agencies
* @return the List of agencies with information
*/
@Override
public ResponseEntity<List<FullAgencyResponseDTO>> getTenantAgencies(String postcode,
Integer topicId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public interface AgencyRepository extends JpaRepository<Agency, Long> {
String GROUP_BY_ORDER_BY = "GROUP BY a.id "
+ "ORDER BY a.postcode DESC";

String TOPIC_ORDER_BY = "ORDER BY at.topic_id";
String ORDER_BY_TOPIC = "ORDER BY at.topic_id";

/**
* Returns a list of {@link Agency}s that are assigned to the given post code.
Expand Down Expand Up @@ -87,7 +87,7 @@ List<Agency> searchWithTopic(@Param(value = "postcode") String postCode,

@Query(
value = SELECT_ALL_AGENCIES_TOPICS
+ TOPIC_ORDER_BY,
+ ORDER_BY_TOPIC,
nativeQuery = true)
List<Integer> findAllAgenciesTopics();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ List<Agency> searchWithTopic(@Param(value = "postcode") String postCode,
@Query(
value = SELECT_ALL_AGENCIES_TOPICS
+ AND_A_TENANT_ID_FILTER
+ TOPIC_ORDER_BY,
+ ORDER_BY_TOPIC,
nativeQuery = true)
List<Integer> findAllAgenciesTopics();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ List<Agency> searchWithTopic(@Param(value = "postcode") String postCode,

@Query(
value = SELECT_ALL_AGENCIES_TOPICS
+ TOPIC_ORDER_BY,
+ ORDER_BY_TOPIC,
nativeQuery = true)
List<Integer> findAllAgenciesTopics();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,9 @@ public List<FullAgencyResponseDTO> getAgencies(Optional<String> postCode,
return mutableResponseDTO;
}

/**
* Returns a list of {@link AgencyResponseDTO} from the current tenant, which match to the provided
* postCode. If no agency is found, returns empty list.
*
* @param postCode the postcode for regarding agencies
* @param topicId the topic id used for filtering agencies
* @return a list containing regarding agencies
*/
public List<FullAgencyResponseDTO> getAgencies(String postCode, Integer topicId) {

var agencies = findAgencies(postCode, topicId);
Collections.shuffle(agencies);
var agencies = findAgenciesForCurrentTenant(postCode, topicId);
return agencies.stream()
.map(this::convertToFullAgencyResponseDTO)
.toList();
Expand Down Expand Up @@ -203,7 +194,7 @@ private List<Agency> findAgencies(AgencySearch agencySearch) {
}
}

private List<Agency> findAgencies(String postCode, Integer topicId) {
private List<Agency> findAgenciesForCurrentTenant(String postCode, Integer topicId) {

AgencySearch agencySearch = buildAgencySearch(Optional.of(postCode),
Optional.empty(), Optional.of(topicId), Optional.empty(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import de.caritas.cob.agencyservice.api.model.AgencyTopicsDTO;
import de.caritas.cob.agencyservice.topicservice.generated.web.model.TopicDTO;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -32,6 +32,8 @@ public List<AgencyTopicsDTO> enrichTopicIdsWithTopicData(List<Integer> topicIds)
private static void logAvailableTopicsList(List<TopicDTO> availableTopics) {
if (availableTopics == null) {
log.debug("Available topics list is null ");
} else if (availableTopics.isEmpty()) {
log.debug("Available topics list is empty");
} else {
log.debug("Available topics list has size: {} ", availableTopics.size());
}
Expand All @@ -40,7 +42,7 @@ private static void logAvailableTopicsList(List<TopicDTO> availableTopics) {
private List<AgencyTopicsDTO> enrichTopicIds(List<TopicDTO> availableTopics, List<Integer> topicIds) {

if (availableTopics == null) {
return new ArrayList<>();
return Collections.emptyList();
}
// Create a map of availableTopics to quickly access TopicDTO by id
Map<Long, String> topicMap = availableTopics.stream()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package de.caritas.cob.agencyservice.api.controller;

import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES_PRIVATE;
import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES_BY_TENANT;
import static de.caritas.cob.agencyservice.testHelper.TestConstants.AGENCY_RESPONSE_DTO;
import static de.caritas.cob.agencyservice.testHelper.TestConstants.FULL_AGENCY_RESPONSE_DTO;
import static de.caritas.cob.agencyservice.testHelper.TestConstants.INVALID_POSTCODE_QUERY;
Expand Down Expand Up @@ -52,7 +52,7 @@ void getTenantAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() thr
.thenReturn(null);

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isNoContent());
Expand All @@ -63,7 +63,7 @@ void getTenantAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() thr
void getTenantAgencies_Should_ReturnBadRequest_When_PostcodeParamIsInvalid() throws Exception {

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + INVALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + INVALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest());
Expand All @@ -74,7 +74,7 @@ void getTenantAgencies_Should_ReturnBadRequest_When_PostcodeParamIsInvalid() thr
void getTenantAgencies_Should_ReturnBadRequest_When_topicIdParamIsNotProvided()
throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

Expand All @@ -83,7 +83,7 @@ void getTenantAgencies_Should_ReturnBadRequest_When_topicIdParamIsNotProvided()
void getTenantAgencies_Should_ReturnBadRequest_When_PostCodeParamIsNotProvided()
throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_TOPIC_ID_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

Expand All @@ -98,7 +98,7 @@ void getTenantAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws E
.thenReturn(agencies);

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
Expand All @@ -110,15 +110,15 @@ void getTenantAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws E
@Test
void getTenantAgencies_Should_ReturnUnauthorized_When_UserHasNoAuthority() throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_TOPIC_ID_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isUnauthorized());
}

@Test
@WithMockUser(authorities = {AuthorityValue.SEARCH_AGENCIES})
void getTenantAgencies_Should_ReturnForbidden_When_UserHasWrongAuthority() throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_TOPIC_ID_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isForbidden());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_AGENCIES_WITH_IDS;
import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES;
import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES_PRIVATE;
import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES_BY_TENANT;
import static de.caritas.cob.agencyservice.testHelper.PathConstants.PATH_GET_LIST_OF_AGENCIES_TOPICS;
import static de.caritas.cob.agencyservice.testHelper.TestConstants.AGENCY_ID;
import static de.caritas.cob.agencyservice.testHelper.TestConstants.AGENCY_RESPONSE_DTO;
Expand All @@ -18,6 +18,8 @@
import static de.caritas.cob.agencyservice.testHelper.TestConstants.VALID_TOPIC_ID_QUERY;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -41,7 +43,6 @@
import java.util.Optional;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.slf4j.Logger;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
Expand Down Expand Up @@ -87,7 +88,7 @@ class AgencyControllerTest {
@Test
void getAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() throws Exception {

when(agencyService.getAgencies(Mockito.any(Optional.class), Mockito.anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class)))
when(agencyService.getAgencies(any(Optional.class), anyInt(), any(Optional.class), any(Optional.class), any(Optional.class), any(Optional.class)))
.thenReturn(null);

mvc.perform(
Expand Down Expand Up @@ -140,7 +141,7 @@ void getAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws Excepti
List<FullAgencyResponseDTO> agencies = new ArrayList<>();
agencies.add(FULL_AGENCY_RESPONSE_DTO);

when(agencyService.getAgencies(Mockito.any(Optional.class), Mockito.anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class)))
when(agencyService.getAgencies(any(Optional.class), anyInt(), any(Optional.class), any(Optional.class), any(Optional.class), any(Optional.class)))
.thenReturn(agencies);

mvc.perform(
Expand All @@ -150,15 +151,15 @@ void getAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws Excepti
.andExpect(status().isOk())
.andExpect(jsonPath("[0].name").value(AGENCY_RESPONSE_DTO.getName()));

verify(agencyService, atLeastOnce()).getAgencies(Mockito.any(Optional.class),
Mockito.anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class));
verify(agencyService, atLeastOnce()).getAgencies(any(Optional.class),
anyInt(), any(Optional.class), any(Optional.class), any(Optional.class), any(Optional.class));
}

@Test
void getAgencies_With_Ids_Should_ReturnNoContent_When_ServiceReturnsNoAgency()
throws Exception {

when(agencyService.getAgencies(Mockito.anyList())).thenReturn(Collections.emptyList());
when(agencyService.getAgencies(anyList())).thenReturn(Collections.emptyList());

mvc.perform(get(PATH_GET_AGENCIES_WITH_IDS + AGENCY_ID).accept(MediaType.APPLICATION_JSON))
.andExpect(status().isNotFound());
Expand All @@ -176,23 +177,23 @@ void getAgencies_With_Ids_Should_ReturnBadRequest_When_IdInvalid() throws Except
void getAgencies_With_Ids_Should_ReturnAgencyAndOk_When_ServiceReturnsAgency()
throws Exception {

when(agencyService.getAgencies(Mockito.anyList())).thenReturn(AGENCY_RESPONSE_DTO_LIST);
when(agencyService.getAgencies(anyList())).thenReturn(AGENCY_RESPONSE_DTO_LIST);

mvc.perform(get(PATH_GET_AGENCIES_WITH_IDS + AGENCY_ID + "," + AGENCY_ID)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("[0].name").value(AGENCY_RESPONSE_DTO.getName()));

verify(agencyService, atLeastOnce()).getAgencies(Mockito.anyList());
verify(agencyService, atLeastOnce()).getAgencies(anyList());
}

@Test
void getListOfAgencies_Should_ReturnServerErrorAndLogDatabaseError_OnAgencyServiceThrowsServerErrorException()
void getListOfAgencies_Should_ReturnServerErrorAndLogDatabaseError_When_AgencyServiceThrowsServerErrorException()
throws Exception {

InternalServerErrorException dbEx = new InternalServerErrorException(
LogService::logDatabaseError, "message");
when(agencyService.getAgencies(any(), anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class))).thenThrow(dbEx);
when(agencyService.getAgencies(any(), anyInt(), any(Optional.class), any(Optional.class), any(Optional.class), any(Optional.class))).thenThrow(dbEx);

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_CONSULTING_TYPE_QUERY)
Expand Down Expand Up @@ -220,7 +221,7 @@ void getListOfAgencies_Should_ReturnServerErrorAndLogNumberFormatError_OnAgencyS
InternalServerErrorException nfEx = new InternalServerErrorException(
LogService::logNumberFormatException, "message");

when(agencyService.getAgencies(any(), anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class))).thenThrow(nfEx);
when(agencyService.getAgencies(any(), anyInt(), any(Optional.class), any(Optional.class), any(Optional.class), any(Optional.class))).thenThrow(nfEx);

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_CONSULTING_TYPE_QUERY)
Expand Down Expand Up @@ -273,24 +274,24 @@ void getAgenciesTopics_Should_ReturnListAndOk_When_ServiceReturnsList() throws E
List<AgencyTopicsDTO> agenciesTopics = new ArrayList<>();
agenciesTopics.add(AGENCY_TOPICS_DTO);

when(topicEnrichmentService.enrichTopicIdsWithTopicData(Mockito.anyList()))
when(topicEnrichmentService.enrichTopicIdsWithTopicData(anyList()))
.thenReturn(agenciesTopics);

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_TOPICS))
.andExpect(status().isOk());

verify(topicEnrichmentService, atLeastOnce()).enrichTopicIdsWithTopicData(Mockito.anyList());
verify(topicEnrichmentService, atLeastOnce()).enrichTopicIdsWithTopicData(anyList());
}

@Test
void getTenantAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() throws Exception {

when(agencyService.getAgencies(Mockito.anyString(), Mockito.anyInt()))
when(agencyService.getAgencies(anyString(), anyInt()))
.thenReturn(null);

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isNoContent());
Expand All @@ -300,7 +301,7 @@ void getTenantAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() thr
void getTenantAgencies_Should_ReturnBadRequest_When_PostcodeParamIsInvalid() throws Exception {

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + INVALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + INVALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest());
Expand All @@ -310,15 +311,15 @@ void getTenantAgencies_Should_ReturnBadRequest_When_PostcodeParamIsInvalid() thr
void getTenantAgencies_Should_ReturnBadRequest_When_topicIdParamIsNotProvided()
throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

@Test
void getTenantAgencies_Should_ReturnBadRequest_When_PostCodeParamIsNotProvided()
throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_TOPIC_ID_QUERY)
mvc.perform(get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

Expand All @@ -328,17 +329,17 @@ void getTenantAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws E
List<FullAgencyResponseDTO> agencies = new ArrayList<>();
agencies.add(FULL_AGENCY_RESPONSE_DTO);

when(agencyService.getAgencies(Mockito.anyString(), Mockito.anyInt()))
when(agencyService.getAgencies(anyString(), anyInt()))
.thenReturn(agencies);

mvc.perform(
get(PATH_GET_LIST_OF_AGENCIES_PRIVATE + "?" + VALID_POSTCODE_QUERY + "&"
get(PATH_GET_LIST_OF_AGENCIES_BY_TENANT + "?" + VALID_POSTCODE_QUERY + "&"
+ VALID_TOPIC_ID_QUERY)
.accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("[0].name").value(AGENCY_RESPONSE_DTO.getName()));

verify(agencyService, atLeastOnce()).getAgencies(Mockito.anyString(), Mockito.anyInt());
verify(agencyService, atLeastOnce()).getAgencies(anyString(), anyInt());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class PathConstants {
public static final String PAGE_PARAM = "page";
public static final String PER_PAGE_PARAM = "perPage";
public static final String PATH_GET_LIST_OF_AGENCIES = "/agencies";
public static final String PATH_GET_LIST_OF_AGENCIES_PRIVATE = "/agencies/by-tenant";
public static final String PATH_GET_LIST_OF_AGENCIES_BY_TENANT = "/agencies/by-tenant";
public static final String PATH_GET_LIST_OF_AGENCIES_TOPICS = "/agencies/topics";
public static final String PATH_GET_AGENCIES_WITH_IDS = "/agencies/";
public static final String GET_AGENCY_PATH = ROOT_PATH + "/agencies";
Expand Down
Loading

0 comments on commit 49e7325

Please sign in to comment.