-
Notifications
You must be signed in to change notification settings - Fork 11
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
Support reporting raids with SMS #24
Conversation
If a subscribed user sends an SMS starting with "report", the text after "report" will be interpreted as an address and sent out as an alert to all subscribers. This operates through the same code path as the alert form on the site. Right now, this only supports plain text addresses. We will be able to integrate it with geocoding in the future to acquire coordinates, as well as correct typos.
Issue #13 |
Also, I'm still very new to Scala, so if there's anything I did which isn't idiomatic or which could be done better in Scala, let me know :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good - The one concern I have is if we want all subscribers to be able to send out alerts system wide. If you're at the tech meeting we can chat about it there, otherwise I'll bring it up with the rest of the group and update the PR with the notes from the discussion.
Oh, I 100% agree! I meant this mainly as a stop-gap until we have some sort of whitelist, but I'd also be happy to build in some basic whitelist functionality into this PR. |
And, unfortunately I won't be able to make it to the meeting, but I'll stay in touch! |
Just double checking if this is good to merge. Since this app is still in development, I think it's OK to submit this without a whitelist and wait to do whitelisting correctly in another change, but if we want I can add an unauthenticated whitelist page to the site as part of this PR and only allow whitelisted users to submit reports. |
Thanks for the follow up - I agree we can submit this without whitelisting as we're not using it in production yet. We can follow up in another PR to add it. Merged! |
If a subscribed user sends an SMS starting with "report", the text after
"report" will be interpreted as an address and sent out as an alert to
all subscribers. This operates through the same code path as the alert
form on the site.
Right now, this only supports plain text addresses. We will be able to
integrate it with geocoding in the future to acquire coordinates, as
well as correct typos.