[refs #3210] Replace realm reads for chats with app-db reads #3225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Let me first resolve the conflicts. |
flexsurfer
reviewed
Feb 6, 2018
src/status_im/chat/events/input.cljs
Outdated
| @@ -477,8 +477,7 @@ | |||
| (handlers/register-handler-fx | |||
| :send-current-message | |||
| [(re-frame/inject-cofx :random-id) | |||
| (re-frame/inject-cofx :get-last-clock-value) | |||
| (re-frame/inject-cofx :get-stored-chat)] | |||
| (re-frame/inject-cofx :get-last-clock-value)] | |||
There was a problem hiding this comment.
i thought we removed :get-last-clock-value, @janherich ?
There was a problem hiding this comment.
@flexsurfer it will be removed after i merge develop
|
Found bug with public chats - create new pub chat, send message to it, got: UPD: it only happens in this PR, not in current develop. |
|
@dmitryn That bug should not happen on develop |
@janherich right, i'm just logging it here to remember myself to fix it. |
dmitryn
force-pushed
the
3146-realm-reads-part-2
branch
from
ea9ab0a
to
3c406b2
Compare
|
Re-opening, was accidentally closed while merging conflicts with develop branch |
|
@janherich @flexsurfer it's ready for review after develop has been merged. Please take a look. |
janherich
approved these changes
Feb 6, 2018
src/status_im/chat/handlers.cljs
Outdated
| @@ -172,14 +172,15 @@ | |||
| :added-to-at timestamp | |||
| :timestamp timestamp | |||
| :is-active true} | |||
| exists? (chats/exists? group-id)] | |||
| (when (or (not exists?) (chats/new-update? timestamp group-id)) | |||
| exists? (not (nil? chat))] | |||
There was a problem hiding this comment.
Technically, (nil? chat) is not necessary and (not chat) will work just as good, but minor point.
There was a problem hiding this comment.
@janherich ok, i'll get rid of exists? binding at all, because (if chat) reads as clear as (if exists?)
src/status_im/data_store/chats.cljs
Outdated
| set | ||
| (set/intersection identities) | ||
| seq | ||
| boolean))) |
There was a problem hiding this comment.
Seems quite complicated for simple membership test, won't fn signature [contacts identity] and body:
((into #{} (map :identity) contacts) identity)Work just as well ?
Also because it's a simple helper function used just in the protocol.handlers ns, I would define it there.
There was a problem hiding this comment.
@janherich good point. Set is indeed can be used as function for membership test. Will change that.
janherich
approved these changes
Feb 7, 2018
|
@janherich i'm also thinking if It's being used in 2 other namespaces: chat/handlers and protocol/handlers so i cannot simply move it to both. Maybe move it to |
|
@dmitryn I think |
|
please, ignore e2e tests failures above, all of them are using old 'add contact flow', new flow will be added today |
dmitryn
force-pushed
the
3146-realm-reads-part-2
branch
from
9d71e91
to
8d26de0
Compare
|
Commits have been squashed. |
goranjovic
approved these changes
Feb 8, 2018
|
@flexsurfer Are you happy with the changes? Can we move it forward? |
flexsurfer
approved these changes
Feb 8, 2018
|
@goranjovic sure |
|
@dmitryn conflicts :) |
dmitryn
force-pushed
the
3146-realm-reads-part-2
branch
2 times, most recently
from
f711b4e
to
6e95026
Compare
|
Conflicts have been resolved. |
4 tasks
|
@dmitryn I've tested it and so far it looks good. Could you rebase this branch so I could run end-end tests on the most up to date version of the build? |
dmitryn
force-pushed
the
3146-realm-reads-part-2
branch
from
6e95026
to
8a5ebd3
Compare
Automated test results:test_send_eth_from_wallet_sign_now:x:Test Steps & Error message:
test_browse_link_entering_url_in_dapp_view:white_check_mark::Test Steps & Error message:test_contact_profile_view:x:Test Steps & Error message:
test_one_to_one_chat_messages_and_delete_chat:x:Test Steps & Error message:
test_transaction_send_command_wrong_password:white_check_mark::Test Steps & Error message:test_send_stt_from_wallet_via_enter_recipient_address:white_check_mark::Test Steps & Error message:test_send_eth_from_wallet_sign_later:white_check_mark::Test Steps & Error message:test_send_transaction_from_daap:white_check_mark::Test Steps & Error message:test_network_switch:white_check_mark::Test Steps & Error message:test_transaction_send_command_group_chat:white_check_mark::Test Steps & Error message:test_group_chat_messages:white_check_mark::Test Steps & Error message:test_public_chat:x:Test Steps & Error message:
test_transaction_send_command_one_to_one_chat:x:Test Steps & Error message:
test_send_eth_to_request_in_one_to_one_chat:white_check_mark::Test Steps & Error message:test_send_eth_to_request_in_group_chat:white_check_mark::Test Steps & Error message:test_send_eth_to_request_from_wallet:white_check_mark::Test Steps & Error message: |
Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
flexsurfer
force-pushed
the
3146-realm-reads-part-2
branch
from
8a5ebd3
to
8abdd77
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #3210
This second part of ongoing work, part 1 is here #3212
Summary:
Replace realm read for chats data with app-db read as we don't need to touch realm since data is already in app-db.
Review notes:
This is part of further work at #3210
Testing notes:
App behavior should not change. Code changed related to interactions with the chats:
status: ready