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: Add UnVerified role removal after verification #3

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

nihalxkumar
Copy link
Member

  • Add functionality to remove UnVerified role after successful verification
  • Rename CodeModal to OTPModal
  • Add section comments in OTPModal
  • Fix one missed out change from last commit of renaming "Enter Code" to "Enter OTP"

@nihalxkumar nihalxkumar added the enhancement New feature or request label Oct 30, 2024 — with GitHub Codespaces
@nihalxkumar nihalxkumar requested a review from sattwyk October 30, 2024 11:31
Copy link
Member

@sattwyk sattwyk left a comment

Choose a reason for hiding this comment

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

Please change the order of actions when handling roles. First, remove the "Unverified" role and then add the "Verified" role. This way, you avoid the situation where the "Verified" role is added but the "Unverified" role has not been removed. Additionally, include a check to see if the user has the "Unverified" role before making any changes.

bot/bot.py Show resolved Hide resolved
Copy link
Member

@sattwyk sattwyk left a comment

Choose a reason for hiding this comment

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

LGTM! - there might me some edge cases I gotta figure out but it's good for now

@sattwyk sattwyk merged commit 35384a8 into main Nov 5, 2024
1 check failed
@nihalxkumar nihalxkumar deleted the nihalxkumar-removeNotVerified branch November 5, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants