From bfbba2bf6872999ea8c9980fd62607bec66c6405 Mon Sep 17 00:00:00 2001 From: Alexander Sorokin Date: Wed, 8 Jun 2016 06:03:14 -0700 Subject: [PATCH 1/2] Remove HashWithIndifferentAccess altogether. Using symbols is pretty fragile except for simple cases where the symbol exactly maps to field name. --- lib/airtable/record.rb | 54 +++++++++++++++++++++++++++++++++--------- test/record_test.rb | 2 +- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/lib/airtable/record.rb b/lib/airtable/record.rb index 03ba2c6..d70fb9e 100644 --- a/lib/airtable/record.rb +++ b/lib/airtable/record.rb @@ -10,14 +10,33 @@ def id=(val); @attrs["id"] = val; end # Return given attribute based on name or blank otherwise def [](name) - @attrs.has_key?(to_key(name)) ? @attrs[to_key(name)] : "" + if (name.is_a? Symbol) + name = @key_to_name[name] + end + @attrs.has_key?(name) ? @attrs[name] : "" end # Set the given attribute to value def []=(name, value) - @column_keys << name - @attrs[to_key(name)] = value - define_accessor(name) unless respond_to?(name) + if (name.is_a? Symbol) + if @key_to_name.has_key?(name) + name = @key_to_name[name] + else + # This is pretty fragile. It only works when the field name + # is exactly the same as the symbol (to_key(name) == name). + # In other cases, this will fail with 422 while saving to Airtable. + @key_to_name[name] = name.to_s + name = name.to_s + @column_names << name + end + else + if !@attrs.has_key?(name) + @column_names << name + @key_to_name[to_key(name)] = name + end + end + @attrs[name] = value + define_accessor_if_needed(name) end def inspect @@ -29,18 +48,28 @@ def attributes; @attrs; end # Removes old and add new attributes for the record def override_attributes!(attrs={}) - @column_keys = attrs.keys - @attrs = HashWithIndifferentAccess.new(Hash[attrs.map { |k, v| [ to_key(k), v ] }]) - @attrs.map { |k, v| define_accessor(k) } + @column_names ||= []; + @key_to_name ||= Hash[] + attrs.keys.each do |k| + column_key = k.is_a?(Symbol) ? k : to_key(k) + # k.to_s fallback is fragile + column_name = k.is_a?(Symbol) ? @key_to_name[column_key] || k.to_s : k + if !@key_to_name.has_key?(column_key) + @column_names << column_name; + @key_to_name[column_key] = column_name; + end + end + @attrs = Hash[attrs.map {|k, v| [ k.is_a?(Symbol) ? @key_to_name[k] : k, v]} ] + @attrs.map { |k, v| define_accessor_if_needed(k) } end # Hash with keys based on airtable original column names def fields - HashWithIndifferentAccess.new(Hash[@column_keys.map { |k| [ k, @attrs[to_key(k)] ] }]) + Hash[@attrs] end # Airtable will complain if we pass an 'id' as part of the request body. - def fields_for_update; fields.except(:id); end + def fields_for_update; fields.except('id'); end def method_missing(name, *args, &blk) # Accessor for attributes @@ -68,8 +97,11 @@ def underscore(string) gsub(/\s/, '_').tr("-", "_").downcase end - def define_accessor(name) - self.class.send(:define_method, name) { @attrs[name] } + def define_accessor_if_needed(name) + key = to_key(name) + if !respond_to?(key) + self.class.send(:define_method, key) { @attrs[name] } + end end end # Record diff --git a/test/record_test.rb b/test/record_test.rb index 1ee836a..87c9775 100644 --- a/test/record_test.rb +++ b/test/record_test.rb @@ -10,7 +10,7 @@ it "returns new columns in fields_for_update" do record = Airtable::Record.new(:name => "Sarah Jaine", :email => "sarah@jaine.com", :id => 12345) record[:website] = "http://sarahjaine.com" - record.fields_for_update.must_include(:website) + record.fields_for_update.must_include('website') end it "returns fields_for_update in original capitalization" do From 41c4e802882895debfd47cac8eb072a8693e22c0 Mon Sep 17 00:00:00 2001 From: Alexander Sorokin Date: Wed, 8 Jun 2016 07:36:12 -0700 Subject: [PATCH 2/2] Add support for sorting records by multiple fields --- .../httparty_patched_hash_conversions.rb | 49 +++++++++++++++++++ lib/airtable/resource.rb | 8 ++- lib/airtable/table.rb | 39 +++++++++++++-- test/airtable_test.rb | 7 +++ test/test_helper.rb | 2 +- 5 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 lib/airtable/httparty_patched_hash_conversions.rb diff --git a/lib/airtable/httparty_patched_hash_conversions.rb b/lib/airtable/httparty_patched_hash_conversions.rb new file mode 100644 index 0000000..9d779df --- /dev/null +++ b/lib/airtable/httparty_patched_hash_conversions.rb @@ -0,0 +1,49 @@ +module HTTParty + module PatchedHashConversions + # @return This hash as a query string + # + # @example + # { name: "Bob", + # address: { + # street: '111 Ruby Ave.', + # city: 'Ruby Central', + # phones: ['111-111-1111', '222-222-2222'] + # } + # }.to_params + # #=> "name=Bob&address[city]=Ruby Central&address[phones][0]=111-111-1111&address[phones][1]=222-222-2222&address[street]=111 Ruby Ave." + def self.to_params(hash) + hash.to_hash.map { |k, v| normalize_param(k, v) }.join.chop + end + + # @param key The key for the param. + # @param value The value for the param. + # + # @return This key value pair as a param + # + # @example normalize_param(:name, "Bob Jones") #=> "name=Bob%20Jones&" + def self.normalize_param(key, value) + param = '' + stack = [] + + if value.respond_to?(:to_ary) + param << value.to_ary.each_with_index.map { |element, index| normalize_param("#{key}[#{index}]", element) }.join + elsif value.respond_to?(:to_hash) + stack << [key, value.to_hash] + else + param << "#{key}=#{ERB::Util.url_encode(value.to_s)}&" + end + + stack.each do |parent, hash| + hash.each do |k, v| + if v.respond_to?(:to_hash) + stack << ["#{parent}[#{k}]", v.to_hash] + else + param << normalize_param("#{parent}[#{k}]", v) + end + end + end + + param + end + end +end diff --git a/lib/airtable/resource.rb b/lib/airtable/resource.rb index 26ae69a..20e0e6f 100644 --- a/lib/airtable/resource.rb +++ b/lib/airtable/resource.rb @@ -1,12 +1,18 @@ +require 'airtable/httparty_patched_hash_conversions' + module Airtable # Base class for authorized resources sending network requests class Resource include HTTParty - base_uri 'https://api.airtable.com/v0/' + base_uri (ENV['AIRTABLE_ENDPOINT_URL'] || 'https://api.airtable.com/') + 'v0/' # debug_output $stdout attr_reader :api_key, :app_token, :worksheet_name + query_string_normalizer(proc do |query| + HTTParty::PatchedHashConversions.to_params(query) + end) + def initialize(api_key, app_token, worksheet_name) @api_key = api_key @app_token = app_token diff --git a/lib/airtable/table.rb b/lib/airtable/table.rb index 18173ee..bc42684 100644 --- a/lib/airtable/table.rb +++ b/lib/airtable/table.rb @@ -20,10 +20,21 @@ def all(options={}) # Fetch records from the sheet given the list options # Options: limit = 100, offset = "as345g", sort = ["Name", "asc"] + # sort could be an array # records(:sort => ["Name", :desc], :limit => 50, :offset => "as345g") def records(options={}) - options["sortField"], options["sortDirection"] = options.delete(:sort) if options[:sort] - results = self.class.get(worksheet_url, query: options).parsed_response + update_sort_options!(options) + raw_response = self.class.get(worksheet_url, query: options) + case raw_response.code + when 200 + # ok + when 422 + raise 'Involid request' + when 500...600 + puts "Server error #{response.code}" + raise 'Server error' + end + results = raw_response.parsed_response RecordSet.new(results) end @@ -33,7 +44,7 @@ def records(options={}) # # select(limit: 10, sort: ["Name", "asc"], formula: "Order < 2") def select(options={}) - options['sortField'], options['sortDirection'] = options.delete(:sort) if options[:sort] + update_sort_options!(options) options['maxRecords'] = options.delete(:limit) if options[:limit] if options[:formula] @@ -45,6 +56,28 @@ def select(options={}) RecordSet.new(results) end + def update_sort_options!(options) + sortOption = options.delete(:sort) || options.delete('sort') + if sortOption && sortOption.is_a?(Array) + if sortOption.length > 0 + if sortOption[0].is_a? String + singleSortField, singleSortDirection = sortOption + options["sort"] = [{field: singleSortField, direction: singleSortDirection}] + elsif sortOption.is_a?(Array) && sortOption[0].is_a?(Array) + options["sort"] = sortOption.map {|(sortField, sortDirection)| {field: sortField, direction: sortDirection.downcase} } + else + raise ArgumentError.new("Unknown sort options format.") + end + end + elsif sortOption + options["sort"] = sortOption + end + + if options["sort"] + raise ArgumentError.new("Unknown sort direction") unless options["sort"].all? {|sortObj| ['asc', 'desc'].include? sortObj[:direction]} + end + end + def raise_bad_formula_error raise ArgumentError.new("The value for filter should be a String.") end diff --git a/test/airtable_test.rb b/test/airtable_test.rb index 90dec42..28c6e49 100644 --- a/test/airtable_test.rb +++ b/test/airtable_test.rb @@ -32,6 +32,13 @@ assert_equal @select_records.records, [] end + it "should sort by multiple pairs" do + stub_airtable_response!("https://api.airtable.com/v0/#{@app_key}/#{@sheet_name}?sort[0][field]=foo&sort[0][direction]=asc&sort[1][field]=bar&sort[1][direction]=desc", { "records" => [], "offset" => "abcde" }) + @table = Airtable::Client.new(@client_key).table(@app_key, @sheet_name) + @select_records = @table.select(sort: [['foo', 'asc'], ['bar', 'desc']]) + assert_equal @select_records.records, [] + end + it "should raise an ArgumentError if a formula is not a string" do stub_airtable_response!("https://api.airtable.com/v0/#{@app_key}/#{@sheet_name}", { "records" => [], "offset" => "abcde" }) @table = Airtable::Client.new(@client_key).table(@app_key, @sheet_name) diff --git a/test/test_helper.rb b/test/test_helper.rb index fbfea0a..e9fd3a1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -9,4 +9,4 @@ def stub_airtable_response!(url, response, method=:get) FakeWeb.register_uri(method, url, :body => response.to_json, :content_type => "application/json") -end \ No newline at end of file +end