-
Notifications
You must be signed in to change notification settings - Fork 29
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
Message thread #12
base: master
Are you sure you want to change the base?
Message thread #12
Conversation
Add message thread, and tests
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.
Thanks for this contribution @ArdusJax! Sorry for the very late response. Are you still up for finishing this?
I didn't run the tests or check if it works yet, but if you'd like to get this merged I'll see what can be done :)
@@ -21,7 +21,9 @@ class IncomingMessageProcessor(eventBus: MessageEventBus) extends Actor with Act | |||
val incomingMessage: IncomingMessage = mType match { | |||
case MessageType("hello", _) => Hello | |||
case MessageType("pong", _) => Pong | |||
case MessageType("message", None) => s.parseJson.convertTo[BaseMessage] | |||
case MessageType("message", _) => s.parseJson.convertTo[BaseMessage] |
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 this case will match also Some
, unless the order is reversed.
implicit object RepliesJsonReader extends RootJsonFormat[Replies] { | ||
|
||
def read(value: JsValue) = { | ||
println(value.getClass) |
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 remove ;)
println(value.getClass) | ||
value match { | ||
case JsArray(replies) => | ||
val r = replies.map{ x => |
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.
Let's add a space before {
. Also, since x
and r
aren't very descriptive, we might as well do
Replies {
replies.map(_.convertTo[Reply])
}
} | ||
} | ||
|
||
implicit object RepliesJsonReader extends RootJsonFormat[Replies] { |
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.
Do we need a Format
? Would Reader
be enough?
override def write(obj: Replies): JsValue = serializationError("not supported") | ||
} | ||
|
||
implicit object ReplyJsonReader extends RootJsonFormat[Reply] { |
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 question about format vs reader
@@ -10,7 +10,7 @@ class CommandsRecognizerBot(override val bus: MessageEventBus) extends IncomingM | |||
|
|||
def receive: Receive = { | |||
|
|||
case bm@BaseMessage(text, channel, user, dateTime, edited) => | |||
case bm@BaseMessage(text, channel, user, dateTime, Some(thread_ts),edited) => |
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 happens if we get None
in place of thread_ts
?
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.
Missing space before edited
Add message thread, and tests. This allows bots to processes and respond in a message thread.