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

Feature/#365 알림 response 변경, 알림 검증 로직 추가, 중복 함수 리팩토링 #366

Merged
merged 14 commits into from
Dec 4, 2024

Conversation

swkim12345
Copy link
Collaborator

@swkim12345 swkim12345 commented Dec 4, 2024

close #365

✅ 작업 내용

  • 알림 response alarmExpiredDate로 변경
  • 알림 검증 로직 create, update에 적용
  • 중복 함수 리팩토링

📌 이슈 사항

  • 알림 구독을 했을 때 api만 사용해서 구독을 했을 경우 검증로직이 제대로 작동되지 않을 위험이 있습니다.

😎 체크 사항

  • label 설정 확인
  • 브랜치 방향 확인

@swkim12345 swkim12345 added 🐞 bugfix Something isn't working 🔨 refactor 코드 리팩토링 BE labels Dec 4, 2024
@swkim12345 swkim12345 requested a review from a team December 4, 2024 15:00
Copy link
Collaborator

@xjfcnfw3 xjfcnfw3 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@@ -33,14 +33,14 @@ export class AlarmResponse {
example: 10,
nullable: true,
})
alarmDate?: Date;
alarmExpiredDate?: Date;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 아마 영국시간 기준으로 전달될거 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

로컬상에서는 괜찮았는데... 확인해볼게요!

): boolean {
if (
alarm.alarmExpiredDate &&
this.isAlarmNotExpired(alarm.alarmExpiredDate, recent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

예외 처리 관련로직을 함수로 분리한거 굿입니다!

Copy link
Collaborator

@baegyeong baegyeong left a comment

Choose a reason for hiding this comment

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

LGTM~

): boolean {
if (
alarm.alarmExpiredDate &&
this.isAlarmNotExpired(alarm.alarmExpiredDate, recent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿 좋네요

@swkim12345 swkim12345 merged commit b48f975 into dev-be Dec 4, 2024
@swkim12345 swkim12345 deleted the feature/#365 branch December 5, 2024 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 🐞 bugfix Something isn't working 🔨 refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] - 알림 들어올 때 가격, 거래량 등 검증 로직 추가, docs 추가
3 participants