Skip to content
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

Bugfix #7319 - Fixed incorrect saving for HabitTranslation for ua fields. #7898

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import org.springframework.validation.annotation.Validated;
import java.util.List;
import java.util.Set;

@Validated
@NoArgsConstructor
@AllArgsConstructor
@Builder
Expand All @@ -29,6 +31,7 @@ public class CustomHabitDtoRequest {
@NotNull(message = ServiceValidationConstants.HABIT_COMPLEXITY)
private Integer complexity;
private Integer defaultDuration;
@Valid
private List<HabitTranslationDto> habitTranslations;
private String image;
private List<CustomToDoListItemResponseDto> customToDoListItemDto;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package greencity.dto.habittranslation;

import java.io.Serializable;
import jakarta.validation.constraints.NotBlank;
import lombok.NoArgsConstructor;
import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
Expand All @@ -15,9 +16,12 @@
@EqualsAndHashCode
@Builder
public class HabitTranslationDto implements Serializable {
@NotBlank
private String description;
private String habitItem;
@NotBlank
private String languageCode;
@NotBlank
private String name;
private String descriptionUa;
private String nameUa;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,37 @@ protected HabitTranslation convert(HabitTranslationDto habitTranslationDto) {
.build();
}

/**
* Additional method that build {@link HabitTranslation} from
* {@link HabitTranslationDto} but from nameUa, descriptionUa, habitItemUa
* fields if they not null.
*
* @param habitTranslationDto {@link HabitTranslationDto}
* @return {@link HabitTranslation}
*
* @author Chernenko Vitaliy
*/
public HabitTranslation convertUa(HabitTranslationDto habitTranslationDto) {
HabitTranslation habitTranslation = new HabitTranslation();
if (habitTranslationDto.getNameUa() == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that logic can be simplified using defaultIfNull

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very useful thing. I think I'll use it in the future.
Thanks. Fixed.

habitTranslation.setName(habitTranslationDto.getName());
} else {
habitTranslation.setName(habitTranslationDto.getNameUa());
}
if (habitTranslationDto.getDescriptionUa() == null) {
habitTranslation.setDescription(habitTranslationDto.getDescription());
} else {
habitTranslation.setDescription(habitTranslationDto.getDescriptionUa());
}
if (habitTranslationDto.getHabitItemUa() == null) {
habitTranslation.setHabitItem(habitTranslationDto.getHabitItem());
} else {
habitTranslation.setHabitItem(habitTranslationDto.getHabitItemUa());
}

return habitTranslation;
}

/**
* Method that build {@link List} of {@link HabitTranslation} from {@link List}
* of {@link HabitTranslationDto}.
Expand All @@ -29,4 +60,22 @@ protected HabitTranslation convert(HabitTranslationDto habitTranslationDto) {
public List<HabitTranslation> mapAllToList(List<HabitTranslationDto> dtoList) {
return dtoList.stream().map(this::convert).collect(Collectors.toList());
}

/**
* Method that build {@link List} of {@link HabitTranslation} from {@link List}
* of {@link HabitTranslationDto} and {@link String} language.
*
* @param dtoList {@link List} of {@link HabitTranslationDto}
* @param language {@link String}
*
* @return {@link List} of {@link HabitTranslation}
*
* @author Chernenko Vitaliy
*/
public List<HabitTranslation> mapAllToList(List<HabitTranslationDto> dtoList, String language) {
if (language.equals("ua")) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move ua to the constant and compare like this to prevent NPE
"ua".equals(language)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll remember it.
Fixed.

return dtoList.stream().map(this::convertUa).collect(Collectors.toList());
}
return dtoList.stream().map(this::convert).collect(Collectors.toList());
}
}
17 changes: 11 additions & 6 deletions service/src/main/java/greencity/service/HabitServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -491,21 +491,26 @@ private void updateHabitTranslationsForCustomHabit(CustomHabitDtoRequest habitDt
}

private void saveHabitTranslationListsToHabitTranslationRepo(CustomHabitDtoRequest habitDto, Habit habit) {
List<HabitTranslation> habitTranslationListForUa = mapHabitTranslationFromAddCustomHabitDtoRequest(habitDto);
final String UA = "ua";
final String EN = "en";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move to the constants

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done

List<HabitTranslation> habitTranslationListForUa =
mapHabitTranslationFromAddCustomHabitDtoRequest(habitDto, UA);
habitTranslationListForUa.forEach(habitTranslation -> habitTranslation.setHabit(habit));
habitTranslationListForUa.forEach(habitTranslation -> habitTranslation.setLanguage(
languageRepo.findByCode("ua").orElseThrow(NoSuchElementException::new)));
languageRepo.findByCode(UA).orElseThrow(NoSuchElementException::new)));
habitTranslationRepo.saveAll(habitTranslationListForUa);

List<HabitTranslation> habitTranslationListForEn = mapHabitTranslationFromAddCustomHabitDtoRequest(habitDto);
List<HabitTranslation> habitTranslationListForEn =
mapHabitTranslationFromAddCustomHabitDtoRequest(habitDto, EN);
habitTranslationListForEn.forEach(habitTranslation -> habitTranslation.setHabit(habit));
habitTranslationListForEn.forEach(habitTranslation -> habitTranslation.setLanguage(
languageRepo.findByCode("en").orElseThrow(NoSuchElementException::new)));
languageRepo.findByCode(EN).orElseThrow(NoSuchElementException::new)));
habitTranslationRepo.saveAll(habitTranslationListForEn);
}

private List<HabitTranslation> mapHabitTranslationFromAddCustomHabitDtoRequest(CustomHabitDtoRequest habitDto) {
return habitTranslationMapper.mapAllToList(habitDto.getHabitTranslations());
private List<HabitTranslation> mapHabitTranslationFromAddCustomHabitDtoRequest(CustomHabitDtoRequest habitDto,
String language) {
return habitTranslationMapper.mapAllToList(habitDto.getHabitTranslations(), language);
}

private void setTagsIdsToHabit(CustomHabitDtoRequest habitDto, Habit habit) {
Expand Down
15 changes: 15 additions & 0 deletions service/src/test/java/greencity/ModelUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,12 @@ public class ModelUtils {
public static ZonedDateTime zonedDateTime = ZonedDateTime.now();
public static LocalDateTime localDateTime = LocalDateTime.now();
public static String habitTranslationName = "use shopper";
public static String habitTranslationNameUa = "Назва звички українською";
public static String habitTranslationDescription = "Description";
public static String habitTranslationDescriptionUa = "Опис звички українською";
public static String toDoListText = "buy a shopper";
public static String habitItem = "Item";
public static String habitItemUa = "Айтем звички українською";
public static String habitDefaultImage = "img/habit-default.png";
public static AddEventDtoRequest addEventDtoRequest = AddEventDtoRequest.builder()
.datesLocations(List.of(EventDateLocationDto.builder()
Expand Down Expand Up @@ -2577,6 +2580,18 @@ public static HabitTranslationDto getHabitTranslationDto() {
.build();
}

public static HabitTranslationDto getHabitTranslationDtoEnAndUa() {
return HabitTranslationDto.builder()
.description(habitTranslationDescription)
.habitItem(habitItem)
.name(habitTranslationName)
.languageCode("en")
.nameUa(habitTranslationNameUa)
.descriptionUa(habitTranslationDescriptionUa)
.habitItemUa(habitItemUa)
.build();
}

public static HabitTranslation getHabitTranslationForServiceTest() {
return HabitTranslation.builder()
.id(1L)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ void convertTest() {
assertEquals(expected, habitTranslationMapper.convert(habitTranslationDto));
}

@Test
void convertUaWithValidHabitTranslationDtoSucceeds() {
HabitTranslationDto habitTranslationDto = ModelUtils.getHabitTranslationDtoEnAndUa();

HabitTranslation expected = HabitTranslation.builder()
.description(habitTranslationDto.getDescriptionUa())
.habitItem(habitTranslationDto.getHabitItemUa())
.name(habitTranslationDto.getNameUa())
.build();
assertEquals(expected, habitTranslationMapper.convertUa(habitTranslationDto));
}

@Test
void mapAllToListTest() {
HabitTranslationDto habitTranslationDto = ModelUtils.getHabitTranslationDto();
Expand All @@ -42,4 +54,34 @@ void mapAllToListTest() {
List<HabitTranslation> expectedList = List.of(expected);
assertEquals(expectedList, habitTranslationMapper.mapAllToList(habitTranslationDtoList));
}

@Test
void mapAllToListWithEnLanguageReturnsList() {
HabitTranslationDto habitTranslationDto = ModelUtils.getHabitTranslationDtoEnAndUa();
List<HabitTranslationDto> habitTranslationDtoList = List.of(habitTranslationDto);

HabitTranslation expected = HabitTranslation.builder()
.description(habitTranslationDto.getDescription())
.habitItem(habitTranslationDto.getHabitItem())
.name(habitTranslationDto.getName())
.build();
List<HabitTranslation> expectedList = List.of(expected);

assertEquals(expectedList, habitTranslationMapper.mapAllToList(habitTranslationDtoList, "en"));
}

@Test
void mapAllToListWithUaLanguageReturnsList() {
HabitTranslationDto habitTranslationDto = ModelUtils.getHabitTranslationDtoEnAndUa();
List<HabitTranslationDto> habitTranslationDtoList = List.of(habitTranslationDto);

HabitTranslation expected = HabitTranslation.builder()
.description(habitTranslationDto.getDescriptionUa())
.habitItem(habitTranslationDto.getHabitItemUa())
.name(habitTranslationDto.getNameUa())
.build();
List<HabitTranslation> expectedList = List.of(expected);

assertEquals(expectedList, habitTranslationMapper.mapAllToList(habitTranslationDtoList, "ua"));
}
}
37 changes: 26 additions & 11 deletions service/src/test/java/greencity/service/HabitServiceImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,9 @@ void addCustomHabitTestWithImagePathInDto() throws IOException {
when(userRepo.findByEmail(user.getEmail())).thenReturn(Optional.of(user));
when(habitRepo.save(customHabitMapper.convert(addCustomHabitDtoRequest))).thenReturn(habit);
when(tagsRepo.findById(20L)).thenReturn(Optional.of(tag));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto)))
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "ua"))
.thenReturn(List.of(habitTranslationUa));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "en"))
.thenReturn(List.of(habitTranslationUa));
when(languageRepo.findByCode("ua")).thenReturn(Optional.of(languageUa));
when(languageRepo.findByCode("en")).thenReturn(Optional.of(languageEn));
Expand All @@ -641,7 +643,8 @@ void addCustomHabitTestWithImagePathInDto() throws IOException {
verify(habitRepo).save(customHabitMapper.convert(addCustomHabitDtoRequest));
verify(customHabitMapper, times(3)).convert(addCustomHabitDtoRequest);
verify(tagsRepo).findById(20L);
verify(habitTranslationMapper, times(2)).mapAllToList(List.of(habitTranslationDto));
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "ua");
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "en");
verify(languageRepo, times(2)).findByCode(anyString());
verify(customToDoListItemRepo).findAllByUserIdAndHabitId(1L, 1L);
verify(customToDoListMapper).mapAllToList(anyList());
Expand Down Expand Up @@ -692,8 +695,10 @@ void addCustomHabitTestWithImageFile() throws IOException {
when(languageRepo.findByCode("ua")).thenReturn(Optional.of(languageUa));
when(languageRepo.findByCode("en")).thenReturn(Optional.of(languageEn));
when(customToDoListItemRepo.findAllByUserIdAndHabitId(1L, 1L)).thenReturn(List.of(customToDoListItem));
when(customToDoListMapper.mapAllToList(List.of(customToDoListItemResponseDto)))
.thenReturn(List.of(customToDoListItem));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "ua"))
.thenReturn(List.of(habitTranslationUa));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "en"))
.thenReturn(List.of(habitTranslationUa));
when(modelMapper.map(habit, CustomHabitDtoResponse.class)).thenReturn(addCustomHabitDtoResponse);
when(customToDoListResponseDtoMapper.mapAllToList(List.of(customToDoListItem)))
.thenReturn(List.of(customToDoListItemResponseDto));
Expand All @@ -709,7 +714,8 @@ void addCustomHabitTestWithImageFile() throws IOException {
verify(habitRepo).save(customHabitMapper.convert(addCustomHabitDtoRequest));
verify(customHabitMapper, times(3)).convert(addCustomHabitDtoRequest);
verify(tagsRepo).findById(20L);
verify(habitTranslationMapper, times(2)).mapAllToList(List.of(habitTranslationDto));
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "ua");
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "en");
verify(languageRepo, times(2)).findByCode(anyString());
verify(customToDoListItemRepo).findAllByUserIdAndHabitId(1L, 1L);
verify(customToDoListMapper).mapAllToList(anyList());
Expand Down Expand Up @@ -755,7 +761,9 @@ void addCustomHabitTest2() throws IOException {
when(userRepo.findByEmail(user.getEmail())).thenReturn(Optional.of(user));
when(habitRepo.save(customHabitMapper.convert(addCustomHabitDtoRequest))).thenReturn(habit);
when(tagsRepo.findById(20L)).thenReturn(Optional.of(tag));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto)))
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "ua"))
.thenReturn(List.of(habitTranslationUa));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto), "en"))
.thenReturn(List.of(habitTranslationUa));
when(languageRepo.findByCode("ua")).thenReturn(Optional.of(languageUa));
when(languageRepo.findByCode("en")).thenReturn(Optional.of(languageEn));
Expand All @@ -777,7 +785,8 @@ void addCustomHabitTest2() throws IOException {
verify(habitRepo).save(customHabitMapper.convert(addCustomHabitDtoRequest));
verify(customHabitMapper, times(3)).convert(addCustomHabitDtoRequest);
verify(tagsRepo).findById(20L);
verify(habitTranslationMapper, times(2)).mapAllToList(List.of(habitTranslationDto));
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "ua");
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "en");
verify(languageRepo, times(2)).findByCode(anyString());
verify(customToDoListItemRepo).findAllByUserIdAndHabitId(1L, 1L);
verify(customToDoListMapper).mapAllToList(anyList());
Expand Down Expand Up @@ -807,7 +816,9 @@ void addCustomHabitNoSuchElementExceptionWithNotExistingLanguageCodeTestUa() thr
when(userRepo.findByEmail(user.getEmail())).thenReturn(Optional.of(user));
when(habitRepo.save(customHabitMapper.convert(addCustomHabitDtoRequest))).thenReturn(habit);
when(tagsRepo.findById(20L)).thenReturn(Optional.of(tag));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto)))
when(habitTranslationMapper.mapAllToList(addCustomHabitDtoRequest.getHabitTranslations(), "ua"))
.thenReturn(List.of(habitTranslation));
when(habitTranslationMapper.mapAllToList(addCustomHabitDtoRequest.getHabitTranslations(), "en"))
.thenReturn(List.of(habitTranslation));
when(languageRepo.findByCode("ua")).thenReturn(Optional.empty());

Expand All @@ -818,7 +829,8 @@ void addCustomHabitNoSuchElementExceptionWithNotExistingLanguageCodeTestUa() thr
verify(habitRepo).save(customHabitMapper.convert(addCustomHabitDtoRequest));
verify(customHabitMapper, times(3)).convert(addCustomHabitDtoRequest);
verify(tagsRepo).findById(20L);
verify(habitTranslationMapper).mapAllToList(addCustomHabitDtoRequest.getHabitTranslations());
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "ua");
verify(habitTranslationMapper, times(0)).mapAllToList(List.of(habitTranslationDto), "en");
verify(languageRepo).findByCode(anyString());
}

Expand All @@ -842,7 +854,9 @@ void addCustomHabitNoSuchElementExceptionWithNotExistingLanguageCodeEn() throws
when(userRepo.findByEmail(user.getEmail())).thenReturn(Optional.of(user));
when(habitRepo.save(customHabitMapper.convert(addCustomHabitDtoRequest))).thenReturn(habit);
when(tagsRepo.findById(20L)).thenReturn(Optional.of(tag));
when(habitTranslationMapper.mapAllToList(List.of(habitTranslationDto)))
when(habitTranslationMapper.mapAllToList(addCustomHabitDtoRequest.getHabitTranslations(), "ua"))
.thenReturn(List.of(habitTranslationUa));
when(habitTranslationMapper.mapAllToList(addCustomHabitDtoRequest.getHabitTranslations(), "en"))
.thenReturn(List.of(habitTranslationUa));
when(languageRepo.findByCode("ua")).thenReturn(Optional.of(languageUa));
when(languageRepo.findByCode("en")).thenReturn(Optional.empty());
Expand All @@ -855,7 +869,8 @@ void addCustomHabitNoSuchElementExceptionWithNotExistingLanguageCodeEn() throws
verify(customHabitMapper, times(3)).convert(addCustomHabitDtoRequest);
verify(tagsRepo).findById(20L);

verify(habitTranslationMapper, times(2)).mapAllToList(addCustomHabitDtoRequest.getHabitTranslations());
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "ua");
verify(habitTranslationMapper, times(1)).mapAllToList(List.of(habitTranslationDto), "en");
verify(languageRepo, times(2)).findByCode(anyString());
}

Expand Down
Loading