From 3e761c3b85440807dc772f845de159faeb2359f8 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Wed, 19 Dec 2018 22:19:22 +0100 Subject: [PATCH] Extend message hash confirmation for group chats Signed-off-by: Andrea Maria Piana --- src/status_im/constants.cljs | 1 + src/status_im/events.cljs | 4 +- src/status_im/group_chats/core.cljs | 10 +-- src/status_im/transport/message/core.cljs | 33 ++++++--- src/status_im/transport/shh.cljs | 16 ++--- .../ui/screens/desktop/main/chat/views.cljs | 16 ++++- .../ui/screens/group/add_contacts/views.cljs | 12 ++-- .../ui/screens/profile/group_chat/views.cljs | 14 ++-- .../cljs/status_im/test/group_chats/core.cljs | 43 +++++++++++- test/cljs/status_im/test/transport/core.cljs | 69 +++++++++++++++++++ 10 files changed, 181 insertions(+), 37 deletions(-) diff --git a/src/status_im/constants.cljs b/src/status_im/constants.cljs index d22cf583f0..63489177cb 100644 --- a/src/status_im/constants.cljs +++ b/src/status_im/constants.cljs @@ -17,6 +17,7 @@ (def min-password-length 6) (def max-chat-name-length 20) +(def max-group-chat-participants 10) (def response-suggesstion-resize-duration 100) (def default-number-of-messages 20) (def blocks-per-hour 120) diff --git a/src/status_im/events.cljs b/src/status_im/events.cljs index 466682adf1..82c298ed63 100644 --- a/src/status_im/events.cljs +++ b/src/status_im/events.cljs @@ -1292,8 +1292,8 @@ (handlers/register-handler-fx :transport/message-sent - (fn [cofx [_ chat-id message-id message-type envelope-hash-js]] - (transport.message/set-message-envelope-hash cofx chat-id message-id message-type envelope-hash-js))) + (fn [cofx [_ chat-id message-id message-type envelope-hash-js messages-count]] + (transport.message/set-message-envelope-hash cofx chat-id message-id message-type envelope-hash-js messages-count))) (handlers/register-handler-fx :transport/contact-message-sent diff --git a/src/status_im/group_chats/core.cljs b/src/status_im/group_chats/core.cljs index ee0d1b2495..44373a3034 100644 --- a/src/status_im/group_chats/core.cljs +++ b/src/status_im/group_chats/core.cljs @@ -17,7 +17,6 @@ [status-im.transport.message.protocol :as protocol] [status-im.transport.message.group-chat :as message.group-chat] [status-im.transport.message.public-chat :as transport.public-chat] - [status-im.transport.chat.core :as transport.chat] [status-im.utils.fx :as fx] [status-im.chat.models :as models.chat] @@ -465,7 +464,7 @@ (map #(assoc % :from from) events)) all-updates)) -(defn get-inviter-pk [my-public-key {:keys [membership-updates] :as chat}] +(defn get-inviter-pk [my-public-key {:keys [membership-updates]}] (->> membership-updates unwrap-events (keep (fn [{:keys [from type members]}] @@ -501,12 +500,15 @@ (let [previous-chat (get-in cofx [:db :chats chat-id]) all-updates (clojure.set/union (set (:membership-updates previous-chat)) (set (:membership-updates membership-update))) + my-public-key (accounts.db/current-public-key cofx) unwrapped-events (unwrap-events all-updates) - new-group (build-group unwrapped-events)] + new-group (build-group unwrapped-events) + member? (contains? (:contacts new-group) my-public-key)] (fx/merge cofx (models.chat/upsert-chat {:chat-id chat-id :name (:name new-group) - :is-active (get previous-chat :is-active true) + :is-active (or member? + (get previous-chat :is-active true)) :group-chat true :membership-updates (into [] all-updates) :admins (:admins new-group) diff --git a/src/status_im/transport/message/core.cljs b/src/status_im/transport/message/core.cljs index d69de9d94a..3ba3b7d603 100644 --- a/src/status_im/transport/message/core.cljs +++ b/src/status_im/transport/message/core.cljs @@ -57,6 +57,20 @@ {:db (assoc-in db [:transport/chats chat-id :resend?] nil) :data-store/tx [(transport-store/save-transport-tx {:chat-id chat-id :chat updated-chat})]})) +(fx/defn check-confirmations [{:keys [db] :as cofx} status chat-id message-id] + (when-let [{:keys [pending-confirmations not-sent]} + (get-in db [:transport/message-ids->confirmations message-id])] + (if (zero? (dec pending-confirmations)) + (fx/merge cofx + {:db (update db :transport/message-ids->confirmations dissoc message-id)} + (models.message/update-message-status chat-id message-id (if not-sent + :not-sent + status))) + (let [confirmations {:pending-confirmations (dec pending-confirmations) + :not-sent (or not-sent + (= :not-sent status))}] + {:db (assoc-in db [:transport/message-ids->confirmations message-id] confirmations)})))) + (fx/defn update-envelope-status [{:keys [db] :as cofx} envelope-hash status] (let [{:keys [chat-id message-type message-id]} @@ -72,7 +86,7 @@ (let [{:keys [fcm-token]} (get-in db [:contacts/contacts chat-id])] (fx/merge cofx (remove-hash envelope-hash) - (models.message/update-message-status chat-id message-id status) + (check-confirmations status chat-id message-id) (models.message/send-push-notification fcm-token status))))))) (fx/defn set-contact-message-envelope-hash @@ -82,18 +96,19 @@ :message-type :contact-message})}) (fx/defn set-message-envelope-hash - "message-type is used for tracking - TODO (cammellos): For group messages it returns multiple hashes, for now - we naively assume that if one is sent the batch is ok" - [{:keys [db] :as cofx} chat-id message-id message-type envelope-hash-js] + "message-type is used for tracking" + [{:keys [db] :as cofx} chat-id message-id message-type envelope-hash-js messages-count] (let [envelope-hash (js->clj envelope-hash-js) hash (if (vector? envelope-hash) (last envelope-hash) envelope-hash)] - {:db (assoc-in db [:transport/message-envelopes hash] - {:chat-id chat-id - :message-id message-id - :message-type message-type})})) + {:db (-> db + (assoc-in [:transport/message-envelopes hash] + {:chat-id chat-id + :message-id message-id + :message-type message-type}) + (update-in [:transport/message-ids->confirmations message-id] + #(or % {:pending-confirmations messages-count})))})) (defn- own-info [db] (let [{:keys [name photo-path address]} (:account/account db) diff --git a/src/status_im/transport/shh.cljs b/src/status_im/transport/shh.cljs index 66b545964b..b6a5b3c885 100644 --- a/src/status_im/transport/shh.cljs +++ b/src/status_im/transport/shh.cljs @@ -59,11 +59,11 @@ (on-success resp) (on-error err)))))) -(defn handle-response [success-event error-event] +(defn handle-response [success-event error-event messages-count] (fn [err resp] (if-not err (if success-event - (re-frame/dispatch (conj success-event resp)) + (re-frame/dispatch (conj success-event resp messages-count)) (log/debug :shh/post-success)) (re-frame/dispatch [error-event err resp])))) @@ -82,12 +82,12 @@ -shh (sendDirectMessage direct-message - (handle-response success-event error-event))))))) + (handle-response success-event error-event 1))))))) (re-frame/reg-fx :shh/send-pairing-message (fn [params] - (let [{:keys [web3 payload src dsts success-event error-event] + (let [{:keys [web3 payload src success-event error-event] :or {error-event :protocol/send-status-message-error}} params message (clj->js {:sig src :chat constants/contact-discovery @@ -98,7 +98,7 @@ -shh (sendPairingMessage message - (handle-response success-event error-event)))))) + (handle-response success-event error-event 1)))))) (re-frame/reg-fx :shh/send-group-message @@ -118,7 +118,7 @@ -shh (sendDirectMessage message - (handle-response success-event error-event)))))))) + (handle-response success-event error-event (count dsts))))))))) (re-frame/reg-fx :shh/send-public-message @@ -134,7 +134,7 @@ -shh (sendPublicMessage message - (handle-response success-event error-event))))))) + (handle-response success-event error-event 1))))))) (re-frame/reg-fx :shh/post @@ -145,7 +145,7 @@ :whisper-message (update message :payload (comp transport.utils/from-utf8 transit/serialize)) :on-success (if success-event - #(re-frame/dispatch (conj success-event %)) + #(re-frame/dispatch (conj success-event % 1)) #(log/debug :shh/post-success)) :on-error #(re-frame/dispatch [error-event %])})))) diff --git a/src/status_im/ui/screens/desktop/main/chat/views.cljs b/src/status_im/ui/screens/desktop/main/chat/views.cljs index 17c9dec12f..d7e0a7a723 100644 --- a/src/status_im/ui/screens/desktop/main/chat/views.cljs +++ b/src/status_im/ui/screens/desktop/main/chat/views.cljs @@ -7,6 +7,8 @@ [status-im.ui.screens.chat.message.message :as message] [taoensso.timbre :as log] [reagent.core :as reagent] + [status-im.chat.models :as models.chat] + [status-im.group-chats.core :as models.group-chats] [status-im.ui.screens.chat.utils :as chat-utils] [status-im.utils.gfycat.core :as gfycat] [status-im.constants :as constants] @@ -22,6 +24,7 @@ [status-im.ui.components.icons.vector-icons :as vector-icons] [status-im.ui.screens.desktop.main.chat.styles :as styles] [status-im.contact.db :as contact.db] + [status-im.ui.screens.chat.views :as views.chat] [status-im.ui.components.popup-menu.views :refer [show-desktop-menu get-chat-menu-items]] [status-im.i18n :as i18n] @@ -323,12 +326,21 @@ (re-frame/dispatch [:chat.ui/set-chat-input-text text])))}] [send-button inp-ref disconnected?]]))) +(defn not-joined-group-chat? [chat current-public-key] + (and (models.chat/group-chat? chat) + (models.group-chats/invited? current-public-key chat) + (not (models.group-chats/joined? current-public-key chat)))) + (views/defview chat-view [] - (views/letsubs [{:keys [input-text chat-id] :as current-chat} [:chats/current-chat]] + (views/letsubs [{:keys [input-text chat-id] :as current-chat} [:chats/current-chat] + current-public-key [:account/public-key]] + [react/view {:style styles/chat-view} [toolbar-chat-view current-chat] [react/view {:style styles/separator}] - [messages-view current-chat] + (if (not-joined-group-chat? current-chat current-public-key) + [views.chat/group-chat-join-section current-public-key current-chat] + [messages-view current-chat]) [react/view {:style styles/separator}] [reply-message-view] [chat-text-input chat-id input-text]])) diff --git a/src/status_im/ui/screens/group/add_contacts/views.cljs b/src/status_im/ui/screens/group/add_contacts/views.cljs index 6d3b346037..b402c213b4 100644 --- a/src/status_im/ui/screens/group/add_contacts/views.cljs +++ b/src/status_im/ui/screens/group/add_contacts/views.cljs @@ -4,6 +4,7 @@ [status-im.i18n :as i18n] [status-im.utils.platform :as utils.platform] [status-im.ui.components.button.view :as buttons] + [status-im.constants :as constants] [status-im.ui.components.contact.contact :refer [toggle-contact-view]] [status-im.ui.components.list.views :as list] [status-im.ui.components.list-selection :as list-selection] @@ -86,8 +87,6 @@ (i18n/label :t/group-chat-no-contacts)] [buttons/secondary-button {:on-press handle-invite-friends-pressed} (i18n/label :t/invite-friends)]]) -(def max-participants 10) - (defn number-of-participants-disclaimer [number-of-participants-available] [react/view {:style styles/number-of-participants-disclaimer} [react/text (if (> number-of-participants-available @@ -105,9 +104,10 @@ :label (i18n/label :t/next) :count (pos? selected-contacts-count)} (i18n/label :t/group-chat)] - [number-of-participants-disclaimer (- (dec max-participants) selected-contacts-count)] + (when (seq contacts) + [number-of-participants-disclaimer (- (dec constants/max-group-chat-participants) selected-contacts-count)]) (if (seq contacts) - [toggle-list contacts (partial group-toggle-contact (< selected-contacts-count (dec max-participants)))] + [toggle-list contacts (partial group-toggle-contact (< selected-contacts-count (dec constants/max-group-chat-participants)))] [no-contacts])])) ;; Add participants to existing group chat @@ -125,7 +125,7 @@ :label (i18n/label :t/add)} name] - [number-of-participants-disclaimer (- max-participants current-participants-count)] + [number-of-participants-disclaimer (- constants/max-group-chat-participants current-participants-count)] (when (seq contacts) [toggle-list contacts (partial group-toggle-participant (< (+ current-participants-count - selected-contacts-count) max-participants))])]))) + selected-contacts-count) constants/max-group-chat-participants))])]))) diff --git a/src/status_im/ui/screens/profile/group_chat/views.cljs b/src/status_im/ui/screens/profile/group_chat/views.cljs index 7e2c5a867f..702e37924e 100644 --- a/src/status_im/ui/screens/profile/group_chat/views.cljs +++ b/src/status_im/ui/screens/profile/group_chat/views.cljs @@ -1,6 +1,7 @@ (ns status-im.ui.screens.profile.group-chat.views (:require-macros [status-im.utils.views :refer [defview letsubs]]) (:require [status-im.utils.platform :as platform] + [status-im.constants :as constants] [status-im.ui.screens.profile.group-chat.styles :as styles] [status-im.ui.components.react :as react] [status-im.ui.screens.profile.components.styles :as profile.components.styles] @@ -37,9 +38,9 @@ :icon-opts {:color colors/blue :accessibility-label :done-button}}]]) -(defn actions [admin? chat-id] +(defn actions [allow-adding-members? chat-id] (concat - (when admin? + (when allow-adding-members? [{:label (i18n/label :add-members) :icon :icons/add :action #(re-frame/dispatch [:navigate-to :add-participants-toggle-list])}]) @@ -96,11 +97,14 @@ (defview group-chat-profile [] (letsubs [{:keys [admins chat-id] :as current-chat} [:chats/current-chat] editing? [:get :group-chat-profile/editing?] + members [:contacts/current-chat-contacts] changed-chat [:get :group-chat-profile/profile] current-pk [:account/public-key]] (when current-chat - (let [shown-chat (merge current-chat changed-chat) - admin? (admins current-pk)] + (let [shown-chat (merge current-chat changed-chat) + admin? (admins current-pk) + allow-adding-members? (and admin? + (< (count members) constants/max-group-chat-participants))] [react/view profile.components.styles/profile [status-bar/status-bar] (if editing? @@ -113,7 +117,7 @@ :editing? editing? :allow-icon-change? false :on-change-text-event :group-chats.ui/name-changed}] - [list/action-list (actions admin? chat-id) + [list/action-list (actions allow-adding-members? chat-id) {:container-style styles/action-container :action-style styles/action :action-label-style styles/action-label diff --git a/test/cljs/status_im/test/group_chats/core.cljs b/test/cljs/status_im/test/group_chats/core.cljs index f784be8e45..a05e52a9aa 100644 --- a/test/cljs/status_im/test/group_chats/core.cljs +++ b/test/cljs/status_im/test/group_chats/core.cljs @@ -75,7 +75,7 @@ (is (not actual))))) (testing "an already existing chat" (let [cofx (assoc - (group-chats/handle-membership-update {:now 0 :db {}} initial-message "payload" admin) + (group-chats/handle-membership-update {:now 0 :db {:account/account {:public-key member-3}}} initial-message "payload" admin) :now 0)] (testing "the message has already been received" (let [actual (group-chats/handle-membership-update cofx initial-message "payload" admin)] @@ -83,6 +83,47 @@ (is (= (get-in cofx [:db :chats chat-id]) (get-in actual [:db :chats chat-id])))))) + (testing "a chat we have deleted" + (let [after-leaving-cofx (-> (group-chats/handle-membership-update cofx + {:chat-id chat-id + :membership-updates [{:from member-1 + :events [{:type "chat-created" + :clock-value 1 + :name "group-name"} + {:type "admins-added" + :clock-value 10 + :members [member-2]} + {:type "admin-removed" + :clock-value 11 + :member member-1}]} + {:from member-3 + :events [{:type "member-removed" + :clock-value 12 + :member member-3}]}]} + "payload" + member-3) + (assoc-in [:db :chats chat-id :is-active] false)) + after-been-invited-again-cofx (group-chats/handle-membership-update (assoc after-leaving-cofx :now 0) + {:chat-id chat-id + :membership-updates [{:from member-1 + :events [{:type "chat-created" + :clock-value 1 + :name "group-name"} + {:type "admins-added" + :clock-value 10 + :members [member-2]} + {:type "admin-removed" + :clock-value 11 + :member member-1}]} + {:from member-2 + :events [{:type "members-added" + :clock-value 13 + :members [member-3]}]}]} + "payload" + member-2)] + + (testing "it sets the chat active after been invited again" + (is (get-in after-been-invited-again-cofx [:db :chats chat-id :is-active]))))) (testing "a new message comes in" (let [actual (group-chats/handle-membership-update cofx {:chat-id chat-id diff --git a/test/cljs/status_im/test/transport/core.cljs b/test/cljs/status_im/test/transport/core.cljs index b999d6c3c1..8ea9754450 100644 --- a/test/cljs/status_im/test/transport/core.cljs +++ b/test/cljs/status_im/test/transport/core.cljs @@ -1,5 +1,6 @@ (ns status-im.test.transport.core (:require [cljs.test :refer-macros [deftest is testing]] + [status-im.utils.fx :as fx] [status-im.protocol.core :as protocol] [status-im.transport.core :as transport] [status-im.transport.message.core :as message])) @@ -82,3 +83,71 @@ (let [actual (message/receive-whisper-messages {:db {}} nil messages sig)] (testing "it add an fx for the message" (is (:chat-received-message/add-fx actual)))))) + +(deftest message-envelopes + (let [chat-id "chat-id" + from "from" + message-id "message-id" + initial-cofx {:db {:chats {chat-id {:messages {message-id {:from from}}}}}}] + + (testing "a single envelope message" + (let [cofx (message/set-message-envelope-hash initial-cofx chat-id message-id :message-type "hash-1" 1)] + (testing "it sets the message-infos" + (is (= {:chat-id chat-id + :message-id message-id + :message-type :message-type} + (get-in cofx [:db :transport/message-envelopes "hash-1"])))) + (testing "the message is sent" + (is (= :sent + + (get-in + (message/update-envelope-status cofx "hash-1" :sent) + [:db :chats chat-id :message-statuses message-id from :status])))) + + (testing "the message is not sent" + (is (= :not-sent + (get-in + (message/update-envelope-status cofx "hash-1" :not-sent) + [:db :chats chat-id :message-statuses message-id from :status])))))) + (testing "multi envelope message" + (testing "only inserts" + (let [cofx (fx/merge + initial-cofx + (message/set-message-envelope-hash chat-id message-id :message-type "hash-1" 3) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-2" 3) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-3" 3))] + (testing "it sets the message count" + (is (= {:pending-confirmations 3} + (get-in cofx [:db :transport/message-ids->confirmations message-id])))))) + (testing "message sent correctly" + (let [cofx (fx/merge + initial-cofx + (message/set-message-envelope-hash chat-id message-id :message-type "hash-1" 3) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-2" 3) + (message/update-envelope-status "hash-1" :sent) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-3" 3) + (message/update-envelope-status "hash-2" :sent) + (message/update-envelope-status "hash-3" :sent))] + (testing "it removes the confirmations" + (is (not (get-in cofx [:db :transport/message-ids->confirmations message-id])))) + (testing "the message is sent" + (is (= :sent + (get-in + cofx + [:db :chats chat-id :message-statuses message-id from :status])))))) + (testing "message not sent" + (let [cofx (fx/merge + initial-cofx + (message/set-message-envelope-hash chat-id message-id :message-type "hash-1" 3) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-2" 3) + (message/update-envelope-status "hash-1" :sent) + (message/set-message-envelope-hash chat-id message-id :message-type "hash-3" 3) + (message/update-envelope-status "hash-2" :not-sent) + (message/update-envelope-status "hash-3" :sent))] + (testing "it removes the confirmations" + (is (not (get-in cofx [:db :transport/message-ids->confirmations message-id])))) + (testing "the message is sent" + (is (= :not-sent + (get-in + cofx + [:db :chats chat-id :message-statuses message-id from :status])))))))))