From ca144fbe1b0932795e6f0318dd336ed6d2b4e6f5 Mon Sep 17 00:00:00 2001 From: Icaro Motta Date: Tue, 27 Sep 2022 10:05:03 -0300 Subject: [PATCH] [#13965 #13966 #13969] Implement accept/decline/open chat for contact requests (#14073) Notification reconciliation is now implemented, which means new notifications are "merged" with existing app db notifications. The original implementation was not compatible with the way notifications are fetched by read/unread status. Unit tests were written to cover event handlers in the new status-im.activity-center.core namespace. New test utilities were added for two main reasons: 1) reduce test clutter to arrange data, and 2) to spy on effects to make sure they are dispatched correctly. --- .../notifications/activity_logs.cljs | 1 + src/status_im/activity_center/core.cljs | 40 ++++- src/status_im/activity_center/core_test.cljs | 152 ++++++++++++++++++ src/status_im/test_helpers.cljs | 87 ++++++++++ src/status_im/transport/message/core.cljs | 13 +- src/status_im/ui/components/topnav.cljs | 22 +-- .../ui/screens/activity_center/views.cljs | 54 ++++--- src/status_im/ui/screens/home/views.cljs | 4 +- src/status_im/utils/config.cljs | 5 + 9 files changed, 339 insertions(+), 39 deletions(-) create mode 100644 src/status_im/activity_center/core_test.cljs create mode 100644 src/status_im/test_helpers.cljs diff --git a/src/quo2/components/notifications/activity_logs.cljs b/src/quo2/components/notifications/activity_logs.cljs index 7686ea9aca..b768bda0e0 100644 --- a/src/quo2/components/notifications/activity_logs.cljs +++ b/src/quo2/components/notifications/activity_logs.cljs @@ -94,6 +94,7 @@ :align-items :flex-start :flex 1} [status-tags/status-tag {:size :small + :label (:label status) :status status}]]) (defn- activity-title diff --git a/src/status_im/activity_center/core.cljs b/src/status_im/activity_center/core.cljs index dd7762da79..2dad121816 100644 --- a/src/status_im/activity_center/core.cljs +++ b/src/status_im/activity_center/core.cljs @@ -5,6 +5,34 @@ [status-im.utils.fx :as fx] [taoensso.timbre :as log])) +;;;; Notification reconciliation + +(defn- update-notifications + [old new] + (let [ids-to-be-removed (->> new + (filter #(or (:dismissed %) (:accepted %))) + (map :id)) + grouped-new (apply dissoc (group-by :id new) ids-to-be-removed) + grouped-old (apply dissoc (group-by :id old) ids-to-be-removed)] + (->> (merge grouped-old grouped-new) + vals + (map first)))) + +(fx/defn notifications-reconcile + {:events [:activity-center.notifications/reconcile]} + [{:keys [db]} new-notifications] + (let [{read-new true + unread-new false} (group-by :read new-notifications) + read-old (get-in db [:activity-center :notifications-read :data]) + unread-old (get-in db [:activity-center :notifications-unread :data])] + {:db (-> db + (assoc-in [:activity-center :notifications-read :data] + (update-notifications read-old read-new)) + (assoc-in [:activity-center :notifications-unread :data] + (update-notifications unread-old unread-new)))})) + +;;;; Notifications fetching and pagination + (def notifications-per-page 20) @@ -29,11 +57,11 @@ {:db (assoc-in db [:activity-center notifications-group :loading?] true) ::json-rpc/call [{:method (notifications-group->rpc-method notifications-group) :params [cursor notifications-per-page] - :on-success #(re-frame/dispatch [:activity-center/notifications-fetch-success notifications-group %]) - :on-error #(re-frame/dispatch [:activity-center/notifications-fetch-error notifications-group %])}]})) + :on-success #(re-frame/dispatch [:activity-center.notifications/fetch-success notifications-group %]) + :on-error #(re-frame/dispatch [:activity-center.notifications/fetch-error notifications-group %])}]})) (fx/defn notifications-fetch-first-page - {:events [:activity-center/notifications-fetch-first-page]} + {:events [:activity-center.notifications/fetch-first-page]} [{:keys [db] :as cofx} {:keys [status-filter] :or {status-filter :unread}}] (let [notifications-group (notifications-read-status->group status-filter)] (fx/merge cofx @@ -44,7 +72,7 @@ (notifications-fetch start-or-end-cursor notifications-group)))) (fx/defn notifications-fetch-next-page - {:events [:activity-center/notifications-fetch-next-page]} + {:events [:activity-center.notifications/fetch-next-page]} [{:keys [db] :as cofx}] (let [status-filter (get-in db [:activity-center :current-status-filter]) notifications-group (notifications-read-status->group status-filter) @@ -53,7 +81,7 @@ (notifications-fetch cofx cursor notifications-group)))) (fx/defn notifications-fetch-success - {:events [:activity-center/notifications-fetch-success]} + {:events [:activity-center.notifications/fetch-success]} [{:keys [db]} notifications-group {:keys [cursor notifications]}] {:db (-> db (update-in [:activity-center notifications-group] dissoc :loading?) @@ -63,7 +91,7 @@ (map data-store.activities/<-rpc notifications)))}) (fx/defn notifications-fetch-error - {:events [:activity-center/notifications-fetch-error]} + {:events [:activity-center.notifications/fetch-error]} [{:keys [db]} notifications-group error] (log/warn "Failed to load Activity Center notifications" error) {:db (update-in db [:activity-center notifications-group] dissoc :loading?)}) diff --git a/src/status_im/activity_center/core_test.cljs b/src/status_im/activity_center/core_test.cljs new file mode 100644 index 0000000000..8822666c19 --- /dev/null +++ b/src/status_im/activity_center/core_test.cljs @@ -0,0 +1,152 @@ +(ns status-im.activity-center.core-test + (:require [cljs.test :refer [deftest is testing]] + [day8.re-frame.test :as rf-test] + [re-frame.core :as rf] + [status-im.ethereum.json-rpc :as json-rpc] + [status-im.test-helpers :as h] + status-im.events)) + +(defn setup [] + (h/register-helper-events) + (rf/dispatch [:init/app-started])) + +(deftest notifications-reconcile-test + (testing "does nothing when there are no new notifications" + (rf-test/run-test-sync + (setup) + (let [read [{:id "0x1" :read true}] + unread [{:id "0x4" :read false}]] + (rf/dispatch [:test/assoc-in [:activity-center :notifications-read :data] read]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :data] unread]) + + (rf/dispatch [:activity-center.notifications/reconcile nil]) + + (is (= read (get-in (h/db) [:activity-center :notifications-read :data]))) + (is (= unread (get-in (h/db) [:activity-center :notifications-unread :data])))))) + + (testing "removes dismissed or accepted notifications" + (rf-test/run-test-sync + (setup) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-read :data] + [{:id "0x1" :read true} + {:id "0x2" :read true} + {:id "0x3" :read true}]]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :data] + [{:id "0x4" :read false} + {:id "0x5" :read false} + {:id "0x6" :read false}]]) + + (rf/dispatch [:activity-center.notifications/reconcile + [{:id "0x1" :read true :dismissed true} + {:id "0x3" :read true :accepted true} + {:id "0x4" :read false :dismissed true} + {:id "0x5" :read false :accepted true}]]) + + (is (= [{:id "0x2" :read true}] + (get-in (h/db) [:activity-center :notifications-read :data]))) + (is (= [{:id "0x6" :read false}] + (get-in (h/db) [:activity-center :notifications-unread :data]))))) + + (testing "replaces old notifications with newly arrived ones" + (rf-test/run-test-sync + (setup) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-read :data] + [{:id "0x1" :read true} + {:id "0x2" :read true}]]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :data] + [{:id "0x3" :read false} + {:id "0x4" :read false}]]) + + (rf/dispatch [:activity-center.notifications/reconcile + [{:id "0x1" :read true :name "ABC"} + {:id "0x3" :read false :name "XYZ"}]]) + + (is (= [{:id "0x1" :read true :name "ABC"} + {:id "0x2" :read true}] + (get-in (h/db) [:activity-center :notifications-read :data]))) + (is (= [{:id "0x3" :read false :name "XYZ"} + {:id "0x4" :read false}] + (get-in (h/db) [:activity-center :notifications-unread :data])))))) + +(deftest notifications-fetch-test + (testing "fetches first page" + (rf-test/run-test-sync + (setup) + (let [spy-queue (atom [])] + (h/stub-fx-with-callbacks ::json-rpc/call + :on-success (constantly {:cursor "10" + :notifications [{:chatId "0x1"}]})) + (h/spy-fx spy-queue ::json-rpc/call) + + (rf/dispatch [:activity-center.notifications/fetch-first-page]) + + (is (= :unread (get-in (h/db) [:activity-center :current-status-filter]))) + (is (nil? (get-in (h/db) [:activity-center :notifications-unread :loading?]))) + (is (= "10" (get-in (h/db) [:activity-center :notifications-unread :cursor]))) + (is (= "" (get-in @spy-queue [0 :args 0 :params 0]))) + (is (= [{:chat-id "0x1"}] + (->> (get-in (h/db) [:activity-center :notifications-unread :data]) + (map #(select-keys % [:chat-id])))))))) + + (testing "does not fetch next page when pagination cursor reached the end" + (rf-test/run-test-sync + (setup) + (let [spy-queue (atom [])] + (h/spy-fx spy-queue ::json-rpc/call) + (rf/dispatch [:test/assoc-in [:activity-center :current-status-filter] :unread]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :cursor] ""]) + + (rf/dispatch [:activity-center.notifications/fetch-next-page]) + + (is (= [] @spy-queue))))) + + (testing "fetches next page when pagination cursor is not empty" + (rf-test/run-test-sync + (setup) + (let [spy-queue (atom [])] + (h/stub-fx-with-callbacks ::json-rpc/call + :on-success (constantly {:cursor "" + :notifications [{:chatId "0x1"}]})) + (h/spy-fx spy-queue ::json-rpc/call) + (rf/dispatch [:test/assoc-in [:activity-center :current-status-filter] :unread]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :cursor] "10"]) + + (rf/dispatch [:activity-center.notifications/fetch-next-page]) + + (is (= "wakuext_unreadActivityCenterNotifications" (get-in @spy-queue [0 :args 0 :method]))) + (is (= "10" (get-in @spy-queue [0 :args 0 :params 0]))) + (is (= "" (get-in (h/db) [:activity-center :notifications-unread :cursor]))) + (is (= [{:chat-id "0x1"}] + (->> (get-in (h/db) [:activity-center :notifications-unread :data]) + (map #(select-keys % [:chat-id])))))))) + + (testing "does not fetch next page while it is still loading" + (rf-test/run-test-sync + (setup) + (let [spy-queue (atom [])] + (h/spy-fx spy-queue ::json-rpc/call) + (rf/dispatch [:test/assoc-in [:activity-center :current-status-filter] :read]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-read :cursor] "10"]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-read :loading?] true]) + + (rf/dispatch [:activity-center.notifications/fetch-next-page]) + + (is (= [] @spy-queue))))) + + (testing "resets loading flag after an error" + (rf-test/run-test-sync + (setup) + (let [spy-queue (atom [])] + (h/stub-fx-with-callbacks ::json-rpc/call :on-error (constantly :fake-error)) + (h/spy-event-fx spy-queue :activity-center.notifications/fetch-error) + (h/spy-fx spy-queue ::json-rpc/call) + (rf/dispatch [:test/assoc-in [:activity-center :current-status-filter] :unread]) + (rf/dispatch [:test/assoc-in [:activity-center :notifications-unread :cursor] ""]) + + (rf/dispatch [:activity-center.notifications/fetch-first-page]) + + (is (nil? (get-in (h/db) [:activity-center :notifications-unread :loading?]))) + (is (= [:activity-center.notifications/fetch-error + :notifications-unread + :fake-error] + (:args (last @spy-queue)))))))) diff --git a/src/status_im/test_helpers.cljs b/src/status_im/test_helpers.cljs new file mode 100644 index 0000000000..590cb63ebf --- /dev/null +++ b/src/status_im/test_helpers.cljs @@ -0,0 +1,87 @@ +(ns status-im.test-helpers + "Utilities for simplifying the process of writing tests and improving test + readability. + + Avoid coupling this namespace with particularities of the Status' domain, thus + prefer to use it for more general purpose concepts, such as the re-frame event + layer." + (:require [re-frame.core :as rf] + [re-frame.registrar :as rf-registrar] + [re-frame.events :as rf-events] + [re-frame.db :as rf-db])) + +(defn db + "A simple wrapper to get the latest value from the app db." + [] + @rf-db/app-db) + +(defn register-helper-events + "Register utility events for testing. + + Note that re-frame-test removes such events if they're declared in the scope + of the macro `day8.re-frame.test/run-test-sync` (or the async variant)." + [] + (rf/reg-event-db + :test/assoc-in + (fn [db [_ keys value]] + (assoc-in db keys value)))) + +(defn spy-event-fx + "Re-register event effect using id `id`, but conj to `state` the event + arguments before calling the original handler. + + Callers of this function can later on deref `state` to make assertions. + + It's recommended to run this function in the scope of the macro + `day8.re-frame.test/run-test-sync` (or the async variant) as they + automatically clean up effects." + [state id] + (let [interceptors (get-in @rf-registrar/kind->id->handler [:event id])] + (rf-events/register + id + (concat (butlast interceptors) + (list {:id :test/spy-event-fx + :before (fn [context] + (swap! state conj {:id id :args (get-in context [:coeffects :event])}) + context)} + (last interceptors)))))) + +(defn spy-fx + "Re-register effect using id `id`, but conj to `state` the effect arguments + before calling the original effect handler. + + Callers of this function can later on inspect `state` to make assertions. + + It's recommended to run this function in the scope of the macro + `day8.re-frame.test/run-test-sync` (or the async variant) as they + automatically clean up effects." + [state id] + (let [original-fn (get-in @rf-registrar/kind->id->handler [:fx id])] + (rf/reg-fx + id + (fn [fx-args] + (swap! state conj {:id id :args fx-args}) + (original-fn fx-args))))) + +(defn stub-fx-with-callbacks + "Re-register effect using id `id` with a no-op version. + + This function is useful to redefine effects that expect callbacks, usually to + pass downstream dummy data to successful/failure events. In re-frame parlance, + such effects accept on-success and on-error keywords. + + The original effect handler for `id` is expected to receive a single map as + argument with either :on-success or :on-error keywords. + + This function expects to receive either `on-success` or `on-error`, but not + both. If both are passed, `on-error` will be preferred." + [id & {:keys [on-success on-error]}] + (rf/reg-fx + id + (fn [[fx-map]] + (let [original-on-error (:on-error fx-map) + original-on-success (:on-success fx-map)] + (cond (and original-on-error on-error) + (original-on-error (on-error fx-map)) + (and original-on-success on-success) + (original-on-success (on-success fx-map))))))) diff --git a/src/status_im/transport/message/core.cljs b/src/status_im/transport/message/core.cljs index c5d05d8f39..41728dc131 100644 --- a/src/status_im/transport/message/core.cljs +++ b/src/status_im/transport/message/core.cljs @@ -1,9 +1,11 @@ (ns ^{:doc "Definition of the StatusMessage protocol"} status-im.transport.message.core - (:require [status-im.chat.models.message :as models.message] + (:require [status-im.activity-center.core :as activity-center] + [status-im.chat.models.message :as models.message] [status-im.chat.models.pin-message :as models.pin-message] [status-im.chat.models :as models.chat] [status-im.chat.models.reactions :as models.reactions] + [status-im.utils.config :as config] [status-im.contact.core :as models.contact] [status-im.communities.core :as models.communities] [status-im.pairing.core :as models.pairing] @@ -70,8 +72,13 @@ (do (js-delete response-js "activityCenterNotifications") (fx/merge cofx - (notifications-center/handle-activities (map data-store.activities/<-rpc - (types/js->clj activity-notifications))) + (if (and @config/new-ui-enabled? @config/new-activity-center-enabled?) + (->> activity-notifications + types/js->clj + (map data-store.activities/<-rpc) + activity-center/notifications-reconcile) + (notifications-center/handle-activities (map data-store.activities/<-rpc + (types/js->clj activity-notifications)))) (process-next response-js sync-handler))) (seq installations) diff --git a/src/status_im/ui/components/topnav.cljs b/src/status_im/ui/components/topnav.cljs index efc5d70149..0b245ee586 100644 --- a/src/status_im/ui/components/topnav.cljs +++ b/src/status_im/ui/components/topnav.cljs @@ -1,12 +1,12 @@ (ns status-im.ui.components.topnav - (:require - [re-frame.core :as re-frame] - [quo2.components.buttons.button :as quo2.button] - [status-im.qr-scanner.core :as qr-scanner] - [status-im.utils.handlers :refer [evt [:contact-requests.ui/decline-request id])} + :button-2 {:label (i18n/label :t/accept) + :type :success + :on-press #(>evt [:contact-requests.ui/accept-request id])}} nil)) +(defn activity-pressable + [notification & children] + (case (get-in notification [:message :contact-request-state]) + constants/contact-request-message-state-accepted + ;; NOTE [2022-09-21]: We need to dispatch to + ;; `:contact.ui/send-message-pressed` instead of + ;; `:chat.ui/navigate-to-chat`, otherwise the chat screen looks completely + ;; broken if it has never been opened before for the accepted contact. + [animation/pressable {:on-press #(>evt [:contact.ui/send-message-pressed {:public-key (:author notification)}])} + children] + [:<> children])) + (defn render-notification [notification index] [rn/view {:flex 1 :flex-direction :column :margin-top (if (= 0 index) 0 4)} - [activity-logs/activity-log - (merge {:context (activity-context notification) - :icon (activity-icon notification) - :message (activity-message notification) - :status (activity-status notification) - :timestamp (datetime/timestamp->relative (:timestamp notification)) - :title (activity-title notification) - :unread? (not (:read notification))} - (activity-buttons notification))]]) + [activity-pressable notification + [activity-logs/activity-log + (merge {:context (activity-context notification) + :icon (activity-icon notification) + :message (activity-message notification) + :status (activity-status notification) + :timestamp (datetime/timestamp->relative (:timestamp notification)) + :title (activity-title notification) + :unread? (not (:read notification))} + (activity-buttons notification))]]]) (defn notifications-list [] @@ -94,12 +110,12 @@ [rn/flat-list {:style {:padding-horizontal 8} :data notifications :key-fn :id - :on-end-reached #(>evt [:activity-center/notifications-fetch-next-page]) + :on-end-reached #(>evt [:activity-center.notifications/fetch-next-page]) :render-fn render-notification}])) (defn activity-center [] (reagent/create-class - {:component-did-mount #(>evt [:activity-center/notifications-fetch-first-page {:status-filter :unread}]) + {:component-did-mount #(>evt [:activity-center.notifications/fetch-first-page {:status-filter :unread}]) :reagent-render (fn [] [:<> @@ -107,8 +123,8 @@ :title (i18n/label :t/activity)}] ;; TODO(ilmotta): Temporary solution to switch between read/unread ;; notifications while the Design team works on the mockups. - [button/button {:on-press #(>evt [:activity-center/notifications-fetch-first-page {:status-filter :unread}])} + [button/button {:on-press #(>evt [:activity-center.notifications/fetch-first-page {:status-filter :unread}])} "Unread"] - [button/button {:on-press #(>evt [:activity-center/notifications-fetch-first-page {:status-filter :read}])} + [button/button {:on-press #(>evt [:activity-center.notifications/fetch-first-page {:status-filter :read}])} "Read"] [notifications-list]])})) diff --git a/src/status_im/ui/screens/home/views.cljs b/src/status_im/ui/screens/home/views.cljs index c10a305d53..9d7013e5c7 100644 --- a/src/status_im/ui/screens/home/views.cljs +++ b/src/status_im/ui/screens/home/views.cljs @@ -274,7 +274,9 @@ :accessibility-label :notifications-button :on-press #(do (re-frame/dispatch [:mark-all-activity-center-notifications-as-read]) - (re-frame/dispatch [:navigate-to :notifications-center]))} + (if (and @config/new-ui-enabled? @config/new-activity-center-enabled?) + (re-frame/dispatch [:navigate-to :activity-center]) + (re-frame/dispatch [:navigate-to :notifications-center])))} [icons/icon :main-icons/notification2 {:color (quo2.colors/theme-colors quo2.colors/black quo2.colors/white)}]] (when (pos? notif-count) [react/view {:style (merge (styles/counter-public-container) {:top 5 :right 5}) diff --git a/src/status_im/utils/config.cljs b/src/status_im/utils/config.cljs index c64f8e01ad..8e85eae267 100644 --- a/src/status_im/utils/config.cljs +++ b/src/status_im/utils/config.cljs @@ -177,3 +177,8 @@ (def wallet-connect-project-id "87815d72a81d739d2a7ce15c2cfdefb3") (def new-ui-enabled? (atom false)) + +;; TODO: Remove this (highly) temporary flag once the new Activity Center is +;; usable enough to replace the old one **in the new UI**. +(def new-activity-center-enabled? + (atom false))