feat(datasource-pylon): custom fields introspection (EXT-10)

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb

@@ -98,6 +98,21 @@ def fetch_team(id)
       fetch_resource('teams', id)
     end
 
+    # The custom-field definitions of one object type. `object_type` is
+    # mandatory on this endpoint, so a schema spanning several collections costs
+    # one call per collection rather than one call in total.
+    #
+    # Degrades to an empty list: this is read while the agent boots, and a token
+    # missing the permission — or a Pylon that happens to be down right then —
+    # has to cost the operator the custom columns, not the whole datasource.
+    def fetch_custom_fields(object_type)
+      params = { 'object_type' => object_type }
+
+      best_effort("fetch_custom_fields(#{object_type})", default: []) do
+        must_succeed('custom-fields') { collect_pages('custom-fields', params) }
+      end
+    end
+
     private
 
     def search_resource(path, limit:, cursor: nil, filter: nil, search_text: nil)
@@ -126,15 +141,19 @@ def fetch_all(path, params = {})
     # sent. `CursorWalker` answers the other question — the offset/limit window a
     # list view asks for — and is not what this needs.
     #
+    # `params` ride along on every page, cursor included: a mandatory parameter
+    # dropped on the second request answers a different question than the first.
+    #
     # An empty page and a cursor that does not move both stop the loop: neither
     # happens today, but a walk driven by a remote value stops on its own terms.
-    def collect_pages(path)
+    def collect_pages(path, params = {})
       records = []
       cursor = nil
       pages = 0
 
       loop do
-        page = to_search_page(connection.get(path, cursor.nil? ? {} : { 'cursor' => cursor }).body)
+        query = cursor.nil? ? params : params.merge('cursor' => cursor)
+        page = to_search_page(connection.get(path, query).body)
         records.concat(page.records)
         pages += 1
         break if page.next_cursor.nil? || page.next_cursor == cursor || page.records.empty?

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/collections/record_serialization.rb

@@ -13,7 +13,7 @@ def nested_id(value)
       def add_custom_field_values(record, values)
         custom_fields.each do |cf|
           entry = values.is_a?(Hash) ? values[cf[:column_name]] : nil
-          record[cf[:column_name]] = custom_field_value(entry)
+          record[cf[:column_name]] = coerce_custom_field(custom_field_value(entry), cf[:schema].column_type)
         end
       end
 
@@ -24,6 +24,52 @@ def custom_field_value(entry)
 
         entry.key?('value') ? entry['value'] : entry['values']
       end
+
+      # The API reference documents what a custom field *is*, never the form its
+      # value is read back in, so a Number answering `"42"` is not ruled out. A
+      # value that is not already of its column's type is therefore converted --
+      # the in-memory pass of the primary-key short-circuit would otherwise drop
+      # a row on `"42" == 42.0`, comparing a read string with the float
+      # `ConditionTreeParser.cast_to_type` casts the filter to.
+      #
+      # A date stays the string it is: the filter carries an ISO8601 string too,
+      # and comparing two of those is the ordering itself.
+      def coerce_custom_field(value, column_type)
+        return nil if value.nil?
+
+        case column_type
+        when 'Number'  then coerce_number(value)
+        when 'Boolean' then coerce_boolean(value)
+        else                value
+        end
+      end
+
+      # A value already numeric is handed back untouched: `ConditionTreeLeaf#match`
+      # compares with `==` and `Array#include?`, both of which hold across Integer
+      # and Float, so nothing needs widening -- and widening would make an integer
+      # field display the `12.0` it does not hold, where `FilterValue#format_float`
+      # narrows the very same value on the way out. A string is read to the
+      # tightest form for that reason, the two halves agreeing on what an integer
+      # looks like.
+      #
+      # A number that cannot be read reads as absent rather than as zero.
+      def coerce_number(value)
+        return value if value.is_a?(Numeric)
+
+        float = Float(value, exception: false)
+        return nil if float.nil?
+
+        float == float.to_i ? float.to_i : float
+      end
+
+      # An empty boolean reads as absent as well: `false` is an answer of its own,
+      # and Pylon gave none.
+      def coerce_boolean(value)
+        return value if [true, false].include?(value)
+        return nil if value.to_s.strip.empty?
+
+        !%w[false 0 no].include?(value.to_s.strip.downcase)
+      end
     end
   end
 end

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/datasource.rb

@@ -15,10 +15,25 @@ def initialize(api_key:, **options)
     # The five collections are registered together: each one declares relations
     # pointing at the others, and a relation whose foreign collection is missing
     # is a schema the agent refuses to boot on.
+    #
+    # Their custom fields are introspected here, one call per object type, as
+    # Pylon indexes its definitions by object type and asks for one on every
+    # call. Each entry stays on the collection it belongs to: a custom field is
+    # filtered through the very slug it is read by, which the collection's
+    # `api_filters` already carries, so there is no datasource-wide mapping to
+    # hold — and two Pylon datasources in the same agent share nothing.
+    #
+    # An introspection that fails costs the custom columns, not the datasource:
+    # `fetch_custom_fields` degrades to an empty list and the agent boots on the
+    # native schema.
     def register_collections
-      add_collection(Collections::Issue.new(self))
-      add_collection(Collections::Account.new(self))
-      add_collection(Collections::Contact.new(self))
+      custom_fields = Schema::CustomFieldsIntrospector.new(@client)
+
+      add_collection(Collections::Issue.new(self, custom_fields: custom_fields.issue_custom_fields))
+      add_collection(Collections::Account.new(self, custom_fields: custom_fields.account_custom_fields))
+      add_collection(Collections::Contact.new(self, custom_fields: custom_fields.contact_custom_fields))
+      # Pylon carries custom fields on issues, accounts and contacts only:
+      # neither an agent nor a team has any.
       add_collection(Collections::User.new(self))
       add_collection(Collections::Team.new(self))
     end

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/condition_tree_translator.rb

@@ -24,6 +24,13 @@ class ConditionTreeTranslator
       LIST_OPERATORS      = %w[in not_in].freeze
       VALUELESS_OPERATORS = %w[is_set is_unset].freeze
 
+      # The comparisons Pylon reads as a moment in time: what tells FilterValue
+      # that a bare date is a date, and not a piece of text a field happens to
+      # hold. Read off the emitted operator rather than off the column type,
+      # which the translator does not see -- and it is the operator that decides
+      # the format anyway.
+      TIME_OPERATORS      = %w[time_is_after time_is_before].freeze
+
       def self.call(condition_tree, api_filters: {}, timezone: nil)
         return nil if condition_tree.nil?
 
@@ -89,7 +96,7 @@ def with_value(filter, operator, leaf)
         return filter if VALUELESS_OPERATORS.include?(operator)
         return filter.merge('values' => @value.list(leaf)) if LIST_OPERATORS.include?(operator)
 
-        filter.merge('value' => @value.single(leaf))
+        filter.merge('value' => @value.single(leaf, time: TIME_OPERATORS.include?(operator)))
       end
 
       # `present`, `blank` and `missing` are advertised on every field carrying

packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/query/filter_value.rb

@@ -1,3 +1,4 @@
+require 'date'
 require 'active_support/core_ext/time/zones'
 
 module ForestAdminDatasourcePylon
@@ -6,20 +7,30 @@ module Query
     # Split from the translator, which knows the shape of the condition tree but
     # not the format of what the leaves carry.
     class FilterValue
+      # A date carrying no time of day, which is what the frontend sends for a
+      # Dateonly column -- a shape only a custom field has, no native column
+      # being typed that way.
+      DATE_ONLY = /\A\d{4}-\d{2}-\d{2}\z/
+
       def initialize(timezone: nil)
         @timezone = timezone.to_s.strip.empty? ? 'UTC' : timezone
       end
 
-      def single(leaf)
+      # `time` says the operator this value travels with is one of Pylon's time
+      # comparisons, which is what decides whether a bare date is a date or a
+      # piece of text: the same string on a text field is a value of its own.
+      def single(leaf, time: false)
         raise_nil_value(leaf.field) if leaf.value.nil?
 
-        format(leaf.value)
+        format(leaf.value, time: time)
       end
 
       # Dropping the blanks would silently answer a different question: `not_in
       # [nil, 'open']` was asked to exclude the blank records and would come
       # back including them. An empty list is just as bad the other way round,
       # translating to a filter matching everything.
+      #
+      # No time comparison takes a list, so nothing here is read as a date.
       def list(leaf)
         values = Array(leaf.value)
         raise_empty_list(leaf) if values.empty?
@@ -30,21 +41,49 @@ def list(leaf)
 
       private
 
-      # Numbers and booleans travel as they are: the filter is JSON, not a query
-      # string, so only the date types need a wire format.
-      def format(value)
+      # Booleans travel as they are: the filter is JSON, not a query string, so
+      # only the dates and the numbers the agent widened need a wire format.
+      def format(value, time: false)
         case value
         when Time, DateTime then value.to_time.utc.iso8601
         when Date           then format_date(value)
+        when Float          then format_float(value)
+        when String         then time ? format_time_string(value) : value
         else                     value
         end
       end
 
+      # The agent casts every Number column with `to_f`
+      # (`ConditionTreeParser.cast_to_type`), so an integer custom field would be
+      # filtered with `42.0` -- a form none of its values carry. A float with
+      # nothing after the point travels as the integer it is; a decimal keeps its
+      # own.
+      def format_float(value)
+        value == value.to_i ? value.to_i : value
+      end
+
+      # `time_is_after` receives a timestamp everywhere else in this datasource,
+      # a native date column being read and filtered as one: a Dateonly custom
+      # field cannot be the one field sending the same operator another shape.
+      # The bound is the one a Ruby `Date` already gets -- midnight in the
+      # timezone of the caller.
+      #
+      # A string this operator cannot read as a date is left to Pylon, which
+      # names what it refuses better than a guess here would.
+      def format_time_string(value)
+        return value unless DATE_ONLY.match?(value)
+
+        format_date(Date.parse(value))
+      rescue Date::Error
+        value
+      end
+
       # Only reached by a condition tree built in Ruby -- a segment or a scope
-      # written as code. Everything coming through HTTP arrives as an ISO8601
-      # string, already expressed in the timezone of the caller: the agent casts
-      # a date filter with `value.to_s`, and the toolkit formats the bounds it
-      # derives from Today / Previous* itself.
+      # written as code -- and by the bare date above. Everything else coming
+      # through HTTP arrives as an ISO8601 timestamp, already expressed in the
+      # timezone of the caller: the agent casts a date filter with `value.to_s`,
+      # and the toolkit formats the bounds it derives from Today / Previous*
+      # itself.
       def format_date(value)
         Time.use_zone(@timezone) { Time.zone.local(value.year, value.month, value.day).utc.iso8601 }
       rescue ArgumentError