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.

Sign up for GitHub

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

Preventing Bots from having Top 10 coin holder role #470

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Preventing Bots from having Top 10 coin holder role #470

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05fe7a5
cron job now checks if the member is a bot before adding the role
probro27 Apr 4, 2023
8a6ab44
getting additional 5 members from leaderboard to resolve issue
probro27 Jun 6, 2023
9e40606
users made to be exact 10 in a role assignment loop
probro27 Jun 13, 2023
2b10c1b
Merge branch 'main' into 467-bots-not-have-top-10-coin-holder-role
probro27 Jun 13, 2023
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
18 changes: 11 additions & 7 deletions src/components/cron.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export const createCoffeeChatCron = (client: Client): CronJob =>
// Gives Codey coin role to those on the leaderboard list everyday
export const assignCodeyRoleForLeaderboard = (client: Client): CronJob =>
new CronJob('0 0 0 */1 * *', async function () {
const leaderboard = await getCoinLeaderboard(NUMBER_USERS_TO_ASSIGN_ROLE);
const leaderboard = await getCoinLeaderboard(NUMBER_USERS_TO_ASSIGN_ROLE + 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

This might give more users than 10 if there are less than 5 bots that are in the top 10 codey coin users. I would make sure that in the code that is responsible for displaying the leaderboard, you only display the top ten people (right now it displays everyone in the leaderboard).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mcpenguin So the code for getCoinLeaderboard uses a SQL query and I am not sure how I can change the SQL query itself to remove the bots.
What I can do is something like this -

let users = 0;
while (users < 10) {
  const leaderboard = await getCointLeaderboard(NUMBER_USERS_TO_ASSIGN_ROLE + 5);
 ... // the rest of the code 
    if (memberToUpdate && !memberToUpdate.user?.bot) {
        await updateMemberRole(memberToUpdate, await getRoleName(CODEY_COIN_ROLE_ID), true);
       users++; // add that we have added 1 user
      }
}

This can ensure that only 10 users get the role. Let me know your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should get the leaderboard before the while loop, but yeah, that was how I was thinking we could proceed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay cool, yeah I will do that and update the pull request.

const leaderboardIds: Set<string> = new Set(leaderboard.map((entry) => entry.user_id));
const guild = client.guilds.resolve(TARGET_GUILD_ID);
if (!guild) {
Expand All @@ -155,10 +155,14 @@ export const assignCodeyRoleForLeaderboard = (client: Client): CronJob =>
await updateMemberRole(member, roleName, false);
}
});
leaderboardIds.forEach(async (user_id) => {
const memberToUpdate = members.get(user_id);
if (memberToUpdate && !previousIds.has(user_id)) {
await updateMemberRole(memberToUpdate, roleName, true);
}
});
let users = 0;
while (users < 10) {
leaderboardIds.forEach(async (user_id) => {
const memberToUpdate = members.get(user_id);
if (memberToUpdate && !previousIds.has(user_id)) {
await updateMemberRole(memberToUpdate, roleName, true);
users++;
}
});
}
});