Skip to content

Commit

Permalink
Merge pull request #203 from Onlineberatung/OB-10084-make-postcode-op…
Browse files Browse the repository at this point in the history
…tional

fix: make postcode optional for agency search
  • Loading branch information
tkuzynow authored Feb 28, 2024
2 parents 8a38787 + d088926 commit 0613f9b
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 36 deletions.
2 changes: 1 addition & 1 deletion api/agencyservice.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ paths:
parameters:
- name: postcode
in: query
required: true
required: false
description: The postcode the user entered
schema:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import de.caritas.cob.agencyservice.generated.api.controller.AgenciesApi;
import io.swagger.annotations.Api;
import java.util.List;
import java.util.Optional;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -38,14 +39,14 @@ public class AgencyController implements AgenciesApi {
*/
@Override
public ResponseEntity<List<FullAgencyResponseDTO>> getAgencies(
@RequestParam String postcode, @RequestParam Integer consultingType,
@RequestParam Integer consultingType, @RequestParam(required = false) String postcode,
@RequestParam(value = "topicId", required = false) Integer topicId,
@RequestParam(value = "age", required = false) Integer age,
@RequestParam(value = "gender", required = false) String gender,
@RequestParam(value = "counsellingRelation", required = false) String counsellingRelation
) {

var agencies = agencyService.getAgencies(postcode, consultingType,
var agencies = agencyService.getAgencies(Optional.ofNullable(postcode), consultingType,
ofNullable(topicId), ofNullable(age), ofNullable(gender), ofNullable(counsellingRelation));

return !CollectionUtils.isEmpty(agencies)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.CrudRepository;
import org.springframework.data.repository.query.Param;

/**
Expand All @@ -15,10 +14,10 @@ public interface AgencyRepository extends JpaRepository<Agency, Long> {

String AND_WITH_BRACKET = "AND (";
String SELECT_WITH_TOPICS = "SELECT a.*, :tenantId FROM agency a "
+ "INNER JOIN agency_postcode_range r ON a.id = r.agency_id "
+ "LEFT JOIN agency_postcode_range r ON a.id = r.agency_id "
+ "INNER JOIN agency_topic at ON a.id = at.agency_id "
+ "WHERE (CAST(:postcode AS INT) BETWEEN CAST(SUBSTR(r.postcode_from, 1, :length) AS int) "
+ "AND CAST(SUBSTR(r.postcode_to, 1, :length) AS int)) " + "AND a.is_offline = false "
+ "WHERE :postcode is NULL OR ((CAST(:postcode AS INT) BETWEEN CAST(SUBSTR(r.postcode_from, 1, :length) AS int) "
+ "AND CAST(SUBSTR(r.postcode_to, 1, :length) AS int))) " + "AND a.is_offline = false "
+ "AND (:type is NULL OR a.consulting_type = :type) "
+ "AND at.topic_id = :topicId "
+ AND_WITH_BRACKET
Expand All @@ -32,10 +31,10 @@ public interface AgencyRepository extends JpaRepository<Agency, Long> {
+ "AND a.delete_date IS NULL ";

String SELECT_WITHOUT_TOPICS = "SELECT a.*, :tenantId FROM agency a "
+ "INNER JOIN agency_postcode_range r ON a.id = r.agency_id "
+ "LEFT JOIN agency_postcode_range r ON a.id = r.agency_id "
+ "WHERE "
+ "(CAST(:postcode AS INT) BETWEEN CAST(SUBSTR(r.postcode_from, 1, :length) AS int) "
+ "AND CAST(SUBSTR(r.postcode_to, 1, :length) AS int)) " + "AND a.is_offline = false "
+ ":postcode is NULL OR ((CAST(:postcode AS INT) BETWEEN CAST(SUBSTR(r.postcode_from, 1, :length) AS int) "
+ "AND CAST(SUBSTR(r.postcode_to, 1, :length) AS int))) " + "AND a.is_offline = false "
+ "AND (:type is NULL OR a.consulting_type = :type) "
+ AND_WITH_BRACKET
+ " (:age IS NULL) OR (a.age_from <= :age)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
@Data
public class AgencySearch {

private String postCode;
private Optional<String> postCode;
private Optional<Integer> consultingTypeId;
private Optional<Integer> topicId;
private Optional<Integer> age;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public List<AgencyResponseDTO> getAgencies(int consultingTypeId) {
}


public List<FullAgencyResponseDTO> getAgencies(String postCode, int consultingTypeId,
public List<FullAgencyResponseDTO> getAgencies(Optional<String> postCode, int consultingTypeId,
Optional<Integer> topicId) {
return getAgencies(postCode, consultingTypeId, topicId, Optional.empty(), Optional.empty(), Optional.empty());
}
Expand All @@ -116,17 +116,19 @@ public List<FullAgencyResponseDTO> getAgencies(String postCode, int consultingTy
* @param consultingTypeId the consulting type used for filtering agencies
* @return a list containing regarding agencies
*/
public List<FullAgencyResponseDTO> getAgencies(String postCode, int consultingTypeId,
public List<FullAgencyResponseDTO> getAgencies(Optional<String> postCode,
Integer consultingTypeId,
Optional<Integer> topicId,
Optional<Integer> age, Optional<String> gender, Optional<String> counsellingRelation) {

var consultingTypeSettings = retrieveConsultingTypeSettings(
consultingTypeId);

if (doesPostCodeNotMatchMinSize(postCode, consultingTypeSettings)) {
if (postCode.isPresent() && doesPostCodeNotMatchMinSize(postCode.get(),
consultingTypeSettings)) {
return Collections.emptyList();
}


var agencies = findAgencies(postCode, getConsultingTypeIdForSearch(consultingTypeId), topicId,
age, gender, counsellingRelation);
Collections.shuffle(agencies);
Expand All @@ -147,7 +149,7 @@ private Optional<Integer> getConsultingTypeIdForSearch(int consultingTypeId) {
return multitenancyWithSingleDomain ? Optional.empty() : Optional.of(consultingTypeId);
}

private List<Agency> findAgencies(String postCode, Optional<Integer> consultingTypeId,
private List<Agency> findAgencies(Optional<String> postCode, Optional<Integer> consultingTypeId,
Optional<Integer> optionalTopicId, Optional<Integer> age,
Optional<String> gender, Optional<String> counsellingRelation) {

Expand Down Expand Up @@ -175,8 +177,8 @@ private List<Agency> findAgencies(String postCode, Optional<Integer> consultingT
private List<Agency> findAgencies(AgencySearch agencySearch) {
try {
return getAgencyRepositoryForSearch()
.searchWithoutTopic(agencySearch.getPostCode(),
agencySearch.getPostCode().length(), agencySearch.getConsultingTypeId().orElse(null),
.searchWithoutTopic(agencySearch.getPostCode().orElse(null),
agencySearch.getPostCode().orElse("").length(), agencySearch.getConsultingTypeId().orElse(null),
agencySearch.getAge().orElse(null),
agencySearch.getGender().orElse(null),
agencySearch.getCounsellingRelation().orElse(null),
Expand Down Expand Up @@ -238,7 +240,7 @@ public ExtendedConsultingTypeResponseDTO retrieveConsultingTypeSettings(int cons
private List<Agency> findAgenciesWithTopic(AgencySearch agencySearch) {
try {
return getAgencyRepositoryForSearch()
.searchWithTopic(agencySearch.getPostCode(), agencySearch.getPostCode().length(),
.searchWithTopic(agencySearch.getPostCode().orElse(null), agencySearch.getPostCode().orElse("").length(),
agencySearch.getConsultingTypeId().orElse(null),
agencySearch.getTopicId().orElseThrow(),
agencySearch.getAge().orElse(null), agencySearch.getGender().orElse(null),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class AgencyControllerIT {
@Test
void getAgencies_Should_ReturnNoContent_When_ServiceReturnsEmptyList() throws Exception {

when(agencyService.getAgencies(Mockito.anyString(), Mockito.anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class)))
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)))
.thenReturn(null);

mvc.perform(
Expand Down Expand Up @@ -112,11 +112,11 @@ void getAgencies_Should_ReturnBadRequest_When_ConsultingTypeParamIsInvalid()
}

@Test
void getAgencies_Should_ReturnBadRequest_When_PostcodeParamIsNotProvided()
void getAgencies_Should_ReturnRespondWith2XXResponseCode_When_PostcodeParamIsNotProvided()
throws Exception {

mvc.perform(get(PATH_GET_LIST_OF_AGENCIES + "?" + VALID_CONSULTING_TYPE_QUERY)
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isNoContent());
}

@Test
Expand All @@ -133,7 +133,7 @@ void getAgencies_Should_ReturnListAndOk_When_ServiceReturnsList() throws Excepti
List<FullAgencyResponseDTO> agencies = new ArrayList<>();
agencies.add(FULL_AGENCY_RESPONSE_DTO);

when(agencyService.getAgencies(Mockito.anyString(), Mockito.anyInt(), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class), Mockito.any(Optional.class)))
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)))
.thenReturn(agencies);

mvc.perform(
Expand All @@ -143,7 +143,7 @@ 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.anyString(),
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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ void searchWithoutTopic_Should_findAgencyByPostcodeAndConsultingType() {
assertThat(agencyList).hasSize(2);
}

@Test
void searchWithoutTopic_Should_findAgencyByOnlyConsultingTypeSkippingPostCodeFiltering() {
// given, when
var agencyList = agencyRepository.searchWithoutTopic(null, 5, 0, null, null, null, 1L);
// then
assertThat(agencyList).hasSize(1138);
}


@Test
void searchWithTopic_Should_findAgencyByPostcodeAndConsultingTypeAndTopicId() {
Expand All @@ -66,6 +74,14 @@ void searchWithTopic_Should_findAgencyByPostcodeAndConsultingTypeAndTopicId() {
assertThat(agencyList.get(0).getAgencyTopics()).extracting("topicId").containsExactly(0L, 1L);
}

@Test
void searchWithTopic_Should_findAgencyConsultingTypeAndTopicIdSkippingPostCode() {
// given, when
var agencyList = agencyRepository.searchWithTopic(null, 5, 0, 1, null, null, null, 1L);
// then
assertThat(agencyList).hasSize(4);
}

@Test
void searchWithTopic_Should_findAgencyByPostcodeAndConsultingTypeAndTopicId_When_ConsultingTypeIsNotProvided() {
// given, when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void getAgencies_Should_returnMatchingAgencies_When_postcodeAndConsulting
String postCode = "88662";

List<FullAgencyResponseDTO> resultAgencies = agencyService
.getAgencies(postCode, CONSULTING_TYPE_PREGNANCY, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty());
.getAgencies(Optional.of(postCode), CONSULTING_TYPE_PREGNANCY, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty());

assertThat(resultAgencies, hasSize(1));
FullAgencyResponseDTO resultAgency = resultAgencies.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void getAgencies_Should_throwBadRequestException_When_topicIdNotProvidedA
restrictedTenantDTO);

// when
this.agencyService.getAgencies("12123", 1, Optional.empty());
this.agencyService.getAgencies(Optional.of("12123"), 1, Optional.empty());

// then
verify(agencyRepository).searchWithTopic("12123", 5, 1, 2, null,
Expand All @@ -99,7 +99,7 @@ public void getAgencies_Should_searchByTopicId_When_topicIdProvidedAndFeatureEna
restrictedTenantDTO);

// when
this.agencyService.getAgencies("12123", 1, Optional.of(2), Optional.empty(), Optional.empty(), Optional.empty());
this.agencyService.getAgencies(Optional.of("12123"), 1, Optional.of(2), Optional.empty(), Optional.empty(), Optional.empty());

// then
verify(agencyRepository).searchWithTopic("12123", 5, 1, 2, null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Optional;
import javax.swing.text.html.Option;
import org.hamcrest.collection.IsEmptyCollection;
import org.jeasy.random.EasyRandom;
import org.junit.After;
Expand Down Expand Up @@ -117,7 +118,7 @@ public void getListOfAgencies_Should_ReturnServiceExceptionAndLogDatabaseError_O
private void callGetAgencies() {
Optional<Integer> emptyTopicIds = Optional.empty();
try {
agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_SUCHT, emptyTopicIds);
agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_SUCHT, emptyTopicIds);
fail("Expected exception: ServiceException");
} catch (InternalServerErrorException internalServerErrorException) {
assertTrue("Excepted ServiceException thrown", true);
Expand Down Expand Up @@ -163,9 +164,9 @@ public void getListOfAgencies_Should_ReturnListOfFullAgencyResponseDTO_WhenDBSel
when(consultingTypeManager.getConsultingTypeSettings(Mockito.anyInt()))
.thenReturn(CONSULTING_TYPE_SETTINGS_WITH_WHITESPOT_AGENCY);

assertThat(agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_SUCHT, Optional.empty()),
assertThat(agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_SUCHT, Optional.empty()),
everyItem(instanceOf(FullAgencyResponseDTO.class)));
assertThat(agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_SUCHT, Optional.empty()))
assertThat(agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_SUCHT, Optional.empty()))
.extracting(POSTCODE).contains(POSTCODE);
}

Expand All @@ -181,7 +182,7 @@ public void getListOfAgencies_Should_ReturnWhiteSpotAgency_WhenNoAgencyFoundForG
when(consultingTypeManager.getConsultingTypeSettings(Mockito.anyInt()))
.thenReturn(CONSULTING_TYPE_SETTINGS_WITH_WHITESPOT_AGENCY);

assertThat(agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_SUCHT, Optional.empty()))
assertThat(agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_SUCHT, Optional.empty()))
.extracting(FIELD_AGENCY_ID).contains(AGENCY_ID);
}

Expand All @@ -194,7 +195,7 @@ public void getListOfAgencies_Should_ReturnEmptyList_WhenNoAgencyFoundForGivenPo
when(consultingTypeManager.getConsultingTypeSettings(Mockito.anyInt()))
.thenReturn(CONSULTING_TYPE_SETTINGS_WITHOUT_WHITESPOT_AGENCY);

assertThat(agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_SUCHT, Optional.empty()),
assertThat(agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_SUCHT, Optional.empty()),
IsEmptyCollection.empty());
}

Expand All @@ -205,7 +206,7 @@ public void getListOfAgencies_Should_ReturnEmptyList_When_PostcodeSizeIsSmallerT
when(consultingTypeManager.getConsultingTypeSettings(Mockito.anyInt()))
.thenReturn(CONSULTING_TYPE_SETTINGS_EMIGRATION);

assertThat(agencyService.getAgencies(VALID_POSTCODE, CONSULTING_TYPE_EMIGRATION, Optional.empty()),
assertThat(agencyService.getAgencies(Optional.of(VALID_POSTCODE), CONSULTING_TYPE_EMIGRATION, Optional.empty()),
IsEmptyCollection.empty());
}

Expand Down Expand Up @@ -268,7 +269,7 @@ public void getAgencies_Should_ThrowInternalServerError_When_MissingConsultingTy

when(consultingTypeManager.getConsultingTypeSettings(anyInt()))
.thenThrow(new MissingConsultingTypeException(""));
agencyService.getAgencies("", 0, Optional.empty());
agencyService.getAgencies(Optional.of(""), 0, Optional.empty());
}

@Test
Expand Down Expand Up @@ -310,7 +311,7 @@ public void getAgencies_Should_throwBadRequestException_When_TopicIdNotProvidedA
when(tenantService.getRestrictedTenantDataForSingleTenant()).thenReturn(restrictedTenantDTO);

// when
this.agencyService.getAgencies("12123", 1, Optional.empty());
this.agencyService.getAgencies(Optional.of("12123"), 1, Optional.empty());
}

@Test
Expand All @@ -326,7 +327,7 @@ public void getAgencies_Should_searchByTopicId_When_TopicIdProvidedAndFeatureEna
when(tenantService.getRestrictedTenantDataForSingleTenant()).thenReturn(restrictedTenantDTO);

// when
this.agencyService.getAgencies("12123", 1, Optional.of(2));
this.agencyService.getAgencies(Optional.of("12123"), 1, Optional.of(2));

// then
verify(agencyRepository).searchWithTopic("12123", 5, 1, 2, AGE, GENDER, COUNSELLING_RELATION,
Expand Down

0 comments on commit 0613f9b

Please sign in to comment.