New release by simonoff · Pull Request #27 · Airtable/airtable-ruby

simonoff · 2017-12-15T16:00:45Z

No description provided.

syrnick · 2017-12-15T17:44:05Z

airtable.gemspec

+  spec.name        = 'airtable'
+  spec.version     = Airtable::VERSION
+  spec.authors     = ['Nathan Esquenazi', 'Alexander Sorokin']
+  spec.email       = ['nesquena@gmail.com', 'syrnick@gmail.com']


Feel free to put your name here. Nathan wrote original version, so we'll put his name into Readme/Acknowledgements.

@syrnick thx!. Will do it.

syrnick · 2017-12-15T17:52:34Z

lib/airtable/entity/record.rb

+      end
+
+      def __create__(base, name)
+        res = base.__make_request__(:post, name, { fields: fields })


this name needs escaping equivalent to JS encodeURIComponent (e.g. 3cc21ea). I had hard time finding a better way to do it.

syrnick · 2017-12-27T01:02:41Z

lib/airtable/entity/table.rb

+      end
+
+      def create(fields)
+        ::Airtable::Entity::Record.new(nil, fields: fields).__create__(*@args)


it would be clearer to spell out *@args as [@base, @name]

Ok, i will try to change but possible rubocop will not like it.

I see. If it does, lets use a more descriptive name, so it can't be confused with the method arguments. Right now someone might look at this and think that *@args is the arguments to def create.

syrnick · 2017-12-27T01:03:10Z

lib/airtable/entity/record.rb

+        self
+      end
+
+      def __update__(base, name)


Lets rename name to table_name

syrnick · 2017-12-27T01:05:40Z

lib/airtable/entity/record.rb

+      end
+
+      def __update__(base, name)
+        args = [:patch, [name, @id].join('/'), fields: fields]


Important: name (i.e. tableName) must be properly encoded as it may have special characters (e.g. 3cc21ea). Try it with a table that's named Some &.#%?// table

syrnick · 2017-12-27T01:10:38Z

lib/airtable/entity/record.rb

+      end
+
+      class << self
+        def all(base, name, params)


It seems that all and __fetch__ should be class methods on Table, not Record.

No. Table class is already too big. And better to use such methods in Record to follow the Ruby/Ruby on Rails way.

sounds good

syrnick · 2017-12-27T01:14:30Z

lib/airtable/entity/record.rb

+            res << new(*args)
+          end
+          return unless result['offset']
+          __fetch__(base, name, params.merge(offset: result['offset']), res)


lets add a comment that ruby can easily handle 10k deep stack and that should be plenty to fetch any table.

You mean to add to README?

no, just here. recursion is a bit of a red flag unless the language always does tail call optimization (ruby has an option that's off by default). Hence, it helps to add a comment to the implementation justifying why it shouldn't cause trouble. There are esoteric scenarios where this will blow up. E.g. iterating over a large base (50k records) with page_size set to 1 (really bad idea of course).

syrnick · 2017-12-27T01:20:27Z

README.md

+Supported Operations:
+	Get Record (if only RECORD_ID provided)
+	Get Field (if RECORD_ID and FIELD_ID are provided)
+	Update Field (if RECORD_ID, FIELD_ID and VALUE are provided)


Lets make the operation explicit:

airtable get -b Base -t table
airtable get -b Base -t table -f FIELD_NAME

airtable update -b Base -t table -r RECORD_ID -f FIELD_NAME -v newValue

Sure. Will start working on it shortly.

syrnick · 2017-12-27T01:23:31Z

airtable.gemspec

-  spec.require_paths = ["lib"]
+  spec.files                 = `git ls-files -z`.split("\x0")
+  spec.bindir                = 'exe'
+  spec.executables           = `git ls-files -- exe/*`.split("\n").map {|f| File.basename(f)}


is exe/ better than bin/ ?

Yes. More info there - http://bundler.io/blog/2015/03/20/moving-bins-to-exe.html

syrnick · 2017-12-27T01:24:39Z

Gemfile

 # Specify your gem's dependencies in airtable.gemspec
 gemspec
+
+gem 'pry'


Is pry needed here?

Its for development purposes. Really helpful. Gemfile is not pushed to the gem file so used only for development environment of gem file only.

syrnick · 2017-12-27T02:47:21Z

lib/airtable/entity/table.rb

+      end
+
+      def select(options = {})
+        params = {}


Lets add support for view (should be a string if present) and filterByFormula (should be a string if present) parameters.

Ah, yes it will be added. I want to finished with current codebase and functionality first.

kasrak · 2017-12-27T16:25:04Z

README.md

 ```ruby
 # Pass in api key to client
 @client = Airtable::Client.new("keyPCx5W")
+# Or if you have AIRTABLE_KEY varibale you can use


typo: varibale -> variable

is this referring to an environment variable? if so, let's make that clearer

kasrak · 2017-12-27T16:25:15Z

README.md


+### Accessing a Base
+
+Now we can access any base in our Airsheet account by referencing the [API docs](https://airtable.com/api):


Airsheet -> Airtable

kasrak · 2017-12-27T16:29:05Z

README.md

 ```

-We can specify a `sort` order, `limit`, and `offset` as part of our query:
+We can specify a `sort` order, `limit`, `max_records` and `offset` as part of our query:


Maybe take out max_records from here since it can be confusing to see next to limit and most people don't need to use it. We can add a note about it below where we say "This executes a variable number of network requests (100 records per batch) to retrieve all records in a sheet."

instead could say:

"This executes a variable number of network requests (by default 100 records per batch) to retrieve all records in a sheet. You can use max_records to customize the number of records to fetch per batch."

Then I think better to use per_page instead of max_records.

Oh wait that's a good point. I just realized that in the REST API (and Javascript library) these are called maxRecords (instead of limit) and pageSize (instead of max_records)

This seems like a good opportunity to make the ruby library consistent with those

kasrak · 2017-12-27T16:29:42Z

README.md


+## Command Line Tool
+
+This gem is include a very simple command line tool which can show basic functionality of service.


"This gem includes a simple command line tool which shows the basic functionality of the service."

syrnick · 2018-01-10T15:25:30Z

lib/airtable/entity/base.rb


      def __make_request__(method, path, data)
-        url  = [::Airtable.server_url, @id, path].join('/')
+        url  = [::Airtable.server_url, CGI.escape(@id), path].join('/')


@id shouldn't need escaping for now. It can only be 17-character alphanumeric starting with "app".

syrnick · 2018-01-10T15:33:30Z

lib/airtable/entity/table.rb


      def initialize(base, name)
-        @name = name
+        @name = CGI.escape(name)


I don't think CGI.escape works - we need to use an equivalent of encodeUriComponent in JS .

3cc21ea

For example, they differ in handling of spaces - GCI.escape turns them into '+' v.s. %2B.

Separately, storing @name as encoded might be confusing later on. If someone does p table.name, they would n't expect the encoding. It'd be better to either:
a) have an accessor url_encoded_name that does encoding on the fly.
b) wrap name access in url_encode e.g. @table_path = [@base, url_encode(@name)]
c) add in instance variable @url_encoded_name = url_encode(@name)

simonoff added 7 commits December 12, 2017 13:42

Refactoring started. Docker support added. Rspec support added.

f741381

Rubocop changes

f889f87

Sorting by different variants

dcc801a

Make calls more like in Airtable.js

11f0d0b

Create, Update actions

f33cf3f

Replace and Destroy methods

4929bb3

Binary started

ed3bc16

syrnick reviewed Dec 15, 2017

View reviewed changes

simonoff added 2 commits December 19, 2017 17:38

CLI finished. Code refactoring started

984574c

Status error catching

612eee8

syrnick suggested changes Dec 27, 2017

View reviewed changes

syrnick reviewed Dec 27, 2017

View reviewed changes

kasrak reviewed Dec 27, 2017

View reviewed changes

syrnick mentioned this pull request Jan 2, 2018

Allow httparty versions greater than 0.14.0 #28

Closed

Fixes and new functionality updates

4c7cb62

syrnick reviewed Jan 10, 2018

View reviewed changes

Latest fixes

8102ec6


		### Accessing a Base

		Now we can access any base in our Airsheet account by referencing the [API docs](https://airtable.com/api):


		## Command Line Tool

		This gem is include a very simple command line tool which can show basic functionality of service.

Conversation

simonoff commented Dec 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

syrnick Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

syrnick Jan 10, 2018 •

edited

Loading