Skip to content

Feat/2 implement performance #29

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

Merged
merged 22 commits into from
Jul 23, 2023
Merged

Feat/2 implement performance #29

merged 22 commits into from
Jul 23, 2023

Conversation

junha-ahn
Copy link
Member

@junha-ahn junha-ahn commented Jul 12, 2023

What is this PR?

Key Changes

Test Checklist

Controller Layer

Booking

  • POST bookings should return created booking
  • GET bookings should return booking
  • PUT bookings should return updated booking
  • DELETE bookings should return no content

User

  • POST users should return created user

Performance

  • POST performances should return created performance

Service Layer

Booking

  • BookingService_createBooking invoke BookingRepository_save
  • BookingService_getBooking invoke BookingRepository_findById
  • BookingService_updateBooking invoke BookingRepository_save
  • BookingService_deleteBooking invoke BookingRepository_deleteById
  • BookingService_deleteBooking throw IllegalArgumentException

User

  • UserService_createUser invoke UserRepository_save

Performance

  • PerformanceService_getPerformance invoke PerformanceRepository_findById

Copy link
Member Author

@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.

Performance 관련 파일만 체크했습니다.

  • User, Booking 제외

@junha-ahn junha-ahn requested a review from bohblue2 July 12, 2023 13:41
@junha-ahn
Copy link
Member Author

junha-ahn commented Jul 12, 2023

매우 깔끔합니다. 수고하셨습니다!!

Todo

  • 일단 CI actions가 성공하도록 변경해주세요. (일단 main branch 한번 pull 받으셔야할겁니다)
  • Test 시나리오 작성 및 구현 부탁드려요.

@junha-ahn junha-ahn removed the request for review from bohblue2 July 12, 2023 13:45
@codecov
Copy link

codecov bot commented Jul 15, 2023

Codecov Report

Patch coverage: 93.66% and project coverage change: +42.88 🎉

Comparison is base (5a62890) 50.00% compared to head (9856dc7) 92.88%.

❗ Current head 9856dc7 differs from pull request most recent head 4077c48. Consider uploading reports for the commit 4077c48 to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##               main      #29       +/-   ##
=============================================
+ Coverage     50.00%   92.88%   +42.88%     
- Complexity        2       51       +49     
=============================================
  Files             2       15       +13     
  Lines             8      225      +217     
  Branches          0        1        +1     
=============================================
+ Hits              4      209      +205     
- Misses            4       15       +11     
- Partials          0        1        +1     
Impacted Files Coverage Δ
...com/group4/ticketingservice/config/MapperConfig.kt 0.00% <0.00%> (ø)
.../group4/ticketingservice/service/BookingService.kt 78.26% <78.26%> (ø)
.../group4/ticketingservice/util/DateTimeConverter.kt 80.00% <80.00%> (ø)
...up4/ticketingservice/service/PerformanceService.kt 87.50% <87.50%> (ø)
...4/ticketingservice/controller/BookingController.kt 100.00% <100.00%> (ø)
...cketingservice/controller/PerformanceController.kt 100.00% <100.00%> (ø)
...oup4/ticketingservice/controller/UserController.kt 100.00% <100.00%> (ø)
...tlin/com/group4/ticketingservice/dto/BookingDto.kt 100.00% <100.00%> (ø)
.../com/group4/ticketingservice/dto/PerformanceDto.kt 100.00% <100.00%> (ø)
.../kotlin/com/group4/ticketingservice/dto/UserDto.kt 100.00% <100.00%> (ø)
... and 4 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member Author

@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.

Todo @bohblue2

  • 해당 테스트 시나리오를 PR 내용 중 Test CheckList에 등록시켜주세요 (Test Title 들을 복붙해도 무관합니다)
BookingController
- BookingService_createBooking invoke BookingRepository_save
- …

BookingService
- BookingService_createBooking invoke BookingRepository_save

Copy link
Member Author

@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.

테스트 관련 수정

@bohblue2
Copy link
Collaborator

bohblue2 commented Jul 17, 2023

Todo @bohblue2

  • 해당 테스트 시나리오를 PR 내용 중 Test CheckList에 등록시켜주세요 (Test Title 들을 복붙해도 무관합니다)

BookingController

- BookingService_createBooking invoke BookingRepository_save

- …



BookingService

- BookingService_createBooking invoke BookingRepository_save

처리했습니다.

@junha-ahn
Copy link
Member Author

junha-ahn commented Jul 17, 2023

테스트 확인했습니다! 감사합니다

그런데 해당 PR은 Performance에 대한 기능 구현인데... 왜 Booking이 메인 테스트를 짜신건지...

#2 이슈 확인 바랍니다

Todo

  • @Transactional 삭제 (당장 필요 없음)
  • Performance select 기능 구현
  • Performance select 기능 테스트
    • Select One: 코멘트 확인 후 해당하는 status 테스트
    • Select List: 코멘트에 존재하는 빈 리스트 반환 테스트
      @hihahayoung 체크 부탁드립니다.

@junha-ahn junha-ahn linked an issue Jul 17, 2023 that may be closed by this pull request
@junha-ahn junha-ahn mentioned this pull request Jul 19, 2023
13 tasks
Copy link
Member Author

@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.

공연 구현 좋습니다! 다만 이전에 남긴 리뷰에 Todo 확인해주십시요.

"Performance select 기능 테스트" 부분입니다.

Copy link
Member Author

@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.

수정 부탁드립니다.

Transaction 어노테이션도 삭제 부탁드립니다.

@junha-ahn
Copy link
Member Author

고생하셨습니다. 마지막으로 Repository 통합 테스트 구현으로 해당 PR 마무리하면 될 것 같습니다.

아래 참고하셔서 Main branch 내용 받아오시고, 통합 테스트 구현 부탁드립니다.

통합테스트 PR

통합 테스트 예시 파일

class BookmarkRepositoryTest(
    @Autowired val bookmarkRepository: BookmarkRepository
) : AbstractIntegrationTest() {
  • AbstractIntegrationTest() 부분 중요합니다! 빠지면 안됩니다!

@junha-ahn
Copy link
Member Author

junha-ahn commented Jul 21, 2023

통합 테스트는 잠시 구현하지 말아주세요

@bohblue2 통합 테스트 구현 부탁드립니다~

@junha-ahn junha-ahn merged commit 86295f5 into main Jul 23, 2023
@junha-ahn junha-ahn deleted the feat/2-implement-performance branch July 23, 2023 06:16
@junha-ahn junha-ahn mentioned this pull request Jul 29, 2023
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.

공연 구현
2 participants