slackapi / java-slack-sdk

Slack Developer Kit (including Bolt for Java) for any JVM language
https://tools.slack.dev/java-slack-sdk/
MIT License
580 stars 218 forks source link

ChatUpdate for blocks doesn't update the message #128

Closed cjcdoomed closed 5 years ago

cjcdoomed commented 5 years ago

https://github.com/seratch/jslack/blob/b484515bcb0d9fbaa15167a98850cb1b13b8948c/src/test/java/com/github/seratch/jslack/Slack_blockkit_Test.java#L95

When you run the test you get ok=true in the response ChatUpdateResponse(ok=true, warning=null, error=null, channel=xxxxx, ts=1555595439.003400, text=Modified text) However if you look at the message, the additional divider block after the ActionsBlock does not display.

I'm not sure if this is a Jslack issue or an issue with chat.update api. I've verified that if you don't build the request with blocks() and just use text() the ChatUpdate works and you can see the updated message in the Slack app. Great API btw!

d-subrahmanyam commented 5 years ago

Seems like the test itself has an issue.

List<LayoutBlock> newBlocks = new ArrayList<>();
        Collections.copy(blocks, newBlocks);
        newBlocks.add(new DividerBlock());
        {
            ChatUpdateRequest request = ChatUpdateRequest.builder()
                    .token(token)
                    .text("Modified text")
                    .channel(postResponse.getChannel())
                    .ts(postResponse.getTs())
                    .blocks(newBlocks).build();

            ChatUpdateResponse response = slack.methods().chatUpdate(request);

            assertThat(response.getError(), is(nullValue()));
            assertThat(response.isOk(), is(true));
        }

There is an issue In the above snippet from BlockKit_Test.java at line 178 - Collections.copy(blocks, newBlocks);. The usage of Collections.copy is wrong. It's supposed to be Collections.copy(dest, src).

I believe that line is supposed to change to Collections.copy(newBlocks, blocks);.

seratch commented 5 years ago

Thanks, both of you. I've fixed the tests.