From a914334feda80d7b2f046da3545946d7c31d49e1 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Fri, 3 Apr 2020 13:32:42 +0200 Subject: [PATCH] Fix incorrect loading of messages In some instances when receiving messages from a mailsever in the chat you are in, the flag `all-loaded?` would not be reset, meaning that messages not in the current view would be added to the db, but would not be seen until actually reloading the chat (go back home, open again). Signed-off-by: Andrea Maria Piana --- src/status_im/chat/models/loading.cljs | 2 +- src/status_im/chat/models/message.cljs | 35 ++++---- src/status_im/ui/screens/chat/state.cljs | 4 +- src/status_im/ui/screens/chat/views.cljs | 4 +- .../status_im/test/chat/models/message.cljs | 80 ++++++++++++++++++- 5 files changed, 101 insertions(+), 24 deletions(-) diff --git a/src/status_im/chat/models/loading.cljs b/src/status_im/chat/models/loading.cljs index 10690ccd97..700d9d65ca 100644 --- a/src/status_im/chat/models/loading.cljs +++ b/src/status_im/chat/models/loading.cljs @@ -162,7 +162,7 @@ (when-let [current-chat-id (:current-chat-id db)] (if-not (get-in db [:chats current-chat-id :messages-initialized?]) (do - ; reset chat viewable-items state + ; reset chat first-not-visible-items state (chat.state/reset) (fx/merge cofx {:db (-> db diff --git a/src/status_im/chat/models/message.cljs b/src/status_im/chat/models/message.cljs index c15bd2ff3a..99db5c3c2c 100644 --- a/src/status_im/chat/models/message.cljs +++ b/src/status_im/chat/models/message.cljs @@ -104,33 +104,32 @@ (fx/defn add-received-message [{:keys [db] :as cofx} - {:keys [from - message-id - chat-id - clock-value - content] :as message}] + {:keys [chat-id + clock-value] :as message}] (let [{:keys [loaded-chat-id view-id current-chat-id]} db cursor-clock-value (get-in db [:chats current-chat-id :cursor-clock-value]) current-chat? (= chat-id loaded-chat-id)] - (when (and current-chat? - (or (not cursor-clock-value) - (<= cursor-clock-value clock-value))) - (if (or (not @view.state/viewable-item) - (not= current-chat-id - (:chat-id @view.state/viewable-item)) - (<= (:clock-value @view.state/viewable-item) + (when current-chat? + ;; If we don't have any hidden message or the hidden message is before + ;; this one, we add the message to the UI + (if (or (not @view.state/first-not-visible-item) + (<= (:clock-value @view.state/first-not-visible-item) clock-value)) (add-message cofx {:message message :seen-by-user? (and current-chat? (= view-id :chat))}) - ;; Not in the current view, offload to db and update cursor if necessary - (when (and (< clock-value - cursor-clock-value) - (= current-chat-id - (:chat-id @view.state/viewable-item))) - {:db (assoc-in db [:chats chat-id :cursor] (chat-loading/clock-value->cursor clock-value))}))))) + ;; Not in the current view, set all-loaded to false + ;; and offload to db and update cursor if necessary + {:db (cond-> db + (>= clock-value + cursor-clock-value) + (update-in [:chats chat-id] assoc + :cursor (chat-loading/clock-value->cursor clock-value) + :cursor-clock-value clock-value) + :always + (assoc-in [:chats chat-id :all-loaded?] false))})))) (defn- add-to-chat? [{:keys [db]} {:keys [chat-id clock-value message-id from]}] diff --git a/src/status_im/ui/screens/chat/state.cljs b/src/status_im/ui/screens/chat/state.cljs index b8e74276ae..094bde4d87 100644 --- a/src/status_im/ui/screens/chat/state.cljs +++ b/src/status_im/ui/screens/chat/state.cljs @@ -1,6 +1,6 @@ (ns status-im.ui.screens.chat.state) -(defonce viewable-item (atom nil)) +(defonce first-not-visible-item (atom nil)) (defn reset [] - (reset! viewable-item nil)) + (reset! first-not-visible-item nil)) diff --git a/src/status_im/ui/screens/chat/views.cljs b/src/status_im/ui/screens/chat/views.cljs index 8f5a1c5d34..354cbb2d28 100644 --- a/src/status_im/ui/screens/chat/views.cljs +++ b/src/status_im/ui/screens/chat/views.cljs @@ -95,7 +95,7 @@ (defn on-viewable-items-changed [e] (when @messages-list-ref - (reset! state/viewable-item + (reset! state/first-not-visible-item (when-let [last-visible-element (aget (.-viewableItems e) (dec (.-length (.-viewableItems e))))] (let [index (.-index last-visible-element) ;; Get first not visible element, if it's a datemark/gap @@ -158,4 +158,4 @@ [messages-view current-chat]]] (when show-input? [input/container]) - [bottom-sheet]])) \ No newline at end of file + [bottom-sheet]])) diff --git a/test/cljs/status_im/test/chat/models/message.cljs b/test/cljs/status_im/test/chat/models/message.cljs index 106860ed31..84db2b0b57 100644 --- a/test/cljs/status_im/test/chat/models/message.cljs +++ b/test/cljs/status_im/test/chat/models/message.cljs @@ -3,13 +3,91 @@ [status-im.utils.gfycat.core :as gfycat] [status-im.utils.identicon :as identicon] [status-im.constants :as constants] + [status-im.chat.models.loading :as chat-loading] [status-im.utils.datetime :as time] [status-im.transport.message.protocol :as protocol] [status-im.chat.models.message-list :as models.message-list] - + [status-im.ui.screens.chat.state :as view.state] [status-im.chat.models.message :as message] [status-im.utils.datetime :as time])) +(deftest add-received-message-test + (with-redefs [message/add-message (constantly :added)] + (let [chat-id "chat-id" + clock-value 10 + cursor-clock-value (dec clock-value) + cursor (chat-loading/clock-value->cursor cursor-clock-value) + cofx {:now 0 + :db {:loaded-chat-id chat-id + :current-chat-id chat-id + :all-loaded? true + :chats {chat-id {:cursor cursor + :cursor-clock-value cursor-clock-value}}}} + message {:chat-id chat-id + :clock-value clock-value}] + (testing "not current-chat" + (is (nil? (message/add-received-message + (update cofx :db dissoc :loaded-chat-id) + message)))) + ;; <- cursor + ;; <- message + ;; <- top of the chat + (testing "there's no hidden item" + (with-redefs [view.state/first-not-visible-item (atom nil)] + (is (= :added (message/add-received-message + cofx + message))))) + ;; <- cursor + ;; <- first-hidden-item + ;; <- message + ;; <- top of the chat + (testing "the hidden item has a clock value less than the current" + (with-redefs [view.state/first-not-visible-item (atom {:clock-value (dec clock-value)})] + (is (= :added (message/add-received-message + cofx + message))))) + ;; <- cursor + ;; <- message + ;; <- first-hidden-item + ;; <- top of the chat + (testing "the message falls between the first-hidden-item and cursor" + (with-redefs [view.state/first-not-visible-item (atom {:clock-value (inc clock-value)})] + (let [result (message/add-received-message + cofx + message)] + (testing "it sets all-loaded? to false" + (is (not (get-in result [:db :chats chat-id :all-loaded?])))) + (testing "it updates cursor-clock-value & cursor" + (is (= clock-value (get-in result [:db :chats chat-id :cursor-clock-value]))) + (is (= (chat-loading/clock-value->cursor clock-value) (get-in result [:db :chats chat-id :cursor]))))))) + ;; <- message + ;; <- first-hidden-item + ;; <- top of the chat + (testing "the message falls between the first-hidden-item and cursor is nil" + (with-redefs [view.state/first-not-visible-item (atom {:clock-value (inc clock-value)})] + (let [result (message/add-received-message + (update-in cofx [:db :chats chat-id] dissoc :cursor :cursor-clock-value) + message)] + (testing "it sets all-loaded? to false" + (is (not (get-in result [:db :chats chat-id :all-loaded?])))) + (testing "it updates cursor-clock-value & cursor" + (is (= clock-value (get-in result [:db :chats chat-id :cursor-clock-value]))) + (is (= (chat-loading/clock-value->cursor clock-value) (get-in result [:db :chats chat-id :cursor]))))))) + ;; <- message + ;; <- cursor + ;; <- first-hidden-item + ;; <- top of the chat + (testing "the message falls before both the first-hidden-item and cursor" + (with-redefs [view.state/first-not-visible-item (atom {:clock-value (inc clock-value)})] + (let [result (message/add-received-message + cofx + (update message :clock-value #(- % 2)))] + (testing "it sets all-loaded? to false" + (is (not (get-in result [:db :chats chat-id :all-loaded?])))) + (testing "it does not update cursor-clock-value & cursor" + (is (= cursor-clock-value (get-in result [:db :chats chat-id :cursor-clock-value]))) + (is (= cursor (get-in result [:db :chats chat-id :cursor])))))))))) + (deftest add-to-chat? (testing "it returns true when it's not in loaded message" (is (message/add-to-chat? {:db {:chats {"a" {}}}}