-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix farcaster following constraint #628
Conversation
Reviewer's Guide by SourceryThis pull request fixes a bug in the Farcaster following constraint implementation. The changes modify how the follow status is retrieved and accessed in two methods: File-Level Changes
Tips
|
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 @PooyaFekri - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
""" | ||
try: | ||
following_fid = self._get_profile(address)["fid"] | ||
return self._get_follow_status(fid, following_fid) | ||
return self._get_follow_status(fid, following_fid)[following_fid] |
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 (bug_risk): Potential KeyError risk in dictionary access
The changes to is_following
and be_followed_by
methods now assume that _get_followers_status
and _get_follow_status
return dictionaries with specific keys. This could lead to KeyErrors if the expected keys are not present. Consider using the dict.get()
method with a default value or checking for the key's existence before accessing it. Also, ensure that all code relying on these methods is updated to handle the new return type.
""" | |
try: | |
following_fid = self._get_profile(address)["fid"] | |
return self._get_follow_status(fid, following_fid) | |
return self._get_follow_status(fid, following_fid)[following_fid] | |
return self._get_followers_status(follower_fid, fid).get(fid, False) | |
except ( | |
RequestException, | |
IndexError, | |
""" | |
try: | |
following_fid = self._get_profile(address)["fid"] | |
return self._get_follow_status(fid, following_fid).get(following_fid, False) |
Summary by Sourcery
Fix the Farcaster following constraint by correcting the way follower status is accessed in the is_following and be_followed_by methods.
Bug Fixes: