Fix usage of typing API #1088
Fix usage of typing API #1088
Conversation
|
Please could you merge |
- Timeout is a required parameter when starting typing. - The typing parameter takes a bool not an integer. Signed-off-by: Kurt Roeckx <[email protected]>
|
All the test suite errors look like bugs in Synapse and Dendrite. |
| @@ -19,3 +24,50 @@ | |||
| Future->done(1); | |||
| }); | |||
| }; | |||
|
|
|||
| test "PUT /rooms/:room_id/typing/:user_id without timeout fails", | |||
There was a problem hiding this comment.
what makes you think timeout is mandatory? https://spec.matrix.org/unstable/client-server-api/#put_matrixclientr0roomsroomidtypinguserid doesn't mark it as required.
There was a problem hiding this comment.
re #1088 (comment): I don't agree with your interpretation.
There was a problem hiding this comment.
The reason I think it's required when typing is true are:
- It says:
If false, the timeout key can be omitted., which to me implies that it needs to be present when it's true. - It says
This tells the server that the user is typing for the next N milliseconds where N is the value specified in the timeout key. That implies to me that we need to have a value.
Note that I'm not the only one interpreting it that way. If you think this is wrong, please fix the specifications. Adding a default value on the server side is easy enough.
There was a problem hiding this comment.
So to clarify it more, those are the options:
| typing | timeout |
|---|---|
| true | present |
| true | not present |
| false | present |
| false | not present |
The can be omitted to me means that both the 3rd and 4th option are valid. Without the sentence, option 3 and 4 are also valid. But since the sentence is there, to me it says that if it's true you can not omit it.
tests/10apidoc/35room-typing.pl
Outdated
| test "PUT /rooms/:room_id/typing/:user_id with invalid json fails", | ||
| requires => [ local_user_and_room_fixtures() ], | ||
|
|
||
| proves => [qw( can_set_room_typing )], | ||
|
|
||
| do => sub { | ||
| my ( $user, $room_id ) = @_; | ||
|
|
||
| do_request_json_for( $user, | ||
| method => "PUT", | ||
| uri => "/r0/rooms/$room_id/typing/:user_id", | ||
|
|
||
| content => { | ||
| typing => 1, | ||
| timeout => 30000, | ||
| }, | ||
| )->main::expect_http_400() | ||
| ->then( sub { | ||
| my ( $response ) = @_; | ||
| my $body = decode_json( $response->content ); | ||
| assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); | ||
| Future->done( 1 ); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
this looks good, but we can't really merge it until synapse (and preferably dendrite, if it has the same problem) are fixed. Any chance you could raise issues against those projects?
|
what makes you think `timeout` is mandatory? https://spec.matrix.org/unstable/client-server-api/#put_matrixclientr0roomsroomidtypinguserid doesn't mark it as required.
My interpretation is that it's required when typing is true, but not
when it's false.
|
Signed-off-by: Kurt Roeckx <[email protected]>
|
blocked by matrix-org/synapse#10497 |
Signed-off-by: Kurt Roeckx [email protected]