Chat refactoring: updated send-message namespace #2795

alwx · 2017-12-22T11:37:38Z

#2572 should be merged first!

This PR removes side effects from send-message namespace and makes it a little easier.

However, there are things that should be done next (and will be done in the next PRs):

Refactoring of status-im.commands.handlers.jail
Simplification of :chat-send-message/send-command! and :chat-send-message/send-message! (the logic of these two methods is different, but can be generalized)

flexsurfer · 2017-12-25T20:56:50Z

#2572 is merged, @yenda @janherich please review this PR

flexsurfer · 2017-12-25T21:10:28Z

src/status_im/chat/events/send_message.cljs

+                                     :chat-id chat-id}))))
+
+(handlers/register-handler-fx
+  :chat-send-message/send-message!


handler should be pure, it should return data with fx

flexsurfer · 2017-12-25T21:10:55Z

src/status_im/chat/events/send_message.cljs

+                  (send-notification fcm-token (:message options))))))))))
+
+(handlers/register-handler-fx
+  :chat-send-message/send-command!


same here, should be pure

flexsurfer · 2017-12-25T21:12:13Z

src/status_im/chat/models/message.cljs

+  [(re-frame/inject-cofx :random-id)
+   (re-frame/inject-cofx :random-id-seq)
+   (re-frame/inject-cofx :get-stored-chat)


:get-stored-chat can we get this from db ? not from realm

and :get-last-clock-value?

flexsurfer · 2017-12-25T21:14:27Z

src/status_im/chat/models/message.cljs

+
+(defn send-message [{{:keys [network-status] :as db} :db
+                     :keys [now get-stored-chat]}


this function should be pure too

flexsurfer · 2017-12-26T08:16:01Z

src/status_im/chat/events/input.cljs

+                                                :chat-id current-chat-id
+                                                :identity current-public-key
+                                                :address current-account-id})]
+      (assoc cofx' :db (-> (:db cofx')


there is no reason to return cofx it's confusing, better to return effects or fx

flexsurfer · 2017-12-26T12:50:55Z

app is hanging after sending password confirm

oskarth · 2018-01-02T11:32:39Z

@alwx What's the status of this PR?

alwx · 2018-01-04T08:45:17Z

The original issue is fixed. The only thing that is left to be done is rebasing the branch. I've also found some other things that can be simplified, but these parts can probably be skipped now.

alwx · 2018-01-05T09:13:05Z

It's finished. Please, check.

flexsurfer · 2018-01-05T11:49:05Z

src/status_im/chat/events/input.cljs

+                                             :chat-id  current-chat-id
+                                             :identity current-public-key
+                                             :address  current-account-id})]
+      (assoc fx :db (-> (:db fx)


this looks like update no?

janherich · 2017-12-23T17:45:41Z

src/status_im/chat/events/input.cljs

+
+(handlers/register-handler-fx
+  ::send-command
+  message-model/send-interceptors


Isn't the ::send-command handler duplicated ?

janherich · 2018-01-05T10:42:36Z

src/status_im/chat/models/message.cljs

+                           (filter :hidden)
+                           (map :name))
+        command'      (prepare-command current-public-key chat-id (get-last-clock-value chat-id) request content)
+        preview       (get-in db [:message-data :preview (:message-id command')])


there is no :message-data anymore, please check the current code

alwx · 2018-01-08T07:51:30Z

All code style issues have been fixed.

flexsurfer · 2018-01-25T06:32:06Z

@yenda there are no change requests from me, just comments

Serhy · 2018-01-25T12:33:59Z

@alwx , @yenda The latest build of this branch (Android: https://i.diawi.com/Ni4DY3, iOS: https://i.diawi.com/CeuTFs) has been tested on iOS (11.2.2) and Android (7.0)

The only issue is present: it's exactly what described in #2807 (comment) (no indication on sender side that /location, /request, /send in group chat delivered to any of group chat members).

This issue is not present in latest develop build (25th of January), not present in develop build from 22nd of January.

alwx · 2018-01-25T12:59:37Z

@Serhy checking

alwx · 2018-01-29T13:08:01Z

@Serhy
Sorry, there was a problem with Jenkins, that's why it took some time. The problem described by you, by the way, exists in master as well, but maybe due to restructuring it doesn't affect things there the way it affects them here :)

Please, take a look at these builds ASAP, because it's already time to merge this branch. Thanks!

Android: https://i.diawi.com/PftqWL
iOS: https://i.diawi.com/PjAdXb

Serhy · 2018-02-01T15:45:39Z

Builds for Android: https://i.diawi.com/ez6PN8 and iOS: https://i.diawi.com/JcYz9K was tested on Android 7.0 (Samsung real device) and iOS 11.2.2. Upgrade from 0.9.13 tested on Android 6.0 emulator.
No any regression found! Looks good for merge!

…if quickly tap on send button Signed-off-by: Dmitry Novotochinov <trybeee@gmail.com>

alwx self-assigned this Dec 22, 2017

alwx force-pushed the feature/chat-send-message-refactoring branch 2 times, most recently from 5030b44 to 10a3d84 Compare December 22, 2017 11:44

alwx changed the title ~~WIP: Chat refactoring: updated send-message namespace~~ Chat refactoring: updated send-message namespace Dec 23, 2017

alwx requested review from yenda and janherich December 23, 2017 09:15

alwx added the developer-xp label Dec 23, 2017

flexsurfer added tech-debt and removed developer-xp labels Dec 25, 2017

flexsurfer force-pushed the feature/chat-send-message-refactoring branch from 10a3d84 to 046d8cd Compare December 25, 2017 20:48

flexsurfer requested changes Dec 25, 2017

View reviewed changes

flexsurfer reviewed Dec 26, 2017

View reviewed changes

janherich mentioned this pull request Dec 27, 2017

Send message refactoring #2829

Merged

alwx force-pushed the feature/chat-send-message-refactoring branch 3 times, most recently from c8dca07 to ff34aa4 Compare January 5, 2018 09:12

alwx requested a review from flexsurfer January 5, 2018 09:12

flexsurfer reviewed Jan 5, 2018

View reviewed changes

janherich suggested changes Jan 5, 2018

View reviewed changes

alwx requested review from janherich and flexsurfer January 8, 2018 07:51

alwx force-pushed the feature/chat-send-message-refactoring branch from e19a040 to 03a1ed5 Compare January 8, 2018 08:05

yenda removed the Tested - Issues label Jan 25, 2018

yenda force-pushed the feature/chat-send-message-refactoring branch from dc43da1 to 7468955 Compare January 25, 2018 01:05

flexsurfer approved these changes Jan 25, 2018

View reviewed changes

Serhy added the Tested - Issues label Jan 25, 2018

alwx force-pushed the feature/chat-send-message-refactoring branch 4 times, most recently from 2827f5f to ef17f05 Compare January 29, 2018 09:31

alwx force-pushed the feature/chat-send-message-refactoring branch from ef17f05 to d7f9330 Compare January 30, 2018 21:28

alwx force-pushed the feature/chat-send-message-refactoring branch from d7f9330 to 4d733ce Compare February 1, 2018 15:56

Serhy added Tested - OK and removed Tested - Issues labels Feb 1, 2018

[fix #1604]: Commands send, request, location are sent several times …

74f9ea3

…if quickly tap on send button Signed-off-by: Dmitry Novotochinov <trybeee@gmail.com>

dmitryn force-pushed the feature/chat-send-message-refactoring branch from 4d733ce to 74f9ea3 Compare February 1, 2018 16:17

dmitryn merged commit 74f9ea3 into develop Feb 1, 2018

rasom deleted the feature/chat-send-message-refactoring branch April 19, 2018 10:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chat refactoring: updated send-message namespace #2795

Chat refactoring: updated send-message namespace #2795

alwx commented Dec 22, 2017

flexsurfer commented Dec 25, 2017

flexsurfer Dec 25, 2017

flexsurfer Dec 26, 2017

flexsurfer Dec 25, 2017

flexsurfer Dec 25, 2017

flexsurfer Dec 26, 2017

flexsurfer Dec 25, 2017

flexsurfer Dec 26, 2017

flexsurfer commented Dec 26, 2017

oskarth commented Jan 2, 2018

alwx commented Jan 4, 2018 •

edited

Loading

alwx commented Jan 5, 2018

flexsurfer Jan 5, 2018

janherich Dec 23, 2017

janherich Jan 5, 2018

alwx commented Jan 8, 2018

flexsurfer commented Jan 25, 2018 •

edited

Loading

Serhy commented Jan 25, 2018 •

edited

Loading

alwx commented Jan 25, 2018

alwx commented Jan 29, 2018 •

edited

Loading

Serhy commented Feb 1, 2018


		(defn send-message [{{:keys [network-status] :as db} :db
		:keys [now get-stored-chat]}

Chat refactoring: updated send-message namespace #2795

Chat refactoring: updated send-message namespace #2795

Conversation

alwx commented Dec 22, 2017

flexsurfer commented Dec 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flexsurfer commented Dec 26, 2017

oskarth commented Jan 2, 2018

alwx commented Jan 4, 2018 • edited Loading

alwx commented Jan 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alwx commented Jan 8, 2018

flexsurfer commented Jan 25, 2018 • edited Loading

Serhy commented Jan 25, 2018 • edited Loading

alwx commented Jan 25, 2018

alwx commented Jan 29, 2018 • edited Loading

Serhy commented Feb 1, 2018

alwx commented Jan 4, 2018 •

edited

Loading

flexsurfer commented Jan 25, 2018 •

edited

Loading

Serhy commented Jan 25, 2018 •

edited

Loading

alwx commented Jan 29, 2018 •

edited

Loading