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

Role management #294

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

Role management #294

wants to merge 2 commits into from

Conversation

zuuring
Copy link
Member

@zuuring zuuring commented Mar 6, 2024

Notes

This adds a role-management.ts to Valkyrie which allows you to call the function on another script to add/remove roles based on the following criteria:

  • guildID (Discord server ID)
  • memberID (Discord user ID)
  • roleID (Discord role ID)
  • action (add or remove)

Additional features

Whether we would like to include additional functionality into this PR such as invite management, role triggers, etc.

This adds a `role-management` to Valkyrie which allows you to call the function on another script to add/remove roles based on the following criteria:

- guildID (Discord server ID)
- memberID (Discord user ID)
- roleID (Discord role ID)
- action (add or remove)
Prettier fix
@zuuring zuuring marked this pull request as ready for review March 8, 2024 10:20
@zuuring
Copy link
Member Author

zuuring commented Mar 8, 2024

Marking this one as ready to review, so we can merge and built out additional features discussed.

@zuuring zuuring requested a review from Shadowfiend March 8, 2024 10:21
Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Dropped a couple of thoughts but my first reaction is this helper doesn't make sense (yet?) and we might want to skip this PR and pull a helper back in when there's something concrete being repeated that we're trying to reuse/abstract.

@@ -0,0 +1,34 @@
import { Client } from "discord.js"

export default async function manageRole(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add this as an unexported helper to the renamed role-management script from #293 . If we do move to a directory for role management due to complexity, this can end up in a helpers file or similar.

Comment on lines +5 to +7
guildId: string,
memberId: string,
roleId: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced we'll find ourselves wanting to do this without having handles to at least the guild and maybe member objects already... For example, the first use we're pondering, which would be in reaction to the guildMemberAdd event, would receive a GuildMember already—allowing us to bypass most of the steps here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants