-
Notifications
You must be signed in to change notification settings - Fork 39
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
653 fr slack connector post message add option to post to an existing thread #2773
653 fr slack connector post message add option to post to an existing thread #2773
Conversation
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.
Left some minor comments, note that you also need to update the web-modeler templates here is an example
@@ -133,4 +100,42 @@ protected static Stream<String> loadTestCasesFromResourceFile(final String fileW | |||
}) | |||
.map(Arguments::of); | |||
} | |||
|
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 might need to fix your formatted, this bloc has been moved apparently
@@ -54,11 +47,17 @@ class ChatPostMessageDataTest { | |||
} | |||
} | |||
|
|||
@Mock private MethodsClient methodsClient; |
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
@@ -80,7 +79,7 @@ void invoke_shouldThrowExceptionWhenUserWithoutEmail() throws SlackApiException, | |||
void invoke_shouldFindUserIdByEmail(String email) throws SlackApiException, IOException { | |||
// Given | |||
ChatPostMessageData chatPostMessageData = | |||
new ChatPostMessageData(email, "plainText", "test", null); | |||
new ChatPostMessageData(email, "thread_ts", "", "test", null); |
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.
plainText
disappeared is it expected?
} | ||
] | ||
"""; | ||
[ |
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 formatting issue
} | ||
} | ||
"""; | ||
{ |
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.
here as well :)
4dd650b
to
142222f
Compare
…nd connector refactor the test to include the new field
142222f
to
f6ee79c
Compare
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.
🟢 flag, great job!
Description
Users were not able to create a new thread from an existing message or add a message to an existing thread. this solves the issues.
I added a new optional field to the slack outbound connector
thread
which takes the id of a message to start a thread from.This id has to be gotten from the output mapping
message.ts
which contains the id of the posted messageRelated issues
There might be an issue in the future when an user wants to add messages (using camunda) inside a thread that has been create by a normal user (human), as I don't know how to retrieve the id.
closes https://github.com/camunda/team-connectors/issues/653