Skip to content
This repository has been archived by the owner on Jun 7, 2020. It is now read-only.

[NEW] Send messages via HTTP + WebSockets fallback #1053

Merged
merged 5 commits into from
Dec 7, 2017

Conversation

cardoso
Copy link
Collaborator

@cardoso cardoso commented Dec 7, 2017

@RocketChat/ios

  • Messages are confirmed faster now and should be more reliable.
  • Only works on servers >= 0.60.0, otherwise we fallback to the old WebSocket method.

Testable on: https://unstable.rocket.chat

Closes #830
Closes #473

TODO

  • Tests

extension APIResult where T == SendMessageRequest {

}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing Newline Violation: Files should have a single trailing newline. (trailing_newline)

@codecov
Copy link

codecov bot commented Dec 7, 2017

Codecov Report

Merging #1053 into develop will increase coverage by 1.56%.
The diff coverage is 75.75%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1053      +/-   ##
===========================================
+ Coverage    43.39%   44.95%   +1.56%     
===========================================
  Files          246      243       -3     
  Lines        13938    13688     -250     
  Branches       770      770              
===========================================
+ Hits          6048     6154     +106     
+ Misses        7425     7077     -348     
+ Partials       465      457       -8
Impacted Files Coverage Δ
...ket.Chat/Controllers/Chat/ChatViewController.swift 19.19% <0%> (+0.53%) ⬆️
...ket.ChatTests/API/Clients/MessagesClientSpec.swift 100% <100%> (ø)
...Chat/API/Requests/Message/SendMessageRequest.swift 31.25% <31.25%> (ø)
Rocket.Chat/API/Clients/MessagesClient.swift 73.17% <73.17%> (ø)
Rocket.Chat/Extensions/API/APIExtensions.swift 50% <0%> (-50%) ⬇️
...ket.Chat/Controllers/Chat/ChatDataController.swift 72.41% <0%> (-0.62%) ⬇️
.../Controllers/Chat/ChatControllerAutocomplete.swift 3.57% <0%> (-0.38%) ⬇️
Rocket.ChatTests/Models/UserSpec.swift 89.53% <0%> (-0.21%) ⬇️
Rocket.ChatTests/RealmTestCase.swift 100% <0%> (ø) ⬆️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77867be...9c18656. Read the comment docs.

@cardoso cardoso changed the title [WIP][NEW] Send messages via HTTP + WebSockets fallback [NEW] Send messages via HTTP + WebSockets fallback Dec 7, 2017
@cardoso cardoso requested a review from rafaelks December 7, 2017 13:27
case .version:
// old server version: fallback to web sockets
SubscriptionManager.sendTextMessage(message, completion: { response in
updateMessage(json: response.result["result"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the TODO here?👌

}, errored: { error in
switch error {
case .version:
// TODO: Remove SendMessage Fallback + old methods after Rocket.Chat 1.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo Violation: TODOs should be avoided (Remove SendMessage Fallback + ...). (todo)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shhhhhhh....

}, errored: { error in
switch error {
case .version:
// TODO: Remove SendMessage Fallback + old methods after Rocket.Chat 1.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo Violation: TODOs should be avoided (Remove SendMessage Fallback + ...). (todo)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shhhhhhh....

@rafaelks rafaelks merged commit 8f5249e into develop Dec 7, 2017
@rafaelks rafaelks deleted the feat/send_message_http.830 branch December 7, 2017 15:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants