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

feat - 달성현황 정보 불러오기 #50

Merged
merged 19 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cfa4cdd
feat - #20 달성현황 조회 관련 success 상수 메세지 추가
jumining Jan 12, 2024
800ae81
feat - #20 달성현황 조회 get하는 컨트롤러 메서드 구현
jumining Jan 12, 2024
41a0b6a
chore - #20 메소드 public으로 변경해서 ChallengeResponse dto에서 사용
jumining Jan 12, 2024
237f683
feat - #20 ChallengeResponse 정의
jumining Jan 12, 2024
856b3e4
feat - #20 달성현황 정보 불러오는 서비스 메소드 구현
jumining Jan 12, 2024
02251b4
chore - #20 오타 삭제
jumining Jan 12, 2024
8f12863
refactor - #20 필요없는 코드 수정
jumining Jan 13, 2024
e504bb3
feat - #20 유저 아이디로 날짜 비교해서 오늘의 일별 챌린지 리턴하는 함수로 수정
jumining Jan 13, 2024
fa4ce85
feat - #20 챌린지 생성 시 일별 챌린지 모두 생성하게 수정
jumining Jan 13, 2024
c8feab6
Merge branch 'develop' of https://github.com/Team-HMH/HMH-Server into…
jumining Jan 13, 2024
7854d3e
refactor - #20 사용하지 않는 인자 수정
jumining Jan 13, 2024
fd0b12f
refactor - #20 일별챌린지 조회할 때 운영체제에 맞는 앱들만 반환하도록 수정
jumining Jan 13, 2024
084d584
refactor - #20 비즈니스로직과 response 형태를 위한 변형 로직 분리
jumining Jan 14, 2024
699e03b
refactor - #20 비즈니스 로직만 남아있도록 코드 삭제
jumining Jan 14, 2024
33a5d8e
feat - #20 챌린지 생성 시 period만큼 일별챌린지 생성되도록 구현
jumining Jan 14, 2024
edb91b0
refactor - #20 getCreateAt() 수정
jumining Jan 15, 2024
f80bc89
merge - #20 충돌 해결
jumining Jan 15, 2024
513e1c7
chore - #20 이름 오류 해결
jumining Jan 15, 2024
d04abb8
chore - #20 빠진 인자 추가
jumining Jan 15, 2024
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 @@ -6,7 +6,7 @@ public record AppGoalTimeResponse(
String appCode,
Long goalTime
) {
static AppGoalTimeResponse of(App app) {
public static AppGoalTimeResponse of(App app) {
return new AppGoalTimeResponse(app.getAppCode(), app.getGoalTime());
}
}
4 changes: 2 additions & 2 deletions src/main/java/sopt/org/HMH/domain/app/service/AppService.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void addAppByChallengeId(Long dailyChallengeId, List<AppGoalTimeRequest>

@Transactional
public void removeApp(Long userId, AppDeleteRequest request, String os) {
Long todayDailyChallengeId = IdConverter.getTodayDailyChallenge(challengeRepository,
Long todayDailyChallengeId = IdConverter.getTodayDailyChallengeByUserId(challengeRepository,
dailyChallengeRepository, userId).getId();
App app = appRepository.findByDailyChallengeIdAndAppCodeAndOs(todayDailyChallengeId, request.appCode(), os);

Expand All @@ -48,7 +48,7 @@ public void removeApp(Long userId, AppDeleteRequest request, String os) {
public void addAppsByUserId(Long userId, List<AppGoalTimeRequest> requests, String os) {
for (AppGoalTimeRequest request : requests) {
appRepository.save(App.builder()
.dailyChallenge(IdConverter.getTodayDailyChallenge(challengeRepository,
.dailyChallenge(IdConverter.getTodayDailyChallengeByUserId(challengeRepository,
dailyChallengeRepository, userId))
.appCode(request.appCode())
.goalTime(request.goalTime())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestHeader;
Expand All @@ -10,6 +11,7 @@
import sopt.org.HMH.domain.challenge.domain.exception.ChallengeSuccess;
import sopt.org.HMH.domain.challenge.dto.request.ChallengeRequest;
import sopt.org.HMH.domain.challenge.dto.response.AddChallengeResponse;
import sopt.org.HMH.domain.challenge.dto.response.ChallengeResponse;
import sopt.org.HMH.domain.challenge.service.ChallengeService;
import sopt.org.HMH.global.common.response.ApiResponse;
import sopt.org.HMH.global.util.IdConverter;
Expand All @@ -34,4 +36,15 @@ public ResponseEntity<ApiResponse<AddChallengeResponse>> orderAddChallenge(
.body(ApiResponse.success(ChallengeSuccess.ADD_CHALLENGE_SUCCESS,
challengeService.addChallenge(IdConverter.getUserId(principal), request, os)));
}

@GetMapping
public ResponseEntity<ApiResponse<ChallengeResponse>> orderGetChallenge(
Principal principal,
@RequestHeader("OS") final String os
) {
return ResponseEntity
.status(ChallengeSuccess.GET_CHALLENGE_SUCCESS.getHttpStatus())
.body(ApiResponse.success(ChallengeSuccess.GET_CHALLENGE_SUCCESS,
challengeService.getChallenge(IdConverter.getUserId(principal), os)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
public enum ChallengeSuccess implements SuccessBase {

ADD_CHALLENGE_SUCCESS(HttpStatus.OK, "챌린지 생성 성공"),
GET_CHALLENGE_SUCCESS(HttpStatus.OK, "챌린지 달성현황 조회 성공"),
;

private final HttpStatus status;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package sopt.org.HMH.domain.challenge.dto.response;

import lombok.AccessLevel;
import lombok.Builder;
import sopt.org.HMH.domain.app.dto.response.AppGoalTimeResponse;
import sopt.org.HMH.domain.challenge.domain.Challenge;
import sopt.org.HMH.domain.dailychallenge.domain.DailyChallenge;
import sopt.org.HMH.domain.dailychallenge.domain.Status;

import java.util.List;

@Builder(access = AccessLevel.PRIVATE)
public record ChallengeResponse(
Integer period,
List<Status> statuses,
Integer todayIndex,
Long goalTime,
List<AppGoalTimeResponse> apps
) {
public static ChallengeResponse of(Challenge challenge, DailyChallenge dailyChallenge, List<Status> statuses, Integer todayIndex) {
return ChallengeResponse.builder()
.period(challenge.getPeriod())
.statuses(statuses)
.todayIndex(todayIndex)
.goalTime(dailyChallenge.getGoalTime())
.apps(dailyChallenge.getApps().stream().map(AppGoalTimeResponse::of).toList())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,34 +1,58 @@
package sopt.org.HMH.domain.challenge.service;

import lombok.RequiredArgsConstructor;
import lombok.val;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import sopt.org.HMH.domain.challenge.domain.Challenge;
import sopt.org.HMH.domain.challenge.dto.request.ChallengeRequest;
import sopt.org.HMH.domain.challenge.dto.response.AddChallengeResponse;
import sopt.org.HMH.domain.challenge.dto.response.ChallengeResponse;
import sopt.org.HMH.domain.challenge.repository.ChallengeRepository;
import sopt.org.HMH.domain.dailychallenge.domain.DailyChallenge;
import sopt.org.HMH.domain.dailychallenge.domain.Status;
import sopt.org.HMH.domain.dailychallenge.repository.DailyChallengeRepository;
import sopt.org.HMH.domain.dailychallenge.service.DailyChallengeService;
import sopt.org.HMH.domain.user.domain.User;
import sopt.org.HMH.domain.user.repository.UserRepository;

import java.time.LocalDateTime;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.List;

@Service
@RequiredArgsConstructor
public class ChallengeService {

private final ChallengeRepository challengeRepository;
private final UserRepository userRepository;

private final DailyChallengeService dailyChallengeService;
private final DailyChallengeRepository dailyChallengeRepository;

@Transactional
public AddChallengeResponse addChallenge(Long userId,
ChallengeRequest request,
String os) {
public AddChallengeResponse addChallenge(Long userId, ChallengeRequest request, String os) {
Challenge challenge = challengeRepository.save(Challenge.builder()
.period(request.period())
.userId(userId).build());
dailyChallengeService.addDailyChallenge(challenge, request.goalTime(), request.apps(), os);
for (int count = 0; count <= request.period(); count++)
dailyChallengeService.addDailyChallenge(challenge, request.goalTime(), request.apps(), os);

return AddChallengeResponse.of(challenge.getId());
}

public ChallengeResponse getChallenge(Long userId, String os) {
val challenge = challengeRepository.findFirstByUserIdOrderByCreatedAtDesc(userId);
val dailyChallenges = challenge.getDailyChallenges();

val statuses = new ArrayList<Status>();
for (val dailyChallenge : dailyChallenges) {
statuses.add(dailyChallenge.getStatus());
}
Copy link
Member

Choose a reason for hiding this comment

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

P3: statuses도 DailyChallengeResponse의 apps 반환한 방식처럼 response내에서 변환하는 것이 더 가독성 및 service 클래스의 책임 측면에서 더 좋을 것 같습니다!


val startDayOfChallenge = challenge.getDailyChallenges().get(0);
val todayIndex = calculateTodayDailyChallengeIndex(startDayOfChallenge.getCreatedAt());
return ChallengeResponse.of(challenge, dailyChallenges.get(todayIndex), statuses, todayIndex);
}
Copy link
Member

Choose a reason for hiding this comment

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

P3: response를 위한 response 값 세팅은 response dto내에서 한다면 전달하는 매개변수가 적어질 수 있을 것 같습니다!
서비스 클래스 내에서는 비즈니스 로직을 처리하는 것에 중점을 두는 것으로 하는 것이 좋을 것 같습니다!


private Integer calculateTodayDailyChallengeIndex(LocalDateTime startDateOfChallenge) {
return (int) ChronoUnit.DAYS.between(LocalDateTime.now().toLocalDate(), startDateOfChallenge.toLocalDate());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import lombok.AccessLevel;
import lombok.Builder;
import sopt.org.HMH.domain.app.domain.App;
import sopt.org.HMH.domain.app.dto.response.AppGoalTimeResponse;
import sopt.org.HMH.domain.dailychallenge.domain.DailyChallenge;

import java.util.List;
Expand All @@ -20,13 +20,4 @@ public static DailyChallengeResponse of(DailyChallenge dailyChallenge) {
.apps(dailyChallenge.getApps().stream().map(AppGoalTimeResponse::of).toList())
.build();
}

public record AppGoalTimeResponse(
String appCode,
Long goalTime
) {
static AppGoalTimeResponse of(App app) {
return new AppGoalTimeResponse(app.getAppCode(), app.getGoalTime());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public Long addDailyChallenge(Challenge challenge, Long goalTime, List<AppGoalTi
}

public DailyChallengeResponse getDailyChallenge(Long userId, String os) {
DailyChallenge dailyChallenge = IdConverter.getTodayDailyChallenge(challengeRepository,
DailyChallenge dailyChallenge = IdConverter.getTodayDailyChallengeByUserId(challengeRepository,
dailyChallengeRepository, userId);

return DailyChallengeResponse.of(dailyChallenge);
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/sopt/org/HMH/global/util/IdConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import sopt.org.HMH.global.auth.jwt.exception.JwtException;

import java.security.Principal;
import java.time.LocalDateTime;
import java.time.temporal.ChronoUnit;

import static java.util.Objects.isNull;

Expand All @@ -20,11 +22,13 @@ public static Long getUserId(Principal principal) {
return Long.valueOf(principal.getName());
}

public static DailyChallenge getTodayDailyChallenge(ChallengeRepository challengeRepository,
public static DailyChallenge getTodayDailyChallengeByUserId(ChallengeRepository challengeRepository,
DailyChallengeRepository dailyChallengeRepository,
final Long userId) {
val challenge = challengeRepository.findFirstByUserIdOrderByCreatedAtDesc(userId);
val startDateOfChallenge = challenge.getDailyChallenges().get(0).getCreatedAt().toLocalDate();

return dailyChallengeRepository.findFirstByChallengeIdOrderByCreatedAtDesc(challenge.getId());
val todayDailyChallengeIndex = (int) ChronoUnit.DAYS.between(LocalDateTime.now().toLocalDate(), startDateOfChallenge);
return challenge.getDailyChallenges().get(todayDailyChallengeIndex);
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

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

P3: 이 부분에 사용되는 sql query가 너무 많다고 생각됩니다. dailyChllenge에 오늘이 챌린지 기간 내 며칠째인지의 필드를 추가하는 것을 고려해보는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sql query는 가장 최신의 챌린지를 구하는 val challenge = challengeRepository.findFirstByUserIdOrderByCreatedAtDesc(userId);의 findFirst~~에서만 사용되고 나머지는 리스트에서 인덱스를 통해 요소에 접근하는 방식입니다! :) dailyChllenge에 오늘이 챌린지 기간 내 며칠째인지의 필드를 추가하는 경우에도 DailyChallengeIndex를 구하는 코드 한 줄만 삭제될 것 같은데 저 코드가 단순 일수 차이를 반환하는 로직이기 때문에 리소스가 크게 든다고 생각되지 않습니다. erd, db설계 고려 회의시 date필드(createdAt)를 넣고 며칠째인지 나타내는 필드는 db에 저장하지 않도록 논의했던 것 같습니다! 읽고 다시 편하게 의견 주십쇼!!ㅎㅎ😀

Copy link
Member

Choose a reason for hiding this comment

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

아하 제가 sql 쿼리라고 잘못 작성했네요...
이렇게 보니 큰 차이는 없긴한 것 같아요!
제가 이 말씀을 드린 이유는!

// 일단 이 부분은 challenge의 .getCreatedAt을 변경되는 것이 맞을 것 같아요!
val startDateOfChallenge = challenge.getDailyChallenges().get(0).getCreatedAt().toLocalDate(); 
val todayDailyChallengeIndex = (int) ChronoUnit.DAYS.between(LocalDateTime.now().toLocalDate(), startDateOfChallenge);
return challenge.getDailyChallenges().get(todayDailyChallengeIndex);

물론 저도 아직 성능 테스트를 해보지는 않았지만... 로직의 복잡성을 증가시키는 것 보다 DB에 필드를 하나 더 추가하는 것이 성능에 더 유리할 것 같다 생각한 것 같아요. 또 인덱스를 구하는 것은 자주 사용되는 로직일 것 같아서요! 지금 보니 하나의 메서드를 추가로 사용하는 것이면 굳이 DB에 추가하지 않아도 될 것 같습니다! :)
다만, challenge.getDailyChallenges().get(0).getCreatedAt()challenge.getCreatedAt()과 같다고 생각되어 그렇게 수정해주시면 좋을 것 같습니다!

}
}
Copy link
Member

Choose a reason for hiding this comment

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

P3: repository를 매개변수로 넣어준다는 로직이 조금 어색한 것 같아요. 제가 봤었던 코드들에서는 보통 repository는 생성자 주입을 통해 관리했었기 때문입니다...!
getUserId도 global에 있는 것보다 auth 폴더에서 접근하는 것이 더 옳다고 생각하여 변경할 예정인데, getTodayDailyChallengeByUserId도 dailyChallenge의 폴더에서 관리하는 것이 어떨지 여쭤보고 싶습니다!