-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Viber notificator #5097
base: master
Are you sure you want to change the base?
Viber notificator #5097
Conversation
"notificator.viber.key", | ||
List.of(KeyType.CONFIG)); |
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.
Please fix indentation.
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.
What formatter are you using, because my Visual Studio code uses a different formatting and it changes formatting as soon as I save, I had to manually save without formatting.
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.
I use IntelliJ.
/** | ||
* Viber notification send location message. | ||
*/ | ||
public static final ConfigKey<Boolean> NOTIFICATOR_VIBER_SEND_LOCATION = new BooleanConfigKey( |
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.
We should probably unify this config parameter between telegram and viber.
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.
What would we name this unified parameter ? Is NOTIFICATOR_SEND_LOCATION = "notificator.sendLocation" okey ?
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.
I think we may keep this as it is for now to ensure backward compatibility, changing the parameter name would require modifications in traccar.xml when upgrading from an older version for anyone using this features. What do you think ?
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.
NOTIFICATOR_SEND_LOCATION = "notificator.sendLocation"
sounds good to me. I think it's a rarely used key, so it's ok to break backward compatibility. Also, it doesn't really break notifications. You just won't get the location info.
* Copyright 2019 - 2023 Anton Tananaev (anton@traccar.org) | ||
* Copyright 2021 Rafael Miquelino (rafaelmiquelino@gmail.com) |
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.
Fix this. You can either put your name/email or keep mine, but the dates should be right.
locationMessage.location = new LatLng(); | ||
locationMessage.location.latitude = position.getLatitude(); | ||
locationMessage.location.longitude = position.getLongitude(); |
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.
locationMessage.location = new LatLng(); | |
locationMessage.location.latitude = position.getLatitude(); | |
locationMessage.location.longitude = position.getLongitude(); | |
LatLng latLng = new LatLng(); | |
latLng.latitude = position.getLatitude(); | |
latLng.longitude = position.getLongitude(); | |
locationMessage.location = latLng; |
private int minApiVersion = 1; | ||
} | ||
|
||
public static class LatLng { |
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.
I would probably rename this to Location
.
client.target(urlSendText).request().header("X-Viber-Auth-Token", | ||
config.getString(Keys.NOTIFICATOR_VIBER_KEY)).post(Entity.json(message)).close(); |
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.
I think you should get the key once in the constructor and save it in a variable.
Also please reformat the request to something like:
client.target(urlSendText).request().header("X-Viber-Auth-Token", | |
config.getString(Keys.NOTIFICATOR_VIBER_KEY)).post(Entity.json(message)).close(); | |
client.target(urlSendText).request() | |
.header("X-Viber-Auth-Token", ...) | |
.post(Entity.json(message)) | |
.close(); |
client.target(urlSendLocation).request().header("X-Viber-Auth-Token", | ||
config.getString(Keys.NOTIFICATOR_VIBER_KEY)).post( | ||
Entity.json(createLocationMessage(message.chatId, position))) |
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.
Same here.
It is nice, but unfortunatelly a more interesting part is missing: getting viber uids from the viber webhook callbacks. |
I added a ViberNotificator based on the TelegramNotificator, it works the same way.
You need a bot API Key and a user unique ID obtained from webhook sent by viber (read viber documentation for details: https://developers.viber.com/docs/api/rest-bot-api/)
We tested this notificator and we are using it in our current project built on top of Traccar. We built an OTP system where we ask a user to send a code to our bot in order to get his/her viber_uid.
We are sharing this with the community in case you find this interesting.