Preventing Bots from having Top 10 coin holder role

[probro27]

Summary of Changes

Added a check in cron job to see if the member being assigned the role is a bot.

Related Issues

resolves #467

Steps to Reproduce

Check if a bot is in leaderboard but does not get the role.

Demonstration of Changes

Cannot demonstrate since my bot for some reason is not getting added to the leaderboard.

Further Information and Comments

[mcpenguin]

LGTM!

[Picowchew]

As per the description in the issue:

There should still be ten people with the Top 10 Coin Holder role, though they should all be non-bot users.

This code looks like less than ten people would have the role.

[mcpenguin]

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).

[probro27]

@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.

[mcpenguin]

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

[probro27]

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

[mcpenguin]

LGTM!

[probro27]

@Picowchew Please confirm :)

[Picowchew]

This code seems to give an infinite loop. Since you have asynchronous code inside the forEach loop, the forEach loop finishes iterating before the code inside an iteration finishes executing. Then, the users variable remains to be 0, so you get an infinite loop because the condition of the while loop is always satisfied.

I would recommend iterating through leaderboardIds with a for loop, and then break out of this loop once 10 non-bot users have the role.

To test your code, I would recommend having more than 10 users in the user_coin table (e.g. you can give coins to more than 10 users), assigning less than 10 users the role, and shortening the time interval of the cron job. Then, ensure that the result is that only the top 10 non-bot users have the role.