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

Add telegram notification support #33

Merged
merged 11 commits into from
Jun 7, 2021
Merged

Conversation

sohm22
Copy link
Contributor

@sohm22 sohm22 commented May 15, 2021

  • new flag to choose notifier between email and telegram ex :- --notifier-type telegram
  • add other flags to add telegram details --telegram-token --telegram-username
  • add telegram bot initialization and send notification functionality
  • update notification workflow
  • create document for telegram bot integration with covaccine-notifier
  • send telegram notification as file if message length exceeds 4096 characters
  • this PR addresses the issue Instead of Email can we notify to Whatsapp or Telegram  #24

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated
@@ -13,6 +13,7 @@ import (

var (
pinCode, state, district, email, password, date, vaccine, fee string
notifier, tgApiToken, tgUsername string
Copy link
Owner

Choose a reason for hiding this comment

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

Let's declare a interface Notifier which implements Send() method. We can have a separate package notifier for this. Let's refactor the notification workflow

pkg/notify/email.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
pkg/notify/telegram.go Show resolved Hide resolved
pkg/notify/telegram.go Outdated Show resolved Hide resolved
Comment on lines 49 to 52
if len(body) > 4096 {
log.Printf("message body too long, trimming it")
body = body[:4095]
}
Copy link
Owner

Choose a reason for hiding this comment

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

why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when started notifier using state and district name
I have encountered error for message greater than 4096 characters.

Copy link
Owner

Choose a reason for hiding this comment

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

Should we split and send messages into 2 parts? Or send message as a file?

search.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@PrasadG193
Copy link
Owner

I think we should add subcommand for each notifier type instead of adding flags, something like -

covaccine-notifier email --username --password ...
covaccine-notifier telegram --token ...

@sohm22 sohm22 requested a review from PrasadG193 May 31, 2021 15:48
README.md Outdated
covaccine-notifier [command]

Available Commands:
email Notify slots avaliability using email
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
email Notify slots avaliability using email
email Notify slots availability using email

README.md Outdated
Available Commands:
email Notify slots avaliability using email
help Help about any command
telegram Notify slots availability using telegram
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
telegram Notify slots availability using telegram
telegram Notify slots availability using Telegram

main.go Outdated
Use: "telegram [FLAGS]",
Short: "Notify slots availability using telegram",
RunE: func(cmd *cobra.Command, args []string) error {
notifierType = NotifierType(TelegramNotifierType)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we need to typecast, TelegramNotifierType and EmailNotifierType is already of type NotifierType

Copy link
Owner

Choose a reason for hiding this comment

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

We can just initialize and pass notifier.Notifier as an arg to Run from here

main.go Outdated
Comment on lines 73 to 74
EmailNotifierType NotifierType = "email"
TelegramNotifierType NotifierType = "telegram"
Copy link
Owner

Choose a reason for hiding this comment

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

Lets move this to notifier package

main.go Outdated
func Run(args []string) error {
if err := checkFlags(); err != nil {
return err
}
if err := checkSlots(); err != nil {
notifier, err := getNotifier()
Copy link
Owner

Choose a reason for hiding this comment

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

We can pass notify.Notifier as a arg to Run(). We don't need getNotifier()

)

type Email struct {
Id string
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Id string
ID string

just a Go way of naming variables :)

@@ -0,0 +1,5 @@
package notify

type Notifier interface {
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a comment on exported functions and types. This applies to other changes as well

log.Printf("Authorized on account %s", bot.Self.UserName)

u := tgbotapi.NewUpdate(0)
u.Timeout = 60
Copy link
Owner

Choose a reason for hiding this comment

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

Please declare timeout value as const and use here

Comment on lines 49 to 52
if len(body) > 4096 {
log.Printf("message body too long, trimming it")
body = body[:4095]
}
Copy link
Owner

Choose a reason for hiding this comment

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

Should we split and send messages into 2 parts? Or send message as a file?

search.go Outdated
Comment on lines 265 to 268
if err := notifier.SendMessage(buf.String()); err != nil {
return err
}
return nil
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if err := notifier.SendMessage(buf.String()); err != nil {
return err
}
return nil
return notifier.SendMessage(buf.String())

@sohm22 sohm22 requested a review from PrasadG193 June 2, 2021 05:01
README.md Outdated
```

**Note:** Gmail password won't work for 2FA enabled accounts. Follow [this](https://support.google.com/accounts/answer/185833?p=InvalidSecondFactor&visit_id=637554658548216477-2576856839&rd=1) guide to generate app token password and use it with `--password` arg

**Note:** For telegram bot integration with covaccine-notifier follow [this](./docs/telegram-integration.md).
Copy link
Owner

Choose a reason for hiding this comment

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

Let's add a sub heading - Integration with Telegram

pkg/notify/telegram.go Show resolved Hide resolved
@sohm22 sohm22 requested a review from PrasadG193 June 5, 2021 19:11
@PrasadG193 PrasadG193 merged commit 08e980e into PrasadG193:main Jun 7, 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.

None yet

2 participants