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/122 sort with querydsl #125

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Conversation

minjun3021
Copy link
Collaborator

@minjun3021 minjun3021 commented Oct 18, 2023

What is this PR?

Key Changes

#122 에 나와있는 3가지의 정렬기준을 한개의 엔드포인트에서 응답하도록 만듬
querydsl을 사용하여 동적쿼리를 만들어서 적용함

최초 요청시에는 query parameter에 sort라는 이름의 키로 요청을 하면됌
"deadline" ->마감임박순
"startDate" -> 곧다가오는 공연
"createdAt " -> 생성일자
ex)localhost:8081/events?sort=created

value는 위 3개 가 될수있으며 sort를 요청안할시에는 id로 자동 정렬됌
ex)localhost:8081/events

최초 요청이후에는 sort는 계속 같이 보내되
앞선 요청한 데이터 10개중 마지막 데이터의 id와time을 같이 보내야함

ex)localhost:8081/events?sort=createdAt&id=2433031&time=2025-10-15T00:00:00Z

Test Checklist

Copy link
Member

@junha-ahn junha-ahn left a comment

Choose a reason for hiding this comment

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

테스트 코드가 많이 추가되야할거같네요.

unit test로도 각 레이어 모두 테스트해주시고, 특히 integrationTest에서 정렬에 대해서 정확한지 테스트하는데 노력을 써주세요.

근데 querydsl로 쓸꺼면 모두 그렇게 해야하지않을까요? 요번 API는 아니지만, Issue라도 생성해두시는게 어떠실지?

Comment on lines +196 to +207
- name: id
in: query
required: true
required: false
schema:
$ref: '#/components/schemas/Pageable'
type: integer
format: int32
- name: time
in: query
required: false
schema:
type: string
format: date-time
Copy link
Member

@junha-ahn junha-ahn Oct 19, 2023

Choose a reason for hiding this comment

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

키 이름을 last_access_id, last_access_time 등 으로 변경할 필요가 있어보입니다.

  • 더 좋은 이름 있을 수 있으니 찾아보세요.
  • 당근마켓, 인스타, 트위터, 페이스북 등 참고
    • 무한스크롤 형태니까... Open API Docs 또는 개발자 도구 열고 직접 요청을 확인해보세요

docs/open-api.yaml Outdated Show resolved Hide resolved
@junha-ahn junha-ahn linked an issue Oct 19, 2023 that may be closed by this pull request
3 tasks
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (9eca9e6) 82.60% compared to head (a08b918) 82.78%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #125      +/-   ##
============================================
+ Coverage     82.60%   82.78%   +0.17%     
- Complexity      147      169      +22     
============================================
  Files            42       44       +2     
  Lines           730      784      +54     
  Branches         31       37       +6     
============================================
+ Hits            603      649      +46     
- Misses          104      109       +5     
- Partials         23       26       +3     
Files Coverage Δ
...up4/ticketingservice/controller/EventController.kt 100.00% <100.00%> (ø)
...kotlin/com/group4/ticketingservice/dto/EventDto.kt 88.46% <100.00%> (+0.96%) ⬆️
...om/group4/ticketingservice/service/EventService.kt 100.00% <100.00%> (ø)
...om/group4/ticketingservice/utils/ResponseAdvice.kt 55.05% <100.00%> (+1.03%) ⬆️
...up4/ticketingservice/utils/exception/ErrorCodes.kt 100.00% <100.00%> (ø)
...group4/ticketingservice/dto/EventSpecifications.kt 16.66% <50.00%> (ø)
...4/ticketingservice/config/QuerydslConfiguration.kt 0.00% <0.00%> (ø)
...ketingservice/repository/EventRepositorySupport.kt 86.66% <86.66%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@junha-ahn junha-ahn left a comment

Choose a reason for hiding this comment

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

last access id + 정렬기준 조건 테스트도 부탁드립니다.

last access id + price 등

@minjun3021
Copy link
Collaborator Author

나머지 소팅방식 테스트 코드 추가하였고,
lassId 관련 테스트는 이미존재합니다.
integrationTest에 소트테스트 하는거 보시면 getEvent 두번합니다.
한번은 최초 요청을 가정한 함수호출
두번째는 lastId를 파라미터로 넣고 함수호출

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Event 정렬 방식 결정
3 participants