-
Notifications
You must be signed in to change notification settings - Fork 1
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
#456 퀘스트 시스템 활성화 #465
#456 퀘스트 시스템 활성화 #465
Conversation
현재 남아 있는 작업 목록입니다.
|
이벤트 참여 동의시 학번 체크 우회하는 방법 학번 체크는 production 환경에서만 이루어집니다. 따라서, 로컬 개발 환경에서는 기본적으로 학번 체크가 우회됩니다. production 환경에서 우회하려면 src/lottery/services/globalState.js 파일의 84번 ~ 94번 줄을 삭제하시면 됩니다. |
|
properties: { | ||
credit: { | ||
type: "number", | ||
description: "완료 보상 중 재화의 개수입니다.", | ||
example: 100, | ||
}, | ||
ticket1: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swagger랑 노션에 남겨둔 API 명세에서는 ticket1 항목이 전송되지 않는다고 했지만... 사실은 0으로 채워진채로 전송됩니다. 이걸 수정하려면 globalStateHandler 쪽에서 ticket1 항목을 지워서 전송하거나, modules/quests.js의 buildQuests 함수를 수정해야 하는데, 전자는 (물론 큰 문제는 아니겠지만) 성능 이슈가 있을 것 같고, 후자는 lottery 모듈의 재사용성을 떨어뜨리는 것 같아 ticket1 항목을 여전히 전송하도록 냅뒀습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
다만 , Nullcheck 관련해서만 한번 더 고민해주시면 좋을 듯 합니다.
또한 Config에 이벤트 관련 기본값 넣는 것 관련해서, ENV를 사용하지않고 기본값만 사용하거나, ENV만 사용하거나 둘중 하나를 택하는 편이 좋을 듯 합니다.
}, | ||
}, | ||
}; | ||
// 다음 Endpoint는 2024 봄학기 이벤트에서 사용되지 않습니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q. 왜 사용되지 않는 코드를 삭제하지 않으셨나요?
A. 다음 이벤트에서 활용될 수도 있는 코드들은 현재 이벤트에서 사용하지 않더라도 삭제하지 않고 주석 처리하여 유지하는 방향으로 결정했습니다. 매번 GitHub Commit Log나 PR List를 확인하여 필요할 때마다 코드를 복구하는 것이 번거로울 것이라 생각했기 때문입니다.
Q. eventConfig.mode
를 확인하여 이벤트에 따라 적절히 핸들링할 수 있었을텐데, 그렇게 하지 않으신 이유가 무엇인가요?
A. 처음에는 lottery 모듈의 하위호환성(최신 버전의 lottery 모듈을 기존 코드에 이식해도 퀘스트 시스템이 오류 없이 작동하는 것)을 최대한 유지할 수 있도록 코딩하려 했으나, 이벤트마다 세세하게 달라지는 요구사항을 매번 eventConfig.mode
를 확인하며 핸들링하기엔 부담이 너무 컸고, 하위호환성을 유지해서 얻을 수 있는 이점이 없는 것 같아 lottery 모듈은 현재 이벤트에서만 사용될 수 있도록 간결하게 구성하고자 했습니다.
src/lottery/services/globalState.js
Outdated
return res.status(400).json({ | ||
error: "GlobalState/Create : not an undergraduate freshman", | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잉?? 저희 errror 를 { error: message }
형태의 JSON으로 반환하는 이유가 있나용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가을학기에 처음 lottery 모듈을 만들면서 참고했던 코드에서 저런 방식으로 오류 메세지를 전달하고 있었어서, lottery 모듈에서도 같은 방식을 사용하고 있었습니다. (rooms.js
파일 같은 곳에서 저런 방식을 쓰고 있습니다!) 그냥 메세지만 바로 send하도록 변경할까요? 이 부분은 이벤트 끝난 후 이슈를 만들어서 lottery 모듈 밖에 있는 코드들도 같이 손보는게 좋을 것 같기도 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이슈 #469 개설하였습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
await eventStatus.save(); | ||
if ( | ||
req.body.inviter && | ||
(await eventStatusModel.findOne({ _id: req.body.inviter }).lean()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 조건식이 잘못 되었는데, 이슈 #457 에서 추천인 시스템 구현하면서 함께 해결하도록 하겠습니다.
Summary
It closes #456
비활성화 됐던 lottery 모듈을 다시 활성화하고, 2024 봄학기 이벤트에 맞춰 코드를 수정합니다.
Further Work