SparkDevNetwork
Invalid SMS Actions Block The SMS Pipeline
Description
Currently, whenever a new Give, Reply, or SMS Conversation SMS Action is added to the pipeline, it prevents all actions lower in the pipeline from being activated. This is because a blank SMS Action will match all incoming messages and will not continue by default.
This means that whenever a new Give, Reply, or SMS Conversation SMS Action is added to the SMS Pipeline, the whole pipeline is blocked until the user adding the SMS Action edits the new action and adds additional criteria to make the SMS Action specific to only its use case.
This problem is especially severe if someone accidentally leaves a blank SMS Action in the pipeline. This can cause the entire pipeline not to work until someone finds and removes that blank action.
Note. This affects the Give, Reply, and SMS Conversation SMS Actions. It does not affect the Launch Workflow SMS Action since that action checks to make sure there is a workflow before it consumes message traffic.
Actual Behavior
New, blank SMS Actions consume all message traffic they receive and prevent the incoming SMS message from continuing to propagate through the rest of the pipeline.
Expected Behavior
New, blank SMS actions should not consume any message traffic they receive until they have been configured by the user.
This check could be added to the existing ShouldProcessMessage() method on each of the SMS Actions and could check to make sure all of the required fields have a value (which is how the Launch Workflow SMS Action works).
Steps to Reproduce
- Go to an existing SMS Pipeline
- Drag a new Reply SMS Action onto the top of the SMS Pipeline
- Do not configure anything on the new SMS Action
- Use the SMS Pipeline Tester or text into the pipeline with a message that should be handled
- Notice that your message was not handled and was not replied to
Issue Confirmation
- [X] Perform a search on the Github Issues to see if your bug or enhancement is already reported.
- [X] Try to reproduce the problem on a fresh install or on the demo site.
Rock Version
v15.1, v16.2
Client Culture Setting
en-US
@Jon-Corey I am unable to replicate the issue as described with "Give" actions. The ShouldProcessMessage() method of that action is already limited to specific keywords (or messages that begin with a "$" character) and it does not seem to trap other messages in my testing.
If you're convinced that this action exhibits the problem, could you perhaps give specific details of how you are able to cause this issue?
On the other two actions, I think I agree regarding the "Reply" action, that it's ShouldProcessMessage() should return false if the required Reply attribute does not have a value, as the UI does not intentionally let you set the action up that way. However, on the "SMS Conversations" action, a "blank" action appears to be a perfectly valid configuration (and this is how our default SMS pipeline comes out-of-the-box) so I think blocking that from processing would not be a good solution.
@shauncummings Sorry. Yes, you are right that "Give" doesn't have any issues. I didn't look into that one enough. That's my bad.
As for "SMS Conversations" I did not take the time I should have to learn how that system works. Again, my bad.
Sorry for the confusion. I'll make sure to take more time to research next time.
@Jon-Corey FYI, the portion of this issue which qualifies as a bug (unconfigured "Reply" actions blocking the pipeline) will be fixed as of v16.6. As the other portion ("catch all" SMS Conversation actions) is a valid configuration, we are not making any change to that.
Thanks again for pointing this out.
@shauncummings Thank you for all your help with this! It is greatly appreciated.