#341 2023 가을학기 추석 홍보 이벤트 위한 transactions #364
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 tasks
kmc7468
commented
Sep 13, 2023
cokia
requested changes
Sep 14, 2023
withSang
reviewed
Sep 14, 2023
withSang
approved these changes
Sep 14, 2023
src/lottery/modules/events.js
Outdated
Comment on lines
70
to
73
| return { | ||
| event, | ||
| transactionId: transaction._id, | ||
| }; |
There was a problem hiding this comment.
return value가 지금은 안 쓰이는 것 같은데 추가하신 이유가 혹시 있을지 궁금해요!
There was a problem hiding this comment.
현재는 삭제된 instagramRewardAction 쪽에서 transactionId를 이용하여 Admin Log를 남겼었습니다. 나중에 필요한 일이 있을까봐 반환값은 그대로 유지하였습니다!
Merged
|
저번 리뷰 때 건님이 주신 의견이 대부분 반영되었습니다! 코드가 크게 바뀌어 상님께서도 리뷰를 다시 해주시면 좋을 것 같아요. 여전히 |
withSang
reviewed
Sep 16, 2023
| @@ -21,7 +21,7 @@ transactionsDocs[`${apiPrefix}/`] = { | |||
| description: "유저의 재화 입출금 기록의 배열", | |||
| items: { | |||
| type: "object", | |||
| required: ["_id", "type", "amount", "comment", "doneat"], | |||
| required: ["_id", "type", "amount", "comment", "createAt"], | |||
코드간 낮은 결합도를 유지하기 위해 이벤트 모듈에서 생긴 예외가 외부에 익셉션으로 직접 안넘어가는게 개인적으로는 이상적인 구현이라고 생각해요 :) |
14KGun
requested changes
Sep 17, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
It closes #341
기존 모듈의 services의 일부 API에서 response를 전송하기 직전에 event contracts를 호출하도록 하여 lottery 모듈과 기존 모듈 사이의 결합성은 낮추고, 기존 모듈에서 간단히 이벤트 달성을 구현할 수 있도록 하였습니다.
"방 공유하기", "광고성 푸시 알림 수신 동의", "이벤트 인스타그램 스토리에 공유", "아이템 구매 후 인스타그램 스토리에 공유"는 구현되지 않았습니다. (앱 또는 프론트엔드에서 API 호출 필요하기 때문. 이 부분은 @TaehyeonPark 님이 해주실 예정입니다.)
Further Work