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

Jump to bottom

feat: add config to set scanner cooldown by role #1014

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

feat: add config to set scanner cooldown by role #1014

wants to merge 3 commits into from

Conversation

SherlockSometimes
Copy link

@SherlockSometimes SherlockSometimes commented May 18, 2024 •
edited
Loading

Add discordRoleCooldown to scanNext and scanZone config to set cooldown per user role

example config (scanZone accepts the same format)
"scanNext": {
...
// Discord roles that will be allowed to use scanNext
"discordRoles": ["Supporter","Scanner", "Mod"],
// Default cooldown if no role specific cooldown
"userCooldownSeconds": 60,
//List of role/cooldown pair shortest matching time will apply
"rules": [{ "role": "Supporter", "cooldown": 30 },
{ "role": "Mod", "cooldown": 0 }],
...
}

Expected outcome:
Supporter: 30 second cooldown
Mod: 0 second cooldown
Scanner: will get the default 60 second cooldown.
Supporter & Mod role: the lower 0 second cooldown would apply.

TurtIeSocks
TurtIeSocks requested changes May 20, 2024
View reviewed changes
Copy link
Collaborator

@TurtIeSocks TurtIeSocks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the contribution, I'm sure many are interested in this! I've been contemplating since you posted in the Discord for how we want to approach this in a scalable way that makes sense, but I keep ending up with solutions that add a lot of complications to this PR. So I'm going to try and keep this simple-ish for now and I may revise it in the future.

  • Let's change the new config keys to just rules, as it will be used for Discord and Telegram. And I will use this more in the future as well...
  • For the values themselves, we should switch to objects instead of a tuple, to be more consistent with the rest of the config / easier to read. Something like:
{
  "rules": [
      { "role": "12415", "cooldown": 5 },
      { "role": "mod", "cooldown": 1 }
   ]
}
  • In server/src/services/config.js, after replacing the alias roles with their ID counterparts, lets create a new map in each of the scanner modes. Something like:
config.scanner.scanNext.rulesObj = Object.fromEntries(config.scanner.scanNext.rules.map((rule) => [rule.role, rule.cooldown]))
  • Same for ScanZone
  • In server/src/services/DiscordClient.js, remove the code you have now. Add:
    // put this below the scanner declaration
    perms.scannerCooldowns = {}
    // Replace the code where the `scannerPerms` function is being called with
              scannerPerms(userRoles, 'discordRoles', trialActive).forEach(
                (x) => {
                  permSets.scanner.add(x)
                  perms.scannerCooldowns[x] = scanner[x].rules.reduce(
                    (acc, rule) => {
                      if (rule.cooldown < acc) {
                        return rule.cooldown
                      }
                      return acc
                    },
                    scanner[x].userCooldownSeconds,
                  )
                },
              )
  • Add this above code to server/src/services/TelegramClient.js as well, with the appropriate adjustments for TG
  • In server/src/graphql/resolves.js:
    • For the scannerConfig resolver, change these:
    • cooldown: perms?.scannerCooldowns?.[mode] || scanner.scanZone.userCooldownSeconds
    • cooldown: perms?.scannerCooldowns?.[mode] || scanner.scanNext.userCooldownSeconds
    • You also need to adjust the scanner resolver, this one seems to have somehow escaped my attempt to have them in alphabetical order, so it's last in the Query object... Add something like:
const cooldownSeconds = perms?.scannerCooldowns?.[category] || config.getSafe(`scanner.${category}.userCooldownSeconds`)
  • Then adjust the cooldown variable calculation to use this new value
  • I would also like if you could adjust the Permissions type found in packages/types/lib so it has the new property.

I know you mentioned on Discord that JS isn't your preferred language so let me know if you want me to jump in and help. I also didn't test these suggested changes so if something seems off, feel free to reach out. Thanks!

@SherlockSometimes SherlockSometimes force-pushed the main branch 2 times, most recently from e4e8445 to 47cbbdb Compare May 21, 2024 02:34
@SherlockSometimes
Copy link
Author

Didn't add the telegram part, I could add it but it would be untested as I don't use telegram, I can look at it tommorow if so, but figured id post the tested code and do that seperatly.
Wasn't sure if 'rules' needed to be added to 'Permissions' also as its part of the permSet object, so left it out for now.
Also added admin users get 0 cooldown as they skip the permission loop and would just get the default value.

TurtIeSocks
TurtIeSocks requested changes May 21, 2024
View reviewed changes
Copy link
Collaborator

@TurtIeSocks TurtIeSocks left a comment

Choose a reason for hiding this comment

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

Thanks for the adjustments, left some suggested changes on a few small things.

As far as TG, if you just translate the code over to the TelegramClient then I will make an attempt to test it later this week. TG is quite a pain in the ass to test if you don't have already have it set up so I wouldn't ask anyone to go through that 😂

packages/types/lib/server.d.ts Outdated Show resolved Hide resolved
server/src/graphql/resolvers.js Outdated Show resolved Hide resolved
server/src/services/DiscordClient.js Outdated Show resolved Hide resolved
server/src/services/DiscordClient.js Outdated Show resolved Hide resolved
feat: add config to set scanner cooldown by role
7151b22 
Allow users to set specific cooldown by roles
@SherlockSometimes
Copy link
Author

Switched to nullish ?? operator as 0 == false and would set default value when a 0 cooldown role was set.
Added telegram, it compiled and ran but no other testing was done with it.

@kamieniarz
Copy link
Contributor

Hey, I tried to use this PR by updating files manually but after all I could manage cooldown to disappear but in fact when I try to request a new scan within original cooldown timerange I have an error untill that 'virtual' (or actually hidden) original cooldown ends. Any chance this could be updated to latest version? Thanks

@SherlockSometimes
Copy link
Author

SherlockSometimes commented Oct 26, 2024 via email

@kamieniarz
Copy link
Contributor

That's the same what I manually did in my code so the effect is the same as what I explained above

@SherlockSometimes
Copy link
Author

SherlockSometimes commented Oct 27, 2024 via email

@SherlockSometimes
Copy link
Author

SherlockSometimes commented Oct 27, 2024 via email

@kamieniarz
Copy link
Contributor

Here's examply

      "userCooldownSeconds": 20,
      "rules": [{ "role": "ADMIN", "cooldown": 1 }],

My map shows no cooldown but when I scan and try another scan (right after first one) I get an error

@SherlockSometimes
Copy link
Author

SherlockSometimes commented Oct 27, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TurtIeSocks TurtIeSocks Awaiting requested review from TurtIeSocks

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SherlockSometimes @kamieniarz @TurtIeSocks