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

Fix the currently broken facebook connector #1989

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

plantroon
Copy link
Contributor

Description

I wanted to use the facebook connector and found that it was broken. Sending a message from facebook side would result in opsdroid receiving it but then proceeding to use user_id as message's text, due to how the arguments were positioned.

I bumped the API version as well to the newest one.

I did not report an issue prior to this.

Status

READY

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation (fix or adds documentation)

How Has This Been Tested?

Simply by following instructions on setting up the facebook connector in documentation. I tested it with multiple skills and all work fine. Hello.py skill would however use the facebook's user_id instead of his actual name, but that is how this connector must have worked originally, as there doesn't seem to be a straight-forward way to get this name. If this is a problem I can look around the facebook API docs and implement it, though barely any skills use it.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

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

I'm so sorry it took so long to get around to reviewing this. Things have been slow here lately.

Overall I think this looks great. I pushed a commit to fix some linting issues, but otherwise I think this should be good to merge.

@jacobtomlinson jacobtomlinson enabled auto-merge (squash) April 25, 2024 11:14
@jacobtomlinson jacobtomlinson merged commit e69b463 into opsdroid:master Apr 25, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants