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

DayLog의 Item들 순서 변경 기능 구현 #129

Merged
merged 10 commits into from
Jul 25, 2023
13 changes: 11 additions & 2 deletions backend/src/docs/asciidoc/docs.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ include::{snippets}/trip-controller-test/get-trips/http-request.adoc[]

==== 응답
include::{snippets}/trip-controller-test/get-trips/http-response.adoc[]
include::{snippets}/day-log-controller-test/get-trip/response-fields.adoc[]
include::{snippets}/trip-controller-test/get-trips/response-fields.adoc[]

=== 단일 여행 생성 (POST /trips/:tripId/daylogs/:dayLogId)
=== 단일 여행 생성 (POST /trips)

==== 요청
include::{snippets}/trip-controller-test/create-trip/http-request.adoc[]
Expand Down Expand Up @@ -115,6 +115,15 @@ include::{snippets}/day-log-controller-test/update-day-log-title/request-fields.
include::{snippets}/day-log-controller-test/update-day-log-title/http-response.adoc[]


=== DayLog 아이템들 순서 수정 (PATCH /trips/:tripId/daylogs/:dayLogId/order)

==== 요청
include::{snippets}/day-log-controller-test/update-ordinal-of-items/http-request.adoc[]
include::{snippets}/day-log-controller-test/update-ordinal-of-items/path-parameters.adoc[]
include::{snippets}/day-log-controller-test/update-ordinal-of-items/request-fields.adoc[]

==== 응답
include::{snippets}/day-log-controller-test/update-day-log-title/http-response.adoc[]
Comment on lines +118 to +126
Copy link
Collaborator

Choose a reason for hiding this comment

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

RestDocs 💯


== 여행 아이템 API

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ public enum ExceptionCode {
ALREADY_DELETED_TRIP_ITEM(2001, "이미 삭제된 여행 아이템입니다."),
ALREADY_DELETED_DATE(2002, "이미 삭제된 날짜입니다."),

INVALID_RATING(3001, "별점은 N.0점이거나 N.5점 형태이어야 합니다.");
INVALID_RATING(3001, "별점은 N.0점이거나 N.5점 형태이어야 합니다."),

INVALID_ORDERED_ITEM_IDS(4001, "날짜에 속한 모든 여행 아이템들의 ID가 필요합니다.");

private final int code;
private final String message;
Expand Down
32 changes: 11 additions & 21 deletions backend/src/main/java/hanglog/trip/domain/Item.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ public Item(
final String memo,
final Place place,
final DayLog dayLog,
final Expense expense
final Expense expense,
final List<Image> images
) {
this(id, itemType, title, ordinal, rating, memo, place, dayLog, expense, new ArrayList<>(), USABLE);
this(id, itemType, title, ordinal, rating, memo, place, dayLog, expense, images, USABLE);
}

public Item(
Expand All @@ -130,10 +131,9 @@ public Item(
final String memo,
final Place place,
final DayLog dayLog,
final Expense expense,
final List<Image> images
final Expense expense
) {
this(id, itemType, title, ordinal, rating, memo, place, dayLog, expense, images, USABLE);
this(id, itemType, title, ordinal, rating, memo, place, dayLog, expense, new ArrayList<>(), USABLE);
}

public Item(
Expand All @@ -144,10 +144,9 @@ public Item(
final Double rating,
final String memo,
final DayLog dayLog,
final Expense expense,
final List<Image> images
final Expense expense
) {
this(id, itemType, title, ordinal, rating, memo, null, dayLog, expense, images, USABLE);
this(id, itemType, title, ordinal, rating, memo, null, dayLog, expense, new ArrayList<>());
}

public Item(
Expand All @@ -164,22 +163,13 @@ public Item(
this(null, itemType, title, ordinal, rating, memo, place, dayLog, expense, images);
}

public Item(
final Long id,
final ItemType itemType,
final String title,
final Integer ordinal,
final Double rating,
final String memo,
final DayLog dayLog,
final Expense expense
) {
this(id, itemType, title, ordinal, rating, memo, null, dayLog, expense, new ArrayList<>());
}

private void validateRatingFormat(final Double rating) {
if (rating % RATING_DECIMAL_UNIT != 0) {
throw new InvalidDomainException(INVALID_RATING);
}
}

public void changeOrdinal(final int ordinal) {
this.ordinal = ordinal;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package hanglog.trip.dto.request;

import jakarta.validation.constraints.NotEmpty;
import java.util.List;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@AllArgsConstructor
@NoArgsConstructor
public class ItemsOrdinalUpdateRequest {
@NotEmpty(message = "아이템 아이디들을 입력해주세요.")
private List<Long> itemIds;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@


import hanglog.trip.dto.request.DayLogUpdateTitleRequest;
import hanglog.trip.dto.request.ItemsOrdinalUpdateRequest;
import hanglog.trip.dto.response.DayLogGetResponse;
import hanglog.trip.service.DayLogService;
import jakarta.validation.Valid;
Expand Down Expand Up @@ -37,4 +38,13 @@ public ResponseEntity<Void> updateDayLogTitle(
dayLogService.updateTitle(dayLogId, request);
return ResponseEntity.noContent().build();
}

@PatchMapping("/order")
public ResponseEntity<Void> updateOrdinalOfItems(
@PathVariable final Long tripId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 안쓰이는거 같아요!

@PathVariable final Long dayLogId,
@RequestBody @Valid final ItemsOrdinalUpdateRequest itemsOrdinalUpdateRequest) {
dayLogService.updateOrdinalOfDayLogItems(dayLogId, itemsOrdinalUpdateRequest);
return ResponseEntity.noContent().build();
}
}
36 changes: 36 additions & 0 deletions backend/src/main/java/hanglog/trip/service/DayLogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@


import static hanglog.global.exception.ExceptionCode.ALREADY_DELETED_DATE;
import static hanglog.global.exception.ExceptionCode.INVALID_ORDERED_ITEM_IDS;
import static hanglog.global.exception.ExceptionCode.NOT_FOUND_DAY_LOG_ID;
import static hanglog.global.exception.ExceptionCode.NOT_FOUND_TRIP_ID;
import static hanglog.global.exception.ExceptionCode.NOT_FOUND_TRIP_ITEM_ID;

import hanglog.global.exception.BadRequestException;
import hanglog.trip.domain.DayLog;
import hanglog.trip.domain.Item;
import hanglog.trip.domain.repository.DayLogRepository;
import hanglog.trip.domain.repository.ItemRepository;
import hanglog.trip.dto.request.DayLogUpdateTitleRequest;
import hanglog.trip.dto.request.ItemsOrdinalUpdateRequest;
import hanglog.trip.dto.response.DayLogGetResponse;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -20,6 +26,7 @@
public class DayLogService {

private final DayLogRepository dayLogRepository;
private final ItemRepository itemRepository;

@Transactional(readOnly = true)
public DayLogGetResponse getById(final Long id) {
Expand Down Expand Up @@ -50,4 +57,33 @@ private void validateAlreadyDeleted(final DayLog dayLog) {
throw new BadRequestException(ALREADY_DELETED_DATE);
}
}

public void updateOrdinalOfDayLogItems(final Long dayLogId,
final ItemsOrdinalUpdateRequest itemsOrdinalUpdateRequest) {
final DayLog dayLog = dayLogRepository.findById(dayLogId)
.orElseThrow(() -> new BadRequestException(NOT_FOUND_DAY_LOG_ID));
final List<Item> items = dayLog.getItems();

final List<Long> orderedItemIds = itemsOrdinalUpdateRequest.getItemIds();
validateOrderedItemIds(items, orderedItemIds);
changeOrdinalOfItemsByOrderedItemIds(orderedItemIds);
}

private void validateOrderedItemIds(final List<Item> items, final List<Long> orderedItemIds) {
for (final Item item : items) {
if (!orderedItemIds.contains(item.getId())) {
throw new BadRequestException(INVALID_ORDERED_ITEM_IDS);
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private void validateOrderedItemIds(final List<Item> items, final List<Long> orderedItemIds) {
for (final Item item : items) {
if (!orderedItemIds.contains(item.getId())) {
throw new BadRequestException(INVALID_ORDERED_ITEM_IDS);
}
}
}
private void validateOrderedItemIds(final List<Item> items, final List<Long> orderedItemIds) {
items.stream().filter(item -> !orderedItemIds.contains(item.getId())).forEach(item -> {
throw new BadRequestException(INVALID_ORDERED_ITEM_IDS);
});
}

어떨까요

Copy link
Member

Choose a reason for hiding this comment

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

어 이미 바꼈네


private void changeOrdinalOfItemsByOrderedItemIds(final List<Long> orderedItemIds) {
int ordinal = 1;

for (final Long itemId : orderedItemIds) {
final Item item = itemRepository.findById(itemId)
.orElseThrow(() -> new BadRequestException(NOT_FOUND_TRIP_ITEM_ID));
item.changeOrdinal(ordinal++);
}
}
}
Comment on lines +85 to 94
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분 때문에 DayLogServiceItemRepository를 주입받고 있습니다.
ItemRepository에 의존하는 게 싫다면 DayLog.getItem()을 사용하는 방법이 있긴 한데,
Stream으로 해당 id를 가지는 객체를 찾거나(O(n)) 몇 줄의 코드를 더 추가해서 탐색 시간을 줄여야 할 것 같은데... 어떻게 생각하시나요?

13 changes: 12 additions & 1 deletion backend/src/test/java/hanglog/trip/fixture/ItemFixture.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public final class ItemFixture {

private static final DayLog DAYLOG_FOR_ITEM_FIXTURE = new DayLog(
public static final DayLog DAYLOG_FOR_ITEM_FIXTURE = new DayLog(
"첫날",
1,
TripFixture.LONDON_TRIP
Expand Down Expand Up @@ -34,4 +34,15 @@ public final class ItemFixture {
DAYLOG_FOR_ITEM_FIXTURE,
ExpenseFixture.EURO_10000
);

public static final Item AIRPLANE_ITEM = new Item(
3L,
ItemType.NON_SPOT,
"비행기",
3,
4.5,
"런던에서 탄 비행기",
DAYLOG_FOR_ITEM_FIXTURE,
ExpenseFixture.EURO_10000
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@
import static org.springframework.restdocs.request.RequestDocumentation.pathParameters;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import hanglog.trip.dto.request.DayLogUpdateTitleRequest;
import hanglog.trip.dto.request.ItemsOrdinalUpdateRequest;
import hanglog.trip.dto.response.DayLogGetResponse;
import hanglog.trip.restdocs.RestDocsTest;
import hanglog.trip.service.DayLogService;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -122,4 +125,35 @@ void updateDayLogTitle() throws Exception {
)
);
}

@DisplayName("데이로그의 아이템들 순서를 변경할 수 있다.")
@Test
void updateOrdinalOfItems() throws Exception {
//given
final ItemsOrdinalUpdateRequest request = new ItemsOrdinalUpdateRequest(List.of(3L,2L,1L));

doNothing().when(dayLogService).updateOrdinalOfDayLogItems( any(), any());

// when & then
mockMvc.perform(patch("/trips/{tripId}/daylogs/{dayLogId}/order", 1L, 1L)
.contentType(APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isNoContent())
.andDo(
restDocs.document(
pathParameters(
parameterWithName("tripId")
.description("여행 ID"),
parameterWithName("dayLogId")
.description("날짜별 기록 ID")
),
requestFields(
fieldWithPath("itemIds")
.type(JsonFieldType.ARRAY)
.description("정렬된 아이템의 IDs")
Copy link
Collaborator

Choose a reason for hiding this comment

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

새로운 순서의 아이템 IDs 어떤가요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

옼케이

.attributes(field("constraint", "ID 배열"))
)
)
);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
package hanglog.trip.service;

import static hanglog.trip.fixture.DayLogFixture.*;
import static hanglog.trip.fixture.DayLogFixture.LONDON_DAYLOG_1;
import static hanglog.trip.fixture.DayLogFixture.UPDATED_LONDON_DAYLOG;
import static hanglog.trip.fixture.ItemFixture.DAYLOG_FOR_ITEM_FIXTURE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;

import hanglog.trip.domain.DayLog;
import hanglog.trip.domain.Item;
import hanglog.trip.domain.repository.DayLogRepository;
import hanglog.trip.domain.repository.ItemRepository;
import hanglog.trip.dto.request.DayLogUpdateTitleRequest;
import hanglog.trip.dto.request.ItemsOrdinalUpdateRequest;
import hanglog.trip.dto.response.DayLogGetResponse;
import java.time.LocalDate;
import java.util.List;
Expand All @@ -29,6 +35,9 @@ class DayLogServiceTest {
@Mock
private DayLogRepository dayLogRepository;

@Mock
private ItemRepository itemRepository;

@DisplayName("날짜별 여행을 조회할 수 있다.")
@Test
void getDayLogById() {
Expand Down Expand Up @@ -68,4 +77,30 @@ void updateTitle() {
verify(dayLogRepository).findById(UPDATED_LONDON_DAYLOG.getId());
verify(dayLogRepository).save(any(DayLog.class));
}

@DisplayName("정렬된 아이템 ID 리스트를 받아 아이템들의 순서를 변경한다.")
@Test
void updateItemOrdinals() {
// given
final ItemsOrdinalUpdateRequest request = new ItemsOrdinalUpdateRequest(List.of(3L, 2L, 1L));
given(dayLogRepository.findById(1L))
.willReturn(Optional.of(DAYLOG_FOR_ITEM_FIXTURE));
given(itemRepository.findById(1L))
.willReturn(Optional.ofNullable(DAYLOG_FOR_ITEM_FIXTURE.getItems().get(0)));
given(itemRepository.findById(2L))
.willReturn(Optional.ofNullable(DAYLOG_FOR_ITEM_FIXTURE.getItems().get(1)));
given(itemRepository.findById(3L))
.willReturn(Optional.ofNullable(DAYLOG_FOR_ITEM_FIXTURE.getItems().get(2)));

// when
dayLogService.updateOrdinalOfDayLogItems(1L, request);

// then
List<Item> items = DAYLOG_FOR_ITEM_FIXTURE.getItems();
Copy link
Collaborator

Choose a reason for hiding this comment

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

final 가능?

assertSoftly(softly -> {
softly.assertThat(items.get(0).getOrdinal()).isEqualTo(3);
softly.assertThat(items.get(1).getOrdinal()).isEqualTo(2);
softly.assertThat(items.get(2).getOrdinal()).isEqualTo(1);
});
}
}