Chat team MVP4 part 1: block user #7170
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (68)
|
yenda
force-pushed
the
mvp4-1-block-user
branch
2 times, most recently
from
0657808 to
bb8b7ed
Compare
| {:messages (core/all-clj (core/get-by-field @core/account-realm | ||
| :message :from public-key) | ||
| :message) | ||
| :chat (core/single-clj (core/get-by-field @core/account-realm |
There was a problem hiding this comment.
as long as :chat is not used later probably no need to execute this query here
|
We can also simulate some kind of spam attack and check how these changes are useful in that case. |
yenda
force-pushed
the
mvp4-1-block-user
branch
from
bb8b7ed to
fd392cf
Compare
| [message-ids] | ||
| (-> @core/account-realm | ||
| (.objects "messages") | ||
| (.filtered (str "(" (core/in-query "message-id" message-ids) ")")))) |
There was a problem hiding this comment.
this is query is going to be slower than fetching each separate entry using .objectForPrimaryKey
There was a problem hiding this comment.
we'll leave that kind of optimization for later this is a mvp and at that point the visible effects of blocking are already applied so perf doesn't matter that much.
There was a problem hiding this comment.
i agree, but also i would at least consider adding activity indicator because it took 10s on my slower device to block the user which already has sent 1000 messages.
There was a problem hiding this comment.
6s for removing 6 messages from 7 chats with ~2000 messages in total.
There was a problem hiding this comment.
What takes 10s is it the realm part or the whole thing. I would think recomputing the message groups of the affected chats is more expensive. Also this is the MVP of the MVP so I'm not really concerned about slow devices at that point
There was a problem hiding this comment.
most likely realm, because otherwise, each incoming message would have a similar effect. I doubt that those subs have any serious impact here.
It is 1.2-1.5s on galaxy s9 which is not a slow device, that's when removing messages 6 messages with 6 chats and ~2000 messages in total. A realistic test would require 20-30 chats, 15-20k messages in the realm db and removing dozens of messages from different chats. I suppose it might be up to 2.5-3s.
Well if that's not necessary from UX pov then no problem.
yenda
force-pushed
the
mvp4-1-block-user
branch
from
fd392cf to
b9bac18
Compare
|
In general, what's the goal of this particular PR: to give the user an ability to mute an annoying user or to protect from some kind of dos attacks? |
|
@rasom for now it's to block the kind of spam we had a couple of time already with someone posting useless huge messages. |
|
@yenda with the assumption that it will be hard for this person to automate account creation and to send N messages per acc? It's kind of easy thing to implement and in this case, this functionality will be useless. If we just want to introduce user blocking functionality then fine, but we if need spam protection this thing isn't a big obstacle. |
|
Yes it's just to block a user |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
ghost
added
the
There was a problem hiding this comment.
@yenda I've noticed that there is no desktop support yet - is this for future versions of this feature? (desktop's profile screen code hasn't been changed https://github.com/status-im/status-react/blob/b56fd2e29fae97c47a8274449adaa70526665e3b/src/status_im/ui/screens/desktop/main/chat/views.cljs#L348)
|
@siphiuel yes it's for a future iteration |
yenda
force-pushed
the
mvp4-1-block-user
branch
from
b9bac18 to
4228713
Compare
yenda
force-pushed
the
mvp4-1-block-user
branch
from
a22e382 to
ccd6e9f
Compare
|
already done |
98% of end-end tests have passedFailed tests (1)Click to expand
Passed tests (57)Click to expand |
100% of end-end tests have passedPassed tests (1) |
|
@yenda @rachelhamlin
2) In public chat: after blocking user and hiding his messages, "No messages" is shown on "Home" screen 3) Can add blocked users to a group chat |
|
@churik yes please. I imagine this function is most useful for removing users from public chats at this point, so it's okay to fix 1:1 issues in next iteration. |
|
Tested:
|
|
So known behavior for now:
|
|
@churik so we can merge this? |
|
@yenda yes, you can merge it! |
|
@churik yeah sure but please create them in pivotal as well since we are traking progress there now |
- dapp related fields are removed since it is not used anymore - blocked? field is added for futur block user feature
yenda
force-pushed
the
mvp4-1-block-user
branch
from
ccd6e9f to
3645f82
Compare
- add block/unblock action to user profile - blocking deletes all messages from user and ignores future messages - unblocking stops ignoring new messages from user but doesn't recover past ones [feature] add contact list [tests] added scroll to BackupRecoveryPhraseButton [tests] added scroll to public key Signed-off-by: yenda <[email protected]>
yenda
force-pushed
the
mvp4-1-block-user
branch
from
3645f82 to
b80e02d
Compare
partially fixes #7157
Summary:
Currently the user can only been unblocked before you leave the profile. Later we will reintroduce contact list with a list of blocked users so that users can be unblocked
Testing notes:
Platforms
Areas that maybe impacted (optional)
Functional
status: ready