Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block and stop #893

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Block and stop #893

merged 4 commits into from
Jul 30, 2020

Conversation

rowanseymour
Copy link
Member

Adds #889

Captura de Pantalla 2020-07-29 a la(s) 14 03 50

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #893 into master will increase coverage by 0.00%.
The diff coverage is 76.92%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #893   +/-   ##
=======================================
  Coverage   64.03%   64.04%           
=======================================
  Files         178      178           
  Lines        6888     6912   +24     
  Branches     1616     1621    +5     
=======================================
+ Hits         4411     4427   +16     
- Misses       1912     1919    +7     
- Partials      565      566    +1     
Impacted Files Coverage Δ
...c/components/flow/actions/updatecontact/helpers.ts 75.00% <0.00%> (-8.34%) ⬇️
src/config/interfaces.ts 100.00% <ø> (ø)
src/config/typeConfigs.ts 75.67% <ø> (ø)
src/flowTypes.ts 100.00% <ø> (ø)
...nents/flow/actions/updatecontact/UpdateContact.tsx 85.71% <100.00%> (+1.50%) ⬆️
...s/flow/actions/updatecontact/UpdateContactForm.tsx 90.00% <100.00%> (+1.66%) ⬆️
src/components/flow/props.ts 100.00% <100.00%> (ø)
src/components/helpers.ts 72.72% <100.00%> (ø)
src/components/simulator/LogEvent.tsx 62.38% <100.00%> (+0.34%) ⬆️
src/testUtils/assetCreators.ts 89.40% <100.00%> (+0.19%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba81d97...02732b6. Read the comment docs.

@nicpottier
Copy link
Contributor

Thought we'd decided to avoid using "opt-out" because it will collide with possible concepts of "opting-in"?

Occurred to me that the distinction between these two states that maybe is important to users is that a contact that is blocked that sends an incoming message will be ignored while an stopped contact will go through normal flow handling for that message. (and become unstopped in the process)

Not entirely sure how to summarize that though!

@rowanseymour
Copy link
Member Author

Oh I thought we'd only decided not to call the status "Opt out" or have "Opt in" as a status.

I think the important thing to explain on this modal, is not all the ways in which blocking and stopping and treated differently in the system, but the fundamental distinction that stopping is the contact opting to end communication, and blocking is you the user opting to end communication.

We might want to link to documentation from the modal if we want users to understand how the system treats these statuses.

@nicpottier
Copy link
Contributor

Ya, it may be a lot to explain all at once. What about:

blocked - remove from groups, ignore messages
stopped - remove from groups

@rowanseymour
Copy link
Member Author

As I said I think the more important thing to explain on this modal is the distinction between contact initiated and user initiated. Stopping will happen automatically on some channels if a contact wants to opt-out, but this is a chance in a flow to see that a contact wants to opt-out.

It's going to get weird if users don't understand this is the same thing that happens if a channel supports stopping/opting-out.

Maybe..

Blocked - we want to ignore messages from this contact
Stopped - contact is opting out and will be removed from groups

@rowanseymour
Copy link
Member Author

This is what we say on the Stopped page:

Captura de Pantalla 2020-07-29 a la(s) 15 38 22

We should probably have a similar blurb on https://app.rapidpro.io/contact/blocked/

@nicpottier
Copy link
Contributor

Well I mean it is still going to say "Blocked" and "Stopped" there so I don't think there's any way diminishment in communicating that. I don't think we generally use "we" in the system, so maybe that's what getting my goat. But in this case it really isn't that the user is opting out for example, the flow author is opting them out for whatever reason, so telling them what changing that status does seems more logical to me. And describing the group behavior and msg behavior as I tried seemed like the simplest way to do that.

We are doing a lot more than just ignoring messages for the contact by selecting "Blocked", making it clear that they will be removed from all groups seems like something we should be communicating to users.

@ericnewcomer thoughts here?

@rowanseymour
Copy link
Member Author

Well the text I have in the PR is Stopped - the contact wants to opt-out which is accurate, if the flow author is using this correctly. I dunno.. I think we want to ensure flow authors use it consistently with how we use it in message handling. Just telling them Stopped - remove from groups makes it sound it like a handy way to remove a contact from groups.

@nicpottier
Copy link
Contributor

Don't you think removing the contact from all groups is a destructive enough thing that we should be mentioning it front and center? I'm just worried that changing status sounds like a pretty benign thing but really is doing some serious shit that you can't undo.

@rowanseymour
Copy link
Member Author

That's a fair point.. tho it leaves me wondering if the block button on the contact list page warrants a health warning.. or we should figure out a way to not remove from groups.

Maybe we stop trying to stuff all this into the select box and add an actual warning text to the modal, e.g.

Captura de Pantalla 2020-07-29 a la(s) 17 20 10

@ericnewcomer
Copy link
Member

Maybe something like..

blocked - remove from groups, ignore forever
stopped - remove from groups, ignore until they message next

With or without the remove from groups part depending on how important we feel that is.

@rowanseymour
Copy link
Member Author

@ericnewcomer you don't like the idea of pulling that out into an actual warning so we don't have to worry about word count here?

(maybe the portuguese version will be longer than the select box is wide 🤷)

@ericnewcomer
Copy link
Member

ericnewcomer commented Jul 29, 2020

Personally, I prefer this information to be as concise as possible and next to the decision point. It'll wrap if it needs to with localization. Also, the option rendering could instead have a section underneath the title when it is focused describing the status.

@rowanseymour
Copy link
Member Author

Fine, ok we all happy with..

Captura de Pantalla 2020-07-29 a la(s) 18 10 04

@ericnewcomer
Copy link
Member

Sure. If we don't want to style it custom, I'd rather we hyphenate instead of using parentheses.

@nicpottier
Copy link
Contributor

Either works for me!

@rowanseymour
Copy link
Member Author

Updated

Captura de Pantalla 2020-07-30 a la(s) 09 34 52

@rowanseymour rowanseymour merged commit fe9bc54 into master Jul 30, 2020
@rowanseymour rowanseymour deleted the block_and_stop branch July 30, 2020 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants