Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -23,5 +23,5 @@ public class NotificationEvent {
private LocalDateTime sentAt;
private NotificationEventType eventType; // 이벤트 구분 (CREATED, UPDATED, DELETED)
private Map<String, String> payload; // (추가 정보 전달용)

private String slackUserEmail;
}
6 changes: 4 additions & 2 deletions notification-service/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ dependencies {
// ✅ 공통 모듈
implementation 'com.timebank:common:0.0.1-SNAPSHOT'
implementation 'org.springframework.kafka:spring-kafka'

implementation 'org.springframework.cloud:spring-cloud-starter-openfeign'
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'org.springframework.boot:spring-boot-starter-mail'
implementation 'org.springframework.boot:spring-boot-starter-actuator'
implementation 'io.micrometer:micrometer-registry-prometheus'

implementation 'io.github.resilience4j:resilience4j-spring-boot3'
implementation 'io.github.resilience4j:resilience4j-retry'
Comment on lines +46 to +47
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Resilience4j 모듈 버전 명시 필요
resilience4j-spring-boot3resilience4j-retry 의존성에 버전이 지정되어 있지 않습니다. 이는 빌드 시 불확실한 버전이 적용될 수 있어 안정성을 해칩니다. 명시적 버전 또는 BOM 관리를 통해 원하는 버전을 지정해주세요.


runtimeOnly 'com.mysql:mysql-connector-j'
compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.timebank.notification_service.application.client;

import com.timebank.notification_service.application.dto.SlackBotMessage;
import com.timebank.notification_service.application.dto.SlackWebHookMessage;

public interface SlackClient {
void sendMessage(SlackWebHookMessage message);

void sendMessage(SlackBotMessage request);

String getUserIdByEmail(String email);

}
Comment on lines +1 to +13
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

SlackClient 인터페이스 확인
Slack WebHook 전송과 Bot 메시지 전송을 위한 오버로드된 sendMessage 메서드가 정의되어 있습니다. 타입 별로 메서드명을 구분(sendWebHookMessage, sendBotMessage)하면 가독성이 더 높아질 수 있습니다.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.timebank.notification_service.application.dto;

import com.timebank.common.infrastructure.external.notification.dto.NotificationType;
import com.timebank.notification_service.domain.entity.Notification;
import com.timebank.notification_service.domain.entity.NotificationType;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.timebank.notification_service.application.dto;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;

@Getter
@AllArgsConstructor
@Builder
public class SlackBotMessage {
private String channel;
private String text;

Comment on lines +10 to +13
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

클래스 목적 문서화 및 필드 설명 추가

클래스와 필드에 대한 JavaDoc 문서화가 있으면 코드의 이해도를 높일 수 있습니다. 특히 channel 필드가 Slack 사용자 ID를 의미하는지 채널 ID를 의미하는지 명확히 설명하는 것이 좋습니다.

+/**
+ * Slack 봇을 통해 메시지를 전송하기 위한 DTO 클래스입니다.
+ * 사용자 DM 또는 채널에 메시지를 보낼 때 사용됩니다.
+ */
@Getter
@AllArgsConstructor
@Builder
public class SlackBotMessage {
+	/**
+	 * 메시지를 전송할 대상 ID입니다. 사용자 ID(예: "U12345") 또는 채널 ID(예: "C12345")가 될 수 있습니다.
+	 */
	private String channel;
+	/**
+	 * 전송할 메시지 내용입니다.
+	 */
	private String text;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class SlackBotMessage {
private String channel;
private String text;
/**
* Slack 봇을 통해 메시지를 전송하기 위한 DTO 클래스입니다.
* 사용자 DM 또는 채널에 메시지를 보낼 사용됩니다.
*/
@Getter
@AllArgsConstructor
@Builder
public class SlackBotMessage {
/**
* 메시지를 전송할 대상 ID입니다. 사용자 ID(: "U12345") 또는 채널 ID(예: "C12345")가 될 수 있습니다.
*/
private String channel;
/**
* 전송할 메시지 내용입니다.
*/
private String text;

public static SlackBotMessage of(String channel, String text) {
return SlackBotMessage.builder()
.channel(channel)
.text(text)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.timebank.notification_service.application.dto;

import lombok.Getter;

@Getter
public class SlackUserResponse {
private boolean ok;
private SlackUser user;

@Getter
public static class SlackUser {
private String id;
private String teamId;
private String name;
private String realName;
private SlackProfile profile;

@Getter
public static class SlackProfile {
private String email;
}
}
}
Comment on lines +1 to +23
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Slack API 응답 매핑 검증
Lombok @Getter만 사용하여 DTO를 정의했으나, Slack API의 필드명이 snake_case(team_id, real_name)를 사용합니다. Jackson 설정에서 PropertyNamingStrategy.SNAKE_CASE를 활성화하지 않았다면, @JsonProperty("team_id"), @JsonProperty("real_name") 등을 추가해 필드 매핑을 보장해야 합니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.timebank.notification_service.application.dto;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;

@Getter
@AllArgsConstructor
@Builder
public class SlackWebHookMessage {
private String text;

public static SlackWebHookMessage from(String text) {
return SlackWebHookMessage.builder()
.text(text)
.build();
}
}
Comment on lines +10 to +18
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

클래스 목적 문서화 및 기능 확장 고려

이 DTO의 목적과 사용 방법에 대한 JavaDoc이 추가되면 더 좋을 것 같습니다. 또한, Slack webhook은 간단한 텍스트 외에도 attachment, blocks 등 더 복잡한 메시지 형식을 지원합니다. 필요에 따라 이러한 기능을 추가하는 것을 고려해 보세요.

+/**
+ * Slack webhook을 통해 메시지를 전송하기 위한 DTO 클래스입니다.
+ * 현재는 기본 텍스트 메시지만 지원하며, 향후 필요에 따라 attachment, blocks 등의 기능을 추가할 수 있습니다.
+ */
@Getter
@AllArgsConstructor
@Builder
public class SlackWebHookMessage {
	private String text;
+	// 향후 확장을 위한 필드 추가 예시:
+	// private List<SlackAttachment> attachments;
+	// private List<SlackBlock> blocks;

	public static SlackWebHookMessage from(String text) {
		return SlackWebHookMessage.builder()
			.text(text)
			.build();
	}
}

Committable suggestion skipped: line range outside the PR's diff.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import com.timebank.common.infrastructure.external.notification.dto.NotificationEventType;
import com.timebank.notification_service.application.client.SlackClient;
import com.timebank.notification_service.application.dto.NotificationDto;
import com.timebank.notification_service.application.event.NotificationEvent;
import com.timebank.notification_service.application.dto.SlackBotMessage;
import com.timebank.notification_service.application.dto.SlackWebHookMessage;
import com.timebank.notification_service.domain.entity.Notification;
import com.timebank.notification_service.domain.entity.NotificationEventType;
import com.timebank.notification_service.domain.repository.NotificationRepository;

import jakarta.persistence.EntityNotFoundException;
Expand All @@ -29,6 +31,7 @@ public class NotificationService {

private final NotificationRepository notificationRepository;
private final KafkaTemplate<String, Object> kafkaTemplate;
private final SlackClient slackClient;

/**
* 알림 생성
Expand All @@ -46,10 +49,6 @@ public NotificationDto createNotification(NotificationDto notificationDto) {
.build();
notification = notificationRepository.save(notification);

// Kafka 이벤트 발행 (CREATED)
NotificationEvent event = new NotificationEvent(notification, NotificationEventType.CREATED);
kafkaTemplate.send(NotificationEventType.CREATED.getTopic(), event);

return NotificationDto.fromEntity(notification);
}

Expand Down Expand Up @@ -96,10 +95,6 @@ public NotificationDto markAsRead(Long notificationId) {
notification.setIsRead(true);
notification = notificationRepository.save(notification);

// Kafka 이벤트 발행 (UPDATED)
NotificationEvent event = new NotificationEvent(notification, NotificationEventType.UPDATED);
kafkaTemplate.send(NotificationEventType.UPDATED.getTopic(), event);

return NotificationDto.fromEntity(notification);
}

Expand All @@ -113,9 +108,9 @@ public void deleteNotification(Long notificationId) {

notificationRepository.delete(notification);

// Kafka 이벤트 발행 (DELETED)
NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED);
kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event);
// // Kafka 이벤트 발행 (DELETED)
// NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED);
// kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event);
}
Comment on lines +111 to 114
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

주석 처리된 코드 제거 필요

Kafka 이벤트 발행 관련 주석 처리된 코드는 제거하는 것이 좋습니다. 주석 처리된 코드는 유지보수를 어렵게 만들고 코드 이해도를 떨어뜨립니다.

-	// // Kafka 이벤트 발행 (DELETED)
-	// NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED);
-	// kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// // Kafka 이벤트 발행 (DELETED)
// NotificationEvent event = new NotificationEvent(notification, NotificationEventType.DELETED);
// kafkaTemplate.send(NotificationEventType.DELETED.getTopic(), event);
}
}


/**
Expand All @@ -131,4 +126,12 @@ public List<NotificationDto> getNotificationsByUser(Long userId) {
.map(NotificationDto::fromEntity)
.collect(Collectors.toList());
}

public void sendMessage(SlackWebHookMessage message, String email) {
//채널 알림
slackClient.sendMessage(message);

//사용자 DM
slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
}
Comment on lines +130 to +136
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

메소드 이름 및 문서화 개선 필요

sendMessage 메소드는 특별히 Slack 메시지를 보내는 기능이지만, 메소드 이름만으로는 그 목적이 명확하지 않습니다. 또한, JavaDoc이 없어 메소드의 기능과 매개변수에 대한 설명이 부족합니다.

-	public void sendMessage(SlackWebHookMessage message, String email) {
+	/**
+	 * Slack을 통해 알림 메시지를 전송합니다.
+	 * 
+	 * @param message Slack 웹훅을 통해 채널에 전송할 메시지
+	 * @param email 직접 메시지(DM)를 받을 사용자의 이메일
+	 */
+	public void sendSlackNotification(SlackWebHookMessage message, String email) {
		//채널 알림
		slackClient.sendMessage(message);

		//사용자 DM
		slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void sendMessage(SlackWebHookMessage message, String email) {
//채널 알림
slackClient.sendMessage(message);
//사용자 DM
slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
}
/**
* Slack을 통해 알림 메시지를 전송합니다.
*
* @param message Slack 웹훅을 통해 채널에 전송할 메시지
* @param email 직접 메시지(DM) 받을 사용자의 이메일
*/
public void sendSlackNotification(SlackWebHookMessage message, String email) {
//채널 알림
slackClient.sendMessage(message);
//사용자 DM
slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
}

🛠️ Refactor suggestion

에러 처리 추가 필요

현재 메소드는 Slack API 호출 실패 시 예외 처리를 하지 않습니다. 특히 getUserIdByEmail(email) 호출이 실패할 경우 발생할 수 있는 예외에 대한 처리가 필요합니다.

	public void sendMessage(SlackWebHookMessage message, String email) {
		//채널 알림
-		slackClient.sendMessage(message);
+		try {
+			slackClient.sendMessage(message);
+			
+			//사용자 DM
+			String userId = slackClient.getUserIdByEmail(email);
+			if (userId != null && !userId.isEmpty()) {
+				slackClient.sendMessage(SlackBotMessage.of(userId, message.getText()));
+			} else {
+				log.warn("사용자 이메일 {}에 대한 Slack ID를 찾을 수 없습니다.", email);
+			}
+		} catch (Exception e) {
+			log.error("Slack 메시지 전송 중 오류 발생: {}", e.getMessage(), e);
+		}
-
-		//사용자 DM
-		slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void sendMessage(SlackWebHookMessage message, String email) {
//채널 알림
slackClient.sendMessage(message);
//사용자 DM
slackClient.sendMessage(SlackBotMessage.of(slackClient.getUserIdByEmail(email), message.getText()));
}
public void sendMessage(SlackWebHookMessage message, String email) {
//채널 알림
try {
slackClient.sendMessage(message);
//사용자 DM
String userId = slackClient.getUserIdByEmail(email);
if (userId != null && !userId.isEmpty()) {
slackClient.sendMessage(SlackBotMessage.of(userId, message.getText()));
} else {
log.warn("사용자 이메일 {}에 대한 Slack ID를 찾을 수 없습니다.", email);
}
} catch (Exception e) {
log.error("Slack 메시지 전송 중 오류 발생: {}", e.getMessage(), e);
}
}

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.timebank.notification_service.domain.entity;

import com.timebank.common.domain.Timestamped;
import com.timebank.common.infrastructure.external.notification.dto.NotificationEventType;
import com.timebank.common.infrastructure.external.notification.dto.NotificationType;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import org.springframework.kafka.annotation.KafkaListener;
import org.springframework.stereotype.Component;

import com.timebank.notification_service.application.event.NotificationEvent;
import com.timebank.common.infrastructure.external.notification.dto.NotificationEvent;
import com.timebank.notification_service.application.dto.SlackWebHookMessage;
import com.timebank.notification_service.application.service.NotificationService;
import com.timebank.notification_service.domain.entity.Notification;
import com.timebank.notification_service.domain.repository.NotificationRepository;

Expand All @@ -16,20 +18,26 @@
public class NotificationConsumer {

private final NotificationRepository notificationRepository;
private final NotificationService notificationService;

// CREATED 토픽 소비 메서드
@KafkaListener(topics = "notification.events.CREATED", groupId = "notification-group")
public void consumeCreated(NotificationEvent event) {
log.info("Consumed CREATED event: {}", event);
saveNotification(event);
// 필요시 추가 로직 처리 (예: 로깅)

notificationService.sendMessage(
SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
Comment on lines +29 to +30
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

슬랙 메시지 생성 로직 검토 필요

SlackWebHookMessage.from 메서드가 event.getMessage()만 사용하고 있어 정보가 부족할 수 있습니다.

이벤트의 다른 속성(타입, 이벤트 유형 등)을 활용하여 더 풍부한 슬랙 메시지를 생성하는 것이 좋습니다:

-notificationService.sendMessage(
-	SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
+notificationService.sendMessage(
+	SlackWebHookMessage.builder()
+		.text(String.format("[%s] %s", event.getType(), event.getMessage()))
+		.eventType(event.getEventType().name())
+		.build(), 
+	event.getSlackUserEmail());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
notificationService.sendMessage(
SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
notificationService.sendMessage(
SlackWebHookMessage.builder()
.text(String.format("[%s] %s", event.getType(), event.getMessage()))
.eventType(event.getEventType().name())
.build(),
event.getSlackUserEmail());

}
Comment on lines +28 to 31
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

코드 일관성 개선 필요

consumeCreated 메서드에서는 저장 후 메시지를 보내는 반면, consumeUpdated에서는 메시지를 보낸 후 저장합니다.

일관된 동작을 위해 패턴을 통일하세요:

public void consumeCreated(NotificationEvent event) {
	log.info("Consumed CREATED event: {}", event);
	saveNotification(event);

	notificationService.sendMessage(
		SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());
}

혹은 모든 메서드가 같은 순서를 따르도록 하세요. 트랜잭션 경계와 실패 시나리오를 고려하여 순서를 결정하는 것이 중요합니다.

Committable suggestion skipped: line range outside the PR's diff.


// UPDATED 토픽 소비 메서드
@KafkaListener(topics = "notification.events.UPDATED", groupId = "notification-group")
public void consumeUpdated(NotificationEvent event) {
log.info("Consumed UPDATED event: {}", event);
// 업데이트 이벤트의 경우 추가 처리 로직이 있다면 구현합니다.

notificationService.sendMessage(
SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());

saveNotification(event);
}

Expand All @@ -38,7 +46,11 @@ public void consumeUpdated(NotificationEvent event) {
public void consumeDeleted(NotificationEvent event) {
log.info("Consumed DELETED event: {}", event);
// 삭제 이벤트는 삭제 후 로깅 혹은 관련 작업 수행
notificationService.sendMessage(
SlackWebHookMessage.from(event.getMessage()), event.getSlackUserEmail());

Comment on lines +49 to +51
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

DELETE 이벤트 처리 개선 필요

DELETED 이벤트도 데이터베이스에 저장된 후 메시지를 보내고 있습니다. 삭제된 항목이 저장되는 로직이 있습니다.

DELETED 이벤트의 경우 로직 검토가 필요합니다. 정말 저장이 필요한지, 아니면 해당 알림을 DB에서 실제로 삭제해야 하는지 확인하세요.


🏁 Script executed:

#!/bin/bash
# saveNotification 메서드가 어떻게 호출되는지 확인
rg -n "saveNotification" --type java notification-service/

Length of output: 673


🏁 Script executed:

#!/bin/bash
sed -n '1,200p' notification-service/src/main/java/com/timebank/notification_service/infrastructure/kafka/NotificationConsumer.java

Length of output: 2510


DELETED 이벤트의 DB 저장 로직 검토 필요

현재 NotificationConsumer.consumeDeleted()에서 삭제 이벤트 수신 시에도 saveNotification(event)를 호출해 알림을 저장하고 있습니다. 삭제된 알림을 별도로 기록(audit)할 의도가 아니라면, 다음을 검토해주세요:

  • NotificationConsumer.java
    • consumeDeleted() (약 49–57행)에서 saveNotification(event) 호출 여부
  • 삭제 이벤트의 처리 방식
    • DB에 그대로 저장이 필요한지
    • 기존 알림을 삭제(notificationRepository.deleteById(...))하거나, 저장 호출을 제거할지

의도에 맞춰 로직을 수정해주세요.

saveNotification(event);

}

// 공통적으로 Notification DB 저장 처리
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.timebank.notification_service.infrastructure.slack;

import org.springframework.cloud.openfeign.FeignClient;
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;
import org.springframework.web.bind.annotation.RequestParam;

import com.timebank.notification_service.application.dto.SlackBotMessage;
import com.timebank.notification_service.application.dto.SlackUserResponse;

@FeignClient(name = "slackBotClient", url = "https://slack.com/api")
public interface SlackBotClient {
Comment on lines +13 to +14
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

FeignClient 구성 최적화 필요

Slack API URL이 하드코딩되어 있습니다. 이는 환경 별로 설정을 변경하기 어렵게 만들 수 있습니다.

URL을 application.yml에서 설정 가능하도록 변경하는 것이 좋습니다:

-@FeignClient(name = "slackBotClient", url = "https://slack.com/api")
+@FeignClient(name = "slackBotClient", url = "${slack.api.url}")

그리고 application.yml에 다음과 같이 추가:

slack:
  api:
    url: https://slack.com/api


@PostMapping("/chat.postMessage")
void sendMessage(@RequestHeader("Authorization") String authorization,
@RequestBody SlackBotMessage body);
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

응답 처리 개선 필요

sendMessage 메서드가 void 반환 타입을 사용하여 Slack API 응답 정보를 활용할 수 없습니다.

성공 여부와 추가 정보를 포함하는 응답 객체를 반환하도록 수정하세요:

-@PostMapping("/chat.postMessage")
-void sendMessage(@RequestHeader("Authorization") String authorization,
-	@RequestBody SlackBotMessage body);
+@PostMapping("/chat.postMessage")
+SlackMessageResponse sendMessage(@RequestHeader("Authorization") String authorization,
+	@RequestBody SlackBotMessage body);

SlackMessageResponse 클래스를 추가하여 성공 여부와 필요한 정보를 포함하도록 하세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@PostMapping("/chat.postMessage")
void sendMessage(@RequestHeader("Authorization") String authorization,
@RequestBody SlackBotMessage body);
@PostMapping("/chat.postMessage")
SlackMessageResponse sendMessage(@RequestHeader("Authorization") String authorization,
@RequestBody SlackBotMessage body);


@GetMapping("/users.lookupByEmail")
SlackUserResponse lookupByEmail(@RequestHeader("Authorization") String authorization,
@RequestParam("email") String email);
Comment on lines +20 to +22
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

예외 상황 고려 필요

lookupByEmail 메서드는 사용자를 찾지 못했을 때 어떻게 동작할지 명확하지 않습니다.

Slack API가 사용자를 찾지 못했을 때 어떤 응답을 제공하는지 확인하고, 응답 객체에서 이를 처리할 수 있는 방법을 고려하세요.


🌐 Web query:

슬랙 API users.lookupByEmail 메서드가 사용자를 찾지 못했을 때 어떤 응답을 반환하나요?

💡 Result:

Slack의 users.lookupByEmail 메서드를 사용하여 이메일로 사용자를 조회할 때, 해당 이메일에 해당하는 사용자가 존재하지 않으면 다음과 같은 응답을 받게 됩니다:

{
    "ok": false,
    "error": "users_not_found"
}

이러한 응답은 요청한 이메일 주소에 해당하는 사용자가 워크스페이스에 존재하지 않음을 나타냅니다. (api.slack.com)

또한, 사용자가 비활성화된 상태인 경우에도 동일한 users_not_found 오류가 반환됩니다. 이러한 경우에는 users.list 메서드를 사용하여 모든 사용자를 조회하고, 그 중에서 비활성화된 사용자를 필터링하여 확인할 수 있습니다. (api.slack.com)

이러한 오류를 처리할 때는 응답의 ok 필드를 확인하여 요청이 성공했는지 여부를 판단하고, error 필드의 값을 통해 오류의 원인을 파악하는 것이 중요합니다.

Citations:


🏁 Script executed:

#!/bin/bash
set -e
# Locate SlackUserResponse class to inspect its fields
rg "class SlackUserResponse" -n -A5

Length of output: 830


🏁 Script executed:

#!/bin/bash
# 출력: SlackUserResponse 전체 클래스 확인
sed -n '1,200p' notification-service/src/main/java/com/timebank/notification_service/application/dto/SlackUserResponse.java

Length of output: 546


Slack API users.lookupByEmail 예외 처리 구현 필요

Slack API는 사용자를 찾지 못하면

{ "ok": false, "error": "users_not_found" }

를 반환합니다. 현재 SlackUserResponse DTO에는 error 필드가 없어 ok == false인 경우를 식별·처리할 수 없습니다. 다음을 반영해주세요:

  • SlackUserResponseprivate String error; 필드 추가
  • Feign 클라이언트 호출 후 response.isOk() 검사
    • ok == false인 경우 SlackUserNotFoundException 등 적절한 예외를 던지거나 404 응답을 반환하도록 처리
  • 서비스 레이어에서 사용자 조회 결과를 Optional로 반환하거나 예외 흐름을 처리하는 로직 추가

예시 수정안:

 public class SlackUserResponse {
     private boolean ok;
+    private String error;
     private SlackUser user;
     // …
 }
SlackUserResponse res = slackClient.lookupByEmail(token, email);
if (!res.isOk()) {
    throw new SlackUserNotFoundException("Slack API error: " + res.getError());
}
SlackUser user = res.getUser();

}
Loading