From cbd920f0707c9426b650cc9dabf28970ef5ce20c Mon Sep 17 00:00:00 2001 From: Jamie Caprani Date: Tue, 31 Jan 2023 14:57:17 +0000 Subject: [PATCH] chore: fix sorting of pinned messages (#14874) --- .../chat/messages/pin/banner/view.cljs | 14 ++- src/status_im2/subs/chat/messages.cljs | 18 +++- src/status_im2/subs/chat/messages_test.cljs | 102 ++++++++++++++++++ 3 files changed, 123 insertions(+), 11 deletions(-) diff --git a/src/status_im2/contexts/chat/messages/pin/banner/view.cljs b/src/status_im2/contexts/chat/messages/pin/banner/view.cljs index d625ff37f8..970935c15c 100644 --- a/src/status_im2/contexts/chat/messages/pin/banner/view.cljs +++ b/src/status_im2/contexts/chat/messages/pin/banner/view.cljs @@ -4,14 +4,12 @@ (defn banner [chat-id] - (let [pinned-messages (rf/sub [:chats/pinned chat-id]) - latest-pin-id (-> pinned-messages - vals - first - (get :message-id)) - latest-pin-text (get-in (rf/sub [:chats/chat-messages chat-id]) - [latest-pin-id :content :text]) - pins-count (count (seq pinned-messages))] + (let [pinned-messages (rf/sub [:chats/pinned-sorted-list chat-id]) + latest-pin-text (->> pinned-messages + last + :content + :text) + pins-count (count pinned-messages)] [quo/banner {:latest-pin-text latest-pin-text :pins-count pins-count diff --git a/src/status_im2/subs/chat/messages.cljs b/src/status_im2/subs/chat/messages.cljs index b902591aff..5af01f3797 100644 --- a/src/status_im2/subs/chat/messages.cljs +++ b/src/status_im2/subs/chat/messages.cljs @@ -149,14 +149,26 @@ {} pin-messages)))) +;; local messages will not have a :pinned-at key until user navigates away and to +;; chat screen. For this reason we want to retain order of local messages with :pinned-at nil +;; as these will be a stack on top of the messages, however we do want to sort previous messages +;; from backend that have a :pinned-at value. +(defn sort-pinned + [a b] + (cond + (and a b) (- a b) + (or a b) (if b false true) + :else a)) + (re-frame/reg-sub :chats/pinned-sorted-list (fn [[_ chat-id] _] (re-frame/subscribe [:chats/pinned chat-id])) (fn [pin-messages _] - (->> pin-messages - vals - (sort-by :pinned-at <)))) + (let [pin-messages-vals (vals pin-messages)] + + (sort-by :pinned-at sort-pinned pin-messages-vals)))) + (re-frame/reg-sub :chats/pin-modal diff --git a/src/status_im2/subs/chat/messages_test.cljs b/src/status_im2/subs/chat/messages_test.cljs index af5d3ef9ba..121f1bd6ed 100644 --- a/src/status_im2/subs/chat/messages_test.cljs +++ b/src/status_im2/subs/chat/messages_test.cljs @@ -1,6 +1,9 @@ (ns status-im2.subs.chat.messages-test (:require [cljs.test :refer [deftest is testing]] [status-im2.subs.chat.messages :as messages] + [test-helpers.unit :as h] + [utils.re-frame :as rf] + [re-frame.db :as rf-db] [status-im2.constants :as constants])) (def messages-state @@ -50,3 +53,102 @@ (is (= {:type :datemark :value "Dec 31, 1999"} d2))))) + +(def pinned-messages-state + {:0xChat {:0x2 {:chat-id :0xChat + :message-id :0x2 + :pinned-at 2000 + :pinned-by :test-user} + :0x1 {:chat-id :0xChat + :message-id :0x1 + :pinned-at 1000 + :pinned-by :test-user} + :0x3 {:chat-id :0xChat + :message-id :0x3 + :pinned-at 3000 + :pinned-by :test-user}}}) + +(def pinned-messages-state-with-1-new-local-message + {:0xChat {:0x3 {:chat-id :0xChat + :message-id :0x3 + :pinned-at nil + :pinned-by :test-user} + :0x2 {:chat-id :0xChat + :message-id :0x2 + :pinned-at 3000 + :pinned-by :test-user} + :0x1 {:chat-id :0xChat + :message-id :0x1 + :pinned-at 2000 + :pinned-by :test-user}}}) + +(def pinned-messages-state-with-2-new-local-messages + {:0xChat {:0x3 {:chat-id :0xChat + :message-id :0x3 + :pinned-at nil + :pinned-by :test-user} + :0x4 {:chat-id :0xChat + :message-id :0x4 + :pinned-at nil + :pinned-by :test-user} + :0x2 {:chat-id :0xChat + :message-id :0x2 + :pinned-at 3000 + :pinned-by :test-user} + :0x1 {:chat-id :0xChat + :message-id :0x1 + :pinned-at 2000 + :pinned-by :test-user}}}) + +(h/deftest-sub :chats/pinned-sorted-list + [sub-name] + (testing "It sorts three messages with pinned-at property" + (swap! rf-db/app-db assoc :pin-messages pinned-messages-state) + (is (= [{:chat-id :0xChat + :message-id :0x1 + :pinned-at 1000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x2 + :pinned-at 2000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x3 + :pinned-at 3000 + :pinned-by :test-user}] + (rf/sub [sub-name :0xChat])))) + (testing "It sorts messages from backend with pinned-at property and 1 new local pinned message" + (swap! rf-db/app-db assoc :pin-messages pinned-messages-state-with-1-new-local-message) + (is (= [{:chat-id :0xChat + :message-id :0x1 + :pinned-at 2000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x2 + :pinned-at 3000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x3 + :pinned-at nil + :pinned-by :test-user}] + (rf/sub [sub-name :0xChat])))) + (testing "It sorts messages from backend with pinned-at property and 2 new local pinned messages" + (swap! rf-db/app-db assoc :pin-messages pinned-messages-state-with-2-new-local-messages) + (is (= [{:chat-id :0xChat + :message-id :0x1 + :pinned-at 2000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x2 + :pinned-at 3000 + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x3 + :pinned-at nil + :pinned-by :test-user} + {:chat-id :0xChat + :message-id :0x4 + :pinned-at nil + :pinned-by :test-user}] + (rf/sub [sub-name :0xChat]))))) +