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

Do not autorespond to numbers on Blacklist #750

Closed
abbyad opened this issue Dec 16, 2014 · 24 comments
Closed

Do not autorespond to numbers on Blacklist #750

abbyad opened this issue Dec 16, 2014 · 24 comments
Labels
Type: Feature Add something new

Comments

@abbyad
Copy link
Contributor

abbyad commented Dec 16, 2014

Following up on #201, users and administrators need to be able to ignore/hide messages from specific numbers or ranges of numbers. This is particularly important for MNO messages, which we do not want to respond to.

First iteration may just be available in app_settings, but eventually we will need some design work to decide the best way to present this to users.

@abbyad abbyad added the Type: Feature Add something new label Dec 16, 2014
@abbyad abbyad changed the title whitelist Do not autorespond to numbers on Blacklist Dec 16, 2014
@abbyad abbyad added the Help wanted Good for first time contributions label Apr 11, 2015
@mandric
Copy link
Contributor

mandric commented Jun 17, 2015

We should also support blacklisting number like "Orange" since the telecom provider can create messages originating from that string. Also does this feature only apply to auto-responses? What if a valid form is sent via a blacklisted from string? I assume we need to ignore responses setup by registration configurations or any other feature.

@abbyad
Copy link
Contributor Author

abbyad commented Sep 4, 2015

We are finding several projects that end up in message loops with the operator when the gateway is out of credit. Once credit is added, all the pending auto-replies get sent, eating up the newly added credit.

@browndav we already have a similar feature for not sending a number to the gateway, so would it be low effort to do something similar that to avoid sending to any other number as well?

As pointed out by @mandric, we should accept text strings as well as phone numbers.

@abbyad
Copy link
Contributor Author

abbyad commented Oct 5, 2015

If someone jumps in to take this, a good place to start looking is how we avoid message loops with the gateway_number in medic-sentinel/transitions/default_responses.js

@mandric
Copy link
Contributor

mandric commented Oct 5, 2015

How do we plan to define a blacklist? Could it be a comma separated list of strings in app_settings somewhere?

@mandric
Copy link
Contributor

mandric commented Oct 5, 2015

Also assuming we need this feature on all branches/release channels.

@mandric mandric self-assigned this Oct 5, 2015
@abbyad
Copy link
Contributor Author

abbyad commented Oct 5, 2015

Correct, we are getting urgent requests for this from many projects so we should push this to all branches.

Having it as a comma separated field in app_settings only would be acceptable for now. If we can surface this to the configuration/settings/advanced that would be even better.

@mandric mandric modified the milestones: Iteration 2015-39: Mobile Application Ready for Internal Testing, Iteration 2015-41: Mobile application overflow Oct 6, 2015
@mandric
Copy link
Contributor

mandric commented Oct 6, 2015

Should we also consider having a "whitelist" option, so you only setup responses to numbers that are in the facilities list or something?

@mandric
Copy link
Contributor

mandric commented Oct 6, 2015

Thinking the blacklisted function might behave like this:

isBlacklisted(str, from)

string from returns
+123 +123 true
+123 +999 false
+123 +123999999 true
SAFARI SAFARICOM true
Safari SAFARICOM true
+123,+456,+789 +456 true
+123,+456,+789 +4569999999 true
SAFARI, ORANGE ORANGE NET true
SAFARI, ORANGE NET ORANGE false
VIVO EM VIVO false

@abbyad
Copy link
Contributor Author

abbyad commented Oct 6, 2015

That would seem to work well @mandric.

I think that having a whitelist would be good to include as well. My understanding is that you could send to a whitelisted number, even if it is in the blacklisted list. Is that right?

Also, are we checking if it is a valid number before sending? This would be useful to know for creating the blacklists.

@mandric
Copy link
Contributor

mandric commented Oct 6, 2015

Nope, no checking if phone numbers are valid, we just take the number the gateway passed to us in the message and use it.

@mandric
Copy link
Contributor

mandric commented Oct 6, 2015

It might be useful to put a warnings property on a doc or something and when the system attempts to send a message to a blacklisted number the doc is marked with a warning. Otherwise the silent failing to add a message or response could lead to confusion. It might be useful for the system to have a way to say: "You wanted me to send to this phone but it's blacklisted."

@mandric
Copy link
Contributor

mandric commented Oct 6, 2015

Also for the record not crazy about using the term “blacklist” but can’t think of anything else at the moment. Maybe “deny list” or “reject list”?

mandric pushed a commit to medic/medic-sentinel that referenced this issue Oct 7, 2015
…ion.

outgoing_deny_list is a new configuration that is a comma separated list
of strings. If a string in that list matches the beginning of the phone
we are setting up a response to then this transition is ignored.  The
transition will never fire because the filter will fail on those
documents and those message will be left without an auto-reply.

Issue: medic/cht-core#750
@mandric
Copy link
Contributor

mandric commented Nov 11, 2015

screen shot 2015-11-11 at 7 16 19 pm

forms only mode:

screen shot 2015-11-11 at 7 15 48 pm

@mandric mandric removed their assignment Nov 11, 2015
@mandric
Copy link
Contributor

mandric commented Nov 11, 2015

Not sure if I should flag/invalidate the record (add an error to it). Currently just shows up as valid with denied messages.

@garethbowen garethbowen self-assigned this Nov 11, 2015
@garethbowen
Copy link
Member

Looks good. I've added one comment about style preference.

@mandric
Copy link
Contributor

mandric commented Nov 11, 2015

Henok, a new RC build should be available shortly with this update. Let me know if you have any time to acceptance test this one again, thanks!

@mandric
Copy link
Contributor

mandric commented Nov 12, 2015

@henokgetachew or @abbyad when you get a chance please acceptance test. Latest RC is running here: https://rc.dev.medicmobile.org/

@henokgetachew henokgetachew removed their assignment Nov 15, 2015
garethbowen pushed a commit that referenced this issue Oct 24, 2017
Outgoing deny list will now be applied to schedules and responses
generated on reports.  So instead of seeing `scheduled` or `pending`
state on a message you will see `denied`.

Moved `isOutgoingAllowed`, `_isMessageFromGateway` and related tests
into utils library so schedule creation and report validation and
response messages can make use of it.

Calling `messages.addMessage` or `uitls.addScheduledMessage` now
transparently checks the outgoing deny list and assigns a `denied` state
when appropriate.

I removed some odd logic in `addScheduledMessage` because it's not used
or documented anywhere and is error prone.  There was a check for
`doc.muted` field when creating a scheduled message.  I looked through
the code and fairly certain this property is not set or used by
anything.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Add something new
Projects
None yet
Development

No branches or pull requests

4 participants