Skip to content

DRAFT Twilio API#96

Open
hectron wants to merge 9 commits intomasterfrom
use_twilio
Open

DRAFT Twilio API#96
hectron wants to merge 9 commits intomasterfrom
use_twilio

Conversation

@hectron
Copy link
Copy Markdown
Collaborator

@hectron hectron commented Nov 4, 2019

No description provided.

Hector Rios added 6 commits November 3, 2019 23:37
This commit adds the ability to have a variety of different numbers that
belong to a property. In order for someone to send an SMS/MMS to HH,
they need to be configured in their proper property.
end
end

def perform
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes a message of the form:

"500.00 new door for the living room" # expenses it the moment it was texted
"45.00 replaced water faucet @ 11/13" # creates an entry for 11/13/2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can change. This is just a first pass at what a text message could be formatted as.

If we want to change it, we can. The cool thing is that since we have specs, we don't have to do a full end-to-end test to verify the success.

@@ -0,0 +1,20 @@
class MMSController < ApplicationController
def process_message
handler = HappyHouse::MMS::Interface.handler_for(inbound_message)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way that this works is that we set up some webhooks in a SMS/MMS provider such as Twilio or Telnyx.

Those webhooks will send JSON payloads to this endpoint. Depending on what the target number is, we can categorize them.

So we have a phone number for submitting expense reports, another for who knows what, etc.

The number that has submitted the MMS should belong to a property. Later, we can support the need for one number, multiple properties.

Comment on lines +19 to +21
# Auto-load lib
config.autoload_paths << config.root.join('lib')
config.eager_load_paths << config.root.join('lib')
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to keep require'ing files in the spec directory, so I just autoloaded + eager loaded the lib directory.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the spec directory have to do with lib?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, rails doesn’t load the lib directory. Even with rails_helper, I had to explicitly require the lib directory files.

These lines auto-load the lib directory on rails boot. To auto-include the lib directory in the specs, we simply have to require 'rails_helper'

# @param inbound_message [HappyHouse::MMS::InboundMessage] The message to process
# @return [nil]
def self.handler_for(inbound_message)
case inbound_message.to
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Routes the message based on who the recipient should be.

@@ -0,0 +1,5 @@
class AddPhoneNumbersToProperties < ActiveRecord::Migration[5.2]
def change
add_column :properties, :phone_numbers, :string, null: false, array: true, default: []
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a weird way to add a phone number to a property. hmmm

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is kinda funky. I didn't want to create another table for just phone numbers, but :shurg:

I think a properties_phone_numbers table might make sense.

@@ -0,0 +1,23 @@
version: '2'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this file all about?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to mention this! This is a dip file. Dip is a tool to work with docker-compose much easier.

def perform
raise HappyHouse::MMS::Errors::BadRequestError, errors.join(', ').capitalize unless valid?

property.expense_items.create(name: expense_name,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know Rails lets do you do this but I'm not a fan of model.association.create or model.association.build methods tbh

# MMS providers such as Twilio.
module HappyHouse
module MMS
class InboundMessage
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this class is responsible for parsing an inbound message?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! If we wanted to set up a new MMS provider, we just have to map what they send us to the attr_accessor that we need.

module HappyHouse
module MMS
class Interface
PHONE_NUMBER_FOR_EXPENSES = ENV['PHONE_NUMBER_FOR_EXPENSES'].freeze
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an array?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's a single number.

@hebron-george
Copy link
Copy Markdown
Collaborator

Got a working demo you can screenshot?

@@ -0,0 +1,5 @@
class AddPhoneNumbersToProperties < ActiveRecord::Migration[5.2]
def change
add_column :properties, :phone_numbers, :string, null: false, array: true, default: []
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is kinda funky. I didn't want to create another table for just phone numbers, but :shurg:

I think a properties_phone_numbers table might make sense.

module HappyHouse
module MMS
class Interface
PHONE_NUMBER_FOR_EXPENSES = ENV['PHONE_NUMBER_FOR_EXPENSES'].freeze
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, it's a single number.

@@ -0,0 +1,23 @@
version: '2'
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to mention this! This is a dip file. Dip is a tool to work with docker-compose much easier.

Comment on lines +4 to +16
bash:
service: website
command: '/bin/bash'
rake:
service: website
command: bundle exec rake
rails:
service: website
command: bundle exec rails
rspec:
service: website
command: bundle exec rspec
psql:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. dip rspec spec/lib/<> == docker-compose run --rm website bundle exec rspec spec/lib/<>

This is basically saying -- whenever I type in dip <interaction>, actually run the command within the docker-compose service.

Comment on lines +17 to +19
service: db
# The following is set to mirror `config/database.yml`
command: psql -h db -U postgres -d happy_house_development
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome because if you have Postgres in a Docker container, you can simply run dip psql to get the psql console inside your postgres container.

# MMS providers such as Twilio.
module HappyHouse
module MMS
class InboundMessage
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! If we wanted to set up a new MMS provider, we just have to map what they send us to the attr_accessor that we need.

Copy link
Copy Markdown
Collaborator

@hebron-george hebron-george left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get it out and test it.
Looks like your last push broke some migration tests though.

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.

2 participants