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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix group usage permission #149

Merged
merged 2 commits into from
Mar 31, 2023

Conversation

AlexHTW
Copy link
Contributor

@AlexHTW AlexHTW commented Mar 30, 2023

So, originally I just wanted to add the admin list to the loop that checks if an allowed user is in the group (first commit).
But then discovered a bug in is_user_in_group, it only checks the first user of the allowed user list (in all my previous testing, I never put myself in a different position until I tested the addition of the admin list 馃檲)
The current implementation of is_user_in_group raises an error if the first allowed user is not in the group and doesn't continue with the other users.
My fix is using the context (now that I think about it, maybe it would work with the previous implementation as well, if the error would be caught), so I added it to is_allowed and is_within_budget.
I gotta say, I am a bit out of my depth with the best practices of the telegram library. But for my tests it all seems to be at least functional now.

@dkvdm
Copy link

dkvdm commented Mar 31, 2023

ayyy, that works! much better, and clean implementation 馃憤
happy that we got to the bottom of this 馃檪

@n3d1117 n3d1117 merged commit 6f21a3f into n3d1117:main Mar 31, 2023
@AlexHTW AlexHTW deleted the allow-group-usage-if-admin-member branch April 1, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants