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

feat: list 명령어 구현 (현 서버의 모든 유저 보여주기) #39

Closed
wants to merge 2 commits into from

Conversation

roeniss
Copy link

@roeniss roeniss commented Sep 1, 2024

Screenshot 2024-09-02 at 00 15 28 by seonghyeok

Screenshot 2024-09-02 at 00 15 19 by seonghyeok

소개

  • 전체 등록된 유저들을 보고 싶어서 기능을 추가했습니다. 피드백 부탁드립니다!

특이사항

  • 해당 서버의 유저들만 보여줘야 했기에, guildId (server id) 를 user schema 에 추가했습니다.
  • 시간 포맷을 통일하기 위해 유틸 코드를 추가했습니다 (오전 1시 3분 --> 01:03)

Copy link
Collaborator

@synoti21 synoti21 left a comment

Choose a reason for hiding this comment

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

좋은 기능 감사합니다! 프로필 공개 여부에 따라 해당 정보를 불러올 지 말지 또한 결정하면 좋을 것 같아요.

Comment on lines +31 to +34
const userInfo = users.map(user => {
return `백준 아이디: ${user.boj_id} | 매일 알림 시간: ${formatDailyTime(user.daily_time)}`;
}).join('\n')

Copy link
Collaborator

Choose a reason for hiding this comment

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

큰 문제가 될 것 같지는 않습니다만, 혹시라도 자신의 백준 정보 공개를 원치 않는 사람들이 있을까봐 조금은 걱정이 되네요. (티어 공개 원하지 않거나, private하게 사용하고 싶은 사람들 등...)

만약 이 기능을 추가한다면 등록할 시 프로필 공개 여부를 넣어야 할 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

네, 저도 이 기능을 추가하고 뒤늦게 생각한건데... 사실 DB 구조를 많이 바꿔야 될 것 같습니다. 이 PR 은 일단 클로즈하고, 디자인을 조금 다시 잡으면 좋을 것 같다는 생각을 했습니다 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

맞아요 사실 정말 주먹구구식으로 만들다보니 놓친 점이 한 두가지가 아니었어요😅 현재 백준봇 v2를 만들고 있는데 올려주신 pr 참고해서 반영하도록 하겠습니다 감사합니다!

@@ -127,6 +127,20 @@ export class MongoUtil{
}
}

static async findAllUserInGuild(guildId: string): Promise<any>{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Discord에서 Guild의 경우는 일종의 워크스페이스 전체를 지칭하는 단위라, 같은 그룹 내 타 채널에서 봇을 사용하는 유저 정보까지 모조리 불러올 것 같아요.

이 부분은 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

제가 discord sdk 에 대한 이해가 조금 부족했던 것 같네요. 수정해야 될 부분으로 보입니다. (See #39 (comment) please)

@synoti21 synoti21 closed this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[suggestion] list - show all registered user
2 participants