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.

Sign up for GitHub

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

Define a separate method for form submission #173

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Define a separate method for form submission #173

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d955929
Define a separate method for form submission that returns the HTTPart…
amitfriedman12 Dec 25, 2018
e2d5be2
Add 'addedAt' attribute to the output for contact_list
mkyaw Feb 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/hubspot/form.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ def fields(opts={})

# {https://developers.hubspot.com/docs/methods/forms/submit_form}
def submit(opts={})
response = Hubspot::FormsConnection.submit(SUBMIT_DATA_PATH, params: { form_guid: @guid }, body: opts)
[204, 302, 200].include?(response.code)
[204, 302, 200].include?(submit_response(opts).code)
Copy link
Contributor

@SViccari SViccari Jan 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @amitfriedman12,

After reading Chris's WIP PR and understanding his wish to return an instance of the class instead of a more generic Hubspot::Response object, that unblocks this work and I'd love to help you merge your changes.

I think there are two changes that we can make to improve feedback for when form.submit fails:

  1. Update FormsConnection.submit to raise when the response fails.
Example of changes: 
class FormsConnection < Connection
  follow_redirects true

  def self.submit(path, opts)
    headers = { 'Content-Type' => 'application/x-www-form-urlencoded' }
    url = generate_url(path, opts[:params], { base_url: 'https://forms.hubspot.com', hapikey: false })
    response = post(url, body: opts[:body], headers: headers)
    handle_response(response)
  end
end
  1. Update the Hubspot::RequestError to provide more details about the error
Example of changes: 
module Hubspot
  class RequestError < StandardError
    attr_reader :response

    def initialize(response)
      @response = response
      super("Status Code: #{response.code}\nResponse: #{response.inspect}")
    end
  end
end

As for form.submit, I'm curious what you and @cbisnett thinks is the best return value. The HubSpot API for submitted data to a form only returns a status code to indicate success or failure. We could return true on success (as failure will raise) or perhaps self (even though nothing about the form has changed). What are your thoughts?

end

def submit_response(opts={})
Hubspot::FormsConnection.submit(SUBMIT_DATA_PATH, params: { form_guid: @guid }, body: opts)
end

# {https://developers.hubspot.com/docs/methods/forms/update_form}
Expand Down
45 changes: 45 additions & 0 deletions spec/lib/hubspot/form_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,51 @@
end
end

describe '#submit_response' do
cassette 'form_submit_data'

let(:form) { Hubspot::Form.find('561d9ce9-bb4c-45b4-8e32-21cdeaa3a7f0') }

context 'with a valid portal id' do
before do
Hubspot.configure(hapikey: 'demo', portal_id: '62515')
end

it 'returns the HTTParty::Response if the form submission is successful' do
params = {}
result = form.submit_response(params)
result.should be_an_instance_of HTTParty::Response
result.code.should eq 200
end
end

context 'with an invalid portal id' do
before do
Hubspot.configure(hapikey: 'demo', portal_id: 'xxxx')
end

it 'returns the HTTParty::Response in case of errors' do
params = { unknown_field: :bogus_value }
result = form.submit_response(params)
result.should be_an_instance_of HTTParty::Response
result.code.should eq 404
end
end

context 'when initializing Hubspot::Form directly' do
let(:form) { Hubspot::Form.new('guid' => '561d9ce9-bb4c-45b4-8e32-21cdeaa3a7f0') }

before { Hubspot.configure(hapikey: 'demo', portal_id: '62515') }

it 'returns the HTTParty::Response if the form submission is successful' do
params = {}
result = form.submit_response(params)
result.should be_an_instance_of HTTParty::Response
result.code.should eq 200
end
end
end

describe '#update!' do
cassette 'form_update'

Expand Down