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.

Sign up for GitHub

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

Jump to bottom

[FIX] Answer, Question 관련 오류 수정 #39

Merged
merged 11 commits into from
Apr 15, 2024
Merged

[FIX] Answer, Question 관련 오류 수정 #39

merged 11 commits into from
Apr 15, 2024

Conversation

sookyungg
Copy link
Member

@sookyungg sookyungg commented Apr 5, 2024 •
edited
Loading

✒️ 관련 이슈번호

🔑 Key Changes

  1. 내용
    • 파일명, 네이밍 같아서 겹치는 부분 최대한 수정 하고, 날라간 코드 다시 작성
    • 질문 조회 밤 열시 기준으로 수정

📸 Screenshot

📢 To Reviewers

  • ExposedAt 기준으로 질문 조회 하는 기능이 LocalDateTime으로 되어있어서, 딱 정확한 시간 아니면 질문이 안가져와지길래 LocalDate로 바꿔서 (ex:오늘의 질문을 조회하면 오늘 날짜로 질문 조회하게끔) 수정 해놓았습니다. 문제가 있으면 알려주세요..!
  • 조회 시간이 밤 열시 이전이라면 전날 질문, 밤열시 이후라면 오늘 질문 조회 할 수 있도록 작성했습니다. QuestionExplorer 부분 중점적으로 확인해 주시면 될 것 같습니다.
  • TODO: 무한스크롤이랑 좋아요 탑텐 부분 잘 작동하는지 다시 확인 부탁드려요!

@sookyungg sookyungg requested review from PgmJun, daehwan2da and orijoon98 and removed request for PgmJun April 5, 2024 18:43
@sookyungg sookyungg self-assigned this Apr 5, 2024
@sookyungg sookyungg force-pushed the fix/#38 branch 3 times, most recently from cceb655 to 8190c29 Compare April 5, 2024 18:53
@sookyungg sookyungg changed the title ⚡️: fix Answer, Question 관련 오류 수정 ver.1 #38 ⚡️: fix Answer, Question 관련 오류 수정 #38 Apr 6, 2024
@sookyungg sookyungg changed the title ⚡️: fix Answer, Question 관련 오류 수정 #38 [FIX] Answer, Question 관련 오류 수정 Apr 7, 2024
PgmJun
PgmJun requested changes Apr 12, 2024
View reviewed changes
Copy link
Member

@PgmJun PgmJun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

고생하셨어요!!
리뷰 조금 남겨두었는데 확인 및 반영 부탁드립니다 :)

plu-domain/src/main/kotlin/com/th/plu/domain/domain/question/explorer/QuestionExplorer.kt Outdated Show resolved Hide resolved
plu-domain/src/main/kotlin/com/th/plu/domain/domain/answer/Answer.kt Show resolved Hide resolved
plu-api/src/main/kotlin/com/th/plu/api/service/answer/AnswerService.kt Outdated Show resolved Hide resolved
daehwan2da
daehwan2da requested changes Apr 14, 2024
View reviewed changes
Copy link
Collaborator

@daehwan2da daehwan2da left a comment

Choose a reason for hiding this comment

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

우리 LocalDateTime 으로 시간 핸들링하는게 많은것같은데,
이거 서버랑 디비 기준으로 시간이 KST 로 안될 수 있어서 시간 파싱 처리는 한곳에서 하도록 개선이 필요해보이는데요, 이 작업 task 따로 잡아서 내가 해둘까요?

  • 문제점
    • 서비스중에 기대하기는 한국 시간으로 기대하지만 별도 시간에 대한 관리처가 없어서 이상한 시간으로 처리되어질 수 있음

plu-domain/src/main/kotlin/com/th/plu/domain/domain/answer/repository/AnswerRepositoryImpl.kt Outdated
.select(answer._id.count())
.from(answer)
.where(answer.question._id.eq(questionId))
.fetchOne()!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

!! 는 빼야할것같은데요
fun findPublicAnswersCountByQuestionId(questionId: Long): Long? 이렇게 바꿔야할듯

@sookyungg sookyungg merged commit 3710675 into develop Apr 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daehwan2da daehwan2da daehwan2da requested changes

@PgmJun PgmJun PgmJun requested changes

@orijoon98 orijoon98 Awaiting requested review from orijoon98

Assignees

@sookyungg sookyungg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FIX] answer, question 충돌 부분 수정
3 participants
@sookyungg @PgmJun @daehwan2da