-
-
Notifications
You must be signed in to change notification settings - Fork 176
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 muted/blocked users api #180
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request implements new API functionality for retrieving blocked and muted users in the Twikit client. The changes include adding two new methods to the client class: File-Level Changes
Tips
|
WalkthroughThe recent updates introduce new asynchronous methods in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dongruixiao - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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 to tell me if it was helpful.
@@ -2401,6 +2401,40 @@ async def unblock_user(self, user_id: str) -> User: | |||
response, _ = await self.v11.destroy_blocks(user_id) | |||
return User(self, build_user_data(response)) | |||
|
|||
async def get_blocked_users( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider refactoring get_blocked_users and get_muted_users to reduce duplication
The get_blocked_users and get_muted_users methods are very similar. Consider creating a generic method for fetching user lists and then implement these as thin wrappers around it. This would improve maintainability and reduce the chance of inconsistencies.
async def _get_user_list(self, endpoint: str, count: int = 20, cursor: str | None = None) -> Result[User]:
# Implementation here
async def get_blocked_users(self, count: int = 20, cursor: str | None = None) -> Result[User]:
return await self._get_user_list('blocks', count, cursor)
async def get_muted_users(self, count: int = 20, cursor: str | None = None) -> Result[User]:
return await self._get_user_list('mutes', count, cursor)
@@ -690,3 +693,25 @@ async def tweet_result_by_rest_id(self, tweet_id): | |||
return await self.gql_get( | |||
Endpoint.TWEET_RESULT_BY_REST_ID, variables, TWEET_RESULT_BY_REST_ID_FEATURES, extra_params=params | |||
) | |||
|
|||
async def blocked_accounts_all(self, user_id, count, cursor): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Standardize coding style across similar methods
The blocked_accounts_all and muted_accounts methods are quite similar, but use slightly different styles for defining the variables dictionary. Consider standardizing the coding style across these methods for better consistency and readability.
async def blocked_accounts_all(self, user_id, count, cursor): | |
async def blocked_accounts_all(self, user_id: str, count: int, cursor: Optional[str] = None) -> Dict: | |
variables = { | |
"userId": user_id, | |
"count": count, | |
"cursor": cursor, | |
} |
@@ -225,3 +225,29 @@ | |||
'longform_notetweets_inline_media_enabled': True, | |||
'responsive_web_enhance_cards_enabled': False | |||
} | |||
|
|||
BLOCKED_ACCOUNTS_ALL_FEATURES = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider using a single dictionary for feature flags.
The introduction of the BLOCKED_ACCOUNTS_ALL_FEATURES
dictionary increases complexity due to duplication of many entries from the existing feature flags dictionary. This duplication can lead to maintenance challenges and potential inconsistencies if updates are needed. To simplify, consider using a single dictionary for feature flags and introducing logic to handle context-specific differences. This approach will reduce duplication, improve maintainability, and enhance readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
twikit/client/gql.py (1)
697-708
: Review Feature Set for Blocked Accounts QueryThe
BLOCKED_ACCOUNTS_ALL_FEATURES
includes several flags that may not be directly relevant to the blocked accounts query. Consider reviewing the feature set to ensure that only necessary features are included, such as:
rweb_tipjar_consumption_enabled
responsive_web_graphql_exclude_directive_enabled
verified_phone_label_enabled
creator_subscriptions_tweet_preview_api_enabled
responsive_web_graphql_timeline_navigation_enabled
This can help optimize the query and reduce unnecessary complexity.
Analysis chain
Verify Feature Set Usage for Blocked Accounts.
The
blocked_accounts_all
method usesBLOCKED_ACCOUNTS_ALL_FEATURES
. Ensure that these features are appropriate for the blocked accounts query and do not include unnecessary flags.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the appropriateness of `BLOCKED_ACCOUNTS_ALL_FEATURES` for the blocked accounts query. # Test: Check the usage of feature flags in the method. Expect: Only relevant flags are used. rg --type python 'BLOCKED_ACCOUNTS_ALL_FEATURES' -A 5Length of output: 84
Script:
#!/bin/bash # Description: Verify the appropriateness of `BLOCKED_ACCOUNTS_ALL_FEATURES` for the blocked accounts query. # Test: Check the usage of feature flags in the method. Expect: Only relevant flags are used. rg --type py 'BLOCKED_ACCOUNTS_ALL_FEATURES' -A 5Length of output: 1297
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- twikit/client/client.py (2 hunks)
- twikit/client/gql.py (3 hunks)
- twikit/constants.py (1 hunks)
Additional comments not posted (4)
twikit/constants.py (1)
229-253
: Ensure Consistency and Avoid Redundancy in Feature Flags.The
BLOCKED_ACCOUNTS_ALL_FEATURES
dictionary introduces new feature flags specific to blocked accounts. Ensure that these flags do not duplicate existing flags in other dictionaries unless necessary. Additionally, verify that the naming conventions are consistent with other feature flags in the codebase.Verification successful
Feature Flags Are Unique and Consistent
The feature flags defined in
BLOCKED_ACCOUNTS_ALL_FEATURES
are unique within the codebase, ensuring no unnecessary duplication. This confirms consistency in the feature flag definitions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if feature flags in `BLOCKED_ACCOUNTS_ALL_FEATURES` are duplicated in other constants. # Test: Search for feature flags across the file. Expect: No unnecessary duplication. rg --type python '": (True|False)' --no-filename | sort | uniq -c | rg -v '^\s*1\s'Length of output: 116
Script:
#!/bin/bash # Description: Verify if feature flags in `BLOCKED_ACCOUNTS_ALL_FEATURES` are duplicated in other constants. # Corrected Test: Search for feature flags across Python files. Expect: No unnecessary duplication. rg '": (True|False)' --glob '*.py' | sort | uniq -c | rg -v '^\s*1\s'Length of output: 71
twikit/client/gql.py (1)
710-717
: Verify Feature Set Usage for Muted Accounts.The
muted_accounts
method usesBLOCKED_ACCOUNTS_ALL_FEATURES
, which may not be appropriate for muted accounts. Verify if a separate feature set is needed for this query.twikit/client/client.py (2)
2404-2436
: LGTM!The
get_blocked_users
method is well-implemented with clear documentation. It leverages existing methods to retrieve blocked users efficiently.
2480-2512
: LGTM!The
get_muted_users
method is consistent with theget_blocked_users
method and is well-documented. It effectively uses existing methods for data retrieval.
Summary by Sourcery
Add new APIs to retrieve lists of blocked and muted users with pagination support, and integrate new GraphQL endpoints to support these features.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation