Skip to content

Conversation

@nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Oct 10, 2017

The slack backend already splits up large messages into MESSAGE_SIZE_LIMIT for the send_message method. This patch uses the same logic for the send_card method.

@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 11, 2017

This pull request is in response to an issue raise for the err-stackstorm plugin. fmnisme/err-stackstorm#16

@ghost ghost added the in progress label Oct 11, 2017
Since the message may be posted in languages other than English, I think it'd be better to format the part as `[x/y]` rather than `[x of y]`.
@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 12, 2017

@gbin @zoni Does this patch seem like an acceptable fix to be merged for the next point release?

@zoni
Copy link
Member

zoni commented Oct 29, 2017

@nzlosh this looks like a sane approach to me, I'd be happy to merge it. To help me verify this works correctly before I merge it though, could you possibly show me a minimal example plugin which:

  1. Has a command to send a card which fits within the limit.
  2. Has a command to send a card which gets split into multiple parts.

That way I can easily run it against a slack instance and see the behavior for myself.

@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 30, 2017

@zoni I've written a simple plugin that will allow you test the multi-part message for cards. You can find the code here. https://gist.github.com/nzlosh/8230a9c41cdfb5d59df5845a390840dc

@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 30, 2017

Something I noticed when testing was to post to #channel_name you have to provide 2 hashes ##channel_name. Without 2 hashes errbot's build_identifier reports an error. This is the same behaviour in the webclient under Linux and the native client under Mac OSX. From what I can tell the slack client is consuming the # before the message is being picked up by the bot.

@nzlosh
Copy link
Contributor Author

nzlosh commented Oct 30, 2017

I just saw the log in errbot's output:

15:20:38 DEBUG    errbot.backends.slack     Processing slack event: {'type': 'message', 'text': '.testcard <#C7SM7GMHD|ops> 1', 'user': 'U110FGZSQ', 'ts': '1509373238.000164
', 'source_team': 'T0V6H6HCJ', 'team': 'T0V6H6HCJ', 'channel': 'C7SM7GMHD'}

The channel is seen as <#C7SM7GMHD|ops> in the event. It looks like the slack backend may need to be updated to handle this format?

@nzlosh
Copy link
Contributor Author

nzlosh commented Dec 8, 2017

@zoni Did the gist example allow you test this PR?

@nzlosh
Copy link
Contributor Author

nzlosh commented Jan 25, 2018

@zoni or @gbin are there issues that are preventing this pull request from being merged?

@zoni
Copy link
Member

zoni commented Feb 20, 2018

Only a severe lack of time/energy on my part, for which I apologize. I'm going to try looking at this real soon. Please pester me again if I haven't gotten back to you on this in a week's time.

@zoni
Copy link
Member

zoni commented Mar 1, 2018

I finally got around to looking at this, and can confirm it works as intended. My apologies for not getting to it sooner. I'll merge this in now, the build failure should resolve itself as the cause of that has since been fixed on master.

Thanks!

@zoni zoni merged commit 98dbb6d into errbotio:master Mar 1, 2018
@ghost ghost removed the in progress label Mar 1, 2018
gbin-argo pushed a commit that referenced this pull request May 15, 2018
…1118)

Since the message may be posted in languages other than English, I think it'd be better to format the part as `[x/y]` rather than `[x of y]`.
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