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

changes for telegram bot #17

Merged
merged 5 commits into from
Apr 2, 2021
Merged

changes for telegram bot #17

merged 5 commits into from
Apr 2, 2021

Conversation

antonyip
Copy link
Contributor

@antonyip antonyip commented Mar 9, 2021

Hello, first time working on this repo, please let me know if I made any mistakes..

Youtube demo:
https://www.youtube.com/watch?v=xjWYWsJYaL4&ab_channel=AntonYip

First Try of Youtube (OLD)
https://www.youtube.com/watch?v=P2hQGfVEJe8&ab_channel=AntonYip

Gitcoin:
https://gitcoin.co/issue/ceramicnetwork/ceramic/45/100025010

@oed
Copy link
Member

oed commented Mar 11, 2021

Hey @antonyip please check out the API documentation. We want something similar to the flow of the discord verification bot, but that works for telegram.

@oed
Copy link
Member

oed commented Mar 11, 2021

This script is useful when testing: https://gist.github.com/oed/5d0e3923e7c1a9f3ff674a42ddabc622

@antonyip
Copy link
Contributor Author

Hi @oed,

I'm pretty sure I've added all the functions, I am just having 1 last problem trying to generate a jwt to test.
Using your script, I'm running nodejs tester.js
and I get this error
(node:14730) UnhandledPromiseRejectionWarning: Error: DID is not authenticated

Let me know if you've seen this issue before.

@antonyip
Copy link
Contributor Author

I figured out how to test it finally..

https://www.youtube.com/watch?v=xjWYWsJYaL4&ab_channel=AntonYip

Copy link
Member

@oed oed left a comment

Choose a reason for hiding this comment

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

Looks good so far. Can you also add the confirm-telegram endpoint to the API.md file?

How can I test this?

@@ -142,6 +142,19 @@ functions:
method: post
cors: true
path: /api/v0/confirm-discord
verify-telegram:
Copy link
Member

Choose a reason for hiding this comment

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

Can you rename this to "confirm-telegram"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thing, Discord is also using verify..

claimMgr,
analytics
)
module.exports.verify_telegram = (event, context, callback) => {
Copy link
Member

Choose a reason for hiding this comment

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

Rename to confirm. Would be a bit confusing otherwise. All other methods that accept the JWS use the confirm language.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discord is also using verify.. Might want to make more change requests then..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, i just doubled-checked, everything there is using verify.. Would like a confirmation to do the rename..

Signed-off-by: Anton Yip <Anton_Yip@hotmail.com>
@antonyip
Copy link
Contributor Author

Looks good so far. Can you also add the confirm-telegram endpoint to the API.md file?

How can I test this?

Done for confirm-telegram in API.md

Steps to test

  1. Make a bot with telegram -> https://github.com/botgram/botgram/blob/master/docs/tutorial.md
  2. add bot-token to .env in bot folder
  3. Run your redis server
  4. Run the telegram-bot -> "source .env && node src/telegram.js"
  5. Deploy the serverless server -> I used "serverless deploy"
  6. trigger your test script -> https://gist.github.com/oed/5d0e3923e7c1a9f3ff674a42ddabc622

You should be able to proceed with the same steps in my video.
https://www.youtube.com/watch?v=xjWYWsJYaL4&ab_channel=AntonYip

@oed
Copy link
Member

oed commented Mar 25, 2021

Thanks @antonyip sorry for the delay here. I will get back to this soon!

Copy link
Member

@oed oed left a comment

Choose a reason for hiding this comment

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

Confirmed this to be working locally!

API.md Outdated Show resolved Hide resolved
@oed oed merged commit c9085d3 into ceramicstudio:master Apr 2, 2021
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.

2 participants