jbot
jbot copied to clipboard
Added FILE_SHARE subtype for incoming message, new event added
Hi there,
Been using JBot for recieving files sent to slack channel. Faced an issue with FILE_SHARED message, the File object I received was containing nulls for most of the fields.
Looked it up here: https://api.slack.com/tutorials/working-with-files. Turned out that "file_shared" event isn't what I needed to listen to and event "message" with subtype "file_share" is.
This way you'll get pretty populated File object (I needed urls for download) right away in the event. Otherwise all you could get is File ID and then have to make an additional request to Slack API for the url.
So all I've done was adding FILE_SHARE_MESSAGE event type as you can see to catch it properly. Hope it makes sense.
Thanks for the JBot, Fedor
I will merge it in a day or two but by any chance is it possible for you to write a unit test for your changes?
Hi Ram, thanks a lot, looking forward to it!
Actually I don't know if it's possible to unit test my change -- I've added a new event type, so as I understand this should rather be tested by some integration test.
And in this case it would involve quite a few more changes, because for now there is just one simple unit test and no integration test harness implemented.
Hi Ram,
I've found some conflicts, so I pulled the changes from origin/master and resolved them.
Then I've found a failed unit test which helped me to fix an empty subtype handilng. And also with its help I've managed to cover my new feature with the test.
So, it seems like all set now. What do you think, can we merege the PR?
Hey,
Thanks for PR (and the unit test). Will review it and merge shortly.
Hi Ram,
Sorry to bother you, but is there anything I can do to help merge the PR?
We're using this new functionality in a small pet project, and merging to master and build new release version would really ease the building process as I could just update JBot version then.
Thanks a lot, Fedor
Hi Fedor, I haven't merged this PR because there are many subtypes under message event type. So, I was thinking of including everything as events in EventType.java
so that you could listen to them like @Controller(events = EventType.MESSAGE_FILE_SHARE)
and @Controller(events = EventType.MESSAGE_BOT_MESSAGE)
etc. I may do this change shortly but if you can manage to raise a PR for the same, it will be great and if not, you can listen for MESSAGE
event type and check for the subtype inside the controller function.
Also, we need to modify EventType.java
to contain the string value which Slack has (to avoid overlapping between events and subevents) so that we don't have to declare all the strings in Bot.java like you've done in this PR.
+1