-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
7817 fix api response status 500 instead of 400 when coordinates not specified #7850
Changes from 7 commits
c4da69e
3cd6645
e423b69
c7e2d8e
bedca83
499a317
837a9a9
e86e160
b0fc84e
ada75f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,7 +132,7 @@ public class EventServiceImpl implements EventService { | |
public EventDto save(AddEventDtoRequest addEventDtoRequest, String email, | ||
MultipartFile[] images) { | ||
checkingEqualityDateTimeInEventDateLocationDto(addEventDtoRequest.getDatesLocations()); | ||
addAddressToLocation(addEventDtoRequest.getDatesLocations()); | ||
validateEventRequest(addEventDtoRequest); | ||
Event toSave = modelMapper.map(addEventDtoRequest, Event.class); | ||
UserVO userVO = restClient.findByEmail(email); | ||
User organizer = modelMapper.map(userVO, User.class); | ||
|
@@ -601,6 +601,33 @@ private void addNewImages(Event toUpdate, UpdateEventDto updateEventDto, Multipa | |
} | ||
} | ||
|
||
private void validateEventRequest(AddEventDtoRequest addEventDtoRequest) { | ||
checkingEqualityDateTimeInEventDateLocationDto(addEventDtoRequest.getDatesLocations()); | ||
ChernenkoVitaliy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (!validateCoordinates(addEventDtoRequest.getDatesLocations())) { | ||
throw new BadRequestException(ErrorMessage.INVALID_COORDINATES); | ||
} | ||
addAddressToLocation(addEventDtoRequest.getDatesLocations()); | ||
} | ||
|
||
public boolean validateCoordinates(List<EventDateLocationDto> eventDateLocationDtos) { | ||
for (EventDateLocationDto eventDateLocationDto : eventDateLocationDtos) { | ||
AddressDto coordinates = eventDateLocationDto.getCoordinates(); | ||
|
||
if (coordinates == null || coordinates.getLatitude() == null || coordinates.getLongitude() == null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe make sens use Objects.isNull() ckeck There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Corrected |
||
return false; | ||
} | ||
|
||
double latitude = coordinates.getLatitude(); | ||
double longitude = coordinates.getLongitude(); | ||
|
||
if (latitude < -90.0 || latitude > 90.0 || longitude < -180.0 || longitude > 180.0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe move this check to separate method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Corrected |
||
return false; | ||
} | ||
} | ||
return true; | ||
} | ||
|
||
private void addAddressToLocation(List<EventDateLocationDto> eventDateLocationDtos) { | ||
eventDateLocationDtos.stream() | ||
.filter(eventDateLocationDto -> Objects.nonNull(eventDateLocationDto.getCoordinates())) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,6 +157,11 @@ void save() { | |
}.getType())).thenReturn(tags); | ||
when(googleApiService.getResultFromGeoCodeByCoordinates(any())) | ||
.thenReturn(ModelUtils.getAddressLatLngResponse()); | ||
AddressDto build = AddressDto.builder() | ||
.latitude(ModelUtils.getAddressDto().getLatitude()) | ||
.longitude(ModelUtils.getAddressDto().getLongitude()) | ||
.build(); | ||
when(modelMapper.map(ModelUtils.getAddressLatLngResponse(), AddressDto.class)).thenReturn(build); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move this AddressDto builder to ModelsUtils class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Corrected |
||
when(eventRepo.findFavoritesAmongEventIds(eventIds, user.getId())).thenReturn(List.of(event)); | ||
when(eventRepo.findSubscribedAmongEventIds(eventIds, user.getId())).thenReturn(List.of()); | ||
|
||
|
@@ -184,38 +189,16 @@ void save() { | |
@Test | ||
void saveEventWithoutAddress() { | ||
User user = ModelUtils.getUser(); | ||
EventDto eventDtoWithoutCoordinatesDto = ModelUtils.getEventDtoWithoutAddress(); | ||
List<Long> eventIds = List.of(eventDtoWithoutCoordinatesDto.getId()); | ||
AddEventDtoRequest addEventDtoWithoutCoordinates = ModelUtils.addEventDtoWithoutAddressRequest; | ||
Event eventWithoutCoordinates = ModelUtils.getEventWithoutAddress(); | ||
List<Tag> tags = ModelUtils.getEventTags(); | ||
|
||
String email = user.getEmail(); | ||
when(modelMapper.map(addEventDtoWithoutCoordinates, Event.class)).thenReturn(eventWithoutCoordinates); | ||
when(restClient.findByEmail(user.getEmail())).thenReturn(testUserVo); | ||
when(modelMapper.map(testUserVo, User.class)).thenReturn(user); | ||
when(eventRepo.save(eventWithoutCoordinates)).thenReturn(eventWithoutCoordinates); | ||
when(modelMapper.map(eventWithoutCoordinates, EventDto.class)).thenReturn(eventDtoWithoutCoordinatesDto); | ||
List<TagVO> tagVOList = Collections.singletonList(ModelUtils.getTagVO()); | ||
when(tagService.findTagsWithAllTranslationsByNamesAndType(addEventDtoWithoutCoordinates.getTags(), | ||
TagType.EVENT)).thenReturn(tagVOList); | ||
when(modelMapper.map(tagVOList, new TypeToken<List<Tag>>() { | ||
}.getType())).thenReturn(tags); | ||
when(eventRepo.findFavoritesAmongEventIds(eventIds, user.getId())).thenReturn(List.of(eventWithoutCoordinates)); | ||
when(eventRepo.findSubscribedAmongEventIds(eventIds, user.getId())) | ||
.thenReturn(List.of(eventWithoutCoordinates)); | ||
|
||
EventDto resultEventDto = eventService.save(addEventDtoWithoutCoordinates, user.getEmail(), null); | ||
|
||
assertEquals(eventDtoWithoutCoordinatesDto, resultEventDto); | ||
assertTrue(resultEventDto.isSubscribed()); | ||
assertTrue(resultEventDto.isFavorite()); | ||
|
||
verify(restClient).findByEmail(user.getEmail()); | ||
verify(eventRepo).save(eventWithoutCoordinates); | ||
verify(tagService).findTagsWithAllTranslationsByNamesAndType(addEventDtoWithoutCoordinates.getTags(), | ||
TagType.EVENT); | ||
verify(eventRepo).findFavoritesAmongEventIds(eventIds, user.getId()); | ||
verify(eventRepo).findSubscribedAmongEventIds(eventIds, user.getId()); | ||
when(eventRepo.save(any(Event.class))).thenReturn(eventWithoutCoordinates); | ||
BadRequestException exception = assertThrows( | ||
BadRequestException.class, | ||
() -> eventService.save(addEventDtoWithoutCoordinates, email, null)); | ||
assertEquals(ErrorMessage.INVALID_COORDINATES, exception.getMessage()); | ||
verify(eventRepo, times(0)).save(eventWithoutCoordinates); | ||
} | ||
|
||
@Test | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need constants INVALID_LONGITUDE and INVALID_LATITUDE?
You don't use them anywhere.