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

refactor: SlackClient 로직 개선 및 로깅 추가 #637

Merged
merged 6 commits into from
Oct 20, 2022
Merged

refactor: SlackClient 로직 개선 및 로깅 추가 #637

merged 6 commits into from
Oct 20, 2022

Conversation

yeon-06
Copy link
Collaborator

@yeon-06 yeon-06 commented Oct 19, 2022 •
edited
Loading

요약

SlackClient 로직 개선



작업 내용

  • request 및 exception 값 로깅
  • 공통 예외 처리 분리

참고 #639

@yeon-06 yeon-06 added 🎉 BE 백엔드 관련 ✨ FEAT labels Oct 19, 2022
@yeon-06 yeon-06 self-assigned this Oct 19, 2022
@github-actions
Copy link

github-actions bot commented Oct 19, 2022 •
edited
Loading

Unit Test Results

200 tests  ±0   200 ✔️ ±0   16s ⏱️ ±0s
  67 suites ±0       0 💤 ±0 
  67 files   ±0       0 ❌ ±0 

Results for commit d1d7c95. ± Comparison against base commit ab94bc8.

This pull request removes 4 and adds 4 tests. Note that renamed tests count towards both. 
com.pickpick.slackevent.SlackEventTest ‑ [2] request={event={type=message, subtype=message_changed}}, expected=MESSAGE_CHANGED
com.pickpick.slackevent.SlackEventTest ‑ [3] request={event={type=message, subtype=message_deleted}}, expected=MESSAGE_DELETED
com.pickpick.slackevent.SlackEventTest ‑ [4] request={event={type=message, subtype=thread_broadcast}}, expected=MESSAGE_THREAD_BROADCAST
com.pickpick.slackevent.SlackEventTest ‑ [5] request={event={type=message, subtype=file_share}}, expected=MESSAGE_FILE_SHARE

com.pickpick.slackevent.SlackEventTest ‑ [2] request={event={subtype=message_changed, type=message}}, expected=MESSAGE_CHANGED
com.pickpick.slackevent.SlackEventTest ‑ [3] request={event={subtype=message_deleted, type=message}}, expected=MESSAGE_DELETED
com.pickpick.slackevent.SlackEventTest ‑ [4] request={event={subtype=thread_broadcast, type=message}}, expected=MESSAGE_THREAD_BROADCAST
com.pickpick.slackevent.SlackEventTest ‑ [5] request={event={subtype=file_share, type=message}}, expected=MESSAGE_FILE_SHARE

♻️ This comment has been updated with latest results.

yeon-06
yeon-06 commented Oct 19, 2022
View reviewed changes
backend/src/main/java/com/pickpick/support/SlackClient.java
@@ -201,6 +183,7 @@ public void inviteBotToChannel(final Member member, final Channel channel) {
.token(member.getToken())
.users(List.of(member.getWorkspace().getBotSlackId()))
.build();
log.info(LOGGING_INFO, request);

try {
ConversationsInviteResponse response = methodsClient.conversationsInvite(request);
Copy link
Collaborator Author

@yeon-06 yeon-06 Oct 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

inviteBotToChannel() 메서드는 중간에 isBotAlreadyInChannel()이라는 메서드를 실행하면서 다른 메서드들과는 다른 로직을 갖고 있기 때문에 execute()를 사용하지 않았습니다.

@yeon-06 yeon-06 marked this pull request as draft October 19, 2022 10:32
@yeon-06 yeon-06 marked this pull request as ready for review October 19, 2022 10:52
@yeon-06 yeon-06 changed the title refactor: SlackClient 로직 개선 refactor: SlackClient 로직 개선 및 로깅 추가 Oct 19, 2022
@pickpick-sonarqube

This comment has been minimized.

Sign in to view
@yeon-06 yeon-06 linked an issue Oct 19, 2022 that may be closed by this pull request
로깅 추가 #639
Closed
@pickpick-sonarqube

This comment has been minimized.

Sign in to view
1 similar comment
@pickpick-sonarqube
Copy link

Failed

  • 78.57% Coverage on New Code (is less than 80.00%)

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 78.57% Coverage (90.70% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: woowacourse-teams_2022-pickpick_AYKprLeNXDQxKhlck1fc

View in SonarQube

hyewoncc
hyewoncc approved these changes Oct 19, 2022
View reviewed changes
Copy link
Collaborator

@hyewoncc hyewoncc left a comment

Choose a reason for hiding this comment

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

PR 제목이 로직 개선이어서 가벼운 마음으로 보러 들어왔는데... 아주 큰 게 지나갔군요
저 try~catch 언젠가는 없애야한다고 생각했는데 이렇게 깔끔하게 정리됐다니 놀라워요!!!
approve 하겠습니다, 수고하셨어요~
image

backend/src/main/java/com/pickpick/support/SlackClient.java
Comment on lines -76 to +75
try {
OAuthV2AccessResponse response = methodsClient
.oauthV2Access(request);
validateResponse(OAUTH_ACCESS_METHOD_NAME, response);
return response;

} catch (IOException | SlackApiException e) {
throw new SlackApiCallException(OAUTH_ACCESS_METHOD_NAME);
}
return execute(methodsClient::oauthV2Access, OAUTH_ACCESS_METHOD_NAME, request);
Copy link
Collaborator

Choose a reason for hiding this comment

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

try~catch가 어디론가 옮겨졌다 생각하고 음 반복 로직을 더 깔끔하게 못만드나 했는데 이미 멋지게 중복을 없애두셨군요.. 🥺!!!!!

backend/src/test/java/com/pickpick/auth/application/AuthServiceTest.java
Comment on lines -129 to +130
given(slackClient.conversationsList((RequestConfigurator<ConversationsListRequestBuilder>) any()))
given(slackClient.conversationsList(any(ConversationsListRequest.class)))
.willReturn(generateConversationsListResponse());
given(slackClient.usersList((RequestConfigurator<UsersListRequestBuilder>) any()))
given(slackClient.usersList(any(UsersListRequest.class)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 깔끔하게 정리됐군요..!

JangBomi
JangBomi approved these changes Oct 19, 2022
View reviewed changes
Copy link
Collaborator

@JangBomi JangBomi left a comment

Choose a reason for hiding this comment

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

연로그 정말 최고에요 🥺 코드가 정말 깔끔 그 자체가 되었네요 ㅠㅠ 괴물개발자 👍
머지는 너무 늦은 시간인 것 같아서 내일 하면 될 것 같아요!!

@yeon-06 yeon-06 merged commit 63eb3b6 into develop Oct 20, 2022
@yeon-06 yeon-06 deleted the feature/refactor-SlackClient branch October 20, 2022 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyewoncc hyewoncc hyewoncc approved these changes

@JangBomi JangBomi JangBomi approved these changes

Assignees

@yeon-06 yeon-06

Labels
🎉 BE 백엔드 관련 ✨ FEAT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

로깅 추가
3 participants
@yeon-06 @JangBomi @hyewoncc