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

신고 조치 예정 목록 조회 기능 구현 #750

Merged
merged 16 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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 @@ -13,7 +13,7 @@

@RequiredArgsConstructor
@RestController
public class ReportCommandCommandController implements ReportCommandControllerDocs {
public class ReportCommandController implements ReportCommandControllerDocs {

private final ReportCommandService reportCommandService;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.votogether.domain.report.controller;

import com.votogether.domain.report.dto.response.ReportPageResponse;
import com.votogether.domain.report.service.ReportQueryService;
import jakarta.validation.constraints.PositiveOrZero;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@Validated
@RequiredArgsConstructor
@RestController
public class ReportQueryController implements ReportQueryControllerDocs {

private final ReportQueryService reportQueryService;

@GetMapping("/reports/admin")
Copy link
Collaborator

Choose a reason for hiding this comment

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

/reports는 클래스 레벨에 @RequestMapping을 통해 매핑시키는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

다른 컨트롤러 보니까 메서드가 하나만 있는 경우엔 그냥 메서드에 붙여져 있더라구요. 그래서 가독성을 더 챙기기 위해 그렇게 했다고 생각했어서 똑같이 이렇게 하긴 했는데, 다즐의 의견이 궁금합니다 :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

현재는 메서드가 1개밖에 없기에 지금으로도 괜찮다고 생각해요 ㅎㅎ

메서드가 추가된다면 분리하는 것으로 개선해봐도 좋을 것 같아요 👍

public ResponseEntity<ReportPageResponse> getReports(
@RequestParam @PositiveOrZero(message = "페이지는 0이상 정수만 가능합니다.") final int page
) {
final ReportPageResponse reportPageResponse = reportQueryService.getReports(page);
return ResponseEntity.ok(reportPageResponse);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.votogether.domain.report.controller;

import com.votogether.domain.report.dto.response.ReportPageResponse;
import com.votogether.global.exception.ExceptionResponse;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.constraints.PositiveOrZero;
import org.springframework.http.ResponseEntity;

@Tag(name = "신고 조회", description = "신고 조회 API")
public interface ReportQueryControllerDocs {

@Operation(summary = "신고 조치 예정 목록 조회", description = "신고 조치 예정 목록을 조회한다.")
@ApiResponses({
@ApiResponse(
responseCode = "200",
description = "신고 조치 예정 목록 조회 성공"
),
@ApiResponse(
responseCode = "400",
description = "0이상의 정수가 아닌 페이지",
content = @Content(schema = @Schema(implementation = ExceptionResponse.class))
)
})
ResponseEntity<ReportPageResponse> getReports(
@PositiveOrZero(message = "페이지는 0이상 정수만 가능합니다.") final int page
);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.votogether.domain.report.dto;

import com.votogether.domain.report.entity.vo.ReportType;
import java.time.LocalDateTime;

public record ReportAggregateDto(
long reportMaxId,
ReportType reportType,
long targetId,
String reasons,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q
String 타입인데 reasons인 이유가 여러 사유들을 concat하기 때문인가요?
그렇다면 api명세 수정이 필요해보입니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ReportAggregateDto는 api에 관련된 것이라기 보단, ReportCustomRepositoryImpl 클래스의 findReportsGroupedByReportTypeAndTargetId에서 데이터를 조회할 때 매핑할 용도로 만들었습니다.

Service 계층에서 ReportAggregateDto를 ReportPageResponse로 매핑하는 과정이 있어서 괜찮을 것 같다고 생각하는데 어떻게 생각하시나요

Copy link
Collaborator

Choose a reason for hiding this comment

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

String인데 reasons인 이유가 궁금했습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

String이긴 한데 일단 쉼표로 이어져 있는 방식의 String이기 때문에 reasons로 했는데, 좀 더 가독성 좋은 변수명이 있을까요.
제 머리의 한계가...ㅋㅋ

LocalDateTime createdAt
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.votogether.domain.report.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;

@Schema(description = "신고 조치 예정 목록 응답")
public record ReportPageResponse(
@Schema(description = "신고 조치 예정 목록 전체 페이지 수", example = "20")
long totalPageNumber,

@Schema(description = "신고 조치 예정 목록 중 현재 페이지", example = "3")
long currentPageNumber,

@Schema(description = "신고 조치 예정 목록")
List<ReportResponse> reports
) {

public static ReportPageResponse of(
final int totalPageNumber,
final int currentPageNumber,
final List<ReportResponse> reportResponses
) {
return new ReportPageResponse(
totalPageNumber,
currentPageNumber,
reportResponses
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.votogether.domain.report.dto.response;

import com.fasterxml.jackson.annotation.JsonFormat;
import com.votogether.domain.report.dto.ReportAggregateDto;
import com.votogether.domain.report.entity.Report;
import com.votogether.domain.report.entity.vo.ReportType;
import io.swagger.v3.oas.annotations.media.Schema;
import java.time.LocalDateTime;
import java.util.Arrays;
import java.util.List;

@Schema(description = "신고 정보 응답")
public record ReportResponse(
@Schema(description = "신고 ID", example = "1")
long id,

@Schema(description = "신고 유형", example = "POST")
ReportType type,

@Schema(description = "신고 이유들")
List<String> reasons,

@Schema(description = "신고 당한 요소의 내용", example = "2")
String target,

@Schema(description = "신고 생성시간", example = "2023-08-01 13:56")
@JsonFormat(pattern = "yyyy-MM-dd HH:mm")
LocalDateTime createdAt
) {

public static ReportResponse of(final ReportAggregateDto reportAggregateDto, final String target) {
return new ReportResponse(
reportAggregateDto.reportMaxId(),
reportAggregateDto.reportType(),
Arrays.stream(reportAggregateDto.reasons().split(",")).toList(),
target,
reportAggregateDto.createdAt()
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.votogether.domain.report.repository;

import com.votogether.domain.report.dto.ReportAggregateDto;
import java.util.List;
import org.springframework.data.domain.Pageable;

public interface ReportCustomRepository {

List<ReportAggregateDto> findReportsGroupedByReportTypeAndTargetId(final Pageable pageable);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.votogether.domain.report.repository;

import static com.votogether.domain.report.entity.QReport.report;

import com.querydsl.core.types.Projections;
import com.querydsl.core.types.dsl.Expressions;
import com.querydsl.jpa.impl.JPAQueryFactory;
import com.votogether.domain.report.dto.ReportAggregateDto;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Repository;

@RequiredArgsConstructor
@Repository
public class ReportCustomRepositoryImpl implements ReportCustomRepository {

private final JPAQueryFactory jpaQueryFactory;

@Override
public List<ReportAggregateDto> findReportsGroupedByReportTypeAndTargetId(final Pageable pageable) {
return jpaQueryFactory.select(
Projections.constructor(
ReportAggregateDto.class,
report.id.max(),
report.reportType,
report.targetId,
Expressions.stringTemplate("group_concat({0})", report.reason),
Copy link
Collaborator

Choose a reason for hiding this comment

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

사유 여러 개를 이런 방법으로 묶을 수 있군요 배워갑니다 🙇🏻‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

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

Q
이 식이 이해가 잘 안가는데 어떻게 동작하는건가요?
id와 createdAt에 max를 걸어준 이유가 무엇인가요?

Copy link
Collaborator Author

@tjdtls690 tjdtls690 Oct 17, 2023

Choose a reason for hiding this comment

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

동작 방식은reportType, targetId를 group by로 묶어서 reportType, targetId가 같은 레코드끼리는 하나로 합쳐져서 반환됩니다.

여기서 문제는 reportType, targetId는 group by로 묶였기 때문에 1개씩 잘 나오게 되는데, report의 id, reason, createdAt은 여러 행이 나올 수 있는 컬럼들입니다. 그래서 max()는 여러개 묶인 것 중 가장 최신 것을 가져오기 위해 max()를 사용하였습니다.

.orderBy(report.id.max().desc())

위처럼 정렬할 때도 최신순으로 정렬하도록 구현했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

레전드 설명 잘 들었습니다.
이해 되었습니다!

report.createdAt.max()
)
)
.from(report)
.orderBy(report.id.max().desc())
Copy link
Collaborator

Choose a reason for hiding this comment

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

최대 ID로 정렬하는 방법 좋네요 👍

.groupBy(report.reportType, report.targetId)
.offset(pageable.getOffset())
.limit(pageable.getPageSize())
.fetch();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

public interface ReportRepository extends JpaRepository<Report, Long> {
public interface ReportRepository extends JpaRepository<Report, Long>, ReportCustomRepository {

int countByReportTypeAndTargetId(final ReportType reportType, final Long targetId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,21 @@

import com.votogether.domain.member.entity.Member;
import com.votogether.domain.report.dto.request.ReportRequest;
import com.votogether.domain.report.entity.vo.ReportType;
import com.votogether.domain.report.service.strategy.ReportCommentStrategy;
import com.votogether.domain.report.service.strategy.ReportNicknameStrategy;
import com.votogether.domain.report.service.strategy.ReportPostStrategy;
import com.votogether.domain.report.service.strategy.ReportActionProvider;
import com.votogether.domain.report.service.strategy.ReportStrategy;
import java.util.EnumMap;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Transactional
@Service
public class ReportCommandService {

private final Map<ReportType, ReportStrategy> reportActions;

public ReportCommandService(
final ReportPostStrategy reportPostStrategy,
final ReportCommentStrategy reportCommentStrategy,
final ReportNicknameStrategy reportNicknameStrategy
) {
this.reportActions = new EnumMap<>(ReportType.class);
this.reportActions.put(ReportType.POST, reportPostStrategy);
this.reportActions.put(ReportType.COMMENT, reportCommentStrategy);
this.reportActions.put(ReportType.NICKNAME, reportNicknameStrategy);
}
private final ReportActionProvider reportActionProvider;
Comment on lines 12 to +16
Copy link
Collaborator

Choose a reason for hiding this comment

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

provider로 묶으니까 훨씬 깔끔해지군요!


public void report(final Member reporter, final ReportRequest request) {
final ReportStrategy reportStrategy = reportActions.get(request.type());
final ReportStrategy reportStrategy = reportActionProvider.getStrategy(request.type());
reportStrategy.report(reporter, request);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.votogether.domain.report.service;

import com.votogether.domain.report.dto.ReportAggregateDto;
import com.votogether.domain.report.dto.response.ReportResponse;
import com.votogether.domain.report.dto.response.ReportPageResponse;
import com.votogether.domain.report.repository.ReportRepository;
import com.votogether.domain.report.service.strategy.ReportActionProvider;
import com.votogether.domain.report.service.strategy.ReportStrategy;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Transactional(readOnly = true)
@Service
public class ReportQueryService {

private static final int BASIC_PAGE_SIZE = 20;

private final ReportRepository reportRepository;
private final ReportActionProvider reportActionProvider;

public ReportPageResponse getReports(final int page) {
final long totalCount = reportRepository.count();
final int totalPageNumber = (int) Math.ceil((double) totalCount / BASIC_PAGE_SIZE);

final Pageable pageable = PageRequest.of(page, BASIC_PAGE_SIZE);
final List<ReportAggregateDto> reportAggregateDtos = reportRepository
.findReportsGroupedByReportTypeAndTargetId(pageable);
final List<ReportResponse> reportResponses = parseReportResponses(reportAggregateDtos);

return ReportPageResponse.of(totalPageNumber, page, reportResponses);
}

private List<ReportResponse> parseReportResponses(final List<ReportAggregateDto> reportAggregateDtos) {
return reportAggregateDtos.stream()
.map(this::parseReportResponse)
.toList();
}

private ReportResponse parseReportResponse(final ReportAggregateDto reportAggregateDto) {
final ReportStrategy strategy = reportActionProvider.getStrategy(reportAggregateDto.reportType());
return ReportResponse.of(reportAggregateDto, strategy.parseTarget(reportAggregateDto.targetId()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q
parseTarget이라는 메서드 명이 적절한 지 의문이 듭니다!
Target을 파싱해주는 역할이 맞나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

뭔가 target으로 파싱한다는 의미로 했었었는데, 이게 Post의 id로 변할수도, 댓글 내용이나 멤버의 닉네임으로 변할 수도 있어서 어떤 이름이 좋을지 잘 생각이 안나네요.

targetId를 파싱한다는 의미에서 parseTargetId가 나을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

GPT한테 추천 메서드명을 물어보니,

  1. retrieveTargetInfo(): "검색"이라는 뜻을 갖는 retrieve와 "정보"라는 뜻을 갖는 info를 합쳐서 대상의 정보를 검색한다는 의미를 잘 전달하는 이름입니다.
  2. getTargetDescription(): 대상의 설명이나 정보를 얻어오는 것에 중점을 둔 이름입니다.
  3. identifyTarget(): 대상을 식별한다는 의미로, 어떠한 식별자를 사용하여 특정 대상을 구분하거나 식별하는 작업에 적합한 이름입니다.
  4. lookupTargetDetails(): "조회"라는 뜻의 lookup와 "세부 사항"이라는 뜻의 details를 합친 이름으로, 대상의 세부 사항을 조회한다는 의미를 가집니다.

이렇게 나오네요...ㅋㅋ

Copy link
Collaborator

Choose a reason for hiding this comment

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

뭔가 parse라는 단어의 의미랑 맞지 않아서 한번에 이해가 잘 되지 않았어요.
제가 아는 parse의 의미는 "분해하다"라서요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

개인적으로 parse 라는 이름을 많이 쓰는 이유가, 자바에서 제공하는 라이브러리에서도 Integer의 parseInt를 참고했었는데요. parse가 분석하다라는 뜻을 가지고 있어서, 분석해서 변환한다 라는 의미로 받아들였습니다.

}

}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.votogether.domain.report.service.strategy;

import com.votogether.domain.report.entity.vo.ReportType;
import java.util.EnumMap;
import java.util.Map;
import org.springframework.stereotype.Component;

@Component
public class ReportActionProvider {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q

  1. 클래스 명을 위와 같이 지으신 이유가 궁금합니다.
  2. 신고 타입에 대해 전략을 뱉어주는 방법에서 이 방식 외에 어떤 다른 방식도 생각해보셨는지 궁금합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. 클래스 명은 신고 관련 동작을 제공해준다는 의미에서 이렇게 붙였습니다.
  2. 먼저 이걸 분리한 이유는 여러 서비스에서 공통으로 사용하게 되어서 중복 로직 제거, 서비스 크기 단축을 위해서였습니다. 처음 생각은 ReportType과 나머지 파라미터 하나를 전부 전달해서, 이 클래스 안에서 전략 메서드의 실행까지 역할을 맡도록 구현할까도 생각했었는데, 그렇게 되면 이 안에서 분기점이 많이 나올 수밖에 없다고 생각했습니다. 또한 파라미터에 따라 어떤 메서드를 실행시켜줄 지 판단하는 것도 굉장히 애매하다고 느껴져서 그냥 전략을 매핑해주는 용도로 사용하게 되었습니다.
    이것 외엔 서비스의 Map 공통 로직을 처리할 방법을 생각해보진 못했습니다.


private final Map<ReportType, ReportStrategy> reportActions;

public ReportActionProvider(
final ReportPostStrategy reportPostStrategy,
final ReportCommentStrategy reportCommentStrategy,
final ReportNicknameStrategy reportNicknameStrategy
) {
this.reportActions = new EnumMap<>(ReportType.class);
this.reportActions.put(ReportType.POST, reportPostStrategy);
this.reportActions.put(ReportType.COMMENT, reportCommentStrategy);
this.reportActions.put(ReportType.NICKNAME, reportNicknameStrategy);
}

public ReportStrategy getStrategy(ReportType type) {
return reportActions.get(type);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,11 @@ private void validateCommentMine(final Comment comment, final Member member) {
}
}

@Override
public String parseTarget(final Long targetId) {
final Comment reportedComment = commentRepository.findById(targetId)
.orElseThrow(() -> new NotFoundException(CommentExceptionType.NOT_FOUND));
return reportedComment.getContent();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,11 @@ private void changeNicknameByReport(final Member reportedMember, final ReportReq
}
}

@Override
public String parseTarget(final Long targetId) {
final Member reportedMember = memberRepository.findById(targetId)
.orElseThrow(() -> new NotFoundException(MemberExceptionType.NONEXISTENT_MEMBER));
return reportedMember.getNickname();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,9 @@ private void validatePostMine(final Post post, final Member member) {
}
}

@Override
public String parseTarget(final Long targetId) {
return targetId.toString();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
import com.votogether.domain.report.repository.ReportRepository;
import com.votogether.global.exception.BadRequestException;

@FunctionalInterface
public interface ReportStrategy {

void report(final Member reporter, final ReportRequest request);

String parseTarget(Long targetId);

default void validateDuplicatedReport(
final Member reporter,
final ReportRequest request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.springframework.http.HttpStatus;
import org.springframework.test.web.servlet.MockMvc;

@WebMvcTest(ReportCommandCommandController.class)
@WebMvcTest(ReportCommandController.class)
class ReportCommandControllerTest extends ControllerTest {

@MockBean
Expand Down
Loading