From 32cc9b7d6fb4a6d55b5747e328078a980e87c6aa Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 18:26:07 +0200 Subject: [PATCH 01/10] fix(intercom): page id lookups and report explicit sorts Two findings from the lot 1 review, both places where a cursor collection answers something other than what it was asked for. An `id in [...]` read returned every record it fetched whatever page was asked for, so page 1 and page 2 of a related-record list rendered the same rows. The window is now cut out of the ids before they are read, which also spares the requests the discarded records cost: Intercom reads them one request each. `default_pk_sort?` read a symbol-keyed `false` as an absent key, so an explicit `?sort=-id` was taken for the ascending default the agent injects, and the warning about Intercom silently ignoring a sort never fired. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../collections/cursor_collection.rb | 21 ++++++++++++---- .../collections/conversation_spec.rb | 24 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb index eb8fa4721..a73f5be14 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb @@ -106,7 +106,12 @@ def walker def fetch_records(filter) ids = id_lookup(filter) - return records_by_ids(ids) if ids + # The window is cut out of the ids rather than out of the records they + # read: Intercom reads them one request each, so paging after the read + # would pay for a whole page to hand back a slice of it -- and page 2 of + # a set larger than the cap would come back empty, the records it names + # having been dropped by the truncation before the window was applied. + return records_by_ids(page_window(ids, filter)) if ids refuse_filter!(filter) unless browsing?(filter) @@ -230,9 +235,17 @@ def warn_ignored_sort(sort) end def default_pk_sort?(clauses) - clauses.size == 1 && - (clauses.first[:field] || clauses.first['field']).to_s == primary_key && - (clauses.first[:ascending] || clauses.first['ascending']) != false + return false unless clauses.size == 1 + + clause = clauses.first + return false unless (clause[:field] || clause['field']).to_s == primary_key + + # `key?` rather than `||`: a descending clause carries `false`, which an + # `||` fallback reads as "absent" -- so an explicit `?sort=-id` would be + # taken for the ascending default the agent injects, and the one order + # Intercom silently drops would go unreported. + ascending = clause.key?(:ascending) ? clause[:ascending] : clause['ascending'] + ascending != false end def warn_truncated_ids(asked) diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb index eb4a24ad2..4d1d8ac02 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb @@ -229,6 +229,19 @@ def ids(rows) expect(ids(rows)).to eq(%w[1 2]) end + # A pointing collection pages through the records it named, and every page + # must name different ones: cut out of the records instead of out of the + # ids, the window would render the same rows on page 1 and page 2 -- and + # a page past the cap would come back empty, its ids having been dropped + # by the truncation before the window was applied. + it 'reads only the ids the page window names' do + stub_record('2', conversation('2')) + page = ForestAdminDatasourceToolkit::Components::Query::Page.new(offset: 1, limit: 1) + tree = leaf('id', operators::IN, %w[1 2 3]) + + expect(ids(collection.list(nil, filter(condition_tree: tree, page: page), %w[id]))).to eq(%w[2]) + end + it 'reads the first of too many and says the result is truncated' do allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) asked = (1..(Collections::CursorCollection::MAX_ID_READS + 3)).map(&:to_s) @@ -278,6 +291,17 @@ def ids(rows) expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/ignores a sort/) end + # `?sort=-id` is an order the operator asked for, not the ascending default + # the agent injects when a request names none. + it 'reports an explicit descending order on the primary key' do + stub_list(conversation('1')) + sort = ForestAdminDatasourceToolkit::Components::Query::Sort.new([{ field: 'id', ascending: false }]) + + collection.list(nil, filter(sort: sort), %w[id]) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/ignores a sort/) + end + it 'stays quiet on the primary-key order the agent injects by default' do stub_list(conversation('1')) sort = ForestAdminDatasourceToolkit::Components::Query::Sort.new([{ field: 'id', ascending: true }]) From ab35af2eecbc4df598cdabee598afb429bacd6be Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 18:34:15 +0200 Subject: [PATCH 02/10] feat(intercom): table of what the search endpoints filter The list of fields `/tickets/search` really filters is not the list the specification gives: measured during lot 1, it refuses `company_id` with `invalid_field` although a ticket carries one. So the first deliverable of the filtering lot is not code, it is the table of (endpoint x field x operator) the schema will derive its operators from. `query/search_fields.yml` holds it, one row per column, each row carrying its provenance: `measured` for what was observed against a workspace, `spec` for what was only read off the documentation. The date operators are measured and differ per endpoint. Everything else is a candidate until the probe runs. `Query::SearchFields` reads and validates it. An unknown operator, a type nothing knows how to send, a column filed as both filterable and refused: the file ships with the gem and is written against a script's output, so it fails at boot rather than producing a schema nobody can explain. `bin/probe_search_fields` is what measures it, against the customer's workspace: one search per cell, reading Intercom's refusal codes, and skipping the rest of a row whose field comes back `invalid_field`. It writes evidence rather than rewriting the table, which carries the prose saying why a column stays refused. The refused tables also record the R7 arbitration: a ticket attribute is filtered through an id that differs per ticket type, so the union column cannot say which id to use and the attributes stay display-only until the customer says otherwise. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../bin/probe_search_fields | 254 ++++++++++++ .../lib/forest_admin_datasource_intercom.rb | 1 + .../query/search_fields.rb | 152 ++++++++ .../query/search_fields.yml | 361 ++++++++++++++++++ .../query/search_fields_spec.rb | 132 +++++++ .../spec/probe_search_fields_spec.rb | 172 +++++++++ 6 files changed, 1072 insertions(+) create mode 100755 packages/forest_admin_datasource_intercom/bin/probe_search_fields create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb diff --git a/packages/forest_admin_datasource_intercom/bin/probe_search_fields b/packages/forest_admin_datasource_intercom/bin/probe_search_fields new file mode 100755 index 000000000..664b8e9a4 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/bin/probe_search_fields @@ -0,0 +1,254 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Enumerates what Intercom's search endpoints really filter, and with which +# operators, against a real workspace. The answer is what `search_fields.yml` +# holds and what the schema derives its filters from -- and it cannot be read +# off the documentation: measured during lot 1, `/tickets/search` refuses +# `company_id` with `invalid_field` although a ticket carries one. +# +# INTERCOM_ACCESS_TOKEN=... bin/probe_search_fields --endpoint tickets +# INTERCOM_ACCESS_TOKEN=... bin/probe_search_fields --region eu --out measured.yml +# +# It sends one search per (field, operator) cell, asking for a single record, +# and reads Intercom's refusal codes: `invalid_field` for a field the endpoint +# does not filter at all -- the rest of its row is then skipped -- `data_invalid` +# for an operator it refuses on that field, `invalid_value` for a value shape it +# refuses. Read-only: a search changes nothing, and no payload is printed or +# written, only field names, operators and error codes. +# +# What it writes with `--out` is *evidence*, deliberately not the table itself: +# `search_fields.yml` carries the prose that says why a column stays refused, +# and a generated file would drop it. Merge the measurements into the table by +# hand, which is also where deciding to expose a newly discovered field belongs. + +$LOAD_PATH.unshift(File.expand_path('../lib', __dir__)) + +require 'forest_admin_datasource_intercom' +require 'optparse' + +module ProbeSearchFields + SearchFields = ForestAdminDatasourceIntercom::Query::SearchFields + + # A cell is probed with the value shapes its field plausibly takes, most + # likely first: a wrong shape is refused with `data_invalid` just like an + # unsupported operator, so a single attempt would report an operator as + # refused when only the value was wrong. + VALUES = { + 'date' => ['1', 1, '2026-01-01'], + 'number' => [0, '0'], + 'boolean' => [true, 'true'], + 'string' => ['forest-probe', 0], + 'text' => ['forest-probe'], + 'id_list' => ['forest-probe', 0] + }.freeze + + LIST_OPERATORS = %w[IN NIN].freeze + + # A candidate carries no declared type -- discovering it is the point -- so + # one is guessed from its name, and the fallbacks above cover a wrong guess. + def self.guess_type(field) + case field + when /(_at|_since|_until)\z/ then 'date' + when /\Acount_|_count\z|\Atime_to_|_time\z/ then 'number' + when /\A(open|read|is_|has_|.*_participated)/ then 'boolean' + when /_ids\z/ then 'id_list' + else 'string' + end + end + + class Probe + def initialize(client, endpoint) + @client = client + @endpoint = endpoint + end + + # Every operator of the alphabet on every field of the row: what the table + # already declares, plus the candidates. Nothing is assumed from the + # declared operators -- an operator missing from the table is exactly what + # this is here to find. + def run(field, type) + results = {} + + SearchFields::KNOWN_OPERATORS.each do |operator| + outcome = probe(field, operator, type) + return { unfilterable: outcome } if outcome[:code] == 'invalid_field' + + results[operator] = outcome + end + + { operators: results } + end + + private + + def probe(field, operator, type) + last = nil + + VALUES.fetch(type, VALUES['string']).each do |value| + outcome = attempt(field, operator, operator_value(operator, value)) + # `invalid_field` is about the field, not about what was sent with it: + # another value shape cannot make a field the endpoint does not filter + # appear, so the row ends here rather than paying a request per shape. + return outcome if outcome[:ok] || outcome[:code] == 'invalid_field' + + last = outcome + end + + last + end + + def attempt(field, operator, value) + @client.search_page(@endpoint.path, query: { 'field' => field, 'operator' => operator, 'value' => value }, + per_page: 1, list_key: list_key) + { ok: true } + rescue ForestAdminDatasourceIntercom::APIError => e + { ok: false, code: error_code(e), status: e.status } + end + + def operator_value(operator, value) + LIST_OPERATORS.include?(operator) ? [value] : value + end + + # `/tickets/search` answers under `tickets` and `/conversations/search` + # under `conversations`; a body read under the wrong key raises before the + # outcome is classified. + def list_key = @endpoint.name + + def error_code(error) + errors = error.body.is_a?(Hash) ? error.body['errors'] : nil + first = Array(errors).first + + (first.is_a?(Hash) ? first['code'] : nil) || "http_#{error.status}" + end + end + + class Report + def initialize(endpoint) + @endpoint = endpoint + @rows = {} + end + + def record(field, column, result) + @rows[field] = { 'column' => column, 'result' => result } + end + + # What the run says about the committed table, which is the only reason to + # run it: an operator the table promises and Intercom refuses is a filter + # the interface offers and the read cannot honour. + def print_diff + @rows.each do |field, row| + column = row['column'] + result = row['result'] + + if result[:unfilterable] + declared = column ? " <- table declares it on column '#{column}'" : '' + puts " #{field.ljust(42)} NOT FILTERABLE (#{result[:unfilterable][:code]})#{declared}" + next + end + + print_operators(field, column, result[:operators]) + end + end + + def to_yaml_document + { 'endpoint' => @endpoint.name, 'path' => @endpoint.path, 'measured_at' => Time.now.utc.strftime('%Y-%m-%d'), + 'fields' => @rows.to_h { |field, row| [field, measured_row(row)] } }.to_yaml + end + + private + + def print_operators(field, column, outcomes) + supported = outcomes.select { |_, outcome| outcome[:ok] }.keys + declared = column ? Array(@endpoint.field(column)&.operators) : [] + puts " #{field.ljust(42)} #{supported.empty? ? "(no operator answered)" : supported.join(" ")}" + + report_drift(declared - supported, supported - declared) + end + + def report_drift(promised, discovered) + puts " ! the table promises #{promised.join(", ")} here, Intercom refuses it" unless promised.empty? + puts " + Intercom also accepts #{discovered.join(", ")}" unless discovered.empty? + end + + def measured_row(row) + result = row['result'] + return { 'filterable' => false, 'code' => result[:unfilterable][:code] } if result[:unfilterable] + + { 'filterable' => true, + 'operators' => result[:operators].select { |_, outcome| outcome[:ok] }.keys, + 'refused' => result[:operators].reject { |_, outcome| outcome[:ok] } + .transform_values { |outcome| outcome[:code] } } + end + end + + class CLI + def self.call(argv) + new(options(argv)).run + end + + def self.options(argv) + options = { endpoints: SearchFields.endpoints, region: 'us', token: ENV.fetch('INTERCOM_ACCESS_TOKEN', nil) } + + OptionParser.new do |parser| + parser.banner = 'Usage: INTERCOM_ACCESS_TOKEN=... bin/probe_search_fields [options]' + parser.on('--endpoint NAME', "one of #{SearchFields.endpoints.join(", ")}") { |v| options[:endpoints] = [v] } + parser.on('--region NAME', 'us (default), eu or au') { |v| options[:region] = v } + parser.on('--token TOKEN', 'defaults to $INTERCOM_ACCESS_TOKEN') { |v| options[:token] = v } + parser.on('--out PATH', 'write the measurements as YAML evidence') { |v| options[:out] = v } + end.parse!(argv) + + options + end + + def initialize(options) + @options = options + end + + def run + abort('No token: set INTERCOM_ACCESS_TOKEN or pass --token.') if @options[:token].to_s.empty? + + documents = @options[:endpoints].map { |name| probe_endpoint(SearchFields.fetch(name)) } + write(documents) if @options[:out] + 0 + end + + private + + def probe_endpoint(endpoint) + puts "\n#{endpoint.path} -- #{endpoint.fields.size} declared field(s), #{endpoint.candidates.size} candidate(s)" + probe = Probe.new(client, endpoint) + report = Report.new(endpoint) + + targets(endpoint).each { |field, column, type| report.record(field, column, probe.run(field, type)) } + report.print_diff + report.to_yaml_document + end + + # The declared fields first, with the type the table gives them; then the + # candidates, whose type is guessed. A candidate already covered by a + # declared field is not probed twice. + def targets(endpoint) + declared = endpoint.fields.values.map { |field| [field.field, field.column, field.type] } + candidates = (endpoint.candidates - declared.map(&:first)).map { |field| [field, nil, guess_type(field)] } + + declared + candidates + end + + def guess_type(field) = ProbeSearchFields.guess_type(field) + + def client + @client ||= ForestAdminDatasourceIntercom::Client.new( + ForestAdminDatasourceIntercom::Configuration.new(access_token: @options[:token], region: @options[:region]) + ) + end + + def write(documents) + File.write(@options[:out], documents.join("\n")) + puts "\nWritten to #{@options[:out]}. Merge the measurements into search_fields.yml by hand: " \ + 'the table carries the prose a generated file would drop.' + end + end +end + +exit(ProbeSearchFields::CLI.call(ARGV)) if $PROGRAM_NAME == __FILE__ diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom.rb index 43d8fe9c9..b90198801 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom.rb @@ -5,6 +5,7 @@ require 'set' require 'time' require 'uri' +require 'yaml' require 'zeitwerk' require 'faraday' require 'faraday/retry' diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb new file mode 100644 index 000000000..b5735896e --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb @@ -0,0 +1,152 @@ +module ForestAdminDatasourceIntercom + module Query + # Reads `search_fields.yml`, the table of what Intercom's search endpoints + # filter, and hands it to the schema and to the translator as objects rather + # than as nested hashes. + # + # The table is data rather than code for one reason: `bin/probe_search_fields` + # rewrites it from a real workspace. Anything derived from it -- which + # columns are filterable, with which Forest operators, and what an operator + # is told about the ones that are not -- therefore follows a measurement + # instead of a hand-written list that drifts from the endpoint. + # + # Every row is validated on load. The file ships with the gem and is written + # by a script, so a typo in an operator, or a column filed as both filterable + # and refused, is a defect of this package: it fails at boot rather than + # producing a schema nobody can explain. + module SearchFields + PATH = File.expand_path('search_fields.yml', __dir__) + + # The operators Intercom's search DSL spells, and nothing else: `=`, `!=`, + # `>`, `<`, `>=`, `<=`, the substring pair `~` / `!~`, the anchors `^` / + # `$`, and the membership pair. Which of them an endpoint honours on a + # given field is the table's business; this is only the alphabet. + KNOWN_OPERATORS = ['=', '!=', '>', '<', '>=', '<=', '~', '!~', '^', '$', 'IN', 'NIN'].freeze + KNOWN_TYPES = %w[string text date boolean number id_list].freeze + KNOWN_SOURCES = %w[measured spec].freeze + + # `source` says where a row comes from, and `measured?` is what the boot + # report and the README section read: a row taken from the documentation is + # a candidate the probe has not confirmed. + Field = Struct.new(:column, :field, :type, :operators, :source, keyword_init: true) do + def measured? = source == 'measured' + end + + # A column that stays unfilterable, and why. The reason travels into the + # refusal the operator reads, so it names what to filter on instead + # wherever there is something to name. + Refusal = Struct.new(:column, :reason, :source, keyword_init: true) do + def measured? = source == 'measured' + end + + Endpoint = Struct.new(:name, :path, :measured_at, :fields, :refused, :candidates, :ticket_attributes, + keyword_init: true) do + # Whether the probe has run against a real workspace for this endpoint. + # False means every `spec` row is still a candidate. + def measured? = !measured_at.nil? + + def field(column) = fields[column] + def refusal(column) = refused[column] + def filterable_columns = fields.keys + def unmeasured_fields = fields.values.reject(&:measured?) + end + + class << self + def fetch(name) + table[name.to_s] || + raise(ConfigurationError, "Unknown Intercom search endpoint #{name.inspect}; " \ + "the table declares #{table.keys.join(", ")}.") + end + + def endpoints = table.keys + + def table + @table ||= build(YAML.safe_load_file(PATH)) + end + + # Public so a spec can feed it a table of its own: this validation is the + # reason the file can be rewritten by a script without the package + # trusting whatever comes back. + def build(raw) + raw.fetch('endpoints').to_h { |name, definition| [name, endpoint(name, definition)] }.freeze + end + + private + + def endpoint(name, definition) + Endpoint.new( + name: name, + path: definition.fetch('path'), + measured_at: definition['measured_at'], + fields: fields(name, definition['fields']), + refused: refusals(name, definition['refused']), + candidates: Array(definition['candidates']).freeze, + ticket_attributes: definition['ticket_attributes'] + ).freeze + end + + def fields(endpoint, declared) + (declared || {}).to_h do |column, row| + field = Field.new(column: column, field: row.fetch('field'), type: row.fetch('type'), + operators: Array(row['operators']).freeze, source: row.fetch('source')).freeze + validate_field!(endpoint, field) + + [column, field] + end.freeze + end + + def refusals(endpoint, declared) + (declared || {}).to_h do |column, row| + refusal = Refusal.new(column: column, reason: squish(row.fetch('reason')), + source: row.fetch('source')).freeze + validate_source!(endpoint, column, refusal) + + [column, refusal] + end.freeze + end + + def validate_field!(endpoint, field) + validate_source!(endpoint, field.column, field) + validate_type!(endpoint, field) + validate_operators!(endpoint, field) + end + + def validate_type!(endpoint, field) + return if KNOWN_TYPES.include?(field.type) + + malformed!(endpoint, field.column, "type #{field.type.inspect} is not one of #{KNOWN_TYPES.join(", ")}") + end + + # An empty operator list is how a table stops short of saying anything: + # it would publish a filterable column no operator can reach. A column + # Intercom does not filter belongs in the refused table, where it comes + # with the reason an operator reads. + def validate_operators!(endpoint, field) + if field.operators.empty? + malformed!(endpoint, field.column, + 'it declares no operator; a column Intercom cannot filter belongs in the refused table') + end + + unknown = field.operators - KNOWN_OPERATORS + return if unknown.empty? + + malformed!(endpoint, field.column, "Intercom's search DSL has no operator #{unknown.join(", ")}") + end + + def validate_source!(endpoint, column, row) + return if KNOWN_SOURCES.include?(row.source) + + malformed!(endpoint, column, "source #{row.source.inspect} is neither #{KNOWN_SOURCES.join(" nor ")}") + end + + def malformed!(endpoint, column, detail) + raise ConfigurationError, "#{File.basename(PATH)} is malformed at #{endpoint}.#{column}: #{detail}." + end + + # A YAML folded block keeps the newlines the file needs to stay readable; + # the reason travels into a one-line message. + def squish(text) = text.to_s.split.join(' ') + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml new file mode 100644 index 000000000..e9549e236 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml @@ -0,0 +1,361 @@ +# The table of (endpoint x field x operator): what Intercom's search endpoints +# really filter, and with which operators. It is the single source of truth of +# this datasource's filtering -- the schema derives every column's +# filter_operators from it, so no collection can advertise a filter the +# translator would then refuse. +# +# `source` is the provenance of a row, and it is not decoration: +# +# measured -- observed against a real workspace, by `bin/probe_search_fields` +# or during a spike, and recorded here with the date; +# spec -- read off Intercom's documentation and nothing else. +# +# The distinction exists because the two disagree. Measured during lot 1: +# `company_id` is documented on a ticket and refused by `/tickets/search` with +# `invalid_field`. A `spec` row is therefore a candidate, never a promise, and +# the probe is what turns it into one. Run it against the customer's workspace +# and commit what it writes. +# +# `type` decides how a column's Forest operators are derived from the Intercom +# ones, and how a value reaches the wire: +# +# string -- an exact-match field +# text -- a field Intercom also matches per word (`~`) +# date -- epoch seconds on the wire, truncated to the UTC day on read +# boolean, number +# id_list -- a field holding several ids, matched against one of them +version: 1 + +endpoints: + conversations: + path: conversations/search + # Nothing has been probed yet: fill this in with the probe's output. + measured_at: null + fields: + state: + field: state + type: string + operators: ['=', '!='] + source: spec + open: + field: open + type: boolean + operators: ['='] + source: spec + read: + field: read + type: boolean + operators: ['='] + source: spec + priority: + field: priority + type: string + operators: ['=', '!='] + source: spec + title: + field: title + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + admin_assignee_id: + field: admin_assignee_id + type: string + operators: ['=', '!='] + source: spec + team_assignee_id: + field: team_assignee_id + type: string + operators: ['=', '!='] + source: spec + contact_ids: + field: contact_ids + type: id_list + operators: ['=', '!='] + source: spec + source_type: + field: source.type + type: string + operators: ['=', '!='] + source: spec + source_delivered_as: + field: source.delivered_as + type: string + operators: ['=', '!='] + source: spec + source_subject: + field: source.subject + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + # The full-text search of the collection: Intercom matches `~` per word, + # not as a substring, which the README states rather than the interface + # implying otherwise. + source_body: + field: source.body + type: text + operators: ['~', '!~'] + source: spec + source_author_email: + field: source.author.email + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + # The date operators are measured, and they are not the same on every + # endpoint: `/contacts/search` refuses `>=`, `<=` and `!=` where this one + # accepts them (25 August 2026, API 2.16). Whatever this table allows, a + # Date column publishes the two bounds alone -- see the translator. + created_at: + field: created_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: measured + updated_at: + field: updated_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: measured + waiting_since: + field: waiting_since + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + snoozed_until: + field: snoozed_until + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + closed_at: + field: statistics.last_close_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + first_closed_at: + field: statistics.first_close_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + closed_by_id: + field: statistics.last_closed_by_id + type: string + operators: ['=', '!='] + source: spec + first_contact_reply_at: + field: statistics.first_contact_reply_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + last_contact_reply_at: + field: statistics.last_contact_reply_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + last_admin_reply_at: + field: statistics.last_admin_reply_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: spec + reopen_count: + field: statistics.count_reopens + type: number + operators: ['=', '!=', '>', '<'] + source: spec + part_count: + field: statistics.count_conversation_parts + type: number + operators: ['=', '!=', '>', '<'] + source: spec + ai_agent_participated: + field: ai_agent_participated + type: boolean + operators: ['='] + source: spec + # Columns this collection publishes and will not filter, with the reason an + # operator reads when they try. A column absent from both tables is refused + # by name too -- these are the ones whose refusal is permanent and has an + # explanation worth giving. + refused: + tag_names: + reason: >- + Intercom filters conversations by tag id, and this column holds the + tag names. Filtering it would mean resolving a name to an id per + request, and a name the workspace renamed would silently match + nothing. + source: spec + company_name: + reason: >- + The conversation carries its company as an object read from the + payload; the search endpoint filters no company field. + source: spec + company_id: + reason: >- + The search endpoint filters no company field. Measured on + `/tickets/search`, which refuses `company_id` with `invalid_field` + although a ticket carries one; the same is assumed here until probed. + source: spec + timeline: + reason: >- + Built by the agent from the parts of a conversation, which the search + endpoint does not read. + source: measured + contact_name: + reason: >- + Read from the Contacts endpoint, not from the conversation. Filtering + on it arrives with the Contacts collection. + source: spec + contact_email: + reason: >- + Read from the Contacts endpoint, not from the conversation. Filter on + `source_author_email` instead, which is on the conversation itself. + source: spec + contact_count: + reason: Counted by the agent from the contacts the payload carries. + source: measured + # What `bin/probe_search_fields` enumerates on top of the fields above: + # names the documentation mentions, or that an ops team would plausibly + # search on. A candidate that turns out to be filterable becomes a field + # above -- with a column to expose it on, or with none, in which case it is + # a column the next lot may add. + candidates: + - id + - source.id + - source.author.id + - source.author.type + - source.author.name + - statistics.time_to_assignment + - statistics.time_to_admin_reply + - statistics.time_to_first_close + - statistics.median_time_to_reply + - statistics.first_assignment_at + - statistics.first_admin_reply_at + - statistics.last_assignment_at + - statistics.count_assignments + - conversation_rating.score + - conversation_rating.remark + - conversation_rating.contact_id + - ai_agent.resolution_state + - ai_agent.last_answer_type + - ai_agent.rating + - channel_initiated + - tag_ids + - teammate_ids + - company_id + - topics + + tickets: + path: tickets/search + measured_at: null + fields: + open: + field: open + type: boolean + operators: ['='] + source: spec + category: + field: category + type: string + operators: ['=', '!='] + source: spec + ticket_type_id: + field: ticket_type_id + type: string + operators: ['=', '!='] + source: spec + admin_assignee_id: + field: admin_assignee_id + type: string + operators: ['=', '!='] + source: spec + team_assignee_id: + field: team_assignee_id + type: string + operators: ['=', '!='] + source: spec + contact_ids: + field: contact_ids + type: id_list + operators: ['=', '!='] + source: spec + created_at: + field: created_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: measured + updated_at: + field: updated_at + type: date + operators: ['>', '<', '>=', '<=', '=', '!='] + source: measured + refused: + company_id: + reason: >- + Measured during lot 1: `/tickets/search` refuses `company_id` with + `invalid_field`, although a ticket carries one. Filtering tickets by + account is not something this endpoint does. + source: measured + closed_at: + reason: >- + Derived by the agent from the parts of the ticket. The search endpoint + filters nothing of the sort, and ignores a sort on it without a word. + source: measured + closed_by_name: + reason: Derived by the agent from the parts of the ticket. + source: measured + last_reply_at: + reason: Derived by the agent from the parts of the ticket. + source: measured + last_responder_name: + reason: Derived by the agent from the parts of the ticket. + source: measured + last_responder_type: + reason: Derived by the agent from the parts of the ticket. + source: measured + state_label: + reason: >- + Read off the state object the ticket embeds. Whether the endpoint + filters a ticket state at all, and under which name, is one of the + probe's questions. + source: spec + state_external_label: + reason: Read off the state object the ticket embeds. + source: spec + ticket_type_name: + reason: >- + Read off the type object the ticket embeds. Filter on + `ticket_type_id`, which the endpoint does take. + source: spec + part_count: + reason: Counted by the agent from the parts the payload carries. + source: measured + # R7, and it is a product decision rather than an implementation choice: a + # ticket attribute is filtered as `ticket_attribute.{id}`, and a same-named + # attribute carries a different id per ticket type (measured: + # `_default_title_` is 14162161 on one type and 14162165 on another). One + # union column cannot know which id a row's type uses, so filtering it + # server-side would take one collection per ticket type -- more collections + # in the interface, and a schema that changes shape when the customer adds a + # type. Until the customer says that trade is worth it, the attributes stay + # display-only, as lot 1 published them. + ticket_attributes: + filterable: false + reason: >- + An Intercom ticket attribute is filtered through an id that differs from + one ticket type to the next, so a column showing the attribute of every + type at once cannot say which id to filter on. Filter on the ticket type + and a native field instead. + source: measured + + candidates: + - id + - ticket_id + - state + - ticket_state.category + - ticket_state.id + - state_id + - is_shared + - previous_state_id + - contact_id + - source.author.email + - source.subject + - source.body + - title + - description diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb new file mode 100644 index 000000000..6f2d20a57 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb @@ -0,0 +1,132 @@ +module ForestAdminDatasourceIntercom + RSpec.describe Query::SearchFields do + def table(fields: {}, refused: {}, path: 'tickets/search', measured_at: nil, candidates: []) + described_class.build( + 'endpoints' => { 'tickets' => { 'path' => path, 'measured_at' => measured_at, 'fields' => fields, + 'refused' => refused, 'candidates' => candidates } } + )['tickets'] + end + + def field_row(overrides = {}) + { 'field' => 'created_at', 'type' => 'date', 'operators' => ['>', '<'], 'source' => 'spec' }.merge(overrides) + end + + describe 'the committed table' do + # The schema derives its filters from this file, so a malformed row is a + # boot failure rather than a column nobody can explain. Reading it here is + # what turns that guarantee into a test. + it 'reads the two search endpoints' do + expect(described_class.endpoints).to eq(%w[conversations tickets]) + end + + it 'names the path each endpoint is searched through' do + expect(described_class.fetch('conversations').path).to eq('conversations/search') + expect(described_class.fetch('tickets').path).to eq('tickets/search') + end + + # The one thing lot 1 measured about the field lists: `/tickets/search` + # refuses `company_id`, which the specification lists on a ticket. + it 'refuses company_id on tickets, with the measurement as its reason' do + refusal = described_class.fetch('tickets').refusal('company_id') + + expect(refusal.reason).to include('invalid_field') + expect(refusal).to be_measured + end + + # The columns the agent derives from the parts of a ticket: Intercom + # filters none of them, and lot 1 published them without an operator. + it 'refuses every column derived from the parts of a ticket' do + refused = described_class.fetch('tickets').refused.keys + + expect(refused).to include('closed_at', 'closed_by_name', 'last_reply_at', 'last_responder_name', + 'last_responder_type') + end + + it 'keeps the ticket attributes unfilterable while the arbitration stands' do + expect(described_class.fetch('tickets').ticket_attributes['filterable']).to be(false) + end + + # Until the probe runs against the customer's workspace, the date rows are + # the only ones a measurement backs. + it 'reports which rows nothing has measured yet' do + conversations = described_class.fetch('conversations') + + expect(conversations).not_to be_measured + expect(conversations.unmeasured_fields.map(&:column)).not_to include('created_at', 'updated_at') + end + + it 'declares no column both filterable and refused' do + described_class.endpoints.each do |name| + endpoint = described_class.fetch(name) + + expect(endpoint.filterable_columns & endpoint.refused.keys).to be_empty + end + end + + # A candidate is what the probe enumerates on top of the table; one that + # is already declared would be probed twice and read as a discovery. + it 'names no candidate already declared as a field' do + described_class.endpoints.each do |name| + endpoint = described_class.fetch(name) + + expect(endpoint.candidates & endpoint.fields.values.map(&:field)).to be_empty + end + end + end + + describe 'a table that cannot be trusted' do + it 'refuses an operator Intercom has no spelling for' do + expect { table(fields: { 'created_at' => field_row('operators' => ['~=']) }) } + .to raise_error(ConfigurationError, /has no operator ~=/) + end + + it 'refuses a type nothing knows how to send' do + expect { table(fields: { 'created_at' => field_row('type' => 'timestamp') }) } + .to raise_error(ConfigurationError, /type "timestamp" is not one of/) + end + + # An empty list would publish a filterable column no operator can reach. + it 'refuses a field declaring no operator, and says where it belongs' do + expect { table(fields: { 'created_at' => field_row('operators' => []) }) } + .to raise_error(ConfigurationError, /belongs in the refused table/) + end + + it 'refuses a provenance that is neither measured nor read off the documentation' do + expect { table(fields: { 'created_at' => field_row('source' => 'guessed') }) } + .to raise_error(ConfigurationError, /source "guessed" is neither measured nor spec/) + end + + it 'refuses a refusal with no provenance of its own' do + expect { table(refused: { 'company_id' => { 'reason' => 'no', 'source' => 'hearsay' } }) } + .to raise_error(ConfigurationError, /tickets.company_id/) + end + + it 'names the endpoint and the column it choked on' do + expect { table(fields: { 'created_at' => field_row('type' => 'timestamp') }) } + .to raise_error(ConfigurationError, /search_fields\.yml is malformed at tickets\.created_at/) + end + end + + describe 'what it hands to the schema' do + it 'carries the Intercom field a column is filtered through' do + expect(described_class.fetch('conversations').field('closed_at').field).to eq('statistics.last_close_at') + end + + it 'reads a refusal reason as one line, whatever the YAML wrapping' do + reason = table(refused: { 'company_id' => { 'reason' => "one\ntwo\n", 'source' => 'spec' } }) + .refusal('company_id').reason + + expect(reason).to eq('one two') + end + + it 'refuses an endpoint nothing declares, rather than filtering nothing' do + expect { described_class.fetch('contacts') } + .to raise_error(ConfigurationError, /Unknown Intercom search endpoint "contacts"/) + end + + it 'is measured once the probe has stamped a date on it' do + expect(table(measured_at: '2026-09-01')).to be_measured + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb b/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb new file mode 100644 index 000000000..6432df153 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb @@ -0,0 +1,172 @@ +require 'tmpdir' + +load File.expand_path('../bin/probe_search_fields', __dir__) + +module ForestAdminDatasourceIntercom + RSpec.describe ProbeSearchFields do + let(:base) { Configuration::REGION_HOSTS[:us] } + let(:endpoint) { Query::SearchFields.fetch('tickets') } + let(:client) { Client.new(Configuration.new(access_token: 's3cr3t', rate_limiter: nil)) } + + def json(payload, status = 200) + { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } } + end + + # Every probe is a search asking for one record; what a stub answers is + # therefore an empty page or one of Intercom's refusal codes. + def stub_search(code: nil, operator: nil, value: nil) + body = if code + { 'type' => 'error.list', 'errors' => [{ 'code' => code, 'message' => 'nope' }] } + else + { 'type' => 'list', 'tickets' => [], 'total_count' => 0 } + end + + stub_request(:post, "#{base}/tickets/search").with { |request| matches?(request, operator, value) } + .to_return(json(body, code ? 400 : 200)) + end + + def matches?(request, operator, value) + query = JSON.parse(request.body)['query'] + + (operator.nil? || query['operator'] == operator) && (value.nil? || query['value'] == value) + end + + describe 'the type guessed for a candidate field' do + # A candidate is a name and nothing else -- discovering what it is is the + # point -- so the value shape sent with it is guessed from that name. + it 'reads a timestamp, a count, a flag and a list of ids off the name' do + expect(described_class.guess_type('statistics.last_close_at')).to eq('date') + expect(described_class.guess_type('count_reopens')).to eq('number') + expect(described_class.guess_type('open')).to eq('boolean') + expect(described_class.guess_type('contact_ids')).to eq('id_list') + expect(described_class.guess_type('state')).to eq('string') + end + end + + describe described_class::Probe do + subject(:probe) { described_class.new(client, endpoint) } + + it 'keeps the operators the endpoint answers and drops the ones it refuses' do + stub_search(code: 'data_invalid') + stub_search(operator: '=') + stub_search(operator: '!=') + + result = probe.run('category', 'string') + + expect(result[:operators].select { |_, outcome| outcome[:ok] }.keys).to eq(['=', '!=']) + end + + # A field the endpoint does not filter at all is worth one request, not + # twelve: `invalid_field` ends the row. + it 'stops at the first invalid_field and reports the field unfilterable' do + stub_search(code: 'invalid_field') + + result = probe.run('company_id', 'string') + + expect(result[:unfilterable][:code]).to eq('invalid_field') + expect(a_request(:post, "#{base}/tickets/search")).to have_been_made.once + end + + # A wrong value shape is refused with the same code as an unsupported + # operator, so a single attempt would report a filter Intercom does answer + # as refused. + it 'retries a refused cell with the other value shapes its type takes' do + stub_search(code: 'data_invalid') + stub_search(operator: '>', value: '2026-01-01') + + result = probe.run('created_at', 'date') + + expect(result[:operators]['>'][:ok]).to be(true) + expect(result[:operators]['<'][:ok]).to be(false) + end + + it 'names the failure by its HTTP status when Intercom sends no error code' do + stub_request(:post, "#{base}/tickets/search").to_return(json({ 'nope' => true }, 500)) + + result = probe.run('category', 'string') + + expect(result[:operators]['='][:code]).to eq('http_500') + end + end + + describe described_class::Report do + subject(:report) { described_class.new(endpoint) } + + def run_probe(field, type) + ProbeSearchFields::Probe.new(client, endpoint).run(field, type) + end + + # The only reason to run the probe: an operator the table promises and + # Intercom refuses is a filter the interface offers and the read cannot + # honour. + it 'reports an operator the table promises and Intercom refuses' do + stub_search(code: 'data_invalid') + stub_search(operator: '=') + report.record('category', 'category', run_probe('category', 'string')) + + expect { report.print_diff }.to output(/! the table promises != here, Intercom refuses it/).to_stdout + end + + it 'reports an operator the table does not know about yet' do + stub_search(code: 'data_invalid') + ['=', '!=', '~'].each { |operator| stub_search(operator: operator) } + report.record('category', 'category', run_probe('category', 'string')) + + expect { report.print_diff }.to output(/\+ Intercom also accepts ~/).to_stdout + end + + it 'names the column a field the endpoint refuses is declared on' do + stub_search(code: 'invalid_field') + report.record('category', 'category', run_probe('category', 'string')) + + expect { report.print_diff }.to output(/NOT FILTERABLE \(invalid_field\).*column 'category'/).to_stdout + end + + it 'writes the measurement as evidence, refusal codes included' do + stub_search(code: 'data_invalid') + stub_search(operator: '=') + report.record('category', 'category', run_probe('category', 'string')) + + written = YAML.safe_load(report.to_yaml_document) + + expect(written['fields']['category']['operators']).to eq(['=']) + expect(written['fields']['category']['refused']['!=']).to eq('data_invalid') + end + + it 'writes a field the endpoint refuses as unfilterable' do + stub_search(code: 'invalid_field') + report.record('company_id', nil, run_probe('company_id', 'string')) + + written = YAML.safe_load(report.to_yaml_document) + + expect(written['fields']['company_id']).to eq({ 'filterable' => false, 'code' => 'invalid_field' }) + end + end + + describe described_class::CLI do + # A probe with no token would abort on the first request with an Intercom + # 401, which reads as a workspace problem rather than as a missing option. + it 'refuses to start without a token' do + expect { described_class.call(['--endpoint', 'tickets']) } + .to raise_error(SystemExit).and output(/No token/).to_stderr + end + + it 'refuses an endpoint the table does not declare' do + expect { described_class.call(['--endpoint', 'contacts', '--token', 's3cr3t']) } + .to raise_error(ConfigurationError, /Unknown Intercom search endpoint/) + end + + it 'probes every declared endpoint and writes the evidence where it was asked to' do + out = File.join(Dir.tmpdir, 'intercom-probe.yml') + stub_search(code: 'invalid_field') + stub_request(:post, "#{base}/conversations/search") + .to_return(json({ 'type' => 'error.list', 'errors' => [{ 'code' => 'invalid_field' }] }, 400)) + + expect { described_class.call(['--token', 's3cr3t', '--out', out]) }.to output(/NOT FILTERABLE/).to_stdout + expect(YAML.safe_load_file(out)['endpoint']).to eq('conversations') + ensure + FileUtils.rm_f(out) + end + end + end +end From 382c1d89e297c370b5763f4ac876ee23940b2335 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 18:52:18 +0200 Subject: [PATCH 03/10] feat(intercom): translate a condition tree into a search query The query layer of the filtering lot, derived from the measured table and from nothing else. Nothing is wired into a collection yet: publishing a filter before the read can honour it is the failure this lot exists to prevent, so the schema follows in the commit that switches the reads to the search endpoints. `OperatorTable` spells a Forest operator the way the search DSL does, per kind of field, and narrows it by what the table says the endpoint accepts. Two rules are worth stating out loud. A date field carries the two bounds alone, even where the endpoint accepts six: declaring an equality on a Date column makes the toolkit republish `in`, which its own validator then refuses (PRD-989), and the twenty date operators an operator actually uses are all rewritten into a pair of bounds before they reach here. And `not_i_contains` is left out although Intercom would answer it, `Rules` not allowing it on a String column. A spec asserts the invariant behind both: for every column of every endpoint, everything the agent publishes from what the column declares is an operator its own validator allows. That is the check PRD-989 says nothing currently makes. `FilterValue` converts what a condition carries into what Intercom reads: epoch seconds for a date, a bare day being midnight in the timezone of the caller; the integer a whole float came from; a real boolean. It refuses the rest by name -- an unparseable date, a cast that overflowed, an empty list, a list holding a blank, and every present / blank / missing condition, which the agent rewrites into a comparison against an empty value that Intercom would answer as if it were a value of its own. `ConditionTreeTranslator` walks the tree, unwrapping a branch that carries a single condition rather than spending one of the two nesting levels Intercom allows on it. A column the endpoint does not filter is refused with the reason the table records, a relation is refused by name, and an operator the field does not answer is refused with the list of the ones it does. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../bin/probe_search_fields | 4 +- .../query/condition_tree_translator.rb | 116 ++++++++++++ .../query/filter_value.rb | 154 ++++++++++++++++ .../query/operator_table.rb | 79 +++++++++ .../query/search_fields.rb | 5 +- .../query/search_fields.yml | 27 +-- .../query/condition_tree_translator_spec.rb | 114 ++++++++++++ .../query/filter_value_spec.rb | 166 ++++++++++++++++++ .../query/operator_table_spec.rb | 76 ++++++++ .../spec/probe_search_fields_spec.rb | 3 +- 10 files changed, 727 insertions(+), 17 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/operator_table.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb diff --git a/packages/forest_admin_datasource_intercom/bin/probe_search_fields b/packages/forest_admin_datasource_intercom/bin/probe_search_fields index 664b8e9a4..2ba05fabf 100755 --- a/packages/forest_admin_datasource_intercom/bin/probe_search_fields +++ b/packages/forest_admin_datasource_intercom/bin/probe_search_fields @@ -39,8 +39,7 @@ module ProbeSearchFields 'number' => [0, '0'], 'boolean' => [true, 'true'], 'string' => ['forest-probe', 0], - 'text' => ['forest-probe'], - 'id_list' => ['forest-probe', 0] + 'text' => ['forest-probe'] }.freeze LIST_OPERATORS = %w[IN NIN].freeze @@ -52,7 +51,6 @@ module ProbeSearchFields when /(_at|_since|_until)\z/ then 'date' when /\Acount_|_count\z|\Atime_to_|_time\z/ then 'number' when /\A(open|read|is_|has_|.*_participated)/ then 'boolean' - when /_ids\z/ then 'id_list' else 'string' end end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb new file mode 100644 index 000000000..fc3d2994b --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb @@ -0,0 +1,116 @@ +module ForestAdminDatasourceIntercom + module Query + # Turns a Forest condition tree into the query `POST /conversations/search` + # and `POST /tickets/search` take: + # + # leaf -> { 'field' => ..., 'operator' => ..., 'value' => ... } + # branch -> { 'operator' => 'AND' | 'OR', 'value' => [...] } + # + # What it will not translate, it refuses. A condition dropped on the way to + # Intercom comes back as a page of unfiltered records that looks filtered, + # and every refusal below therefore names the field, the operator, or the + # thing to change -- an operator reads that message and nothing else. + # + # Which field the endpoint filters, and with which operator, is not decided + # here: it is `search_fields.yml`, measured against a real workspace. This + # walks the tree and formats what the table allows. + class ConditionTreeTranslator + Branch = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeBranch + Leaf = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf + + AGGREGATORS = { 'and' => 'AND', 'or' => 'OR' }.freeze + + def self.call(condition_tree, endpoint:, collection:, timezone: nil) + return nil if condition_tree.nil? + + new(endpoint: endpoint, collection: collection, timezone: timezone).translate(condition_tree) + end + + def initialize(endpoint:, collection:, timezone: nil) + @endpoint = endpoint + @collection = collection + @value = FilterValue.new(collection: collection, timezone: timezone) + end + + def translate(node) + case node + when Branch then translate_branch(node) + when Leaf then translate_leaf(node) + else raise UnsupportedOperatorError, "#{@collection} cannot read #{node.class} as a condition." + end + end + + private + + def translate_branch(branch) + conditions = Array(branch.conditions) + refuse_empty_branch!(branch) if conditions.empty? + + # Read before the unwrap below, so a branch is refused on the aggregator + # it carries rather than on how many conditions it holds. + operator = aggregator(branch) + + # A branch holding one condition needs no group of its own. The agent + # builds a tree one branch at a time -- a scope, then a segment, then the + # operator's own filter -- and the nesting Intercom allows is shallow + # enough that a wrapper around nothing is a level worth not spending. + return translate(conditions.first) if conditions.size == 1 + + { 'operator' => operator, 'value' => conditions.map { |condition| translate(condition) } } + end + + def aggregator(branch) + AGGREGATORS[branch.aggregator.to_s.downcase] || + raise(UnsupportedOperatorError, + "#{@collection} cannot read #{branch.aggregator.inspect} as a condition tree aggregator; " \ + "expected 'And' or 'Or'.") + end + + def translate_leaf(leaf) + field = @endpoint.field(leaf.field.to_s) || refuse_unfilterable!(leaf.field.to_s) + spelling = OperatorTable.intercom_operator(field, leaf.operator) || refuse_operator!(leaf, field) + + { 'field' => field.field, 'operator' => spelling, 'value' => @value.call(leaf, field, spelling) } + end + + # A column the endpoint does not filter, and the reason it does not, taken + # from the table when it carries one: those reasons are the difference + # between "no" and a message an operator can do something with. + def refuse_unfilterable!(column) + raise UnsupportedOperatorError, "#{@collection} cannot filter #{column.inspect}: #{unfilterable_reason(column)}" + end + + def unfilterable_reason(column) + return relation_reason(column) if column.include?(':') + + refusal = @endpoint.refusal(column) + return refusal.reason if refusal + + "#{@endpoint.path} takes no filter on it. Filter on one of: #{@endpoint.filterable_columns.join(", ")}." + end + + # A relation reaches the translator as `relation:field`. None of the + # collections this endpoint serves declares one yet, so the condition can + # only come from a scope or a segment written against a schema this + # datasource does not have. + def relation_reason(column) + "#{@collection} declares no relation, so #{column.inspect} names a field it cannot reach. Filter on one " \ + "of its own columns: #{@endpoint.filterable_columns.join(", ")}." + end + + def refuse_operator!(leaf, field) + supported = OperatorTable.forest_operators(field) + + raise UnsupportedOperatorError, + "#{@collection} cannot filter #{leaf.field.inspect} with #{leaf.operator.inspect}: " \ + "#{@endpoint.path} answers #{supported.join(", ")} on #{field.field.inspect} and nothing else." + end + + def refuse_empty_branch!(branch) + raise UnsupportedOperatorError, + "#{@collection} was given a #{branch.aggregator} branch carrying no condition, which names no record " \ + 'and no filter.' + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb new file mode 100644 index 000000000..d2ab270e4 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb @@ -0,0 +1,154 @@ +module ForestAdminDatasourceIntercom + module Query + # How the value of a Forest condition reaches Intercom's search DSL, and + # every way it can fail to. Split from the translator, which knows the shape + # of the tree but not what a field expects on the wire. + # + # Intercom types its filter values: a date is epoch seconds, a number is a + # number, a flag is a boolean. A value of the wrong shape is refused with + # `data_invalid`, the same code an unsupported operator returns, so the + # conversion belongs here rather than in a rescue reading error codes. + class FilterValue + # What the frontend sends for a Dateonly column, and what a segment written + # in Ruby carries: a day with no time of day, which is midnight in the + # timezone of whoever wrote the filter rather than in the server's. + DATE_ONLY = /\A\d{4}-\d{2}-\d{2}\z/ + + INTEGER = /\A-?\d+\z/ + + def initialize(collection:, timezone: nil) + @collection = collection + identifier = timezone.to_s.strip + # Stored stripped rather than only checked stripped: kept as it came, a + # `" Europe/Paris "` passes the blank guard and then fails the zone + # lookup, and a day boundary lands an offset away from where the filter + # meant it. + @timezone = identifier.empty? ? 'UTC' : identifier + end + + def call(leaf, field, spelling) + return list(leaf, field) if OperatorTable.list_operator?(spelling) + + refuse_absence!(leaf, field) if blank?(leaf.value) + + scalar(leaf.value, leaf, field) + end + + private + + # Dropping the blanks would answer a different question: `not_in [nil, + # 'open']` was asked to exclude the records carrying neither and would come + # back including them. An empty list is as bad the other way round, being a + # filter that matches everything. + def list(leaf, field) + values = Array(leaf.value) + refuse_empty_list!(leaf) if values.empty? + refuse_absence!(leaf, field) if values.any? { |value| blank?(value) } + + values.map { |value| scalar(value, leaf, field) } + end + + def scalar(value, leaf, field) + case field.type + when 'date' then epoch(value, leaf) + when 'number' then number(value, leaf) + when 'boolean' then boolean(value, leaf) + else value.to_s + end + end + + # Intercom stores and compares its dates as epoch seconds. What arrives + # here is an ISO8601 string most of the time -- the frontend sends one, and + # so does every interval operator the toolkit rewrites into a pair of + # bounds -- but a scope or a segment written in Ruby carries a Time or a + # Date, and neither has a timezone of its own. + def epoch(value, leaf) + case value + when DateTime then value.to_time.to_i + when Date then start_of_day(value) + when Time, Numeric then value.to_i + when String then parse(value, leaf) + else refuse_value!(leaf, value, 'a date') + end + end + + def parse(value, leaf) + return start_of_day(Date.parse(value)) if DATE_ONLY.match?(value) + + Time.parse(value).to_i + rescue ArgumentError, TypeError + refuse_value!(leaf, value, 'a date') + end + + # A day with no time of day is the caller's day, not the server's: it is + # the timezone the filter was written in that says when that day starts. + def start_of_day(date) + Time.use_zone(@timezone) { Time.zone.local(date.year, date.month, date.day).to_i } + rescue ArgumentError + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] unknown timezone #{@timezone.inspect}, reading the day boundary of a " \ + 'date filter in UTC instead.' + ) + Time.utc(date.year, date.month, date.day).to_i + end + + # The agent casts every Number column with `to_f`, so an integer 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 cast that overflowed to Infinity, or a NaN, is refused rather than + # passed on: the JSON encoder raises on both, a step later, as a 500 naming + # nothing the operator can act on. + def number(value, leaf) + case value + when Integer then value + when Float then finite(value, leaf) + when String then value.match?(INTEGER) ? value.to_i : finite(Float(value, exception: false), leaf) + else refuse_value!(leaf, value, 'a number') + end + end + + def finite(value, leaf) + refuse_value!(leaf, value, 'a number') unless value.is_a?(Float) && value.finite? + + value == value.to_i ? value.to_i : value + end + + def boolean(value, leaf) + case value + when true, false then value + when 'true' then true + when 'false' then false + else refuse_value!(leaf, value, 'a true or a false') + end + end + + def blank?(value) = value.nil? || value.to_s.empty? + + # `present`, `blank` and `missing` are derived by the agent from an + # equality, above this datasource, and rewritten into a comparison with an + # empty value. Intercom's search matches values and has no spelling for the + # absence of one, so the rewritten condition is refused here rather than + # sent as a comparison against the empty string -- which Intercom would + # answer as if it were a value of its own. + def refuse_absence!(leaf, field) + raise UnsupportedOperatorError, + "#{@collection} cannot filter #{leaf.field.inspect} for absence: Intercom's search matches values " \ + "and has no operator for the lack of one, so a #{leaf.operator} condition on " \ + "#{field.field.inspect} cannot be translated. Filter on a value instead." + end + + def refuse_empty_list!(leaf) + raise UnsupportedOperatorError, + "#{@collection} was asked to filter #{leaf.field.inspect} with #{leaf.operator} and an empty list, " \ + 'which names no record and no filter. Pass at least one value.' + end + + def refuse_value!(leaf, value, expected) + raise UnsupportedOperatorError, + "#{@collection} cannot filter #{leaf.field.inspect} with #{value.inspect}: Intercom expects " \ + "#{expected} on this field." + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/operator_table.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/operator_table.rb new file mode 100644 index 000000000..e08f14f74 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/operator_table.rb @@ -0,0 +1,79 @@ +module ForestAdminDatasourceIntercom + module Query + # How a Forest operator is spelled in Intercom's search DSL, per kind of + # field. Two things read this table and they must never disagree: the schema, + # which publishes a column's `filter_operators`, and the translator, which + # writes the filter. Both go through `forest_operators` and + # `intercom_operator`, so a column cannot advertise a filter the translator + # would then refuse. + # + # What an endpoint really accepts on a given field is not here -- that is + # `search_fields.yml`, measured. This is only the spelling, and the set is + # narrowed by the table before anything is published. + module OperatorTable + Operators = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators + + EQUALITY = { Operators::EQUAL => '=', Operators::NOT_EQUAL => '!=', + Operators::IN => 'IN', Operators::NOT_IN => 'NIN' }.freeze + + # `contains` and `i_contains` both land on `~`: Intercom documents one + # substring operator and no case semantics for it, and the frontend sends + # either spelling depending on the column. Declaring one alone would leave + # the other refused at read time on a field Intercom does filter. + # + # `not_i_contains` is deliberately absent although Intercom would answer it + # the same way as `not_contains`: the toolkit's own `Rules` does not allow + # it on a String column, so publishing it would put a filter in the + # interface that the agent rejects before this datasource is ever reached + # -- the failure PRD-989 describes. + SUBSTRING = { Operators::CONTAINS => '~', Operators::I_CONTAINS => '~', + Operators::NOT_CONTAINS => '!~', + Operators::STARTS_WITH => '^', Operators::ENDS_WITH => '$' }.freeze + + BOUNDS = { Operators::GREATER_THAN => '>', Operators::LESS_THAN => '<' }.freeze + + # A date field carries the two bounds and nothing else, even where the + # endpoint accepts `=`, `!=`, `>=` and `<=` -- measured, it does on both + # search endpoints. The reason is on the agent's side: declaring `equal` on + # a Date column makes the toolkit republish `in`, which its own validator + # then refuses (PRD-989), so the interface would offer a date filter + # answered by a 400 having nothing to do with Intercom. + # + # Nothing is lost that an operator can see. From the two bounds the toolkit + # derives `before`, `after`, `today`, `yesterday`, `past`, `future` and the + # whole `previous_*` family -- twenty operators, all rewritten into a pair + # of bounds before they reach here. What stays out is an equality on an + # instant, which day-granular filtering could not honour anyway. + MAPS = { + 'string' => EQUALITY, + 'boolean' => EQUALITY, + 'number' => EQUALITY.merge(BOUNDS), + 'text' => EQUALITY.merge(SUBSTRING), + 'date' => BOUNDS + }.freeze + + # The operators that take a list rather than a value, on the wire. + LIST_OPERATORS = %w[IN NIN].freeze + + class << self + def types = MAPS.keys + + # What the column publishes: the Forest operators whose Intercom spelling + # this endpoint accepts on this field, and no other. + def forest_operators(field) + MAPS.fetch(field.type).select { |_, spelling| field.operators.include?(spelling) }.keys + end + + # nil when the endpoint does not accept that operator on that field, + # which the translator turns into a refusal naming what it does accept. + def intercom_operator(field, forest_operator) + spelling = MAPS.fetch(field.type)[forest_operator] + + spelling if spelling && field.operators.include?(spelling) + end + + def list_operator?(spelling) = LIST_OPERATORS.include?(spelling) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb index b5735896e..554b6d175 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.rb @@ -22,7 +22,10 @@ module SearchFields # `$`, and the membership pair. Which of them an endpoint honours on a # given field is the table's business; this is only the alphabet. KNOWN_OPERATORS = ['=', '!=', '>', '<', '>=', '<=', '~', '!~', '^', '$', 'IN', 'NIN'].freeze - KNOWN_TYPES = %w[string text date boolean number id_list].freeze + # Read off the operator table rather than listed again here: a type with + # no spelling of its own would pass this validation and raise when the + # schema asked what to publish on it. + KNOWN_TYPES = OperatorTable.types KNOWN_SOURCES = %w[measured spec].freeze # `source` says where a row comes from, and `measured?` is what the boot diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml index e9549e236..e2a06b54c 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml @@ -23,7 +23,6 @@ # text -- a field Intercom also matches per word (`~`) # date -- epoch seconds on the wire, truncated to the UTC day on read # boolean, number -# id_list -- a field holding several ids, matched against one of them version: 1 endpoints: @@ -67,11 +66,6 @@ endpoints: type: string operators: ['=', '!='] source: spec - contact_ids: - field: contact_ids - type: id_list - operators: ['=', '!='] - source: spec source_type: field: source.type type: string @@ -174,6 +168,14 @@ endpoints: # by name too -- these are the ones whose refusal is permanent and has an # explanation worth giving. refused: + contact_ids: + reason: >- + Intercom does match a conversation against one of its contact ids, but + the column holds the list and is therefore typed Json, whose filter + values the agent's own validator requires to be Json too -- an id + would be rejected before this datasource saw it. Filtering by contact + arrives with the Contacts collection and a relation. + source: measured tag_names: reason: >- Intercom filters conversations by tag id, and this column holds the @@ -270,11 +272,6 @@ endpoints: type: string operators: ['=', '!='] source: spec - contact_ids: - field: contact_ids - type: id_list - operators: ['=', '!='] - source: spec created_at: field: created_at type: date @@ -286,6 +283,14 @@ endpoints: operators: ['>', '<', '>=', '<=', '=', '!='] source: measured refused: + contact_ids: + reason: >- + Intercom does match a ticket against one of its contact ids, but + the column holds the list and is therefore typed Json, whose filter + values the agent's own validator requires to be Json too -- an id + would be rejected before this datasource saw it. Filtering by contact + arrives with the Contacts collection and a relation. + source: measured company_id: reason: >- Measured during lot 1: `/tickets/search` refuses `company_id` with diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb new file mode 100644 index 000000000..f371239b4 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb @@ -0,0 +1,114 @@ +module ForestAdminDatasourceIntercom + RSpec.describe Query::ConditionTreeTranslator do + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + let(:nodes) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes } + let(:endpoint) { Query::SearchFields.fetch('conversations') } + + def translate(tree, timezone: 'UTC') + described_class.call(tree, endpoint: endpoint, collection: 'IntercomConversation', timezone: timezone) + end + + def leaf(field, operator, value = nil) + nodes::ConditionTreeLeaf.new(field, operator, value) + end + + def branch(aggregator, *conditions) + nodes::ConditionTreeBranch.new(aggregator, conditions) + end + + describe 'a leaf' do + it 'writes the Intercom field, operator and value the endpoint takes' do + expect(translate(leaf('state', operators::EQUAL, 'open'))) + .to eq({ 'field' => 'state', 'operator' => '=', 'value' => 'open' }) + end + + # The column is the operator's name for it; the field is Intercom's. They + # are not the same on a statistic, which the column flattens onto the row. + it 'filters a flattened statistic through the field Intercom nests it in' do + expect(translate(leaf('closed_at', operators::LESS_THAN, '2026-09-01T00:00:00Z'))['field']) + .to eq('statistics.last_close_at') + end + + it 'answers nil for no condition at all, which is a list view' do + expect(translate(nil)).to be_nil + end + end + + describe 'a branch' do + it 'groups its conditions under the aggregator Intercom spells in capitals' do + tree = branch('Or', leaf('state', operators::EQUAL, 'open'), leaf('state', operators::EQUAL, 'snoozed')) + + expect(translate(tree)).to eq({ 'operator' => 'OR', + 'value' => [{ 'field' => 'state', 'operator' => '=', 'value' => 'open' }, + { 'field' => 'state', 'operator' => '=', 'value' => 'snoozed' }] }) + end + + # The agent builds a tree one branch at a time -- a scope, then a segment, + # then the operator's filter -- and Intercom allows two levels of nesting. + # A wrapper around a single condition is a level worth not spending. + it 'unwraps a branch carrying one condition rather than spending a level on it' do + tree = branch('And', branch('And', leaf('open', operators::EQUAL, true))) + + expect(translate(tree)).to eq({ 'field' => 'open', 'operator' => '=', 'value' => true }) + end + + it 'nests a group inside a group' do + tree = branch('And', leaf('open', operators::EQUAL, true), + branch('Or', leaf('state', operators::EQUAL, 'open'), + leaf('state', operators::EQUAL, 'snoozed'))) + + expect(translate(tree)['value'].last['operator']).to eq('OR') + end + + it 'refuses an aggregator that is neither and nor or' do + expect { translate(branch('Xor', leaf('open', operators::EQUAL, true))) } + .to raise_error(UnsupportedOperatorError, /cannot read "Xor" as a condition tree aggregator/) + end + + # A branch with nothing in it names no record and no filter, so sending it + # would answer a filtered question with the whole collection. + it 'refuses a branch carrying no condition' do + expect { translate(branch('And')) } + .to raise_error(UnsupportedOperatorError, /carrying no condition/) + end + + it 'refuses a node that is neither a leaf nor a branch' do + expect { translate(Object.new) } + .to raise_error(UnsupportedOperatorError, /cannot read Object as a condition/) + end + end + + describe 'what it will not translate' do + # The whole point of the lot: a condition dropped on the way to Intercom + # comes back as an unfiltered page that looks filtered. + it 'refuses a column the endpoint does not filter, and names what it does' do + expect { translate(leaf('contact_name', operators::EQUAL, 'Camille')) } + .to raise_error(UnsupportedOperatorError, /cannot filter "contact_name".*Contacts endpoint/m) + end + + it 'refuses a column nothing declares, listing the ones it takes' do + expect { translate(leaf('nope', operators::EQUAL, 'x')) } + .to raise_error(UnsupportedOperatorError, /takes no filter on it. Filter on one of: state, open/) + end + + # None of these collections declares a relation yet, so a `relation:field` + # can only come from a scope or a segment written against another schema. + it 'refuses a condition on a relation by name' do + expect { translate(leaf('contact:email', operators::EQUAL, 'camille@acme.test')) } + .to raise_error(UnsupportedOperatorError, /declares no relation/) + end + + it 'refuses an operator the endpoint does not answer on that field' do + expect { translate(leaf('state', operators::CONTAINS, 'op')) } + .to raise_error(UnsupportedOperatorError, /answers equal, not_equal on "state" and nothing else/) + end + + # Published on a date column by the toolkit and refused by this endpoint: + # the datasource cannot express an equality on a day it truncates. + it 'refuses an equality on a date, which is not one of the two bounds' do + expect { translate(leaf('created_at', operators::EQUAL, '2026-09-01T00:00:00Z')) } + .to raise_error(UnsupportedOperatorError, /answers greater_than, less_than/) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb new file mode 100644 index 000000000..bd0a103d6 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb @@ -0,0 +1,166 @@ +module ForestAdminDatasourceIntercom + RSpec.describe Query::FilterValue do + subject(:formatter) { described_class.new(collection: 'IntercomConversation', timezone: timezone) } + + let(:timezone) { 'Europe/Paris' } + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + let(:nodes) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes } + + def field(type, operators = ['=']) + Query::SearchFields::Field.new(column: 'c', field: 'c', type: type, operators: operators, source: 'spec') + end + + def call(type, value, spelling: '=', operator: nil) + leaf = nodes::ConditionTreeLeaf.new('c', operator || operators::EQUAL, value) + + formatter.call(leaf, field(type), spelling) + end + + describe 'a date' do + # Intercom stores and compares its dates as epoch seconds. + it 'sends an ISO8601 timestamp as the second it names' do + expect(call('date', '2026-09-01T08:30:00Z')).to eq(1_788_251_400) + end + + it 'keeps the offset an ISO8601 timestamp carries' do + expect(call('date', '2026-09-01T10:30:00+02:00')).to eq(1_788_251_400) + end + + # A day with no time of day is the caller's day: it is the timezone the + # filter was written in that says when that day starts, not the server's. + it 'reads a bare date as midnight in the timezone of the caller' do + expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 8, 31, 22).to_i) + end + + it 'reads a Ruby Date the same way' do + expect(call('date', Date.new(2026, 9, 1))).to eq(Time.utc(2026, 8, 31, 22).to_i) + end + + it 'takes a Time and a DateTime as the instant they are' do + expect(call('date', Time.utc(2026, 9, 1, 8, 30))).to eq(1_788_251_400) + expect(call('date', DateTime.new(2026, 9, 1, 8, 30, 0))).to eq(1_788_251_400) + end + + it 'leaves epoch seconds alone' do + expect(call('date', 1_788_251_400)).to eq(1_788_251_400) + end + + # Falling back to UTC silently would move a day boundary by the offset, + # which is the failure a timezone is read for in the first place. + context 'when the caller names a timezone nothing knows' do + let(:timezone) { 'Middle-Earth/Shire' } + + it 'reads the day boundary in UTC and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 9, 1).to_i) + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/unknown timezone/) + end + end + + context 'when the caller names no timezone at all' do + let(:timezone) { ' ' } + + it 'reads the day boundary in UTC' do + expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 9, 1).to_i) + end + end + + it 'refuses a string that is not a date' do + expect { call('date', 'last tuesday') } + .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) + end + + it 'refuses a value that is not a date at all' do + expect { call('date', { 'day' => 1 }) } + .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) + end + end + + describe 'a number' do + # The agent casts every Number column with `to_f`, so an integer field + # would otherwise be filtered with `42.0`, a form none of its values carry. + it 'sends a whole float as the integer it is' do + expect(call('number', 42.0)).to eq(42) + end + + it 'keeps a decimal, and an integer, as they are' do + expect(call('number', 42.5)).to eq(42.5) + expect(call('number', 42)).to eq(42) + end + + it 'reads an integer written as a string' do + expect(call('number', '42')).to eq(42) + end + + it 'reads a decimal written as a string' do + expect(call('number', '42.5')).to eq(42.5) + end + + # `to_i` raises on both, and so does the JSON encoder a step later, as a + # 500 naming nothing the operator can act on. + it 'refuses a cast that overflowed and a value that is not a number' do + expect { call('number', Float::INFINITY) }.to raise_error(UnsupportedOperatorError, /expects a number/) + expect { call('number', 'many') }.to raise_error(UnsupportedOperatorError, /expects a number/) + expect { call('number', []) }.to raise_error(UnsupportedOperatorError, /expects a number/) + end + end + + describe 'a boolean' do + it 'sends a flag as a boolean, whichever way the filter spelled it' do + expect(call('boolean', true)).to be(true) + expect(call('boolean', false)).to be(false) + expect(call('boolean', 'true')).to be(true) + expect(call('boolean', 'false')).to be(false) + end + + it 'refuses anything else, rather than reading it as truthy' do + expect { call('boolean', 'yes') }.to raise_error(UnsupportedOperatorError, /expects a true or a false/) + end + end + + describe 'a list' do + it 'formats every value of an IN the way the field takes it' do + expect(call('date', %w[2026-09-01 2026-09-02], spelling: 'IN', operator: operators::IN)) + .to eq([Time.utc(2026, 8, 31, 22).to_i, Time.utc(2026, 9, 1, 22).to_i]) + end + + # A filter matching everything is not what an empty list was asked for. + it 'refuses an empty list' do + expect { call('string', [], spelling: 'IN', operator: operators::IN) } + .to raise_error(UnsupportedOperatorError, /empty list/) + end + + # `not_in [nil, 'open']` was asked to exclude the records carrying neither + # and would come back including the blank ones. + it 'refuses a list holding a blank, rather than dropping it' do + expect { call('string', [nil, 'open'], spelling: 'NIN', operator: operators::NOT_IN) } + .to raise_error(UnsupportedOperatorError, /cannot filter "c" for absence/) + end + end + + describe 'a condition on the absence of a value' do + # `present`, `blank` and `missing` are derived from an equality above this + # datasource and rewritten into a comparison with an empty value. Intercom + # would answer it as if the empty string were a value of its own. + it 'refuses the rewritten comparison and says to filter on a value' do + expect { call('string', nil) } + .to raise_error(UnsupportedOperatorError, /matches values and has no operator for the lack of one/) + end + + it 'refuses an empty string the same way' do + expect { call('string', '') }.to raise_error(UnsupportedOperatorError, /for absence/) + end + end + + describe 'a string' do + it 'sends it as it came' do + expect(call('string', 'open')).to eq('open') + end + + it 'sends a text the same way' do + expect(call('text', 'facture')).to eq('facture') + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb new file mode 100644 index 000000000..b8177dc30 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb @@ -0,0 +1,76 @@ +module ForestAdminDatasourceIntercom + RSpec.describe Query::OperatorTable do + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + let(:rules) { ForestAdminDatasourceToolkit::Validations::Rules } + let(:equivalent) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::ConditionTreeEquivalent } + + def field(type, operators) + Query::SearchFields::Field.new(column: 'c', field: 'c', type: type, operators: operators, source: 'spec') + end + + describe '.forest_operators' do + it 'publishes only what the endpoint accepts on that field' do + expect(described_class.forest_operators(field('string', ['=']))).to eq([operators::EQUAL]) + expect(described_class.forest_operators(field('string', ['=', '!=']))) + .to eq([operators::EQUAL, operators::NOT_EQUAL]) + end + + # Intercom documents one substring operator and no case semantics for it, + # and the frontend sends either spelling depending on the column. + it 'reads both spellings of contains onto the one operator Intercom has' do + published = described_class.forest_operators(field('text', ['~'])) + + expect(published).to eq([operators::CONTAINS, operators::I_CONTAINS]) + end + + # The point of the whole table: a date column carries the two bounds even + # where the endpoint accepts more, because declaring an equality on a Date + # makes the toolkit republish `in`, which its own validator refuses. + it 'keeps a date column to the two bounds whatever the endpoint accepts' do + published = described_class.forest_operators(field('date', ['>', '<', '>=', '<=', '=', '!='])) + + expect(published).to eq([operators::GREATER_THAN, operators::LESS_THAN]) + end + + it 'publishes nothing for a field the endpoint answers no known operator on' do + expect(described_class.forest_operators(field('string', ['~']))).to be_empty + end + end + + describe '.intercom_operator' do + it 'spells a Forest operator the way the search DSL does' do + expect(described_class.intercom_operator(field('number', ['>']), operators::GREATER_THAN)).to eq('>') + end + + it 'answers nil for an operator the endpoint does not accept on that field' do + expect(described_class.intercom_operator(field('string', ['=']), operators::NOT_EQUAL)).to be_nil + end + + it 'answers nil for an operator no field of that type carries' do + expect(described_class.intercom_operator(field('boolean', ['=']), operators::CONTAINS)).to be_nil + end + end + + describe 'the operators every published column ends up with' do + # The invariant PRD-989 says nothing checks: everything the agent publishes + # from what a column declares must be an operator its own validator allows. + # A column advertising a filter the agent then rejects is a 400 in the + # interface for a reason that has nothing to do with Intercom. + it 'is a set the toolkit validator allows, for every column of every endpoint' do + column_types = { 'string' => 'String', 'text' => 'String', 'date' => 'Date', + 'boolean' => 'Boolean', 'number' => 'Number' } + + Query::SearchFields.endpoints.each do |name| + Query::SearchFields.fetch(name).fields.each_value do |searchable| + declared = described_class.forest_operators(searchable) + column_type = column_types.fetch(searchable.type) + published = operators.all.select { |o| equivalent.equivalent_tree?(o, declared, column_type) } + + expect(published - rules.get_allowed_operators_for_column_type(column_type)) + .to be_empty, "#{name}.#{searchable.column} publishes an operator Rules refuses" + end + end + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb b/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb index 6432df153..405d2f85f 100644 --- a/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/probe_search_fields_spec.rb @@ -34,11 +34,10 @@ def matches?(request, operator, value) describe 'the type guessed for a candidate field' do # A candidate is a name and nothing else -- discovering what it is is the # point -- so the value shape sent with it is guessed from that name. - it 'reads a timestamp, a count, a flag and a list of ids off the name' do + it 'reads a timestamp, a count and a flag off the name' do expect(described_class.guess_type('statistics.last_close_at')).to eq('date') expect(described_class.guess_type('count_reopens')).to eq('number') expect(described_class.guess_type('open')).to eq('boolean') - expect(described_class.guess_type('contact_ids')).to eq('id_list') expect(described_class.guess_type('state')).to eq('string') end end From 3e96c7ae5aaa89d922afa17fbe12b56e4d0bbf5b Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 18:56:04 +0200 Subject: [PATCH 04/10] fix(intercom): answer the day a date filter names Intercom truncates a date search to the day, and at the UTC boundary rather than the workspace's, whatever its documentation promises. `> V` answers from the start of the day after V; `< V` answers before the start of V's own day. Sent as they come, the two bounds the toolkit rewrites an interval into cancel each other out. `today` reaches the datasource as `> 00:00` and `< 23:59` of one day, which Intercom reads as "from tomorrow" and "before today": no rows, to the most ordinary filter there is, and nothing in the answer saying why. `DayBounds` moves each bound to the boundary that makes Intercom answer the day the filter named. A lower bound goes back a day, an upper bound forward a day -- unless it already sits on a boundary, where the day it names is exactly the one to leave out. A caller in UTC therefore gets that day and nothing else; a caller in another timezone gets the UTC days their window overlaps, up to a day wider at each end, and is told so once per filter rather than once per bound. The window is day-granular either way. That is the granularity the Intercom interface filters on, and the README section of this lot will say it plainly rather than let a column imply otherwise. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../query/day_bounds.rb | 69 +++++++++++++++ .../query/filter_value.rb | 4 +- .../query/condition_tree_translator_spec.rb | 15 ++++ .../query/filter_value_spec.rb | 87 ++++++++++++++----- 4 files changed, 152 insertions(+), 23 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/day_bounds.rb diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/day_bounds.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/day_bounds.rb new file mode 100644 index 000000000..c5df547db --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/day_bounds.rb @@ -0,0 +1,69 @@ +module ForestAdminDatasourceIntercom + module Query + # Where a date bound lands once Intercom's day truncation is accounted for. + # + # Intercom truncates a date search to the day, at the **UTC** boundary -- + # measured, and it contradicts the documentation, which promises the + # workspace's timezone. `> V` answers from the start of the day *after* V, + # and `< V` answers before the start of V's own day. + # + # Sent as they come, the two bounds of an interval cancel each other out: + # `today` reaches here as `> 00:00` and `< 23:59` of one day, which Intercom + # reads as "from tomorrow" and "before today" -- an empty answer to the most + # ordinary filter there is. So each bound is moved to the day boundary that + # makes Intercom answer the day the filter named: + # + # `>` V -> the day before V's day, so the answer starts at V's day; + # `<` V -> the day after V's day, so the answer runs through V's day, + # except when V already sits on a boundary, where V's day is + # exactly what was asked to be left out. + # + # The window is therefore day-granular: a bound naming a time of day matches + # from the start of that day, or through the end of it. That is the + # granularity the Intercom interface filters on, and the README says so. + # + # Split from FilterValue, which knows how to read a date out of whatever a + # condition carries but has no business knowing that Intercom answers a + # different question from the one it was asked. + class DayBounds + UTC_DAY = 86_400 + + def initialize(collection:, timezone:) + @collection = collection + @timezone = timezone + end + + def call(seconds, spelling) + day = seconds - (seconds % UTC_DAY) + report_utc_day(seconds, day) + + return day - UTC_DAY if spelling == '>' + + seconds == day ? seconds : day + UTC_DAY + end + + private + + # A window written in another timezone is answered on the UTC days it + # overlaps, which is up to a day wider at each end. Reported once per + # filter rather than per bound: what an operator needs to know is that the + # day boundary is not theirs, not how many bounds crossed it. + def report_utc_day(seconds, day) + return if @reported || same_day_locally?(seconds, day) + + @reported = true + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{@collection} was filtered on a date written in #{@timezone}, and " \ + 'Intercom truncates a date search to the UTC day whatever the workspace timezone says. The rows come ' \ + 'back for the UTC days the window overlaps, which is up to a day wider at each end.' + ) + end + + def same_day_locally?(seconds, day) + Time.use_zone(@timezone) { Time.zone.at(seconds).to_date } == Time.at(day).utc.to_date + rescue ArgumentError + true + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb index d2ab270e4..db5469b80 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb @@ -24,6 +24,7 @@ def initialize(collection:, timezone: nil) # lookup, and a day boundary lands an offset away from where the filter # meant it. @timezone = identifier.empty? ? 'UTC' : identifier + @day_bounds = DayBounds.new(collection: collection, timezone: @timezone) end def call(leaf, field, spelling) @@ -31,7 +32,8 @@ def call(leaf, field, spelling) refuse_absence!(leaf, field) if blank?(leaf.value) - scalar(leaf.value, leaf, field) + value = scalar(leaf.value, leaf, field) + field.type == 'date' ? @day_bounds.call(value, spelling) : value end private diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb index f371239b4..89ccc306e 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb @@ -78,6 +78,21 @@ def branch(aggregator, *conditions) end end + # The headline failure this lot had to avoid: Intercom truncates a date + # search to the UTC day, so the pair of bounds the toolkit rewrites `today` + # into reads as "from tomorrow" and "before today" -- an empty answer to the + # most ordinary filter there is -- unless each bound is moved to the day + # boundary that makes Intercom answer the day the filter named. + describe 'the two bounds an interval is rewritten into' do + it 'asks for the day the interval names rather than cancelling out' do + tree = branch('And', leaf('created_at', operators::GREATER_THAN, '2026-09-01T00:00:00Z'), + leaf('created_at', operators::LESS_THAN, '2026-09-01T23:59:59Z')) + + expect(translate(tree)['value'].map { |bound| Time.at(bound['value']).utc.iso8601 }) + .to eq(['2026-08-31T00:00:00Z', '2026-09-02T00:00:00Z']) + end + end + describe 'what it will not translate' do # The whole point of the lot: a condition dropped on the way to Intercom # comes back as an unfiltered page that looks filtered. diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb index bd0a103d6..1ddd8eb34 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb @@ -16,44 +16,88 @@ def call(type, value, spelling: '=', operator: nil) formatter.call(leaf, field(type), spelling) end - describe 'a date' do - # Intercom stores and compares its dates as epoch seconds. - it 'sends an ISO8601 timestamp as the second it names' do - expect(call('date', '2026-09-01T08:30:00Z')).to eq(1_788_251_400) + # Intercom truncates a date search to the UTC day -- measured, and against + # its own documentation, which promises the workspace timezone. `>` answers + # from the start of the day after the value, `<` before the start of the + # day of the value. Sent as they come, the two bounds of an interval cancel + # each other out and `today` answers nothing. + describe 'a date, and the UTC day Intercom truncates it to' do + def bound(value, spelling) + Time.at(call('date', value, spelling: spelling, + operator: spelling == '>' ? operators::GREATER_THAN : operators::LESS_THAN)) + .utc.iso8601 + end + + it 'moves a lower bound back a day, so Intercom answers from the day it names' do + expect(bound('2026-09-01T08:30:00Z', '>')).to eq('2026-08-31T00:00:00Z') + end + + it 'moves an upper bound forward a day, so Intercom answers through the day it names' do + expect(bound('2026-09-01T08:30:00Z', '<')).to eq('2026-09-02T00:00:00Z') + end + + # An upper bound already sitting on a day boundary names the day to leave + # out, which is what Intercom answers on its own. + it 'leaves an upper bound already on a UTC day boundary where it is' do + expect(bound('2026-09-01T00:00:00Z', '<')).to eq('2026-09-01T00:00:00Z') + end + + # The pair the toolkit rewrites `today` into, from a caller in UTC: what + # comes back is that day and nothing else. + it 'answers the day itself for the two bounds of an interval' do + expect(bound('2026-09-01T00:00:00Z', '>')).to eq('2026-08-31T00:00:00Z') + expect(bound('2026-09-01T23:59:59Z', '<')).to eq('2026-09-02T00:00:00Z') end it 'keeps the offset an ISO8601 timestamp carries' do - expect(call('date', '2026-09-01T10:30:00+02:00')).to eq(1_788_251_400) + expect(bound('2026-09-01T10:30:00+02:00', '<')).to eq('2026-09-02T00:00:00Z') end # A day with no time of day is the caller's day: it is the timezone the - # filter was written in that says when that day starts, not the server's. + # filter was written in that says when that day starts. it 'reads a bare date as midnight in the timezone of the caller' do - expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 8, 31, 22).to_i) + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + expect(bound('2026-09-01', '>')).to eq('2026-08-30T00:00:00Z') end - it 'reads a Ruby Date the same way' do - expect(call('date', Date.new(2026, 9, 1))).to eq(Time.utc(2026, 8, 31, 22).to_i) + it 'reads a Ruby Date, a Time, a DateTime and epoch seconds the same way' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + expect(bound(Date.new(2026, 9, 1), '>')).to eq('2026-08-30T00:00:00Z') + expect(bound(Time.utc(2026, 9, 1, 8, 30), '<')).to eq('2026-09-02T00:00:00Z') + expect(bound(DateTime.new(2026, 9, 1, 8, 30, 0), '<')).to eq('2026-09-02T00:00:00Z') + expect(bound(1_788_251_400, '<')).to eq('2026-09-02T00:00:00Z') end - it 'takes a Time and a DateTime as the instant they are' do - expect(call('date', Time.utc(2026, 9, 1, 8, 30))).to eq(1_788_251_400) - expect(call('date', DateTime.new(2026, 9, 1, 8, 30, 0))).to eq(1_788_251_400) + # A window written in another timezone is answered on the UTC days it + # overlaps, and an operator has no way of guessing that from the rows. + it 'reports the UTC day boundary once, when it is not the day of the caller' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + bound('2026-08-31T22:00:00Z', '>') + bound('2026-08-31T23:00:00Z', '>') + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).once.with(/truncates a date search/) end - it 'leaves epoch seconds alone' do - expect(call('date', 1_788_251_400)).to eq(1_788_251_400) + it 'stays quiet when the window and the UTC day are the same day anyway' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + bound('2026-09-01T08:30:00Z', '>') + + expect(ForestAdminDatasourceIntercom.logger).not_to have_received(:warn) end - # Falling back to UTC silently would move a day boundary by the offset, - # which is the failure a timezone is read for in the first place. context 'when the caller names a timezone nothing knows' do let(:timezone) { 'Middle-Earth/Shire' } + # Falling back to UTC silently would move a day boundary by the offset, + # which is the failure a timezone is read for in the first place. it 'reads the day boundary in UTC and says so' do allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) - expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 9, 1).to_i) + expect(bound('2026-09-01', '>')).to eq('2026-08-31T00:00:00Z') expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/unknown timezone/) end end @@ -62,17 +106,17 @@ def call(type, value, spelling: '=', operator: nil) let(:timezone) { ' ' } it 'reads the day boundary in UTC' do - expect(call('date', '2026-09-01')).to eq(Time.utc(2026, 9, 1).to_i) + expect(bound('2026-09-01', '>')).to eq('2026-08-31T00:00:00Z') end end it 'refuses a string that is not a date' do - expect { call('date', 'last tuesday') } + expect { call('date', 'last tuesday', spelling: '>') } .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) end it 'refuses a value that is not a date at all' do - expect { call('date', { 'day' => 1 }) } + expect { call('date', { 'day' => 1 }, spelling: '>') } .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) end end @@ -121,8 +165,7 @@ def call(type, value, spelling: '=', operator: nil) describe 'a list' do it 'formats every value of an IN the way the field takes it' do - expect(call('date', %w[2026-09-01 2026-09-02], spelling: 'IN', operator: operators::IN)) - .to eq([Time.utc(2026, 8, 31, 22).to_i, Time.utc(2026, 9, 1, 22).to_i]) + expect(call('number', ['4', 5.0], spelling: 'IN', operator: operators::IN)).to eq([4, 5]) end # A filter matching everything is not what an empty list was asked for. From 6557a0be175e3df1ae6794e6b05b647e59de3581 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 18:57:19 +0200 Subject: [PATCH 05/10] feat(intercom): refuse a tree past what a search can nest Intercom nests a search two levels deep and takes fifteen conditions per group. Past either, it answers a 400 whose body names neither the limit nor the part of the filter that reached it -- and the operator reading it has no way of knowing that their scope, plus their segment, plus their own filter is what went over. Both are checked before the request leaves, and both refusals name what to simplify rather than a number alone. A group inside a group inside a group is one level too many. A group of sixteen says that a condition naming several values arrives here expanded into one condition per value, Intercom accepting no membership operator on these fields, so shortening the list is often what brings it back under. The branches the agent wraps around a single condition still spend no level: it assembles a tree one branch at a time, and unwrapping them is what keeps an ordinary scope-plus-segment-plus-filter inside two levels. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../query/condition_tree_translator.rb | 44 ++++++++++++++++--- .../query/condition_tree_translator_spec.rb | 44 +++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb index fc3d2994b..3418a7951 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/condition_tree_translator.rb @@ -20,6 +20,15 @@ class ConditionTreeTranslator AGGREGATORS = { 'and' => 'AND', 'or' => 'OR' }.freeze + # Intercom nests a search two levels deep and takes fifteen conditions per + # group. Both are checked here rather than left to the API: over either, + # Intercom answers a 400 whose body names neither the limit nor the part of + # the filter that reached it, and an operator reading it has no way of + # knowing that their segment plus their scope plus their own filter is what + # went over. + MAX_DEPTH = 2 + MAX_GROUP_SIZE = 15 + def self.call(condition_tree, endpoint:, collection:, timezone: nil) return nil if condition_tree.nil? @@ -32,9 +41,9 @@ def initialize(endpoint:, collection:, timezone: nil) @value = FilterValue.new(collection: collection, timezone: timezone) end - def translate(node) + def translate(node, depth = 1) case node - when Branch then translate_branch(node) + when Branch then translate_branch(node, depth) when Leaf then translate_leaf(node) else raise UnsupportedOperatorError, "#{@collection} cannot read #{node.class} as a condition." end @@ -42,7 +51,7 @@ def translate(node) private - def translate_branch(branch) + def translate_branch(branch, depth) conditions = Array(branch.conditions) refuse_empty_branch!(branch) if conditions.empty? @@ -54,9 +63,34 @@ def translate_branch(branch) # builds a tree one branch at a time -- a scope, then a segment, then the # operator's own filter -- and the nesting Intercom allows is shallow # enough that a wrapper around nothing is a level worth not spending. - return translate(conditions.first) if conditions.size == 1 + return translate(conditions.first, depth) if conditions.size == 1 + + refuse_too_deep!(depth) if depth > MAX_DEPTH + refuse_too_wide!(branch, conditions.size) if conditions.size > MAX_GROUP_SIZE + + { 'operator' => operator, 'value' => conditions.map { |condition| translate(condition, depth + 1) } } + end - { 'operator' => operator, 'value' => conditions.map { |condition| translate(condition) } } + # What reaches this depth is a group inside a group inside a group. The + # message names the shape rather than a number, since the tree an operator + # can act on is the segment and the scope they wrote, not the one the agent + # assembled out of them. + def refuse_too_deep!(depth) + raise UnsupportedOperatorError, + "#{@collection} cannot answer this filter: Intercom nests a search #{MAX_DEPTH} levels deep and this " \ + "one reaches #{depth}. A group inside a group inside a group is one level too many -- flatten the " \ + 'segment, the scope or the filter carrying the innermost one.' + end + + # Fifteen is reached without trying: a scope, a segment and a filter add up, + # and a condition naming several values is expanded into one condition per + # value on the way here, Intercom taking no membership operator on these + # fields. + def refuse_too_wide!(branch, size) + raise UnsupportedOperatorError, + "#{@collection} cannot answer this filter: Intercom takes #{MAX_GROUP_SIZE} conditions per group and " \ + "this #{branch.aggregator} carries #{size}. A filter naming several values counts one condition per " \ + 'value here, so narrowing the list, the segment or the scope is what brings it back under the limit.' end def aggregator(branch) diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb index 89ccc306e..e947fb0fa 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb @@ -93,6 +93,50 @@ def branch(aggregator, *conditions) end end + # Over either limit Intercom answers a 400 whose body names neither the + # limit nor the part of the filter that reached it. + describe 'the limits of the search DSL, checked before the request leaves' do + def leaves(count) + Array.new(count) { |index| leaf('state', operators::EQUAL, "state-#{index}") } + end + + it 'takes a group nested one level inside another' do + tree = branch('And', leaf('open', operators::EQUAL, true), + branch('Or', *leaves(2))) + + expect(translate(tree)['value'].last['operator']).to eq('OR') + end + + it 'refuses a group inside a group inside a group, naming the shape' do + tree = branch('And', leaf('open', operators::EQUAL, true), + branch('Or', leaf('read', operators::EQUAL, true), + branch('And', *leaves(2)))) + + expect { translate(tree) } + .to raise_error(UnsupportedOperatorError, /nests a search 2 levels deep and this one reaches 3/) + end + + # A branch carrying a single condition is unwrapped, so it spends no level: + # the agent wraps a scope and a segment one branch at a time. + it 'does not spend a level on the branches the agent wraps around one condition' do + tree = branch('And', branch('And', branch('And', leaf('open', operators::EQUAL, true)))) + + expect(translate(tree)).to eq({ 'field' => 'open', 'operator' => '=', 'value' => true }) + end + + it 'takes a group of fifteen conditions' do + expect(translate(branch('Or', *leaves(15)))['value'].size).to eq(15) + end + + # A scope, a segment and a filter add up; and a condition naming several + # values arrives expanded into one condition per value, Intercom taking no + # membership operator on these fields. + it 'refuses a group of sixteen, and says what brings it back under' do + expect { translate(branch('Or', *leaves(16))) } + .to raise_error(UnsupportedOperatorError, /takes 15 conditions per group and this Or carries 16/) + end + end + describe 'what it will not translate' do # The whole point of the lot: a condition dropped on the way to Intercom # comes back as an unfiltered page that looks filtered. From 934208245ec02bc79c07e03b5b2df662733ae863 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 19:02:43 +0200 Subject: [PATCH 06/10] feat(intercom): filter conversations and tickets server-side The lot's point of arrival: a condition switches the read from the listing to the search endpoint, carrying the query the translator wrote, and every column advertises exactly the filters that endpoint answers on it. The schema derives from the measured table and from nothing else. A column the table does not carry advertises none, which is how a refusal is spelled in a schema: the tag names, the account of a ticket, the columns derived from its parts, and every ticket attribute stay display-only, and now do so by construction rather than by a hand-written empty list. `CursorCollection` grew a fourth route rather than a third: no condition walks the listing, `id equals X` reads the record endpoint, and anything else is translated and walked through the search. Counting follows the same path, `total_count` being exact on a search too -- a filtered count stays one request over the whole filtered set rather than over a page of it. A free-text search reaches Intercom as a condition on the one column its endpoint matches text on, `source.body`, folded into the condition tree rather than added to the translated query: written as one tree, the nesting limits are checked over the whole of it. Tickets expose no such column and refuse a search by name. `display_as=plaintext` travels on the search too, in the query string, where Intercom does not document it. The bodies are HTML written by end customers (R10), and a filtered read must not come back as markup where an unfiltered one comes back as text -- if the endpoint ignores the parameter, that is one of the things the probe run will settle. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../client.rb | 20 ++- .../collections/conversation.rb | 10 ++ .../collections/cursor_collection.rb | 129 +++++++++----- .../collections/ticket.rb | 12 +- .../collections/conversation_spec.rb | 161 +++++++++++++++--- .../collections/ticket_spec.rb | 77 ++++++++- 6 files changed, 324 insertions(+), 85 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/client.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/client.rb index e9b278e3a..ecfe093b3 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/client.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/client.rb @@ -61,15 +61,16 @@ def list_page(path, per_page:, starting_after: nil, params: {}, list_key: 'data' must_succeed(path) { to_page(get(path, query, boot: boot).body, path, list_key) } end - # One page of a search endpoint. The query is written by the caller rather - # than translated from a Forest filter -- that translation is lot 2 -- so - # what goes on the wire is what the caller asked for. - def search_page(path, query:, per_page:, starting_after: nil, list_key: 'data') + # One page of a search endpoint. The query is the one the condition-tree + # translator wrote, and it travels in the body; `params` is what still + # belongs in the query string -- `display_as` above all, which is not part + # of the search payload. + def search_page(path, query:, per_page:, starting_after: nil, list_key: 'data', params: {}) pagination = { 'per_page' => self.class.bounded_per_page(per_page) } pagination['starting_after'] = starting_after unless blank?(starting_after) body = { 'query' => query, 'pagination' => pagination } - must_succeed(path) { to_page(post(path, body).body, path, list_key) } + must_succeed(path) { to_page(post(path, body, params: params).body, path, list_key) } end # One record from its own endpoint. Raises on a 404 like on any other @@ -123,8 +124,13 @@ def get(path, params = nil, boot: false) (boot ? boot_connection : connection).get(path, params) end - def post(path, body, boot: false) - (boot ? boot_connection : connection).post(path, body) + def post(path, body, params: {}, boot: false) + http = boot ? boot_connection : connection + + http.post(path) do |request| + request.params.update(params) unless params.nil? || params.empty? + request.body = body + end end # Intercom serves the version its workspace defaults to when the pin is not diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation.rb index 9c0d13fc6..dbe8a53d0 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation.rb @@ -25,12 +25,22 @@ class Conversation < CursorCollection def initialize(datasource) super(datasource, 'IntercomConversation') + # The one collection of this datasource Intercom matches text on: `~` on + # `source.body`, which is the message that opened the conversation. + enable_search end protected def list_endpoint = 'conversations' def list_key = 'conversations' + def searchable = 'conversations' + def search_column = 'source_body' + + # Sent on the search too, where Intercom does not document it: the bodies + # are HTML written by end customers (R10), and a parameter it ignores costs + # a query string while the one it honours saves every filtered row from + # coming back as markup. def read_params = { 'display_as' => 'plaintext' } # The contact identity and the timeline, each read only when the projection diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb index a73f5be14..17c7d3c39 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/cursor_collection.rb @@ -11,10 +11,10 @@ module Collections # * no condition at all -- a list view -- walks the listing endpoint; # * `id equals X` reads the record through its own endpoint, which is what a # record detail is; - # * anything else is **refused**. Translating a Forest condition tree into - # Intercom's search DSL is lot 2, and until it exists a filter that cannot - # be honoured has to say so: an unfiltered page served in answer to a - # filter is the one failure this datasource is built to avoid. + # * anything else is translated into Intercom's search DSL and walked + # through the search endpoint. What the translator will not express, it + # refuses by name: an unfiltered page served in answer to a filter is the + # one failure this datasource is built to avoid. # # Counting is the exception that costs nothing: `total_count` is exact on # every response, filter included, so the record counter is one request. @@ -35,10 +35,10 @@ def initialize(datasource, name) enable_count end - def list(_caller, filter, projection) + def list(caller, filter, projection) warn_ignored_sort(filter&.sort) - records = fetch_records(filter) + records = fetch_records(caller, filter) rows = records.map { |record| project(serialize(record), projection) } enrich(records, rows, projection) rows @@ -48,10 +48,10 @@ def list(_caller, filter, projection) # and grouping over the pages a walk happened to collect would look exact # while answering a fraction. Refused here rather than through the # contract's NotImplementedError, which reads as an oversight. - def aggregate(_caller, filter, aggregation, _limit = nil) + def aggregate(caller, filter, aggregation, _limit = nil) refuse_unsupported_aggregation!(aggregation) - [{ 'group' => {}, 'value' => count_records(filter) }] + [{ 'group' => {}, 'value' => count_records(caller, filter) }] end protected @@ -63,6 +63,19 @@ def record_endpoint = list_endpoint def list_key = 'data' def read_params = {} + # The row of the measured table this collection is filtered through: what + # its columns may advertise, and what the translator is allowed to write. + def searchable = raise(NotImplementedError, "#{self.class} did not implement searchable") + + def search_endpoint + @search_endpoint ||= Query::SearchFields.fetch(searchable) + end + + # The column a free-text search is answered on, for a collection whose + # endpoint has one. Nil elsewhere, and a search is then refused rather than + # answered by a page that ignored it. + def search_column = nil + # One Intercom entity flattened into a record matching the schema. def serialize(_entity) = raise(NotImplementedError, "#{self.class} did not implement serialize") @@ -78,20 +91,32 @@ def max_page_size = Client::MAX_PER_PAGE # One page of the collection. A listing for conversations, a search for # tickets -- Intercom exposes no `GET /tickets` at all -- so the endpoint # and its shape belong to the collection, while walking it does not. - def read_page(per_page:, cursor:) - client.list_page(list_endpoint, per_page: [per_page, max_page_size].min, - starting_after: cursor, params: read_params, list_key: list_key) + def read_page(per_page:, cursor:, query: nil) + size = [per_page, max_page_size].min + + if query.nil? + client.list_page(list_endpoint, per_page: size, starting_after: cursor, + params: read_params, list_key: list_key) + else + client.search_page(search_endpoint.path, query: query, per_page: size, starting_after: cursor, + params: read_params, list_key: list_key) + end end - # A column of this tier advertises no filter and no sort, because the - # collection can honour neither -- except on the primary key, which is - # answered by the record endpoint rather than by a filter. A schema that - # advertised more would put filters in the interface that the read then - # refuses. Read-only for the same reason, on the write side. + # A column advertises exactly the filters the search endpoint answers on + # it, taken from the measured table and derived by the operator table -- + # never written by hand here, so a column cannot offer a filter the + # translator would refuse. A column the table does not carry advertises + # none, which is how a refusal is spelled in a schema. + # + # The primary key is the exception, and it is not a filter: `id equals X` + # and `id in [...]` are answered by the record endpoint. + # + # No column is sortable: Intercom takes no sort on either search endpoint + # and ignores the one it is sent. Read-only, this lot writing nothing. def add_column(name, type, is_primary_key: false) - operators = is_primary_key ? [Operators::EQUAL, Operators::IN] : [] add_field(name, ColumnSchema.new(column_type: type, - filter_operators: operators, + filter_operators: column_operators(name, is_primary_key), is_primary_key: is_primary_key, is_read_only: true, is_sortable: false, @@ -104,7 +129,15 @@ def walker private - def fetch_records(filter) + def column_operators(name, is_primary_key) + return [Operators::EQUAL, Operators::IN] if is_primary_key + + field = search_endpoint.field(name) + + field ? Query::OperatorTable.forest_operators(field) : [] + end + + def fetch_records(caller, filter) ids = id_lookup(filter) # The window is cut out of the ids rather than out of the records they # read: Intercom reads them one request each, so paging after the read @@ -113,13 +146,37 @@ def fetch_records(filter) # having been dropped by the truncation before the window was applied. return records_by_ids(page_window(ids, filter)) if ids - refuse_filter!(filter) unless browsing?(filter) + listed_records(filter, translate(caller, filter)) + end - listed_records(filter) + # The Intercom query a filter comes down to, or nil for a list view, which + # walks the listing endpoint instead. The free-text search is folded into + # the condition tree rather than added to the translated query: written as + # one tree, it is checked against the nesting Intercom allows like every + # other condition, instead of adding a level nothing counted. + def translate(caller, filter) + tree = combined_tree(filter) + + Query::ConditionTreeTranslator.call(tree, endpoint: search_endpoint, collection: name, + timezone: timezone_for(caller)) end - def browsing?(filter) - filter.nil? || (filter.condition_tree.nil? && blank_search?(filter)) + def combined_tree(filter) + conditions = [filter&.condition_tree, search_condition(filter)].compact + return conditions.first if conditions.size < 2 + + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::ConditionTreeFactory.intersect(conditions) + end + + # A free-text search reaches Intercom as a condition on the one column its + # endpoint matches text on -- per word, not as a substring, which the + # README says rather than the interface implying otherwise. + def search_condition(filter) + return nil if blank_search?(filter) + + refuse_search! if search_column.nil? + + Leaf.new(search_column, Operators::CONTAINS, filter.search.to_s.strip) end def blank_search?(filter) @@ -163,10 +220,12 @@ def records_by_ids(ids) end end - def listed_records(filter) + def listed_records(filter, query) offset, limit = translate_page(filter&.page) - walker.walk(offset: offset, limit: limit) { |per_page, cursor| read_page(per_page: per_page, cursor: cursor) } + walker.walk(offset: offset, limit: limit) do |per_page, cursor| + read_page(per_page: per_page, cursor: cursor, query: query) + end end # A filter with no page asks for every record it matched; the walker reads @@ -181,13 +240,11 @@ def translate_page(page) # Exact, and one request: `total_count` counts what the filter names, not # what a page happened to hold. An id lookup counts the records it found, # which is cheaper still. - def count_records(filter) + def count_records(caller, filter) ids = id_lookup(filter) return records_by_ids(ids).size if ids - refuse_filter!(filter) unless browsing?(filter) - - page = read_page(per_page: 1, cursor: nil) + page = read_page(per_page: 1, cursor: nil, query: translate(caller, filter)) return page.total_count if page.total_count raise UnsupportedOperatorError, @@ -205,18 +262,10 @@ def refuse_unsupported_aggregation!(aggregation) 'Chart it on a collection read whole, or wait for the bounded group-by of the reporting lot.' end - def refuse_filter!(filter) - detail = if filter&.condition_tree - 'a condition on this collection' - else - 'a free-text search' - end - + def refuse_search! raise UnsupportedOperatorError, - "#{name} cannot answer #{detail} yet: it reads Intercom's listing endpoint, which takes no filter. " \ - 'Server-side filtering goes through the search endpoint and arrives with the filter translation. ' \ - 'Until then, remove the condition, the scope or the segment carrying it rather than being served a ' \ - 'page that would look filtered without being it.' + "#{name} cannot answer a free-text search: #{search_endpoint.path} matches values field by field, " \ + 'and this collection exposes no text column it searches. Filter on a column instead of searching.' end # Intercom accepts a `sort` on these endpoints and ignores it without a diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket.rb index 69dcf6953..2569e4552 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket.rb @@ -38,16 +38,16 @@ def initialize(datasource, attributes: []) protected - def list_endpoint = 'tickets/search' def record_endpoint = 'tickets' def list_key = 'tickets' + def searchable = 'tickets' def max_page_size = MAX_TICKETS_PER_PAGE - # A search rather than a listing, which is the whole reason this hook - # exists. - def read_page(per_page:, cursor:) - client.search_page(list_endpoint, query: MATCH_EVERY_TICKET, list_key: list_key, - per_page: [per_page, max_page_size].min, starting_after: cursor) + # Intercom exposes no `GET /tickets`, so a list view searches too: with the + # filter it was given, or with the predicate that matches everything when + # it was given none. + def read_page(per_page:, cursor:, query: nil) + super(per_page: per_page, cursor: cursor, query: query || MATCH_EVERY_TICKET) end def enrich(records, rows, projection) diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb index 4d1d8ac02..0114b85e7 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb @@ -69,6 +69,14 @@ def stub_list(*records, next_cursor: nil, total: nil, query: hash_including({})) stub_request(:get, "#{base}/conversations").with(query: query).to_return(json(body)) end + def stub_search(*records, total: nil, next_cursor: nil) + body = { 'type' => 'conversation.list', 'conversations' => records, + 'total_count' => total || records.size, 'pages' => { 'type' => 'pages', 'page' => 1 } } + body['pages']['next'] = { 'starting_after' => next_cursor } if next_cursor + + stub_request(:post, "#{base}/conversations/search").with(query: hash_including({})).to_return(json(body)) + end + def stub_record(id, payload, status = 200) stub_request(:get, "#{base}/conversations/#{id}").with(query: hash_including({})).to_return(json(payload, status)) end @@ -82,14 +90,32 @@ def ids(rows) expect(collection.name).to eq('IntercomConversation') end - # Intercom ignores a sort on this endpoint without a word and filters - # nothing on the listing, so a column advertising either would put in the - # interface what the read then refuses. - it 'declares every column unsortable and unfilterable, except the primary key' do - others = collection.fields.except('id') + # Neither search endpoint takes a sort, and Intercom ignores the one it is + # sent without a word, so no column of this tier may advertise one. + it 'declares every column unsortable' do + expect(collection.fields.values.map(&:is_sortable).uniq).to eq([false]) + end + + # Derived from the measured table, never written by hand: a column + # advertises exactly what the search endpoint answers on it. + it 'advertises the filters the search endpoint answers, and only those' do + expect(collection.fields['state'].filter_operators).to eq(%w[equal not_equal]) + expect(collection.fields['created_at'].filter_operators).to eq(%w[greater_than less_than]) + expect(collection.fields['source_body'].filter_operators).to eq(%w[contains i_contains not_contains]) + end + + # A column the table does not carry advertises nothing, which is how a + # refusal is spelled in a schema: the tag names, the company, the timeline + # and the contact identity are all read from somewhere the endpoint does + # not filter. + it 'advertises no filter on a column the endpoint does not filter' do + %w[tag_names company_name contact_email timeline contact_ids].each do |column| + expect(collection.fields[column].filter_operators).to be_empty, "#{column} advertises a filter" + end + end - expect(others.values.map(&:is_sortable).uniq).to eq([false]) - expect(others.values.map(&:filter_operators).flatten.uniq).to be_empty + it 'is searchable, Intercom matching text on the body of the first message' do + expect(collection.is_searchable?).to be(true) end # The record detail is `id equals X`, answered by the record endpoint @@ -254,25 +280,101 @@ def ids(rows) end end - describe 'a filter it cannot honour' do - # Translating a Forest tree into Intercom's search DSL is the next lot. - # Until then a page that looks filtered without being it is the one answer - # this datasource must not give. - it 'refuses a condition on anything but the primary key' do - expect { collection.list(nil, filter(condition_tree: leaf('state', operators::EQUAL, 'open')), %w[id]) } - .to raise_error(UnsupportedOperatorError, /cannot answer a condition/) + describe 'a filter Intercom answers' do + # A condition switches the read from the listing to the search endpoint, + # which is the only one that takes a filter. + it 'searches instead of listing, with the query the translator wrote' do + search = stub_search + tree = leaf('state', operators::EQUAL, 'open') + + collection.list(nil, filter(condition_tree: tree), %w[id]) + + expect(search.with(body: hash_including('query' => { 'field' => 'state', 'operator' => '=', + 'value' => 'open' }))).to have_been_made end - it 'refuses a free-text search' do - searched = ForestAdminDatasourceToolkit::Components::Query::Filter.new(search: 'facture') + # The bodies are HTML written by end customers (R10), and a filtered read + # must not come back as markup where an unfiltered one comes back as text. + it 'asks the search for plain text too' do + search = stub_search + + collection.list(nil, filter(condition_tree: leaf('open', operators::EQUAL, true)), %w[id]) - expect { collection.list(nil, searched, %w[id]) } - .to raise_error(UnsupportedOperatorError, /cannot answer a free-text search/) + expect(search.with(query: hash_including('display_as' => 'plaintext'))).to have_been_made end - it 'says where the filtering will come from, so the message is actionable' do - expect { collection.list(nil, filter(condition_tree: leaf('state', operators::EQUAL, 'open')), %w[id]) } - .to raise_error(UnsupportedOperatorError, /search endpoint.*filter translation/m) + it 'walks the search cursor for the window a list view asked for' do + stub_request(:post, "#{base}/conversations/search") + .with(query: hash_including({})) + .to_return(json({ 'conversations' => [conversation('1'), conversation('2')], + 'pages' => { 'next' => { 'starting_after' => 'c2' } } })) + stub_request(:post, "#{base}/conversations/search") + .with(query: hash_including({}), + body: hash_including('pagination' => hash_including('starting_after' => 'c2'))) + .to_return(json('conversations' => [conversation('3')])) + page = ForestAdminDatasourceToolkit::Components::Query::Page.new(offset: 2, limit: 1) + + rows = collection.list(nil, filter(condition_tree: leaf('open', operators::EQUAL, true), page: page), %w[id]) + + expect(ids(rows)).to eq(%w[3]) + end + + # Per word rather than as a substring, which the README says out loud. + it 'answers a free-text search on the body of the message that opened the conversation' do + search = stub_search + searched = ForestAdminDatasourceToolkit::Components::Query::Filter.new(search: ' facture ') + + collection.list(nil, searched, %w[id]) + + expect(search.with(body: hash_including('query' => { 'field' => 'source.body', 'operator' => '~', + 'value' => 'facture' }))).to have_been_made + end + + # Written as one tree rather than added to the translated query: the + # nesting Intercom allows is then checked over the whole of it. + it 'ands a free-text search with the condition it came with' do + search = stub_search + searched = ForestAdminDatasourceToolkit::Components::Query::Filter.new( + search: 'facture', condition_tree: leaf('open', operators::EQUAL, true) + ) + + collection.list(nil, searched, %w[id]) + + expect(search.with { |request| JSON.parse(request.body)['query']['operator'] == 'AND' }).to have_been_made + end + + # The date bounds Intercom answers are the ones the day rule moved, which + # is what makes an interval answer the day it names. + it 'sends a date bound on the UTC day boundary that answers the day asked for' do + search = stub_search + tree = leaf('created_at', operators::GREATER_THAN, '2026-09-01T08:30:00Z') + + collection.list(nil, filter(condition_tree: tree), %w[id]) + + expect(search.with(body: hash_including('query' => hash_including('value' => Time.utc(2026, 8, + 31).to_i)))) + .to have_been_made + end + end + + describe 'a filter it cannot honour' do + # A condition dropped on the way to Intercom comes back as an unfiltered + # page that looks filtered, which is the one answer this datasource must + # not give. + it 'refuses a condition on a column the endpoint does not filter' do + expect { collection.list(nil, filter(condition_tree: leaf('tag_names', operators::EQUAL, 'billing')), %w[id]) } + .to raise_error(UnsupportedOperatorError, /cannot filter "tag_names"/) + end + + it 'refuses an operator the endpoint does not answer on that column' do + expect { collection.list(nil, filter(condition_tree: leaf('state', operators::CONTAINS, 'op')), %w[id]) } + .to raise_error(UnsupportedOperatorError, /answers equal, not_equal on "state"/) + end + + it 'makes no request at all when it refuses' do + expect { collection.list(nil, filter(condition_tree: leaf('tag_names', operators::EQUAL, 'x')), %w[id]) } + .to raise_error(UnsupportedOperatorError) + expect(a_request(:post, "#{base}/conversations/search")).not_to have_been_made end end @@ -346,11 +448,22 @@ def aggregation(operation, field: nil, groups: []) .to raise_error(UnsupportedOperatorError, /can only be counted/) end - it 'refuses a condition it could not honour on the list either' do + # `total_count` is exact on a search too, so a filtered count is one + # request over the whole filtered set rather than over a page of it. + it 'counts a filtered collection through the search, in one request' do + stub_search(total: 1_234) + + value = collection.aggregate(nil, filter(condition_tree: leaf('state', operators::EQUAL, 'open')), + aggregation('Count')).first['value'] + + expect(value).to eq(1_234) + end + + it 'refuses to count what it refuses to list' do expect do - collection.aggregate(nil, filter(condition_tree: leaf('state', operators::EQUAL, 'open')), + collection.aggregate(nil, filter(condition_tree: leaf('tag_names', operators::EQUAL, 'billing')), aggregation('Count')) - end.to raise_error(UnsupportedOperatorError, /cannot answer a condition/) + end.to raise_error(UnsupportedOperatorError, /cannot filter "tag_names"/) end # Counting the pages a walk collected would answer a fraction as if it diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/ticket_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/ticket_spec.rb index 55c13c07a..bf1b8fddb 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/ticket_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/ticket_spec.rb @@ -94,13 +94,42 @@ def rows(projection = nil, **options) expect(collection.fields['Due'].column_type).to eq('Date') end - # `/tickets/search` filters none of these and ignores a sort without - # saying so, so nothing but the primary key may advertise anything. - it 'declares every column unfilterable and unsortable, except the primary key' do - others = collection.fields.except('id') + # `/tickets/search` ignores a sort without saying so, on every column. + it 'declares every column unsortable' do + expect(collection.fields.values.map(&:is_sortable).uniq).to eq([false]) + end + + it 'advertises the filters the search endpoint answers, and only those' do + expect(collection.fields['category'].filter_operators).to eq(%w[equal not_equal]) + expect(collection.fields['created_at'].filter_operators).to eq(%w[greater_than less_than]) + end - expect(others.values.map(&:filter_operators).flatten.uniq).to be_empty - expect(others.values.map(&:is_sortable).uniq).to eq([false]) + # Measured during lot 1: `/tickets/search` refuses `company_id` with + # `invalid_field` although a ticket carries one. Filtering tickets by + # account is not something this endpoint does. + it 'advertises no filter on the account, which the endpoint refuses' do + expect(collection.fields['company_id'].filter_operators).to be_empty + end + + # Derived by the agent from the parts of the ticket. A column advertising a + # filter the read cannot honour is what this lot exists to prevent. + it 'advertises no filter on the columns derived from the parts' do + %w[closed_at closed_by_name last_reply_at last_responder_name last_responder_type].each do |column| + expect(collection.fields[column].filter_operators).to be_empty, "#{column} advertises a filter" + end + end + + # R7: an attribute is filtered through an id that differs from one ticket + # type to the next, so the union column cannot say which id to use. + it 'advertises no filter on a ticket attribute while the arbitration stands' do + expect(collection.fields['Due'].filter_operators).to be_empty + expect(collection.fields['_default_title_'].filter_operators).to be_empty + end + + # Intercom matches text field by field, and this endpoint exposes none + # this collection carries. + it 'is not searchable' do + expect(collection).not_to be_is_searchable end # An attribute overwriting a native column would show the attribute where @@ -206,13 +235,45 @@ def rows(projection = nil, **options) expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, '1')).map { |row| row['id'] }).to eq(%w[1]) end - it 'refuses a condition it cannot honour' do + # The search carries the filter it was given, in place of the predicate + # that matches everything. + it 'searches with the query the translator wrote' do + search = stub_search(ticket('1'), body: { 'query' => { 'field' => 'category', 'operator' => '=', + 'value' => 'request' } }) + + rows(%w[id], condition_tree: leaf('category', operators::EQUAL, 'request')) + + expect(search).to have_been_made + end + + it 'refuses a condition on a column the endpoint does not filter, by name' do expect { rows(%w[id], condition_tree: leaf('state_category', operators::EQUAL, 'resolved')) } - .to raise_error(UnsupportedOperatorError, /cannot answer a condition/) + .to raise_error(UnsupportedOperatorError, /cannot filter "state_category"/) + end + + # The account, measured as refused by the endpoint itself. + it 'refuses a condition on the account with the measurement as its reason' do + expect { rows(%w[id], condition_tree: leaf('company_id', operators::EQUAL, '696dd')) } + .to raise_error(UnsupportedOperatorError, /invalid_field/) + end + + it 'refuses a free-text search, having no text column the endpoint matches' do + searched = ForestAdminDatasourceToolkit::Components::Query::Filter.new(search: 'facture') + + expect { collection.list(nil, searched, %w[id]) } + .to raise_error(UnsupportedOperatorError, /cannot answer a free-text search/) end end describe '#aggregate' do + it 'counts a filtered collection through the total_count of its search' do + stub_search(total: 12, body: { 'query' => { 'field' => 'open', 'operator' => '=', 'value' => true } }) + aggregation = ForestAdminDatasourceToolkit::Components::Query::Aggregation.new(operation: 'Count') + + expect(collection.aggregate(nil, filter(condition_tree: leaf('open', operators::EQUAL, true)), + aggregation).first['value']).to eq(12) + end + it 'counts through the total_count of the search, exactly' do stub_search(ticket('1'), total: 81_142) aggregation = ForestAdminDatasourceToolkit::Components::Query::Aggregation.new(operation: 'Count') From 43b053aaee11e667de8856207e89332c38067582 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Tue, 1 Sep 2026 19:05:14 +0200 Subject: [PATCH 07/10] docs(intercom): say what is filterable, what is not and why The README section of the filtering lot. An operator reads it before the interface shows them a filter, so it says out loud what a column can imply but not state: that the table of what each endpoint filters is measured rather than documented, and how to measure it again; that a date filter is day-granular and the day is the UTC one, whatever the workspace timezone says; that the free-text search matches whole words rather than substrings; that no column of either collection is sortable; and what stays refused, one reason per column. A spec checks the section against the table rather than trusting it to stay true: the columns the README lists are exactly the ones the endpoints filter, or the suite fails. Refs PRD-1118 Co-Authored-By: Claude Opus 5 (1M context) --- .../README.md | 121 ++++++++++++++++-- .../query/search_fields_spec.rb | 20 +++ 2 files changed, 133 insertions(+), 8 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/README.md b/packages/forest_admin_datasource_intercom/README.md index 3505508e0..da6d67b56 100644 --- a/packages/forest_admin_datasource_intercom/README.md +++ b/packages/forest_admin_datasource_intercom/README.md @@ -66,7 +66,7 @@ degrades to no attribute column, and a collection whose endpoint answers 403 fai | Collection | Endpoint | Paginated | Countable | | --- | --- | --- | --- | -| `IntercomConversation` | `GET /conversations`, `GET /conversations/{id}` | cursor | yes, exactly | +| `IntercomConversation` | `GET /conversations`, `POST /conversations/search`, `GET /conversations/{id}` | cursor | yes, exactly | | `IntercomTicket` | `POST /tickets/search`, `GET /tickets/{id}` | cursor | yes, exactly | | `IntercomAdmin` | `GET /admins` | read whole | yes, exactly | | `IntercomTeam` | `GET /teams` | read whole | yes, exactly | @@ -82,8 +82,9 @@ this lot, and the only ones a chart may group by. The cost is bandwidth, not cor **Cursor** — conversations and tickets. What is in hand is a page of something far larger, so nothing is filtered or sorted in memory. Three routes and no fourth: no condition walks the listing, -`id equals X` reads the record through its own endpoint, and **anything else is refused** with a -message naming the lot that will answer it. +`id equals X` reads the record through its own endpoint, and anything else is translated into +Intercom's search DSL and walked through the search endpoint. What the translation cannot express is +**refused by name** — see [Filtering](#filtering). ## What the API cannot do, and what this does about it @@ -97,6 +98,9 @@ arrive as a 400 carrying the text. - **Duplicates on a moving dataset.** Intercom documents that records modified between two paginated requests can be served twice; the walk deduplicates by id. The missed counterpart is inherent to cursor pagination and cannot be repaired — it is documented rather than papered over. +- **A search takes no sort at all.** Neither search endpoint accepts one, so **no column of + `IntercomConversation` or `IntercomTicket` is sortable** and an explicit order is reported in the + log. The only collections Intercom sorts are the ones read whole, in memory. - **A sort is accepted and ignored.** Measured: `sort` on these endpoints raises nothing and changes nothing. Since the lack of support is undetectable at runtime, no column is declared sortable and a requested order is reported in the log. The rows come back in the order the API imposes. @@ -114,6 +118,106 @@ arrive as a 400 carrying the text. `/admins` under `admins`, `/teams` under `teams`. A response carrying neither the expected key nor `data` is refused rather than read as an empty page. +## Filtering + +`POST /conversations/search` and `POST /tickets/search` answer the condition trees Forest sends, on +the fields Intercom really filters and with the operators each endpoint really validates. Anything +else is **refused with a message naming what to change** — a condition dropped on the way out comes +back as an unfiltered page that looks filtered, which is the one answer this datasource must not +give. A refusal costs no request: it is raised before anything leaves the process. + +### The table is measured, not documented + +The fields a search endpoint filters are not the fields its specification lists. Measured: +`/tickets/search` refuses `company_id` with `invalid_field` although every ticket carries one. So +the source of truth is a committed table — `lib/forest_admin_datasource_intercom/query/search_fields.yml` +— one row per column, each carrying its provenance: + +| `source` | What it means | +| --- | --- | +| `measured` | observed against a real workspace, by `bin/probe_search_fields` or during the spike | +| `spec` | read off Intercom's documentation, and therefore still a candidate | + +Every `filter_operators` a column publishes is **derived** from that table, so a column cannot +advertise a filter the translator would then refuse, and a column the table does not carry +advertises nothing at all. + +To measure a workspace of your own: + +```bash +INTERCOM_ACCESS_TOKEN=... bin/probe_search_fields --endpoint tickets --out measured.yml +``` + +It sends one search per (field, operator) cell, reads Intercom's refusal codes — `invalid_field` for +a field the endpoint does not filter, `data_invalid` for an operator it refuses on that field — and +prints what the committed table promises that Intercom refuses, plus what Intercom accepts that the +table does not know about. It writes evidence rather than rewriting the table, which carries the +prose a generated file would drop. + +### A date filter is day-granular, and the day is the UTC one + +Intercom truncates a date search to the day, at the **UTC** boundary — measured, and against its own +documentation, which promises the workspace's timezone. `> V` answers from the start of the day +*after* V; `< V` answers before the start of V's own day. + +Sent as they come, the two bounds an interval is rewritten into cancel each other out: `today` +reaches the datasource as `> 00:00` and `< 23:59` of one day, which Intercom reads as "from +tomorrow" *and* "before today" — no rows at all, to the most ordinary filter there is. So each bound +is moved to the boundary that makes Intercom answer the day the filter named. + +What follows from that: + +- a bound naming a time of day matches **from the start of that day, or through the end of it**. It + is the granularity the Intercom interface itself filters on; +- a caller in UTC gets exactly the day they asked for; +- a caller in another timezone gets the UTC days their window overlaps — up to a day wider at each + end — and the agent logs that once per filter; +- a date column publishes `>` and `<` only, and no equality. Everything an operator actually uses — + `before`, `after`, `today`, `yesterday`, `past`, `future`, the whole `previous_*` family — is + rewritten by the agent into a pair of those bounds. An equality on an instant is what stays out, + and a day-granular filter could not have honoured it anyway. + +### What is filterable + +| Collection | Filterable on | +| --- | --- | +| `IntercomConversation` | `state`, `priority`, `open`, `read`, `title`, `admin_assignee_id`, `team_assignee_id`, `source_type`, `source_subject`, `source_body`, `source_delivered_as`, `source_author_email`, `closed_by_id`, `reopen_count`, `part_count`, `ai_agent_participated`, and the dates `created_at`, `updated_at`, `waiting_since`, `snoozed_until`, `closed_at`, `first_closed_at`, `first_contact_reply_at`, `last_contact_reply_at`, `last_admin_reply_at` | +| `IntercomTicket` | `open`, `category`, `ticket_type_id`, `admin_assignee_id`, `team_assignee_id`, `created_at`, `updated_at` | + +**Free-text search** is answered on `IntercomConversation` only, through `~` on `source.body` — the +message that opened the conversation. Intercom matches it **per word, not as a substring**: searching +`fact` does not find `facture`. `IntercomTicket` exposes no text column this endpoint matches and +refuses a search by name. + +### What is not filterable, and why + +- **the columns a ticket derives from its parts** — `closed_at`, `closed_by_name`, `last_reply_at`, + `last_responder_name`, `last_responder_type`. They exist nowhere in Intercom; `/tickets/search` + filters none of them and ignores a sort on them without a word; +- **the account of a ticket** — `company_id`, refused by the endpoint itself with `invalid_field`; +- **the ticket attributes** — filtered as `ticket_attribute.{id}`, and the same attribute carries a + different id per ticket type, so a union column has no single id to translate to. See + [Tickets](#tickets); +- **the tag names, the company name and the contact identity of a conversation** — read from + somewhere the search endpoint does not filter, or filtered by an id the column does not hold; +- **absence** — `present`, `blank` and `missing` are derived by the agent from an equality and + rewritten into a comparison with an empty value. Intercom's search matches values and has no + operator for the lack of one, so the rewritten condition is refused rather than sent as a + comparison against the empty string; +- **group-by**, on either cursor collection: there is no aggregate endpoint, and grouping over the + pages a walk collected would look exact while answering a fraction. + +### The limits of a search, checked before the request leaves + +Intercom nests a search **two levels** deep and takes **fifteen conditions per group**. Past either +it answers a 400 whose body names neither the limit nor the part of the filter that reached it, so +both are checked here and refused with a message naming what to simplify. + +Fifteen is reached without trying: a scope, a segment and an operator's own filter add up, and a +condition naming several values arrives expanded into **one condition per value** — Intercom accepts +no membership operator on these fields. Branches carrying a single condition are unwrapped and spend +no level. + ## Conversations The row carries what a queue is read for: state, priority, assignee and team ids, the company, the @@ -160,16 +264,18 @@ Four things to know about them: ceiling the transition falls out of the window. That case is detected and logged, since a Date column cannot say "unknown". -Both columns are **display only**, and not temporarily: `/tickets/search` filters on neither and -ignores a sort, so neither advertises an operator. +Both are **display only**, and not temporarily: `/tickets/search` filters on neither and ignores a +sort, so neither advertises an operator. The attributes a workspace declares on its ticket types are introspected once at boot and published as the **union** of every type's, keyed by name the way the payload is. Filtering one is a different matter: Intercom filters an attribute by id (`ticket_attribute.{id}`), and the same name carries a different id from one ticket type to the next — measured, `_default_title_` is `14162161` on one type and `14162165` on another. A union column has no single id to translate to, so filtering on a -ticket attribute means one collection per ticket type. The ids are kept per type for the lot that -will need them. +ticket attribute means one collection per ticket type — more collections in the interface, and a +schema that changes shape whenever the customer adds a type. Until that trade is worth paying for, +the attributes stay display-only and advertise no operator. The ids are kept per type so the day the +answer changes costs no second boot round trip. ## Rate limits @@ -215,7 +321,6 @@ Everything else is read when a collection is listed, so an agent boots whatever | Lot | What it brings | | --- | --- | -| 2 | Filter translation into Intercom's search DSL, free-text search, per-endpoint operator tables, UTC date bounds | | 3 | Writes and business actions: reply, close, snooze, reopen, assign, tag, convert | | 4 | Contacts and companies, and the relations promoted from today's denormalized columns | | 5 | Notes, tags, segments | diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb index 6f2d20a57..b203c5545 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/search_fields_spec.rb @@ -74,6 +74,26 @@ def field_row(overrides = {}) end end + # The README is where an operator reads what they may filter on before the + # interface shows it to them, so it is checked against the table rather than + # left to drift from it. + describe 'the README section the table feeds' do + let(:filterable) do + File.read(File.expand_path('../../../README.md', __dir__), encoding: 'UTF-8')[ + /### What is filterable\n(.*?)\n### /m, 1 + ] + end + + it 'lists exactly the columns each endpoint filters' do + { 'IntercomConversation' => 'conversations', 'IntercomTicket' => 'tickets' }.each do |collection, endpoint| + row = filterable.lines.find { |line| line.start_with?("| `#{collection}` |") } + listed = row.to_s.scan(/`([a-z_]+)`/).flatten + + expect(listed).to match_array(described_class.fetch(endpoint).filterable_columns) + end + end + end + describe 'a table that cannot be trusted' do it 'refuses an operator Intercom has no spelling for' do expect { table(fields: { 'created_at' => field_row('operators' => ['~=']) }) } From ba38061e30aaeea7961a53aff8f45b36304a10a9 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Thu, 3 Sep 2026 11:21:22 +0200 Subject: [PATCH 08/10] fix(intercom): filter the primary key next to anything else The key was the one column whose Forest operators were not derived from the table: add_column writes `equal` and `in` on it by hand, the toolkit refusing a collection whose key carries neither. The table had no row for it, so the schema advertised a filter the translator had nothing to write. A bare `id equals X` never noticed, reading the record endpoint before the translator is reached. Nested in an `and` it did: a permission scope turns every record detail into `id equals X and `, which the record endpoint cannot answer -- the ids name a wider set than the scope does -- so it went to the search and was refused by name. A scope on either cursor collection therefore broke the record detail, the count and the CSV export, and a filter on the key from the interface broke as soon as anything was filtered next to it. The row goes into the table as `spec`: Intercom documents `id` on both search endpoints, and the probe is what turns that into a promise. It comes out of the candidates in the same move. The invariant spec could not have caught this. It walks the table, and the gap was a column no row of the table is derived from; walked from the schema instead, it names the offending column and the endpoint that cannot translate it. Verified failing without the row. Co-Authored-By: Claude Opus 5 (1M context) --- .../README.md | 9 +++-- .../query/filter_value.rb | 14 +++++++- .../query/search_fields.yml | 26 ++++++++++++-- .../collections/conversation_spec.rb | 32 +++++++++++++++++ .../query/condition_tree_translator_spec.rb | 23 ++++++++++++- .../query/filter_value_spec.rb | 14 ++++++++ .../query/operator_table_spec.rb | 34 +++++++++++++++++++ 7 files changed, 146 insertions(+), 6 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/README.md b/packages/forest_admin_datasource_intercom/README.md index da6d67b56..38503e773 100644 --- a/packages/forest_admin_datasource_intercom/README.md +++ b/packages/forest_admin_datasource_intercom/README.md @@ -181,8 +181,13 @@ What follows from that: | Collection | Filterable on | | --- | --- | -| `IntercomConversation` | `state`, `priority`, `open`, `read`, `title`, `admin_assignee_id`, `team_assignee_id`, `source_type`, `source_subject`, `source_body`, `source_delivered_as`, `source_author_email`, `closed_by_id`, `reopen_count`, `part_count`, `ai_agent_participated`, and the dates `created_at`, `updated_at`, `waiting_since`, `snoozed_until`, `closed_at`, `first_closed_at`, `first_contact_reply_at`, `last_contact_reply_at`, `last_admin_reply_at` | -| `IntercomTicket` | `open`, `category`, `ticket_type_id`, `admin_assignee_id`, `team_assignee_id`, `created_at`, `updated_at` | +| `IntercomConversation` | `id`, `state`, `priority`, `open`, `read`, `title`, `admin_assignee_id`, `team_assignee_id`, `source_type`, `source_subject`, `source_body`, `source_delivered_as`, `source_author_email`, `closed_by_id`, `reopen_count`, `part_count`, `ai_agent_participated`, and the dates `created_at`, `updated_at`, `waiting_since`, `snoozed_until`, `closed_at`, `first_closed_at`, `first_contact_reply_at`, `last_contact_reply_at`, `last_admin_reply_at` | +| `IntercomTicket` | `id`, `open`, `category`, `ticket_type_id`, `admin_assignee_id`, `team_assignee_id`, `created_at`, `updated_at` | + +**The primary key** is filterable like any other column, but a filter naming it *alone* is not +answered by a search: `id equals X` and `id in [...]` read the record endpoint directly, one request +per record. The search answers it only when something else is filtered alongside it — a permission +scope, a segment, or a second filter. **Free-text search** is answered on `IntercomConversation` only, through `~` on `source.body` — the message that opened the conversation. Intercom matches it **per word, not as a substring**: searching diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb index db5469b80..ca38110cc 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb @@ -68,12 +68,24 @@ def epoch(value, leaf) case value when DateTime then value.to_time.to_i when Date then start_of_day(value) - when Time, Numeric then value.to_i + when Time then value.to_i + when Numeric then seconds(value, leaf) when String then parse(value, leaf) else refuse_value!(leaf, value, 'a date') end end + # An Infinity or a NaN -- a cast that overflowed above this datasource -- + # makes `Integer()` raise a FloatDomainError naming a float where the + # operator asked for a date, and a Complex a RangeError, which is the + # same class one level up. Refused like every other value the field + # cannot take, for the reason `number` refuses them too. + def seconds(value, leaf) + Integer(value) + rescue RangeError, TypeError + refuse_value!(leaf, value, 'a date') + end + def parse(value, leaf) return start_of_day(Date.parse(value)) if DATE_ONLY.match?(value) diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml index e2a06b54c..d2bb0ad2b 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/search_fields.yml @@ -31,6 +31,21 @@ endpoints: # Nothing has been probed yet: fill this in with the probe's output. measured_at: null fields: + # The primary key, and the one column whose Forest operators are not + # derived from this table: every collection publishes `equal` and `in` on + # its key whatever is written here, the toolkit refusing a collection + # whose key carries neither. It has to be declared all the same, or the + # schema advertises a filter the translator has no row to write -- which + # is what a record detail becomes the moment a permission scope turns + # `id equals X` into an `and` the record endpoint cannot answer alone. + # + # A bare `id equals X` still reads the record endpoint, one request + # instead of a search. This row is for the compound case. + id: + field: id + type: string + operators: ['=', 'IN'] + source: spec state: field: state type: string @@ -218,7 +233,6 @@ endpoints: # above -- with a column to expose it on, or with none, in which case it is # a column the next lot may add. candidates: - - id - source.id - source.author.id - source.author.type @@ -247,6 +261,15 @@ endpoints: path: tickets/search measured_at: null fields: + # The primary key, declared here for the same reason as on conversations: + # the schema publishes `equal` and `in` on it whatever this table says, + # so a row has to exist for the translator to write. `GET /tickets/{id}` + # answers the key on its own; the search answers it in an `and`. + id: + field: id + type: string + operators: ['=', 'IN'] + source: spec open: field: open type: boolean @@ -350,7 +373,6 @@ endpoints: source: measured candidates: - - id - ticket_id - state - ticket_state.category diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb index 0114b85e7..3b562221b 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/conversation_spec.rb @@ -16,6 +16,11 @@ def leaf(field, operator, value = nil) .new(field, operator, value) end + def branch(aggregator, *conditions) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeBranch + .new(aggregator, conditions) + end + def json(payload, status = 200) { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } } end @@ -239,6 +244,33 @@ def ids(rows) expect { collection.list(nil, filter(condition_tree: leaf('id', operators::EQUAL, '1')), %w[id]) } .to raise_error(APIError) end + + # A permission scope turns the record detail into `id equals X and `, which the record endpoint cannot answer: the ids name a wider + # set than the scope does, and reading them alone would serve a record the + # scope excludes. It goes to the search, where the key is a field like any + # other -- and the whole condition travels, or none of it does. + it 'reads the key through the search once a scope is filtered alongside it' do + search = stub_search(conversation('1')) + tree = branch('And', leaf('id', operators::EQUAL, '1'), leaf('state', operators::EQUAL, 'closed')) + + expect(ids(collection.list(nil, filter(condition_tree: tree), %w[id]))).to eq(%w[1]) + expect(search).to have_been_requested + end + + it 'sends the scope and the key as the one query, neither dropped' do + stub_search(conversation('1')) + tree = branch('And', leaf('id', operators::IN, %w[1 2]), leaf('open', operators::EQUAL, false)) + + collection.list(nil, filter(condition_tree: tree), %w[id]) + + expected = { 'operator' => 'AND', + 'value' => [{ 'field' => 'id', 'operator' => 'IN', 'value' => %w[1 2] }, + { 'field' => 'open', 'operator' => '=', 'value' => false }] } + + expect(a_request(:post, "#{base}/conversations/search") + .with(query: hash_including({}), body: hash_including('query' => expected))).to have_been_made + end end describe '#list of several records by id' do diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb index e947fb0fa..b252c3867 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/condition_tree_translator_spec.rb @@ -34,6 +34,27 @@ def branch(aggregator, *conditions) end end + # A bare `id equals X` never reaches here -- the collection reads the record + # endpoint instead, one request rather than a search. What does reach here + # is the key nested in an `and`: a permission scope, a segment, or a second + # filter alongside it, which the record endpoint cannot answer on its own. + describe 'the primary key, once something else is filtered alongside it' do + it 'writes the key as the search field it is' do + tree = branch('And', leaf('id', operators::EQUAL, '42'), leaf('state', operators::EQUAL, 'open')) + + expect(translate(tree)).to eq({ 'operator' => 'AND', + 'value' => [{ 'field' => 'id', 'operator' => '=', 'value' => '42' }, + { 'field' => 'state', 'operator' => '=', 'value' => 'open' }] }) + end + + it 'writes a membership on the key as the list Intercom takes' do + tree = branch('And', leaf('id', operators::IN, %w[1 2]), leaf('open', operators::EQUAL, true)) + + expect(translate(tree)['value'].first) + .to eq({ 'field' => 'id', 'operator' => 'IN', 'value' => %w[1 2] }) + end + end + describe 'a branch' do it 'groups its conditions under the aggregator Intercom spells in capitals' do tree = branch('Or', leaf('state', operators::EQUAL, 'open'), leaf('state', operators::EQUAL, 'snoozed')) @@ -147,7 +168,7 @@ def leaves(count) it 'refuses a column nothing declares, listing the ones it takes' do expect { translate(leaf('nope', operators::EQUAL, 'x')) } - .to raise_error(UnsupportedOperatorError, /takes no filter on it. Filter on one of: state, open/) + .to raise_error(UnsupportedOperatorError, /takes no filter on it. Filter on one of: id, state, open/) end # None of these collections declares a relation yet, so a `relation:field` diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb index 1ddd8eb34..8063e19c6 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb @@ -119,6 +119,20 @@ def bound(value, spelling) expect { call('date', { 'day' => 1 }, spelling: '>') } .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) end + + # `to_i` raises a FloatDomainError on either, which would leave the read + # with an error naming a float where the operator asked for a date. The + # number branch already refuses them; a date is no different. + it 'refuses a cast that overflowed to Infinity, and a NaN' do + expect { call('date', Float::INFINITY, spelling: '>') } + .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) + expect { call('date', Float::NAN, spelling: '>') } + .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) + end + + it 'reads a finite number as the epoch seconds Intercom stores' do + expect(bound(1_767_225_600, '>')).to eq('2025-12-31T00:00:00Z') + end end describe 'a number' do diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb index b8177dc30..42a998071 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/operator_table_spec.rb @@ -71,6 +71,40 @@ def field(type, operators) end end end + + # Walked from the schema rather than from the table, which is the only way + # to see the primary key: `add_column` writes its operators by hand -- the + # toolkit refuses a collection whose key carries neither `equal` nor `in` + # -- so the loop above, reading the table, could never reach the one + # column no row of the table is derived from. A column the schema + # publishes and the table cannot spell is a filter the translator refuses + # at read time, which is what a record detail hits the moment a scope + # nests `id equals X` in an `and`. + it 'is a set the table can express, for every column the datasource publishes' do + cursor_collections.each do |collection| + endpoint = collection.send(:search_endpoint) + + collection.fields.each do |column, schema| + published = Array(schema.respond_to?(:filter_operators) ? schema.filter_operators : nil) + next if published.empty? + + searchable = endpoint.field(column) + expect(searchable).not_to be_nil, + "#{collection.name}.#{column} publishes #{published.join(", ")} and " \ + "#{endpoint.path} carries no row to translate it" + expect(published - described_class.forest_operators(searchable)) + .to be_empty, "#{collection.name}.#{column} publishes an operator the translator would refuse" + end + end + end + end + + # Only the two collections a search endpoint backs: the reference ones are + # read whole and filtered in memory, and publish no operator from a table. + def cursor_collections + datasource = Datasource.new(access_token: 's3cr3t', rate_limiter: nil) + + datasource.collections.each_value.grep(Collections::CursorCollection) end end end From e982ef94c893c3edd7a01623a5ce93d2ea4d4b00 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Thu, 3 Sep 2026 11:21:25 +0200 Subject: [PATCH 09/10] fix(intercom): refuse a date that overflowed, not raise `epoch` read a Numeric with `to_i`, which raises FloatDomainError on an Infinity or a NaN -- a cast that overflowed above this datasource. The read then failed with an error naming a float, from a backtrace inside the value formatter, where every other unusable value comes back as the refusal naming the column and what the field expects. The number branch already refuses both, and says why three lines below. A date is no different; only the guard was missing. `Integer()` rather than a finite check: it refuses a Complex too, by the same RangeError, and FloatDomainError is that class one level down. Co-Authored-By: Claude Opus 5 (1M context) --- .../query/filter_value_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb index 8063e19c6..96d3f52a2 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb @@ -120,9 +120,10 @@ def bound(value, spelling) .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) end - # `to_i` raises a FloatDomainError on either, which would leave the read - # with an error naming a float where the operator asked for a date. The - # number branch already refuses them; a date is no different. + # Reading either as epoch seconds raises a FloatDomainError, which would + # leave the read with an error naming a float where the operator asked + # for a date. The number branch already refuses them; a date is no + # different. it 'refuses a cast that overflowed to Infinity, and a NaN' do expect { call('date', Float::INFINITY, spelling: '>') } .to raise_error(UnsupportedOperatorError, /Intercom expects a date on this field/) From af19c4e54f2b4b679722115eb0f5bda072ac5349 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Thu, 3 Sep 2026 16:32:05 +0200 Subject: [PATCH 10/10] fix(intercom): read a date filter in the caller timezone `FilterFactory.get_previous_condition_tree` writes the bounds of a previous period with `strftime('%Y-%m-%d %H:%M:%S')`, which carries no offset at all. `Time.parse` read them in whatever timezone the process happened to run in, never the caller's -- and because those bounds sit on a midnight, any offset at all moves them onto another UTC day once DayBounds truncates. Measured for a Europe/Paris caller asking for the previous month: the filter names 31 July, Intercom is asked from 1 August. A whole day of rows beside the ones the chart named, on a UTC server and on a New York one alike. Reachable through the chart route, which builds the previous-period filter and counts through `aggregate` -- a bare Count with no group is exactly what these collections do answer. `Time.parse` stays, and only for what it refuses: it raises on a string naming no date, where `Time.zone.parse` answers today. A filter on `last tuesday` coming back as a filter on today is the silent wrong answer this datasource exists not to give. A value carrying an offset is untouched, which is every operator the toolkit rewrites into a pair of bounds: those arrive as UTC ISO8601 and read the same in any zone. The timezone moves out to CallerZone, for the reason DayBounds was split off already: FilterValue knows what shape a field expects on the wire, and whether a value carries the caller's wall clock is another question. The UTC fallback for an unknown timezone now really is UTC on both paths; on the timestamp one it had been the process timezone, which is not what its own warning promised. Co-Authored-By: Claude Opus 5 (1M context) --- .../query/caller_zone.rb | 56 +++++++++++++++++++ .../query/filter_value.rb | 32 ++++------- .../query/filter_value_spec.rb | 24 ++++++++ 3 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/caller_zone.rb diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/caller_zone.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/caller_zone.rb new file mode 100644 index 000000000..f06eac6f8 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/caller_zone.rb @@ -0,0 +1,56 @@ +module ForestAdminDatasourceIntercom + module Query + # The timezone a filter was written in, and what reading a value in it + # costs when the caller names one nothing knows. + # + # Split from FilterValue for the reason DayBounds was: that class knows + # what shape a field expects on the wire, and whether a value carries the + # caller's wall clock or the server's is a different question. + class CallerZone + # Kept stripped rather than only checked stripped: as it came, a + # `" Europe/Paris "` passes the blank guard and then fails the zone + # lookup, and a day boundary lands an offset away from where the filter + # meant it. + attr_reader :name + + def initialize(identifier) + stripped = identifier.to_s.strip + @name = stripped.empty? ? 'UTC' : stripped + end + + # A day with no time of day is the caller's day, not the server's: it is + # the timezone the filter was written in that says when that day starts. + def start_of_day(date) + read('the day boundary') { Time.zone.local(date.year, date.month, date.day).to_i } + end + + # A timestamp carrying no offset is the caller's wall clock. `FilterFactory` + # writes the bounds of a previous period as `%Y-%m-%d %H:%M:%S`, so a chart + # comparing to the previous month sent a midnight read in whatever timezone + # the process happened to run in -- and a midnight moved by any offset at + # all lands on another UTC day once truncated, which is a whole day of rows + # beside the ones asked for. + # + # A timestamp carrying an offset is untouched, which is every operator the + # toolkit rewrites into a pair of bounds: those come through as UTC ISO8601 + # and read the same in any zone. + def timestamp(value) + read('the timestamp') { Time.zone.parse(value).to_i } + end + + private + + # Falling back to UTC silently would move a boundary by the offset, which + # is the failure a timezone is read for in the first place. + def read(what, &block) + Time.use_zone(@name, &block) + rescue ArgumentError + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] unknown timezone #{@name.inspect}, reading #{what} of a date " \ + 'filter in UTC instead.' + ) + Time.use_zone('UTC', &block) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb index ca38110cc..c14be8fc3 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/query/filter_value.rb @@ -18,13 +18,8 @@ class FilterValue def initialize(collection:, timezone: nil) @collection = collection - identifier = timezone.to_s.strip - # Stored stripped rather than only checked stripped: kept as it came, a - # `" Europe/Paris "` passes the blank guard and then fails the zone - # lookup, and a day boundary lands an offset away from where the filter - # meant it. - @timezone = identifier.empty? ? 'UTC' : identifier - @day_bounds = DayBounds.new(collection: collection, timezone: @timezone) + @zone = CallerZone.new(timezone) + @day_bounds = DayBounds.new(collection: collection, timezone: @zone.name) end def call(leaf, field, spelling) @@ -67,7 +62,7 @@ def scalar(value, leaf, field) def epoch(value, leaf) case value when DateTime then value.to_time.to_i - when Date then start_of_day(value) + when Date then @zone.start_of_day(value) when Time then value.to_i when Numeric then seconds(value, leaf) when String then parse(value, leaf) @@ -86,26 +81,19 @@ def seconds(value, leaf) refuse_value!(leaf, value, 'a date') end + # `Time.parse` is called for what it refuses, not for what it returns: + # it raises on a string naming no date, where `Time.zone.parse` answers + # today -- a filter on `last tuesday` coming back as a filter on today is + # the silent wrong answer this datasource exists not to give. def parse(value, leaf) - return start_of_day(Date.parse(value)) if DATE_ONLY.match?(value) + return @zone.start_of_day(Date.parse(value)) if DATE_ONLY.match?(value) - Time.parse(value).to_i + Time.parse(value) + @zone.timestamp(value) rescue ArgumentError, TypeError refuse_value!(leaf, value, 'a date') end - # A day with no time of day is the caller's day, not the server's: it is - # the timezone the filter was written in that says when that day starts. - def start_of_day(date) - Time.use_zone(@timezone) { Time.zone.local(date.year, date.month, date.day).to_i } - rescue ArgumentError - ForestAdminDatasourceIntercom.logger.warn( - "[forest_admin_datasource_intercom] unknown timezone #{@timezone.inspect}, reading the day boundary of a " \ - 'date filter in UTC instead.' - ) - Time.utc(date.year, date.month, date.day).to_i - end - # The agent casts every Number column with `to_f`, so an integer 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. diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb index 96d3f52a2..13ee73781 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/query/filter_value_spec.rb @@ -53,6 +53,23 @@ def bound(value, spelling) expect(bound('2026-09-01T10:30:00+02:00', '<')).to eq('2026-09-02T00:00:00Z') end + # `FilterFactory` writes the bounds of a previous period with strftime + # and no offset at all, so a chart comparing to the previous month sends + # `2026-08-01 00:00:00` meaning the caller's midnight. Read in the + # process timezone it is a different instant, and a midnight moved by any + # offset lands on another UTC day once truncated -- a whole day of rows + # beside the ones the chart named. + it 'reads a timestamp carrying no offset in the timezone of the caller' do + # Paris midnight on 1 September is 2026-08-31T22:00:00Z, whose UTC day + # is the 31st, so `>` must answer from the 31st and sit on the 30th. + expect(bound('2026-09-01 00:00:00', '>')).to eq('2026-08-30T00:00:00Z') + end + + it 'reads a whole previous-period window in the timezone of the caller' do + expect(bound('2026-08-01 00:00:00', '>')).to eq('2026-07-30T00:00:00Z') + expect(bound('2026-09-01 00:00:00', '<')).to eq('2026-09-01T00:00:00Z') + end + # A day with no time of day is the caller's day: it is the timezone the # filter was written in that says when that day starts. it 'reads a bare date as midnight in the timezone of the caller' do @@ -100,6 +117,13 @@ def bound(value, spelling) expect(bound('2026-09-01', '>')).to eq('2026-08-31T00:00:00Z') expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/unknown timezone/) end + + it 'reads a timestamp carrying no offset in UTC and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + + expect(bound('2026-09-01 00:00:00', '>')).to eq('2026-08-31T00:00:00Z') + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/unknown timezone/) + end end context 'when the caller names no timezone at all' do