Conversation
Reviewer's Guide by SourceryThis pull request introduces a new helper function, Flow diagram for subscription verification processflowchart TD
Start[Start] --> IsAdmin{Is User Admin?}
IsAdmin -->|Yes| Grant[Grant Access]
IsAdmin -->|No| CheckForceGroup{Force Sub Group?}
CheckForceGroup -->|Yes| CheckGroupMember{Check Group Membership}
CheckForceGroup -->|No| CheckForceChannel{Force Sub Channel?}
CheckGroupMember -->|Member| CheckForceChannel
CheckGroupMember -->|Not Member| Deny[Deny Access]
CheckForceChannel -->|Yes| CheckChannelMember{Check Channel Membership}
CheckForceChannel -->|No| Grant
CheckChannelMember -->|Member| Grant
CheckChannelMember -->|Not Member| Deny
Grant --> End[End]
Deny --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary
|
There was a problem hiding this comment.
Hey @senseiubot - I've reviewed your changes - here's some feedback:
Overall Comments:
- There appears to be duplicate implementations of both
subschannel()andsubsgroup()functions. Consider removing the duplicates and potentially refactoring the common logic into a single helper function that takes the chat_id as a parameter.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return member.status in [ChatMemberStatus.OWNER, ChatMemberStatus.ADMINISTRATOR, ChatMemberStatus.MEMBER] | ||
|
|
||
|
|
||
| async def subsgroup(filter, client, update): |
There was a problem hiding this comment.
issue (complexity): Consider consolidating the duplicate functions subschannel() and subsgroup() into a single function.
The duplicate functions subschannel() and subsgroup() can be consolidated into a single function that handles both cases. This reduces complexity while maintaining all functionality:
async def check_subscription(filter, client, update, force_sub_setting, chat_id):
if not force_sub_setting:
return True
user_id = update.from_user.id
if user_id in ADMINS:
return True
try:
member = await client.get_chat_member(chat_id=chat_id, user_id=user_id)
except UserNotParticipant:
return False
return member.status in [ChatMemberStatus.OWNER, ChatMemberStatus.ADMINISTRATOR, ChatMemberStatus.MEMBER]
# Usage with filters:
subschannel = filters.create(lambda _, c, u: check_subscription(_, c, u, FORCE_SUB_CHANNEL, FORCE_SUB_CHANNEL))
subsgroup = filters.create(lambda _, c, u: check_subscription(_, c, u, FORCE_SUB_GROUP, FORCE_SUB_GROUP))This approach:
- Eliminates code duplication
- Makes future maintenance easier
- Keeps the same filter interface
- Maintains all existing functionality
Summary by Sourcery
New Features: